Documents: Use single-transaction create/update-and-publish endpoints (FE)

[iOvergaard]

Description

Frontend half of the single-transaction save-and-publish work (ADO 68071, under PBI 68081). The backend endpoints come from #22812, and this targets that feature branch so the whole thing lands on v17/dev together.

Save-and-publish used to be two server calls — PUT /document/{id} (save) then PUT /document/{id}/publish — plus a reload. This wires the backoffice to the new combined endpoints so it's a single transaction.

Where it lives

All the publish-related HTTP sits in the publishing domain rather than the detail layer (kept there on review feedback, so other entities/actions can reuse it if they grow combined endpoints of their own):

• UmbDocumentPublishingServerDataSource / UmbDocumentPublishingRepository — new createAndPublish / updateAndPublish, plus a #mapCulturesToPublish helper that emits only real culture codes (invariant content types send an empty array, matching the server contract).
• Shared request-body mappers (umbMapDocumentCreateRequestBody / umbMapDocumentUpdateRequestBody) build the bodies and are used by both the plain create/update and the and-publish calls, so there's no duplication.
• UmbDocumentWorkspaceContext — public finalizeCreate / finalizeUpdate apply the same create/update workspace lifecycle the base save flow would (mark no-longer-new, tree events, reconcile the data state) after the publishing context has made the call. This keeps setIsNew and the workspace event discriminator private, so there are no shared base-class changes. Plus transferPublishedVariantsToCurrent (below).
• UmbDocumentPublishingWorkspaceContext.#performSaveAndPublish orchestrates the flow: one combined call → reload → transfer.

Keeping unpublished edits dirty (ADO 68071)

After save-and-publish, the reload resets both persisted and current state to the full server document. transferPublishedVariantsToCurrent re-applies the still-unpublished, edited variants from the pre-publish draft onto the current state (via UmbMergeContentVariantDataController), so a variant you changed but did not publish stays dirty and the Discard-Changes guard still fires. Verified: edit English + Danish, publish English only → Danish stays dirty with its edit intact.

No spurious "Discard unsaved changes" dialog on create

Creating and publishing a brand-new document flips isNew, which triggers the new→edit redirect, and that redirect's navigation guard was catching the workspace mid-transition and popping a Discard dialog. finalizeCreate now reconciles both persisted and current to the saved data before the flip, so the guard sees a clean workspace whatever the doc type — the dirty check is an order-sensitive JSON comparison, and the saved data can order its values differently from the draft, so reconciling only one side wasn't enough. The unpublished variants are restored (dirty again) by the reload + transfer that follow, so nothing is lost. Thanks to @AndyButland for spotting this and pinning down the ordering cause.

Not changed

• Scheduling still uses the separate publish endpoint (the combined endpoints are immediate-publish only).
• The validation-failure path still falls back to a plain save (no publish).
• No deprecations — the existing publish() methods are still used by scheduling, the publish entity action, and the publish bulk action.

Testing

• Unit tests (89 document tests passing): the shared mappers; the publishing data source maps culturesToPublish correctly; the 68071 data-state behaviour (published variant clean, unpublished edit stays dirty — fails on the pre-fix code, passes after); a single combined call (no separate publish); and the create-and-publish path staying clean through the redirect window then restoring the unpublished edit after reload + transfer.
• OpenAPI client regenerated from the merged OpenApi.json.
• Verified in a production build against a real backend: create-and-publish (an invariant doc with an empty RTE — the case Andy reproduced), update-and-publish, and the variant 68071 flow (publish English only; Danish stays dirty and keeps its edit).

Loading / fetch behaviour

Progress during save & publish shows only on the action button (the in-progress setExecuting state from #22551). The full-workspace loader is intentionally not raised — what's on screen is what was just published, so there's no reason to blank and reload the editor. reload() refreshes the workspace state in place without toggling the loading flag.

Post-publish requests are one GET /document/{id} (reload), GET /document/{id}/published (recomputes the published-vs-draft "pending changes" indicator — pre-existing behaviour, its result is used), and a background GET /tree/document/{id}/siblings. The redundant second GET /document/{id} the data source used to issue after mutating has been removed, so the document is fetched once post-publish.

Related to #21152

🤖 Generated with Claude Code

[claude]

Claude finished @iOvergaard's task in 5m 24s —— View job

---

PR Review

Target: origin/v17/improvement/save-and-publish-take-three · Based on commit: c25740bc · Skipped: 2 files out of 10 total (sdk.gen.ts, types.gen.ts — auto-generated)

Wires the document backoffice to new combined create/update-and-publish endpoints, replacing the previous two-step save-then-publish flow with a single atomic server call; adds logic to re-apply unpublished variant edits after the reload so they remain dirty.

• Modified public API: UmbDocumentServerDataSource (+createAndPublish, +updateAndPublish), UmbDocumentDetailRepository (+createAndPublish, +updateAndPublish), UmbDocumentWorkspaceContext (+performCreateOrUpdateAndPublish, +transferPublishedVariantsToCurrent)
• Other changes: Save-and-publish is now a single atomic server transaction (previously: save → publish → reload). An edited variant that wasn't included in the publish selection now survives the reload as a dirty state.

---

Suggestions

• document-detail.repository.ts:23: parentUnique has no default value in the repository method, while the underlying data source declares parentUnique: string | null = null. Callers of the repository's createAndPublish must always pass the argument explicitly, which is inconsistent with the data source signature and the analogous base-class create method. Adding = null here makes the API surface consistent. Fix this →

• document-workspace.context.ts:408: performCreateOrUpdateAndPublish is public but intentionally does not update persisted/current data — it requires a caller-side reload() + transferPublishedVariantsToCurrent() to leave the workspace in a valid state. The JSDoc communicates this, but a public method with an invisible post-condition is a footgun for any consumer that calls it standalone. Consider whether protected would be more appropriate (only the single in-package caller exists today), or at minimum add a @throws note spelling out the broken state that results if the follow-up is omitted.

• document-workspace-save-and-publish.context.test.ts:109: The single-call verification test mutates UmbDocumentServerDataSource.prototype at runtime to count invocations. The try/finally cleanup is correct, but the test is white-box: it couples to the internal call path rather than observable server behaviour. A more resilient approach would intercept the MSW request log to assert that exactly one PUT /.../update-and-publish was received and zero PUT /document/:id (plain update) calls were made — that way the assertion survives if the implementation layer is refactored.

---

Approved with Suggestions for improvement

Good to go, but please carefully consider the importance of the suggestions.

Labels applied: area/frontend, category/ux

[Copilot]

Pull request overview

This PR updates the Documents backoffice save-and-publish flow to use the new single-transaction Management API endpoints (create-and-publish / update-and-publish), and adds client-side state handling to keep unpublished edits “dirty” after a publish-triggered reload.

Changes:

• Add createAndPublish / updateAndPublish to the document detail server data source (plus culturesToPublish mapping + re-read).
• Wire the publishing workspace to call the combined operation (instead of save + publish) and restore unpublished edits after reload.
• Add unit tests covering the save-and-publish data-state behavior and the single combined call path, plus MSW/mock support for the new endpoints.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/Umbraco.Web.UI.Client/src/packages/documents/documents/workspace/context/document-workspace.context.ts	Adds combined create/update-and-publish orchestration and post-reload “dirty variant” restoration logic.
src/Umbraco.Web.UI.Client/src/packages/documents/documents/workspace/context/document-workspace-save-and-publish.context.test.ts	New unit tests verifying dirty-state behavior after save-and-publish and ensuring only one combined call is used.
src/Umbraco.Web.UI.Client/src/packages/documents/documents/repository/detail/document-detail.server.data-source.ts	Adds createAndPublish / updateAndPublish methods and culturesToPublish mapping with re-read.
src/Umbraco.Web.UI.Client/src/packages/documents/documents/repository/detail/document-detail.server.data-source.test.ts	New unit tests for update-and-publish behavior (requested cultures + invariant case).
src/Umbraco.Web.UI.Client/src/packages/documents/documents/repository/detail/document-detail.repository.ts	Exposes repository pass-throughs for the new server data source methods.
src/Umbraco.Web.UI.Client/src/packages/documents/documents/publishing/workspace-context/document-publishing.workspace-context.ts	Switches save-and-publish flow to the combined endpoint and reapplies unpublished edits after reload.
src/Umbraco.Web.UI.Client/src/packages/core/backend-api/types.gen.ts	Regenerates OpenAPI types to include new request models and routes.
src/Umbraco.Web.UI.Client/src/packages/core/backend-api/sdk.gen.ts	Regenerates OpenAPI SDK methods for the new endpoints.
src/Umbraco.Web.UI.Client/mocks/msw-handlers/document/detail.handlers.ts	Adds MSW handlers for create-and-publish and update-and-publish endpoints.
src/Umbraco.Web.UI.Client/mocks/db/document-publishing.manager.ts	Adds mock DB publishing support for combined create/update-and-publish operations.

[madsrasmussen]

Hi,

Is it possible to maintain the current domain split, with all publishing-related methods located in the publishing repository and publishing context? This would mean that methods like createAndPublish would be moved from the DocumentDetailRepository to the UmbDocumentPublishingRepository. I believe this separation of responsibilities aligns better with how package authors may want to extend a workspace with additional functionality, giving us as much flexibility as possible for workspace extensions. Additionally, it will be beneficial if other entities require publishing functionality, as we would only need to "borrow" the Publishing domain across.

[AndyButland]

Reviewing purely from a testing perspective, I can see the endpoints being called as expected for "create and publish" and "update and publish".

There's an issue with "create and publish" in that after the operation completes there's an immediate and unnecessary display of the "Discard unsaved changes" dialog. If I click "Stay" and then navigate away, there's no secondary display of this dialog, which is correct, so seems there's an issue where this is incorrectly triggered immediately after the create and publish, presumably as it navigates to the "edit" view for the newly created document.

[iOvergaard]

@madsrasmussen Great question!

Agreed on keeping the domain split. One heads-up: moving the createAndPublish/updateAndPublish HTTP methods to UmbDocumentPublishingRepository is easy (the publishing context already has the repo + saveData, and I'll share the request-body mappers).

The catch is the create/update lifecycle around it: setIsNew and the _workspaceEventUnique-stamped UmbEntityUpdatedEvent are protected on the shared base classes, so calling them from the publishing context means exposing them publicly (touches every entity workspace). Are you OK with that?

[iOvergaard]

@AndyButland Interesting finding. I didn't see that myself before, but I will test it out. I think it may be caused by the fact that we now don't overwrite the whole dataset after create/update, because you may not have save-published all variants. Did you have any variants on your page, or any of the known problematic property editors, such as Tiptap with blocks on the document?

[AndyButland]

I checked - I see this on invariant documents, but it does seem to be property editor related, I've tested with an invariant document that does have an RTE on it, but I haven't populated it with anything.

[madsrasmussen]

@iOvergaard I don’t fully remember the entire ceremony around create/update, but it is probably quite complicated. I think there is something interesting in exposing “just enough” so that additional workspace contexts will be able to “prepare” data for a create/save. So maybe we need to introduce new public methods to support this 🤔 I could imagine that other actions might be interested in doing a save/create before their own action and might introduce combined endpoints too.

[AndyButland]

I was still seeing the same issue @iOvergaard after pulling down the latest - but for one particular document type and not others if I tried to recreate. I found evidence of the problem though:

You see the elements in the values array are in a different order.

I'll give you the diff that looks to fix this in UmbDocumentWorkspaceContext (as would prefer you to review and apply if you agree it makes sense).

-	 * Persisted is reconciled to `savedData` (not the full current data) BEFORE flipping isNew: that flip
-	 * triggers the new->edit redirect, whose navigation guard would otherwise see a transient dirty state
-	 * (the caller's reload reconciles only afterwards) and pop a spurious "Discard unsaved changes" dialog
-	 * (reported by Andy Butland). Using `savedData` keeps published variants clean while edited-but-
-	 * unpublished variants correctly stay dirty, so their edits remain guarded during the pre-reload window.
+	 * Both persisted and current are reconciled to `savedData` BEFORE flipping isNew, so the new->edit
+	 * redirect that flip triggers can never see a dirty state and pop a spurious "Discard unsaved changes"
+	 * dialog. Edited-but-unpublished variants are temporarily absent from the current state until the
+	 * caller's reload + transferPublishedVariantsToCurrent restores them (dirty again).
 	 * @param {UmbDocumentDetailModel} savedData - The data that was sent to the server (constructSaveData)
 	 * @returns {Promise<void>}
 	 * @memberof UmbDocumentWorkspaceContext
 	 */
 	public async finalizeCreate(savedData: UmbDocumentDetailModel): Promise<void> {
 		const parent = this._internal_getCreateUnderParent();
 		if (!parent) throw new Error('Parent is not set');
 
+		// Set persisted AND current to the same data BEFORE flipping isNew: that flip triggers the
+		// new->edit redirect, whose navigation guard compares the two states with an order-sensitive
+		// JSON.stringify. `savedData` is a merge-processed projection of the draft (its values array can
+		// be ordered differently, and resolver-backed editors may rewrite values), so reconciling only
+		// persisted still leaves a spurious mismatch -> "Discard unsaved changes" dialog. 
+		// Edited-but-unpublished variants are restored (and become dirty again) by the
+		// caller's reload + transferPublishedVariantsToCurrent.
 		this._data.setPersisted(savedData);
+		this._data.setCurrent(savedData);
 		this.setIsNew(false);

The test document-workspace-create-and-publish.context.test.ts → "keeps an edited-but-unpublished variant dirty after create-and-publish (no data loss)" will need a look at after this, as it fails after this update and so is perhaps asserting the wrong thing.

• ✅ is not dirty immediately after create-and-publish, before the reload reconciles state — passes (the dialog-window
  guarantee, now holds for all doc types)
• ❌ keeps an edited-but-unpublished variant dirty after create-and-publish (no data loss) — fails at da stays dirty
  (edited but not published): expected false to be true (line 98)

[iOvergaard]

Thanks Andy, that's a good find — the values ordering one is easy to miss. Pushed your fix in 9cbd92b0de4.

Confirmed it's what you said: the dirty check is an order-sensitive JSON.stringify compare, and savedData comes back from the merge controller with its values in a different order than the draft (and resolver editors can rewrite values too), so only reconciling persisted still read as dirty on some doc types.

finalizeCreate now sets both persisted and current to savedData before the flip to edit mode, so the redirect can't see unsaved changes on any doc type. The unpublished variants drop out of current briefly, but the reload + transfer straight after restore them (dirty again) — so no data loss.

You were right about the test as well — it was asserting the intermediate state. Moved it to check the end state after reload + transfer, and renamed it. Green here.

Give it another go on the doc type that was reproducing for you when you get a chance? Want to confirm it's gone on your side too.

[AndyButland]

Yes, this is working as expected now @iOvergaard. I'll mark as approved from a testing perspective - but if you are still in discussions with @madsrasmussen and @nielslyngsoe on the code/architecture side of things, obviously please wait for merging until you have resolved everything there.

[iOvergaard]

Thanks, @AndyButland.

@madsrasmussen, I implemented your suggestions. Would you check it, please, and let me know what you think?

[iOvergaard]

@claude please review this PR again after having merged in frontend changes

[claude]

Review: Documents — Single-transaction create/update-and-publish (FE)

This is a well-executed PR. The problem is well-understood, the fix is targeted, and the test coverage is thorough.

---

Navigation-guard fix

The old !newUrl.includes(activeLocalPath) was a substring check — it would match edit/123 anywhere inside edit/1234 or a deeper path like /some-other-workspace/edit/123. The new positional-anchoring approach using the absolute mount path and an exact prefix check is strictly correct. The 8-test suite covers the key edge cases including the create→edit redirect that originally surfaced this bug.

Replacing UmbStringState with plain string | undefined for #localPath/#absolutePath in UmbWorkspaceRouteManager is a good simplification — these aren't observed reactively, so the state overhead was unnecessary.

#applyPersistedData extraction

Pulling this into a shared private method removes the duplication between create and update paths and makes the contract explicit: merge server data into persisted + current for only the selected variants. The UmbMergeContentVariantDataController usage is the right tool — unselected variants are untouched, so unpublished Danish edits survive.

loadWithoutPersist()

Clean escape hatch. Using it inside the custom persist closure keeps the sequencing right: combined endpoint fires, read-back happens inside performCreateOrUpdate, then #applyPersistedData sees the server's authoritative post-publish state. The old #clear() before reload is no longer needed since reconciliation now happens variant-selectively.

Request body mappers

Extracting umbMapDocumentCreateRequestBody / umbMapDocumentUpdateRequestBody into their own file eliminates a future divergence risk between the detail and publishing data sources. The old TODO comment is gone.

Test coverage

The three new test files reproduce the original bugs directly: the spurious Discard dialog on create-and-publish, and the edited-but-unpublished variant losing its edits after save-and-publish. Tests fail on pre-fix code and pass after — good.

---

Two non-blocking observations (inline comments above):

1. loadWithoutPersist() error message says 'Error loading document' but lives in a generic base class. Minor cosmetic — 'Error loading entity' would be accurate for media/member contexts too.

2. If loadWithoutPersist() throws after a successful combined endpoint call (e.g. a transient 500 on the read-back GET), the .catch() in performSaveAndPublish will show the failure toast — even though the content was actually published. Edge case, not a blocker, but something to be aware of for a follow-up.

---

No breaking changes, scheduling path unchanged, validation-failure fallback preserved. This is a solid implementation. ✅

[iOvergaard]

Both addressed in bba87c7:

• Widened the loadWithoutPersist() error message from "document" to "entity" (it's on the shared entity-detail base).
• The publish flow no longer reports a failure when only the read-back fails. Since create/update-and-publish return key-only, the read-back is mandatory — so if it throws after a successful publish we now fall back to the submitted data (lifecycle still completes) and show a soft warning (editContentPublishedReloadFailed) rather than the "could not be published" error.

Browser-tested the EN-only Save & Publish on a variant doc: single success toast, the unsaved Danish draft is preserved, and the discard guard still fires on navigate-away. Thanks for spotting the read-back edge case 👍