From 40b9e2101eff5fb4e1e7d7f489c249c6ecb7cf2f Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Wed, 18 Mar 2026 15:05:04 +0100 Subject: [PATCH 1/3] Preserve segment-specific property values after save and publish. --- .../workspace/content-detail-workspace-base.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/content-detail-workspace-base.ts b/src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/content-detail-workspace-base.ts index 0be1c16c2df2..9f2e7c3abff3 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/content-detail-workspace-base.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/content-detail-workspace-base.ts @@ -500,7 +500,21 @@ export abstract class UmbContentDetailWorkspaceContextBase< data.values = dataValues; */ - return { ...data, values: processedValues }; + // Merge: start with processed values, then add any original server values + // that weren't covered by the preset builder's variant options. + // This prevents segment-specific values from being silently dropped + // when variant options don't cover all culture+segment combinations. + const mergedValues = [...processedValues]; + for (const serverValue of data.values) { + const alreadyIncluded = mergedValues.some( + (v) => v.alias === serverValue.alias && v.culture === serverValue.culture && v.segment === serverValue.segment, + ); + if (!alreadyIncluded) { + mergedValues.push(serverValue); + } + } + + return { ...data, values: mergedValues }; } /** From 04fdc3e8b496fc4333531b22216b398771fdb663 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Wed, 18 Mar 2026 15:49:45 +0100 Subject: [PATCH 2/3] Addressed feedback from code review. --- .../content/workspace/content-detail-workspace-base.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/content-detail-workspace-base.ts b/src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/content-detail-workspace-base.ts index 9f2e7c3abff3..4d80ed73f6ea 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/content-detail-workspace-base.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/content-detail-workspace-base.ts @@ -505,11 +505,13 @@ export abstract class UmbContentDetailWorkspaceContextBase< // This prevents segment-specific values from being silently dropped // when variant options don't cover all culture+segment combinations. const mergedValues = [...processedValues]; + const variantKey = (v: { alias: string; culture: string | null; segment: string | null }) => + `${v.alias}|${UmbVariantId.Create(v).toString()}`; + const coveredKeys = new Set(processedValues.map(variantKey)); for (const serverValue of data.values) { - const alreadyIncluded = mergedValues.some( - (v) => v.alias === serverValue.alias && v.culture === serverValue.culture && v.segment === serverValue.segment, - ); - if (!alreadyIncluded) { + const key = variantKey(serverValue); + if (!coveredKeys.has(key)) { + coveredKeys.add(key); mergedValues.push(serverValue); } } From 7f7071c1d77ba9c59a1264d27c2bacea2f2e073d Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Thu, 23 Apr 2026 12:26:57 +0200 Subject: [PATCH 3/3] Moved fix to a projection in UmbPropertyValuePresetVariantBuilderController. --- .../content-detail-workspace-base.ts | 18 +------- ...ty-value-preset-builder.controller.test.ts | 4 +- ...-preset-variant-builder.controller.test.ts | 42 +++++++++++++++++ ...value-preset-variant-builder.controller.ts | 45 +++++++++++-------- 4 files changed, 71 insertions(+), 38 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/content-detail-workspace-base.ts b/src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/content-detail-workspace-base.ts index c6aeb7bd9055..92e852de7159 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/content-detail-workspace-base.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/content-detail-workspace-base.ts @@ -500,23 +500,7 @@ export abstract class UmbContentDetailWorkspaceContextBase< data.values = dataValues; */ - // Merge: start with processed values, then add any original server values - // that weren't covered by the preset builder's variant options. - // This prevents segment-specific values from being silently dropped - // when variant options don't cover all culture+segment combinations. - const mergedValues = [...processedValues]; - const variantKey = (v: { alias: string; culture: string | null; segment: string | null }) => - `${v.alias}|${UmbVariantId.Create(v).toString()}`; - const coveredKeys = new Set(processedValues.map(variantKey)); - for (const serverValue of data.values) { - const key = variantKey(serverValue); - if (!coveredKeys.has(key)) { - coveredKeys.add(key); - mergedValues.push(serverValue); - } - } - - return { ...data, values: mergedValues }; + return { ...data, values: processedValues }; } /** diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/property/property-value-preset/property-value-preset-builder.controller.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/property/property-value-preset/property-value-preset-builder.controller.test.ts index ed9a344d175d..180bee241130 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/property/property-value-preset/property-value-preset-builder.controller.test.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/property/property-value-preset/property-value-preset-builder.controller.test.ts @@ -14,9 +14,9 @@ import { UmbPropertyValuePresetBuilderController } from './property-value-preset @customElement('umb-test-controller-host') export class UmbTestControllerHostElement extends UmbControllerHostElementMixin(HTMLElement) {} -// TODO: Write test with config, investigate oppertunity to retrieve Config Object, for an simpler DX. [NL] +// TODO: Write test with config, investigate opportunity to retrieve Config Object, for an simpler DX. [NL] -// Test with async APIs, espcially where the first one is slower than the last one. +// Test with async APIs, especially where the first one is slower than the last one. export class TestPropertyValuePresetFirstApi implements UmbPropertyValuePreset { async processValue(value: undefined | string, config: UmbPropertyEditorConfig) { return value ? value + '_first' : 'first'; diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/property/property-value-preset/property-value-preset-variant-builder.controller.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/property/property-value-preset/property-value-preset-variant-builder.controller.test.ts index 84ccf2232bf4..ee0c11d6e5d3 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/property/property-value-preset/property-value-preset-variant-builder.controller.test.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/property/property-value-preset/property-value-preset-variant-builder.controller.test.ts @@ -523,6 +523,48 @@ describe('UmbPropertyValuePresetVariantBuilderController', () => { expect(result.some((r) => r.culture === null)).to.be.false; }); + it('produces one value per distinct segment when variant options include cultures but the property varies by segment only', async () => { + const ctrlHost = new UmbTestControllerHostElement(); + const ctrl = new UmbPropertyValuePresetVariantBuilderController(ctrlHost); + // Variant options reflect a content type that varies by culture AND segment: a cultures × segments + // list plus the invariant option the caller appends for invariant properties. + ctrl.setVariantOptions([ + new UmbVariantId('cultureA', null), + new UmbVariantId('cultureA', 'segmentA'), + new UmbVariantId('cultureA', 'segmentB'), + new UmbVariantId('cultureB', null), + new UmbVariantId('cultureB', 'segmentA'), + new UmbVariantId('cultureB', 'segmentB'), + new UmbVariantId(null, null), + ]); + + // Property varies by segment but not by culture: each culture-bearing option must be projected + // to culture-null and deduped by segment, so one value per distinct segment is produced. + // If the logic instead filtered out everything with a non-null culture, only the invariant value + // would survive and segmented values would be silently dropped. + const propertyTypes: Array = [ + { + alias: 'test', + propertyEditorUiAlias: 'test-editor-ui', + config: [], + typeArgs: { variesByCulture: false, variesBySegment: true }, + }, + ]; + + const result = await ctrl.create(propertyTypes, { + entityType: 'test', + entityUnique: 'some-unique', + }); + + expect(result.length).to.be.equal(3); + expect(result[0]?.culture).to.be.null; + expect(result[0]?.segment).to.be.null; + expect(result[1]?.culture).to.be.null; + expect(result[1]?.segment).to.be.equal('segmentA'); + expect(result[2]?.culture).to.be.null; + expect(result[2]?.segment).to.be.equal('segmentB'); + }); + it('excludes all variant options when property is invariant', async () => { const ctrlHost = new UmbTestControllerHostElement(); const ctrl = new UmbPropertyValuePresetVariantBuilderController(ctrlHost); diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/property/property-value-preset/property-value-preset-variant-builder.controller.ts b/src/Umbraco.Web.UI.Client/src/packages/core/property/property-value-preset/property-value-preset-variant-builder.controller.ts index 432b555df281..16c4cbb39bc4 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/property/property-value-preset/property-value-preset-variant-builder.controller.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/property/property-value-preset/property-value-preset-variant-builder.controller.ts @@ -104,9 +104,17 @@ export class UmbPropertyValuePresetVariantBuilderController extends UmbPropertyV } /** - * Filters variant options based on property type args (variesByCulture/variesBySegment). - * @param {UmbPropertyTypePresetModel | UmbPropertyTypePresetWithSchemaAliasModel} propertyType - Property type model - * @returns {Array} Filtered array of UmbVariantId instances + * Derives the variant ids to generate values for, from the variant options supplied via + * {@link setVariantOptions}. Those options describe the document's variants (driven by the + * content type's variance: e.g. one entry per culture, or per culture × segment), which + * may not match how the property itself varies. + * + * Each option is projected onto the property's variance — the dimensions the property + * varies on (culture and/or segment) are kept, the rest are collapsed to null — and the + * projected ids are deduped. A culture-invariant option is dropped when the property + * varies by culture, because there is no culture to hoist it to. + * @param {UmbPropertyTypePresetModel | UmbPropertyTypePresetWithSchemaAliasModel} propertyType - Property type model, whose variance drives the projection + * @returns {Array} The variant ids to generate values for */ #getFilteredVariantOptions( propertyType: UmbPropertyTypePresetModel | UmbPropertyTypePresetWithSchemaAliasModel, @@ -115,30 +123,29 @@ export class UmbPropertyValuePresetVariantBuilderController extends UmbPropertyV return []; } - const variesByCulture = propertyType.typeArgs.variesByCulture; - const variesBySegment = propertyType.typeArgs.variesBySegment; + const variesByCulture = propertyType.typeArgs.variesByCulture ?? false; + const variesBySegment = propertyType.typeArgs.variesBySegment ?? false; - // Validate that cultures are available when property varies by culture if (variesByCulture && !this.#variantOptions.some((v) => v.culture !== null)) { throw new Error('Cultures must be set when varying by culture.'); } - // Filter options based on property variation settings - return this.#variantOptions.filter((variantId) => { - // If property doesn't vary by culture, only use culture-invariant options - if (!variesByCulture && variantId.culture !== null) { - return false; - } - // If property does vary by culture, exclude culture-invariant options + const seen = new Set(); + const result: Array = []; + for (const variantId of this.#variantOptions) { if (variesByCulture && variantId.culture === null) { - return false; + continue; } - // If property doesn't vary by segment, only use segment-invariant options - if (!variesBySegment && variantId.segment !== null) { - return false; + const projected = variantId.toVariant(variesByCulture, variesBySegment); + const key = projected.toString(); + if (seen.has(key)) { + continue; } - return true; - }); + seen.add(key); + result.push(projected); + } + + return result; } /**