Content: Reintroduce create/save-and-publish as a single operation (closes #22577) - #22812
Merged
AndyButland merged 18 commits intoJul 2, 2026
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
|
@kjac I think this is good to go in, I have a plan for the merge upto 18 rdy to go. |
3 tasks
kjac
marked this pull request as ready for review
May 22, 2026 14:37
|
Claude finished @kjac's task in 6m 54s —— View job PR ReviewTarget: Reintroduces create/save-and-publish as a single atomic operation at all three layers:
CriticalNone Important
Suggestions
Approved with Suggestions for improvementGood to go — the core implementation is clean and well-tested at the service layer. The commented-out dead code block (Important) should be cleaned up before merge. The other items are suggestions worth considering but not blockers. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…4 times, into a single helper method.
…ish, that checks the save result before publishing.
…names already provide details of the method under test).
… combined operation controllers.
AndyButland
approved these changes
May 27, 2026
Contributor
There was a problem hiding this comment.
All looks good to me. Test coverage very thorough, and manual tests check out.
I did spot one security related issue where we needed to validate the users can create/update AND publish, not just create/update, but I've applied that change. Along with a few other smaller things I found in review.
We'll keep this PR open for the FE work to be applied, so approval is for the C# code (management API, service layer and tests).
… (FE) (#23031) * Documents: Use single-transaction create/update-and-publish endpoints (FE) Replace the separate save+publish calls on the document save-and-publish path with the new create-and-publish / update-and-publish endpoints, and keep unpublished but edited variants dirty after reload via a current-state re-merge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Documents: Address review feedback on save-and-publish FE - Deduplicate request-body mapping in the document data source (shared #mapCreateRequestBody / #mapUpdateRequestBody) to remove create vs create-and-publish duplication (CodeScene). - Split performCreateOrUpdateAndPublish into #createAndPublish / #updateAndPublish to reduce method complexity (CodeScene), and clarify its post-condition in JSDoc. - Default parentUnique to null on the repository createAndPublish overload (Claude). - De-duplicate segments before expanding variant lists (Copilot). - Add a createAndPublish data-source unit test (Copilot). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Documents: Drop redundant document re-read on save-and-publish The create/update-and-publish data-source methods re-read the full document after the mutation, but the publishing workspace context immediately calls reload() (which re-fetches and refreshes state) and discards that result. Remove the data-source re-read so save-and-publish fetches the document once (the reload) instead of twice. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Documents: Fix spurious discard dialog after create-and-publish After create-and-publish the workspace flipped isNew=false before its data state was reconciled (that happens in the subsequent reload + transfer). The flip schedules the new->edit redirect, whose navigation guard then saw a transient dirty state and popped a "Discard unsaved changes" dialog — most visible on an invariant document with an empty RTE (reported by Andy Butland). Reconcile persisted to the just-saved data before flipping isNew. Using saveData (not the full current data) keeps published variants clean while edited-but-unpublished variants stay dirty, so their edits remain preserved and navigation-guarded during the brief pre-reload window. Update-and-publish is unaffected (no redirect). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Documents: Move create/update-and-publish into the publishing domain Following review feedback (Mads Rasmussen), keep all publishing-related HTTP in the publishing repository/data source rather than the document detail layer: - Move createAndPublish/updateAndPublish from UmbDocumentServerDataSource + UmbDocumentDetailRepository to UmbDocumentPublishingServerDataSource + UmbDocumentPublishingRepository. Extract the shared create/update request-body mapping to document-detail-request.mappers.ts so it is not duplicated. - The publishing workspace context orchestrates the combined call and asks the document workspace context to apply the create/update lifecycle via new public finalizeCreate/finalizeUpdate methods. This keeps setIsNew and _workspaceEventUnique private — no shared workspace base-class changes. - finalizeCreate carries the create-and-publish fix (reconcile persisted to the saved data before the new->edit redirect) so an edited-but-unpublished variant stays dirty while published variants are clean. Move the and-publish data-source test to the publishing folder; rework the save-and-publish/create-and-publish context tests to drive the publishing data source + finalize methods. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Documents: Drop redundant context guard in save-and-publish orchestrator #createOrUpdateAndPublish re-checked #documentWorkspaceContext although its only caller (#performSaveAndPublish) already guards it; pass the narrowed reference in. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Documents: Reconcile current as well as persisted in finalizeCreate Andy Butland found the create-and-publish "Discard unsaved changes" dialog still appeared for some document types. The dirty guard's jsonStringComparison is order-sensitive, and savedData (a merge-processed projection of the draft) can order its values array differently from current — so reconciling only persisted still left a spurious mismatch. Set both persisted and current to savedData before flipping isNew, so the new->edit redirect can never observe a dirty state. Edited-but-unpublished variants are restored (dirty again) by the caller's reload + transferPublishedVariantsToCurrent. Update the create-and-publish "no data loss" test to assert the end state (after reload + transfer) instead of the intermediate redirect window, since the unpublished variant's edit is briefly absent during it by design. Diagnosis and fix proposed by Andy Butland. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Documents: Align finalizeUpdate with finalizeCreate and tighten method docs Review feedback from Niels: finalizeUpdate now takes the saved data and reconciles persisted/current like finalizeCreate (mirroring the base _update), the finalize*/transfer JSDocs describe only the method's own responsibility, and the transfer parameter is renamed to currentData to match the method name. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * initial correction * second round of refactor * url-pattern-to-string tests * url-pattern-to-string jsdocs * avoid discard changes dialog when navigating between create and edit * keep track of the absolute route as well * check absolute path as part of dirty check * check navigation util * added TODOs * clean up * error handling * error handling for schedule * remove unused import * rename to performDefault... * rename * rename and export interface * simplify #applyPersistedData * fix(documents): avoid double notification on publish/schedule validation failure The validation-failure path already notifies and re-rejects; the shared top-level .catch then fired a second, contradictory toast ("saved for you" followed by "could not be saved"). Scope the failure notification to the publish path so each outcome shows a single message. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(documents): don't report publish failure when only the read-back fails After a successful create/update-and-publish the workspace re-reads the document (the endpoints return key-only). If that read-back fails, the publish has already succeeded server-side, so falling through to the publish-failed toast is misleading. Fall back to the submitted data so the workspace lifecycle completes, and surface a soft warning that the editor could not be refreshed. Also widen the loadWithoutPersist() error message from "document" to "entity" since the method lives on the shared entity-detail base. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * simplify comment --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Niels Lyngsø <niels.lyngso@gmail.com>
…/github.com/umbraco/Umbraco-CMS into v17/improvement/save-and-publish-take-three
AndyButland
changed the title
kjac
pushed a commit
that referenced
this pull request
Aug 4, 2026
…ublish (closes #23523) (#23528) * fix(core): raise ContentSavedNotification on save-and-publish (closes #23523) The combined SaveAndPublish operation introduced in #22812 published the Saving, Publishing and Published notifications but never the paired Saved notification that the standalone Save path raises. Restore it by raising ContentSavedNotification once the document is persisted within CommitDocumentChangesInternal, opted into only by the save-and-publish entry points so the unpublish, branch and scheduled paths are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(core): address review feedback on save-and-publish Saved notification - Capture the Saved notification's cultures using the same expression and timing as the standalone Save path (before persistence resets change tracking), so it honours the documented SavedCultures contract rather than reporting "*" unconditionally for invariant content. - Document the new raiseSavedNotification parameter (CS1573). - Use the concrete Content type for the test document variable (CA1859). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Additional test verifying cultures saved for variant save and publish. * Extracted nested ternary operation into an independent statement. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Testing
A bumch of tests have been added to cover all the things they did in v13 and more.
Fixes #22577