* Use direct imports in core manifests
* Extract theme aliases into constants file
---------
Co-authored-by: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com>
* Backoffice: Add Cache-Control headers to cache-busted backoffice assets (AB#68478)
Adds a UseUmbracoBackOfficeCacheHeaders middleware that sets
Cache-Control: public, max-age=31536000, immutable on responses served
from the cache-busted backoffice path (/umbraco/backoffice/<hash>/*).
The hash in the URL is derived from the Umbraco version, so the URL
itself invalidates on every release - making 'immutable' safe regardless
of whether individual filenames contain a content hash.
In debug mode the cache-bust hash changes per request, so the header is
set to 'no-cache' to avoid filling the browser disk cache with single-use
entries.
Design is non-destructive to consumer customisation, addressing the
review feedback on the v14 attempt (#14475):
- Does not touch StaticFileOptions; consumer
services.Configure<StaticFileOptions>(...) and OnPrepareResponse
callbacks continue to work unchanged.
- Sets the header via Response.OnStarting with a ContainsKey guard, so
any synchronous Cache-Control set upstream wins; consumer OnStarting
callbacks registered later fire first (LIFO) and also win.
- Skips non-2xx responses to avoid long-lived caching of error responses.
Related: GH #21152, PR #22896.
* Backoffice: Correct rationale for no-cache in debug mode
Reword the XML doc on UseUmbracoBackOfficeCacheHeaders to reflect that
IBackOfficePathGenerator is a singleton, so the cache-bust hash is
computed once at startup even in debug mode (per Copilot review on
#22951). The reason for no-cache is not "hash changes per request" but
that built assets may change in place during dev iteration; no-cache
allows fast 304 revalidation while no-store would force full
re-downloads.
No functional change.
* Backoffice: Add unit tests for UseUmbracoBackOfficeCacheHeaders
Covers six scenarios via a minimal in-process pipeline composed with
Microsoft.AspNetCore.TestHost:
- Production: 200 under hash prefix gets immutable header
- Debug: 200 under hash prefix gets no-cache
- Non-2xx under prefix: header not set (status gate)
- Path outside prefix: header not set (path gate)
- Consumer synchronous override: ContainsKey guard skips, consumer wins
- Consumer OnStarting override: LIFO ordering lets consumer win
Adds Microsoft.AspNetCore.TestHost to Umbraco.Tests.UnitTests (standard
Microsoft package, version pinned in tests/Directory.Packages.props).
* Backoffice: Extract cache-headers logic into IMiddleware class
Matches the existing Umbraco middleware convention (BootFailedMiddleware,
PreviewAuthenticationMiddleware, UmbracoRequestMiddleware, etc.) per
Kenn's note: prefer UseMiddleware<T>() with a DI-resolved class over
inline builder.Use lambdas.
The new UmbracoBackOfficeCacheHeadersMiddleware:
- Implements IMiddleware; registered as a singleton in AddWebComponents
- Computes prefix and header value once in the constructor (both
dependencies are singletons themselves, so this is stable)
- Behaviour is unchanged from the inline version
The UseUmbracoBackOfficeCacheHeaders extension method becomes a thin
UseMiddleware<T>() wrapper. Tests updated to register the middleware in
the TestServer DI container so it can be resolved through UseMiddleware.
* Backoffice: Document IMiddleware convention in Web.Common CLAUDE.md
Adds an explicit "Convention" note before the middleware list so future
contributors (and AI assistants) default to the IMiddleware class +
AddSingleton + UseMiddleware<T>() pattern rather than inline
builder.Use(async ...) lambdas. Also lists the new
UmbracoBackOfficeCacheHeadersMiddleware in the folder structure and
middleware reference.
* Backoffice: Tighten middleware convention note with full corroboration
Lists every IMiddleware implementer in the codebase (10/10) and calls
out the two known inline-lambda exceptions (CspNonceExtensions,
WebApplicationExtensions) so the rule reads as the established
convention rather than an absolute, while still steering new work
toward IMiddleware + AddSingleton + UseMiddleware<T>().
* Backoffice: Register cache-headers middleware in AddBackOfficeCore
DI scope validation runs in Development/CI and pre-checks every
singleton's dependency graph can be constructed. The middleware was
registered in AddWebComponents (which runs for every Umbraco bootstrap),
but its IBackOfficePathGenerator dependency is only registered by
AddBackOffice(). The previous CI run on this branch surfaced the
problem in four Delivery-only/Website-only bootstrap tests
(CoreWithDeliveryApi_BootsSuccessfully, DeliveryOnlyScenario_BootsSuccessfully,
etc.) with "Unable to resolve service for type 'IBackOfficePathGenerator'
while attempting to activate 'UmbracoBackOfficeCacheHeadersMiddleware'".
Move the registration alongside IBackOfficePathGenerator in
AddBackOfficeCore (Api.Management), which is the same scope as the
backoffice itself. This also matches the wire-up gate in
UmbracoApplicationBuilder.cs that only calls UseUmbracoBackOfficeCacheHeaders
when IBackOfficeEnabledMarker is registered.
CLAUDE.md updated with the rule ("register the middleware next to its
dependencies' registration") and a pitfall note about DI scope validation.
* Backoffice: Address review feedback from AndyButland (PR #22951)
- Move UseUmbracoBackOfficeCacheHeadersTests from Umbraco.Tests.UnitTests
to Umbraco.Tests.Integration. It uses HostBuilder + TestServer to
exercise the real HTTP pipeline, which is integration-shaped rather
than unit-shaped. Drop Microsoft.AspNetCore.TestHost from UnitTests
(Mvc.Testing in Integration provides it transitively) and from
tests/Directory.Packages.props.
- Soften the misleading "no trailing slash" comment in
UmbracoBackOfficeCacheHeadersMiddleware — we trim anyway, so the
comment is now framed as defensive normalisation.
- Trim the dense middleware convention note in Web.Common/CLAUDE.md to
one paragraph (rule + the two known inline-lambda exceptions). Move
the DI-scope-validation pitfall narrative out of CLAUDE.md and into a
three-line code comment next to the AddSingleton call in
AddBackOfficeCore where it actually applies.
* Backoffice: HTTP verb gate, 304 inclusion, namespace + unused using (PR #22951 review)
Three more from AndyButland's review:
1. Verb gate + 304 inclusion in UmbracoBackOfficeCacheHeadersMiddleware.
Restrict the path-prefix match to GET and HEAD so POST/PUT/DELETE
responses and OPTIONS (CORS preflight) responses don't get tagged as
immutable. Include 304 alongside 2xx in the status gate so
intermediate caches (CDN/proxy) receive the Cache-Control directive on
revalidation responses too. Extended the test suite with four new
cases: NotModifiedResponseUnderPrefix_SetsImmutable,
HeadRequestUnderPrefix_SetsImmutable,
OptionsRequestUnderPrefix_DoesNotSetHeader,
PostRequestUnderPrefix_DoesNotSetHeader. All 10 tests pass.
2. Test namespace updated to Umbraco.Cms.Tests.Integration.* to match
the convention used by ~629 other files in Umbraco.Tests.Integration
(vs the 2 outliers I copied from).
3. Drop unused 'using Umbraco.Extensions;' from the test file.
* Backoffice: Extract conditional checks to satisfy CodeScene complexity gate
CodeScene flagged InvokeAsync with "Complex Conditional" (advisory rule,
code health impact 9.69) after the verb + 304 additions in the prior
commit. Extract the two checks into IsCacheableAssetRequest and
ShouldSetCacheControl helper methods. No behaviour change; tests still
green (10/10, 149 ms).
* Stabilise rollback E2E test by waiting for document reload before asserting.
* Condense rollback wait comment per code-review feedback.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Addressed code review feedback.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* ensure the order from the search endpoints taking a collection of keys is preserved
* Align cosmetic changes to ensure later merge up doesn't run into conflicts.
---------
Co-authored-by: Lan Nguyen Thuy <lnt@umbraco.dk>
Co-authored-by: Andy Butland <abutland73@gmail.com>
* Use direct imports in core manifests
* Extract theme aliases into constants file
---------
Co-authored-by: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com>
* update order search result for element, member type, dictionary...
* undo dictionary search API
* reorder search value
* Apply OrderByRequestedIds
* add unit tests for search order
* Reverted unnecessarily changed files, minor test clean-up, aligned controllers for XML docs.
---------
Co-authored-by: Lan Nguyen Thuy <lnt@umbraco.dk>
Co-authored-by: Andy Butland <abutland73@gmail.com>
* Add IgnoredDelayChanged event to allow updates during back-off
* Make Period and IgnoredDelay settable on RecurringBackgroundJobBase with auto-raising events
* Address PR review: handle CTS race, restore negative-IgnoredDelay guard, clarify setter remarks
- Swallow ObjectDisposedException in OnIgnoredDelayChanged for the shutdown race where an in-flight handler reads the to-be-disposed CTS via Interlocked.Exchange before Dispose disposes it.
- Restore "skip back-off when IgnoredDelay <= TimeSpan.Zero (and not Timeout.InfiniteTimeSpan)" guard in IgnoreAndWaitAsync to defend against direct IRecurringBackgroundJob implementations / property overrides returning a negative value that would otherwise tight-loop via ComputeNextDelay clamping to zero.
- Add regression test for the negative-IgnoredDelay skip path.
- Mirror the constructor "stored without raising" remark on the Period and IgnoredDelay setter doc comments.
* Dispose newly-installed CTS when shutdown race wins the rotate-and-cancel
* Clarify XML docs.
* Introduce helper for cancellation source rotate and cancel.
---------
Co-authored-by: Andy Butland <abutland73@gmail.com>
* Add IgnoredDelayChanged event to allow updates during back-off
* Make Period and IgnoredDelay settable on RecurringBackgroundJobBase with auto-raising events
* Address PR review: handle CTS race, restore negative-IgnoredDelay guard, clarify setter remarks
- Swallow ObjectDisposedException in OnIgnoredDelayChanged for the shutdown race where an in-flight handler reads the to-be-disposed CTS via Interlocked.Exchange before Dispose disposes it.
- Restore "skip back-off when IgnoredDelay <= TimeSpan.Zero (and not Timeout.InfiniteTimeSpan)" guard in IgnoreAndWaitAsync to defend against direct IRecurringBackgroundJob implementations / property overrides returning a negative value that would otherwise tight-loop via ComputeNextDelay clamping to zero.
- Add regression test for the negative-IgnoredDelay skip path.
- Mirror the constructor "stored without raising" remark on the Period and IgnoredDelay setter doc comments.
* Dispose newly-installed CTS when shutdown race wins the rotate-and-cancel
* Clarify XML docs.
* Introduce helper for cancellation source rotate and cancel.
---------
Co-authored-by: Andy Butland <abutland73@gmail.com>
* Support anchor fragments that are included in the data attribute but missing in the href when migrating local links.
* Addressed code review feedback.
* Support anchor fragments that are included in the data attribute but missing in the href when migrating local links.
* Addressed code review feedback.
* Workspace Actions: Restore waiting state for buttons with additional options
The waiting state was suppressed whenever a workspace action reported
hasAdditionalOptions() (e.g. Save and publish on multi-variant sites),
so users saw the button jump straight from idle to the success tick
with no in-flight feedback.
Always set 'waiting' on click (unless the action is a link). The
variant-picker modal still opens on top of the button, so the spinner
is effectively invisible during selection — but it becomes visible
as soon as the modal closes and the publish request is in flight.
Fixes#22551
* Workspace Actions: Spin button only while real work is in flight
Replace the eager always-set-waiting behaviour from the previous commit
with an opt-in `isPending` signal so the spinner appears only while
actual work (validation + HTTP) is happening - not while the variant
picker modal is open, and never as a spurious success tick when the
user cancels the modal.
Changes:
- Add optional `isPending: Observable<boolean>` to UmbWorkspaceAction
and a default UmbBooleanState + protected setPending() on the base
class. Optional + backwards compatible for external implementers.
- Add optional `onActionStarting` callback (via a shared
UmbWorkspaceActionExecutionOptions type) to
UmbPublishableWorkspaceContext.saveAndPublish and
UmbSaveableWorkspaceContext.requestSave. The document publishing
context and content detail workspace base invoke the callback at the
join point right after the variant picker resolves (or is skipped
for the single-variant case), so it never fires when the modal is
cancelled.
- Wire the document save and save-and-publish actions to clear pending
at the start of execute() and pass an onActionStarting callback that
flips it true when work begins.
- Update the workspace action element to observe api.isPending: when
the observable is present the waiting state is driven by the
observable (and the success tick is suppressed if the action
resolves without ever signalling pending - i.e. a cancellation).
When the observable is absent the element falls back to the legacy
eager-waiting behaviour. Failures always surface the failed tick.
Fixes#22551
* Reduce cyclomatic complexity of #onClick and _handleSave
CodeScene Code Health Review flagged two complexity issues:
- UmbWorkspaceActionElement.#onClick reached cyclomatic complexity 9
(threshold is < 9). Extracted the api-execution branch into a new
private #runApiAction helper so #onClick collapses to a simple
link-vs-action dispatch.
- _handleSave was already over the threshold (14); my optional-chain
callback invocation pushed it to 16. Moved the
`executionOptions?.onActionStarting?.()` call into a #notifyActionStarting
helper so the call site is a plain method call and contributes zero
cyclomatic complexity to _handleSave.
No behavioural change.
* Reduce cyclomatic complexity of #handleSaveAndPublish
Same fix as the previous commit's #notifyActionStarting extraction in
content-detail-workspace-base: move the optional-chain callback
invocation into a private helper so #handleSaveAndPublish stays at its
pre-PR cyclomatic complexity (15) instead of degrading to 17.
No behavioural change.
* DRY: extract notifyWorkspaceActionStarting into a shared utility
Both UmbDocumentPublishingWorkspaceContext.#handleSaveAndPublish and
UmbContentDetailWorkspaceContextBase._handleSave had identical private
optional-chain callback off the host method's cyclomatic complexity.
Replace both with a single exported notifyWorkspaceActionStarting()
utility co-located with UmbWorkspaceActionExecutionOptions. This:
- Removes a duplication point between the two contexts.
- Gives future workspace context implementations a ready-made way to
honour the optional callback without re-inventing the helper or
paying the cyclomatic-complexity cost at the call site.
No behavioural change.
* Rename isPending -> isExecuting to mirror the execute() method
Niels suggested correlating the observable's name with the action's
`execute()` method, so the symbol set is now:
- isExecuting (observable on UmbWorkspaceAction interface)
- _isExecuting / setExecuting (UmbWorkspaceActionBase)
- #observeIsExecuting / #executionStarted (workspace-action element)
- isExecutingObserver (observer alias)
Pure rename; no behavioural change.
* Address Copilot review: lazy isExecuting, observer scope, finally reset
Five Copilot findings on PR #22554. Three real regressions + two
contract violations, all addressed:
1. UmbWorkspaceActionBase always exposing `isExecuting` made every
existing subclass appear to opt in to the new modal-aware flow,
suppressing waiting/success states for actions that never call
setExecuting(true). Made `_isExecuting`/`isExecuting` lazy: only
created on the first setExecuting() call. Opt-in subclasses call
`setExecuting(false)` in their constructor so the observable is
exposed before the workspace-action element reads it. Subclasses
that don't opt in keep `isExecuting` undefined and the element
falls back to legacy eager waiting feedback.
2. Element observation of `isExecuting` now lives inside #runApiAction
so it tracks whichever api is actually invoked (`_actionApi ?? #api`),
correctly handling subclasses like UmbSaveAndPreviewWorkspaceActionElement
that swap in a different api at runtime. The shared observer alias
replaces any previous observation on re-clicks.
3. UmbSaveWorkspaceAction and UmbDocumentSaveAndPublishWorkspaceAction
now wrap their execute() body in try/finally and reset
setExecuting(false) on completion so the observable honours the
"true while execute() is performing real work, false otherwise"
contract instead of getting stuck at true between executions.
No behavioural change for actions that already worked correctly before
this PR; the regression-prone "always exposed" behaviour is gone.
* Address Claude review: Elements gap, type placement, tests + cleanup
Three follow-ups on top of c15eb2d0bc:
1. Elements gap — UmbElementSaveAndPublishWorkspaceAction +
UmbElementPublishingWorkspaceContext now wire through the same
onActionStarting/notifyWorkspaceActionStarting handshake as the
Document equivalents, so multi-variant Elements (Forms, Commerce, etc.)
get the spinner-after-modal behaviour rather than no spinner at all.
2. Type placement — moved UmbWorkspaceActionExecutionOptions out of
publishable-workspace-context.interface.ts into its own file so the
saveable interface no longer has a directional dependency on the
publishable one. Both peer contexts now import from the same neutral
location.
3. Unit tests — added blackbox coverage for notifyWorkspaceActionStarting
(no-op on undefined options/callback, invokes when present) and the
UmbWorkspaceActionBase.setExecuting lazy-opt-in contract (undefined
until first call, observable then exposed, value flips, sequential
emissions, stable reference across calls).
Code-review cleanup applied on the same pass:
- Dropped the redundant `setExecuting(false)` at the start of execute()
in the save and save-and-publish actions; the finally block plus
UmbBooleanState's value-dedup already cover idempotency on retries.
- Removed an overlong block comment on `_isExecuting`; the JSDoc on
setExecuting already documents the lazy/opt-in contract for subclasses.
- Trimmed an internal motivation comment from notify-workspace-action-
starting.function.ts that referenced cyclomatic complexity.
- Extracted a tiny makeAction() helper in the controller test to remove
the `{ meta: {} as never }` repetition.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Workspace Actions: Align Save button state with Save & Publish (Andy review feedback)
Two related fixes addressing the variant-Save inconsistency Andy reported:
- Element catch block now only sets `failed` once `#executionStarted` is
true. Pre-flight rejections (user cancelling a variant-picker modal,
context-missing throws, etc.) leave the button idle, matching the
silent-cancel path used by `#handleSaveAndPublish`. Legacy actions
that don't opt in to `isExecuting` are unaffected because they set
`#executionStarted = true` eagerly on click.
- `UmbDocumentWorkspaceContext._handleSave` and
`UmbElementWorkspaceContext._handleSave` now accept and forward the
`UmbWorkspaceActionExecutionOptions` argument to `super._handleSave`.
The previous overrides dropped the parameter, so the
`onActionStarting` callback supplied by `UmbSaveWorkspaceAction` never
fired - which is why Save showed no waiting/success indicator even
on a successful submit.
Result: Save and Save-and-publish now behave identically -
cancel = no indicator, submit = waiting then success - for both
invariant and multi-variant documents and elements.
* Docs: Document the modal-aware execution feedback contract for workspace actions
New 'Button state when the action opens a modal' subsection in
docs/workspaces.md explaining the three-piece contract:
UmbWorkspaceActionExecutionOptions + notifyWorkspaceActionStarting +
UmbWorkspaceActionBase.setExecuting. Covers third-party authoring of
modal-aware buttons, the cancel/pre-flight idle behaviour, and the
silent-parameter-drop pitfall on _handleSave overrides.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Workspace Actions: Restore waiting state for buttons with additional options
The waiting state was suppressed whenever a workspace action reported
hasAdditionalOptions() (e.g. Save and publish on multi-variant sites),
so users saw the button jump straight from idle to the success tick
with no in-flight feedback.
Always set 'waiting' on click (unless the action is a link). The
variant-picker modal still opens on top of the button, so the spinner
is effectively invisible during selection — but it becomes visible
as soon as the modal closes and the publish request is in flight.
Fixes#22551
* Workspace Actions: Spin button only while real work is in flight
Replace the eager always-set-waiting behaviour from the previous commit
with an opt-in `isPending` signal so the spinner appears only while
actual work (validation + HTTP) is happening - not while the variant
picker modal is open, and never as a spurious success tick when the
user cancels the modal.
Changes:
- Add optional `isPending: Observable<boolean>` to UmbWorkspaceAction
and a default UmbBooleanState + protected setPending() on the base
class. Optional + backwards compatible for external implementers.
- Add optional `onActionStarting` callback (via a shared
UmbWorkspaceActionExecutionOptions type) to
UmbPublishableWorkspaceContext.saveAndPublish and
UmbSaveableWorkspaceContext.requestSave. The document publishing
context and content detail workspace base invoke the callback at the
join point right after the variant picker resolves (or is skipped
for the single-variant case), so it never fires when the modal is
cancelled.
- Wire the document save and save-and-publish actions to clear pending
at the start of execute() and pass an onActionStarting callback that
flips it true when work begins.
- Update the workspace action element to observe api.isPending: when
the observable is present the waiting state is driven by the
observable (and the success tick is suppressed if the action
resolves without ever signalling pending - i.e. a cancellation).
When the observable is absent the element falls back to the legacy
eager-waiting behaviour. Failures always surface the failed tick.
Fixes#22551
* Reduce cyclomatic complexity of #onClick and _handleSave
CodeScene Code Health Review flagged two complexity issues:
- UmbWorkspaceActionElement.#onClick reached cyclomatic complexity 9
(threshold is < 9). Extracted the api-execution branch into a new
private #runApiAction helper so #onClick collapses to a simple
link-vs-action dispatch.
- _handleSave was already over the threshold (14); my optional-chain
callback invocation pushed it to 16. Moved the
`executionOptions?.onActionStarting?.()` call into a #notifyActionStarting
helper so the call site is a plain method call and contributes zero
cyclomatic complexity to _handleSave.
No behavioural change.
* Reduce cyclomatic complexity of #handleSaveAndPublish
Same fix as the previous commit's #notifyActionStarting extraction in
content-detail-workspace-base: move the optional-chain callback
invocation into a private helper so #handleSaveAndPublish stays at its
pre-PR cyclomatic complexity (15) instead of degrading to 17.
No behavioural change.
* DRY: extract notifyWorkspaceActionStarting into a shared utility
Both UmbDocumentPublishingWorkspaceContext.#handleSaveAndPublish and
UmbContentDetailWorkspaceContextBase._handleSave had identical private
#notifyActionStarting helpers introduced in this PR purely to keep the
optional-chain callback off the host method's cyclomatic complexity.
Replace both with a single exported notifyWorkspaceActionStarting()
utility co-located with UmbWorkspaceActionExecutionOptions. This:
- Removes a duplication point between the two contexts.
- Gives future workspace context implementations a ready-made way to
honour the optional callback without re-inventing the helper or
paying the cyclomatic-complexity cost at the call site.
No behavioural change.
* Rename isPending -> isExecuting to mirror the execute() method
Niels suggested correlating the observable's name with the action's
`execute()` method, so the symbol set is now:
- isExecuting (observable on UmbWorkspaceAction interface)
- _isExecuting / setExecuting (UmbWorkspaceActionBase)
- #observeIsExecuting / #executionStarted (workspace-action element)
- isExecutingObserver (observer alias)
Pure rename; no behavioural change.
* Address Copilot review: lazy isExecuting, observer scope, finally reset
Five Copilot findings on PR #22554. Three real regressions + two
contract violations, all addressed:
1. UmbWorkspaceActionBase always exposing `isExecuting` made every
existing subclass appear to opt in to the new modal-aware flow,
suppressing waiting/success states for actions that never call
setExecuting(true). Made `_isExecuting`/`isExecuting` lazy: only
created on the first setExecuting() call. Opt-in subclasses call
`setExecuting(false)` in their constructor so the observable is
exposed before the workspace-action element reads it. Subclasses
that don't opt in keep `isExecuting` undefined and the element
falls back to legacy eager waiting feedback.
2. Element observation of `isExecuting` now lives inside #runApiAction
so it tracks whichever api is actually invoked (`_actionApi ?? #api`),
correctly handling subclasses like UmbSaveAndPreviewWorkspaceActionElement
that swap in a different api at runtime. The shared observer alias
replaces any previous observation on re-clicks.
3. UmbSaveWorkspaceAction and UmbDocumentSaveAndPublishWorkspaceAction
now wrap their execute() body in try/finally and reset
setExecuting(false) on completion so the observable honours the
"true while execute() is performing real work, false otherwise"
contract instead of getting stuck at true between executions.
No behavioural change for actions that already worked correctly before
this PR; the regression-prone "always exposed" behaviour is gone.
* Address Claude review: Elements gap, type placement, tests + cleanup
Three follow-ups on top of c15eb2d0bc:
1. Elements gap — UmbElementSaveAndPublishWorkspaceAction +
UmbElementPublishingWorkspaceContext now wire through the same
onActionStarting/notifyWorkspaceActionStarting handshake as the
Document equivalents, so multi-variant Elements (Forms, Commerce, etc.)
get the spinner-after-modal behaviour rather than no spinner at all.
2. Type placement — moved UmbWorkspaceActionExecutionOptions out of
publishable-workspace-context.interface.ts into its own file so the
saveable interface no longer has a directional dependency on the
publishable one. Both peer contexts now import from the same neutral
location.
3. Unit tests — added blackbox coverage for notifyWorkspaceActionStarting
(no-op on undefined options/callback, invokes when present) and the
UmbWorkspaceActionBase.setExecuting lazy-opt-in contract (undefined
until first call, observable then exposed, value flips, sequential
emissions, stable reference across calls).
Code-review cleanup applied on the same pass:
- Dropped the redundant `setExecuting(false)` at the start of execute()
in the save and save-and-publish actions; the finally block plus
UmbBooleanState's value-dedup already cover idempotency on retries.
- Removed an overlong block comment on `_isExecuting`; the JSDoc on
setExecuting already documents the lazy/opt-in contract for subclasses.
- Trimmed an internal motivation comment from notify-workspace-action-
starting.function.ts that referenced cyclomatic complexity.
- Extracted a tiny makeAction() helper in the controller test to remove
the `{ meta: {} as never }` repetition.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Workspace Actions: Align Save button state with Save & Publish (Andy review feedback)
Two related fixes addressing the variant-Save inconsistency Andy reported:
- Element catch block now only sets `failed` once `#executionStarted` is
true. Pre-flight rejections (user cancelling a variant-picker modal,
context-missing throws, etc.) leave the button idle, matching the
silent-cancel path used by `#handleSaveAndPublish`. Legacy actions
that don't opt in to `isExecuting` are unaffected because they set
`#executionStarted = true` eagerly on click.
- `UmbDocumentWorkspaceContext._handleSave` and
`UmbElementWorkspaceContext._handleSave` now accept and forward the
`UmbWorkspaceActionExecutionOptions` argument to `super._handleSave`.
The previous overrides dropped the parameter, so the
`onActionStarting` callback supplied by `UmbSaveWorkspaceAction` never
fired - which is why Save showed no waiting/success indicator even
on a successful submit.
Result: Save and Save-and-publish now behave identically -
cancel = no indicator, submit = waiting then success - for both
invariant and multi-variant documents and elements.
* Docs: Document the modal-aware execution feedback contract for workspace actions
New 'Button state when the action opens a modal' subsection in
docs/workspaces.md explaining the three-piece contract:
UmbWorkspaceActionExecutionOptions + notifyWorkspaceActionStarting +
UmbWorkspaceActionBase.setExecuting. Covers third-party authoring of
modal-aware buttons, the cancel/pre-flight idle behaviour, and the
silent-parameter-drop pitfall on _handleSave overrides.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Workspace Actions: Restore waiting state for buttons with additional options
The waiting state was suppressed whenever a workspace action reported
hasAdditionalOptions() (e.g. Save and publish on multi-variant sites),
so users saw the button jump straight from idle to the success tick
with no in-flight feedback.
Always set 'waiting' on click (unless the action is a link). The
variant-picker modal still opens on top of the button, so the spinner
is effectively invisible during selection — but it becomes visible
as soon as the modal closes and the publish request is in flight.
Fixes#22551
* Workspace Actions: Spin button only while real work is in flight
Replace the eager always-set-waiting behaviour from the previous commit
with an opt-in `isPending` signal so the spinner appears only while
actual work (validation + HTTP) is happening - not while the variant
picker modal is open, and never as a spurious success tick when the
user cancels the modal.
Changes:
- Add optional `isPending: Observable<boolean>` to UmbWorkspaceAction
and a default UmbBooleanState + protected setPending() on the base
class. Optional + backwards compatible for external implementers.
- Add optional `onActionStarting` callback (via a shared
UmbWorkspaceActionExecutionOptions type) to
UmbPublishableWorkspaceContext.saveAndPublish and
UmbSaveableWorkspaceContext.requestSave. The document publishing
context and content detail workspace base invoke the callback at the
join point right after the variant picker resolves (or is skipped
for the single-variant case), so it never fires when the modal is
cancelled.
- Wire the document save and save-and-publish actions to clear pending
at the start of execute() and pass an onActionStarting callback that
flips it true when work begins.
- Update the workspace action element to observe api.isPending: when
the observable is present the waiting state is driven by the
observable (and the success tick is suppressed if the action
resolves without ever signalling pending - i.e. a cancellation).
When the observable is absent the element falls back to the legacy
eager-waiting behaviour. Failures always surface the failed tick.
Fixes#22551
* Reduce cyclomatic complexity of #onClick and _handleSave
CodeScene Code Health Review flagged two complexity issues:
- UmbWorkspaceActionElement.#onClick reached cyclomatic complexity 9
(threshold is < 9). Extracted the api-execution branch into a new
private #runApiAction helper so #onClick collapses to a simple
link-vs-action dispatch.
- _handleSave was already over the threshold (14); my optional-chain
callback invocation pushed it to 16. Moved the
`executionOptions?.onActionStarting?.()` call into a #notifyActionStarting
helper so the call site is a plain method call and contributes zero
cyclomatic complexity to _handleSave.
No behavioural change.
* Reduce cyclomatic complexity of #handleSaveAndPublish
Same fix as the previous commit's #notifyActionStarting extraction in
content-detail-workspace-base: move the optional-chain callback
invocation into a private helper so #handleSaveAndPublish stays at its
pre-PR cyclomatic complexity (15) instead of degrading to 17.
No behavioural change.
* DRY: extract notifyWorkspaceActionStarting into a shared utility
Both UmbDocumentPublishingWorkspaceContext.#handleSaveAndPublish and
UmbContentDetailWorkspaceContextBase._handleSave had identical private
#notifyActionStarting helpers introduced in this PR purely to keep the
optional-chain callback off the host method's cyclomatic complexity.
Replace both with a single exported notifyWorkspaceActionStarting()
utility co-located with UmbWorkspaceActionExecutionOptions. This:
- Removes a duplication point between the two contexts.
- Gives future workspace context implementations a ready-made way to
honour the optional callback without re-inventing the helper or
paying the cyclomatic-complexity cost at the call site.
No behavioural change.
* Rename isPending -> isExecuting to mirror the execute() method
Niels suggested correlating the observable's name with the action's
`execute()` method, so the symbol set is now:
- isExecuting (observable on UmbWorkspaceAction interface)
- _isExecuting / setExecuting (UmbWorkspaceActionBase)
- #observeIsExecuting / #executionStarted (workspace-action element)
- isExecutingObserver (observer alias)
Pure rename; no behavioural change.
* Address Copilot review: lazy isExecuting, observer scope, finally reset
Five Copilot findings on PR #22554. Three real regressions + two
contract violations, all addressed:
1. UmbWorkspaceActionBase always exposing `isExecuting` made every
existing subclass appear to opt in to the new modal-aware flow,
suppressing waiting/success states for actions that never call
setExecuting(true). Made `_isExecuting`/`isExecuting` lazy: only
created on the first setExecuting() call. Opt-in subclasses call
`setExecuting(false)` in their constructor so the observable is
exposed before the workspace-action element reads it. Subclasses
that don't opt in keep `isExecuting` undefined and the element
falls back to legacy eager waiting feedback.
2. Element observation of `isExecuting` now lives inside #runApiAction
so it tracks whichever api is actually invoked (`_actionApi ?? #api`),
correctly handling subclasses like UmbSaveAndPreviewWorkspaceActionElement
that swap in a different api at runtime. The shared observer alias
replaces any previous observation on re-clicks.
3. UmbSaveWorkspaceAction and UmbDocumentSaveAndPublishWorkspaceAction
now wrap their execute() body in try/finally and reset
setExecuting(false) on completion so the observable honours the
"true while execute() is performing real work, false otherwise"
contract instead of getting stuck at true between executions.
No behavioural change for actions that already worked correctly before
this PR; the regression-prone "always exposed" behaviour is gone.
* Address Claude review: Elements gap, type placement, tests + cleanup
Three follow-ups on top of c15eb2d0bc:
1. Elements gap — UmbElementSaveAndPublishWorkspaceAction +
UmbElementPublishingWorkspaceContext now wire through the same
onActionStarting/notifyWorkspaceActionStarting handshake as the
Document equivalents, so multi-variant Elements (Forms, Commerce, etc.)
get the spinner-after-modal behaviour rather than no spinner at all.
2. Type placement — moved UmbWorkspaceActionExecutionOptions out of
publishable-workspace-context.interface.ts into its own file so the
saveable interface no longer has a directional dependency on the
publishable one. Both peer contexts now import from the same neutral
location.
3. Unit tests — added blackbox coverage for notifyWorkspaceActionStarting
(no-op on undefined options/callback, invokes when present) and the
UmbWorkspaceActionBase.setExecuting lazy-opt-in contract (undefined
until first call, observable then exposed, value flips, sequential
emissions, stable reference across calls).
Code-review cleanup applied on the same pass:
- Dropped the redundant `setExecuting(false)` at the start of execute()
in the save and save-and-publish actions; the finally block plus
UmbBooleanState's value-dedup already cover idempotency on retries.
- Removed an overlong block comment on `_isExecuting`; the JSDoc on
setExecuting already documents the lazy/opt-in contract for subclasses.
- Trimmed an internal motivation comment from notify-workspace-action-
starting.function.ts that referenced cyclomatic complexity.
- Extracted a tiny makeAction() helper in the controller test to remove
the `{ meta: {} as never }` repetition.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Workspace Actions: Align Save button state with Save & Publish (Andy review feedback)
Two related fixes addressing the variant-Save inconsistency Andy reported:
- Element catch block now only sets `failed` once `#executionStarted` is
true. Pre-flight rejections (user cancelling a variant-picker modal,
context-missing throws, etc.) leave the button idle, matching the
silent-cancel path used by `#handleSaveAndPublish`. Legacy actions
that don't opt in to `isExecuting` are unaffected because they set
`#executionStarted = true` eagerly on click.
- `UmbDocumentWorkspaceContext._handleSave` and
`UmbElementWorkspaceContext._handleSave` now accept and forward the
`UmbWorkspaceActionExecutionOptions` argument to `super._handleSave`.
The previous overrides dropped the parameter, so the
`onActionStarting` callback supplied by `UmbSaveWorkspaceAction` never
fired - which is why Save showed no waiting/success indicator even
on a successful submit.
Result: Save and Save-and-publish now behave identically -
cancel = no indicator, submit = waiting then success - for both
invariant and multi-variant documents and elements.
* Docs: Document the modal-aware execution feedback contract for workspace actions
New 'Button state when the action opens a modal' subsection in
docs/workspaces.md explaining the three-piece contract:
UmbWorkspaceActionExecutionOptions + notifyWorkspaceActionStarting +
UmbWorkspaceActionBase.setExecuting. Covers third-party authoring of
modal-aware buttons, the cancel/pre-flight idle behaviour, and the
silent-parameter-drop pitfall on _handleSave overrides.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Backoffice: Coalesce small Rollup chunks across all workspaces (AB#67983)
Set experimentalMinChunkSize=10_000 as the default in the shared Vite
helper. Every workspace inherits the coalescing automatically; the
threshold can still be overridden per workspace (pass 0 to disable).
Impact on dist-cms output:
- packages/core .js files: 981 -> 272 (-72%)
- All workspaces combined .js files: 2194 -> 1401 (-36%)
- Welcome dashboard .js requests: 510 -> 497 (-2.5%)
- packages/ufm requests in particular: 23 -> 12 (-48%)
- Gzipped bundle total: -1.2%
- Raw bytes: +1.4% (small overhead from merged chunks; gzip wins it back)
All entry chunks are preserved, so every public
@umbraco-cms/backoffice/<sub> import keeps resolving without changes
to package.json exports or tsconfig paths.
Further consolidation (collapsing core's per-subpath entries into a
single bundle with stubs) was prototyped but hits a TDZ cycle between
the eager entry and its dynamic-import descendants. Tracked for v18,
not part of this change.
* Backoffice: Normalise umbraco-package + manifests shapes (AB#67983)
Aligns the two outliers with the conventions used by the other 38
first-party packages:
- documents/umbraco-package.ts now uses the lazy bundle pattern
(type: 'bundle', js: () => import('./manifests.js')) instead of
eagerly importing manifests at module evaluation. The bundle
initializer auto-loads the manifests at boot, so behaviour is
unchanged.
- umbraco-news/manifests.ts now exports `manifests: Array<...>`
instead of a bare `dashboard` object. The bundle initializer
enumerates exports regardless of name, so behaviour is unchanged.
Preparatory cleanup so future build-time manifest aggregation can
treat every workspace uniformly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Backoffice: Coalesce small Rollup chunks across all workspaces (AB#67983)
Set experimentalMinChunkSize=10_000 as the default in the shared Vite
helper. Every workspace inherits the coalescing automatically; the
threshold can still be overridden per workspace (pass 0 to disable).
Impact on dist-cms output:
- packages/core .js files: 981 -> 272 (-72%)
- All workspaces combined .js files: 2194 -> 1401 (-36%)
- Welcome dashboard .js requests: 510 -> 497 (-2.5%)
- packages/ufm requests in particular: 23 -> 12 (-48%)
- Gzipped bundle total: -1.2%
- Raw bytes: +1.4% (small overhead from merged chunks; gzip wins it back)
All entry chunks are preserved, so every public
@umbraco-cms/backoffice/<sub> import keeps resolving without changes
to package.json exports or tsconfig paths.
Further consolidation (collapsing core's per-subpath entries into a
single bundle with stubs) was prototyped but hits a TDZ cycle between
the eager entry and its dynamic-import descendants. Tracked for v18,
not part of this change.
* Backoffice: Normalise umbraco-package + manifests shapes (AB#67983)
Aligns the two outliers with the conventions used by the other 38
first-party packages:
- documents/umbraco-package.ts now uses the lazy bundle pattern
(type: 'bundle', js: () => import('./manifests.js')) instead of
eagerly importing manifests at module evaluation. The bundle
initializer auto-loads the manifests at boot, so behaviour is
unchanged.
- umbraco-news/manifests.ts now exports `manifests: Array<...>`
instead of a bare `dashboard` object. The bundle initializer
enumerates exports regardless of name, so behaviour is unchanged.
Preparatory cleanup so future build-time manifest aggregation can
treat every workspace uniformly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Backoffice: Coalesce small Rollup chunks across all workspaces (AB#67983)
Set experimentalMinChunkSize=10_000 as the default in the shared Vite
helper. Every workspace inherits the coalescing automatically; the
threshold can still be overridden per workspace (pass 0 to disable).
Impact on dist-cms output:
- packages/core .js files: 981 -> 272 (-72%)
- All workspaces combined .js files: 2194 -> 1401 (-36%)
- Welcome dashboard .js requests: 510 -> 497 (-2.5%)
- packages/ufm requests in particular: 23 -> 12 (-48%)
- Gzipped bundle total: -1.2%
- Raw bytes: +1.4% (small overhead from merged chunks; gzip wins it back)
All entry chunks are preserved, so every public
@umbraco-cms/backoffice/<sub> import keeps resolving without changes
to package.json exports or tsconfig paths.
Further consolidation (collapsing core's per-subpath entries into a
single bundle with stubs) was prototyped but hits a TDZ cycle between
the eager entry and its dynamic-import descendants. Tracked for v18,
not part of this change.
* Backoffice: Normalise umbraco-package + manifests shapes (AB#67983)
Aligns the two outliers with the conventions used by the other 38
first-party packages:
- documents/umbraco-package.ts now uses the lazy bundle pattern
(type: 'bundle', js: () => import('./manifests.js')) instead of
eagerly importing manifests at module evaluation. The bundle
initializer auto-loads the manifests at boot, so behaviour is
unchanged.
- umbraco-news/manifests.ts now exports `manifests: Array<...>`
instead of a bare `dashboard` object. The bundle initializer
enumerates exports regardless of name, so behaviour is unchanged.
Preparatory cleanup so future build-time manifest aggregation can
treat every workspace uniformly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Jacob Overgaard