Skip to content

V13: Move publishing notification after validation#16331

Merged
Migaroez merged 1 commit into
v13/devfrom
v13/bugfix/dont-fire-publishing-notification-early
May 22, 2024
Merged

V13: Move publishing notification after validation#16331
Migaroez merged 1 commit into
v13/devfrom
v13/bugfix/dont-fire-publishing-notification-early

Conversation

@Zeegaan

@Zeegaan Zeegaan commented May 21, 2024

Copy link
Copy Markdown
Member

Notes

How to test

  • Follow steps from discussion

@Zeegaan Zeegaan changed the title Move publishing notification after validation V13: Move publishing notification after validation May 21, 2024

@Migaroez Migaroez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solves the issue/discussion as advertised.

Had some concerns about functional changes but the magic Success based on the bit value of the OperationStatuses seems to take it all into account 👍

@Migaroez Migaroez merged commit 0f3160f into v13/dev May 22, 2024
@Migaroez Migaroez deleted the v13/bugfix/dont-fire-publishing-notification-early branch May 22, 2024 10:17
@nzdev

nzdev commented Sep 9, 2024

Copy link
Copy Markdown
Contributor

With this pr in place, if the content is changed during ContentPublishingNotification, the content is left in Published (Pending Changes) state. E.g. if you want to set a field value at the time of publishing (e.g. a record of when the content was first published) it now shows no extra current version created and Published (Pending Changes) state which is a breaking change.

@markadrake

markadrake commented Dec 1, 2024

Copy link
Copy Markdown
Contributor

@Zeegaan, @Migaroez,
Would you kindly review @nzdev's opened issue: #17023.
This is indeed a breaking change.
Very quick/easy to replicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants