Save and Publish: Publishing notification fires for invalid documents

[piotrbach]

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

11.5.0

Bug summary

If a document encounters errors during the save operation (due to validation failures or missing information), the process should halt, preventing the publication phase from initiating.

The primary concern is the inability to ensure that only validated and fully compliant documents proceed to the publishing stage. This becomes particularly problematic when integrating Umbraco in a headless setup, where consistency and data integrity are paramount. The current functionality does not allow for a reliable check of document validity at the point of publishing using the provided middleware, leading to potential inconsistencies and errors in data sent to external systems or services.

Triggering the ContentPublishingNotification for documents that fail validation essentially undermines the integrity of the publishing workflow, allowing for the possibility of publishing incomplete or incorrect data. While seemingly minor, this oversight poses significant challenges in maintaining data consistency and integrity, especially in scenarios requiring precise control over published content.

In my opinion it's obvious overlooking, that shoud be easy to fix by proper ContentService.StrategyCanPublish implementation and usage.

TIP: Raise PublishCancelable notification after validation https://github.com/umbraco/Umbraco-CMS/blob/v11/dev/src/Umbraco.Core/Services/ContentService.cs#L3068C12-L3068C50.

https://github.com/umbraco/Umbraco-CMS/blob/v11/dev/src/Umbraco.Core/Services/ContentService.cs#L3068C12-L3068C50

Please be aware the thread was opened here https://github.com/umbraco/Umbraco-CMS/discussions/14034 by @wtct.

Of course, splitting Save and Publish is a good idea. However, I am aware that it is breaking change with a big workload.

https://github.com/umbraco/Umbraco-CMS/blob/v11/dev/src/Umbraco.Core/Services/ContentService.cs#L3057C27-L3057C45

https://github.com/umbraco/Umbraco-CMS/blob/release-11.5.0/src/Umbraco.Core/Services/ContentService.cs#L3068

Specifics

No response

Steps to reproduce

• Implement ContentPublishingNotification through INotificationHandler<ContentPublishingNotification>
• Define a document that incorporates mandatory fields or establishes specific validation constraints.
• Click the "Save and Publish" action on a document that lacks the requisite data or contains data that violates the established validation rules.
• Notice that the triggers for both ContentSavingNotification and ContentPublishingNotification are executed.

Expected result / actual result

Publishing notification is firing only when document is valid after Saved event.

---

This item has been added to our backlog AB#38102

[github-actions[bot]]

Hi there @piotrbach!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

• We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
• If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
• We'll replicate the issue to ensure that the problem is as described.
• We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face:

[kjac]

Hi @piotrbach,

Thanks for reaching out 😄

Slight disclaimer: I haven't read the linked thread, so maybe I'm pointing out things here that have already been addressed elsewhere.

First and foremost, the save process must always succeed ... we want to be able to save invalid data, to help editors that might need to abort their content creation flow.

As for publishing, I do agree it is a little strange if the ContentPublishingNotification notification is fired for content with invalid property data. We could consider moving that, so it is fired a little later in the publishing flow (after property validation etc.). Wouldn't that alleviate your immediate issue?

And then for my own curiosity - why would you rely on the ContentPublishingNotification rather than the ContentPublishedNotification to sync with external systems? Since content publishing can be aborted for any number of reasons, the ContentPublishedNotification is the only way to be certain that content was actually published.

[piotrbach]

Hi, Kenn🤝@kjac. Big thanks for the fast response.

1) Yep, that makes sense.

2) "We could consider moving that, so it is fired a little later in the publishing flow (after property validation etc.). Wouldn't that alleviate your immediate issue?". That should work for my project and would be great! Yes, please.

3) We don't want a situation when publishing fails in an external system, and the document is published in Umbraco. We're storing the integration track in a custom table, and ensuring consistency is challenging.

Best, Piotr