From 45db8fd7c41ff1c24070a0d1b458c0359cc7df15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Thu, 29 Aug 2019 11:07:00 +0200 Subject: [PATCH 1/8] Add ISimpleContentType.VariesBySegment() --- src/Umbraco.Core/ContentVariationExtensions.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Umbraco.Core/ContentVariationExtensions.cs b/src/Umbraco.Core/ContentVariationExtensions.cs index 5b157307ab..fe5a82047a 100644 --- a/src/Umbraco.Core/ContentVariationExtensions.cs +++ b/src/Umbraco.Core/ContentVariationExtensions.cs @@ -19,6 +19,11 @@ namespace Umbraco.Core /// public static bool VariesByCulture(this ISimpleContentType contentType) => contentType.Variations.VariesByCulture(); + /// + /// Determines whether the content type varies by segment. + /// + public static bool VariesBySegment(this ISimpleContentType contentType) => contentType.Variations.VariesBySegment(); + /// /// Determines whether the content type is invariant. /// From 74727a179cddcc64277d6db8faf99bc68c52f795 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Thu, 29 Aug 2019 12:05:27 +0200 Subject: [PATCH 2/8] Adds Segment to ContentItemDisplay and update Variants to include segmented variants. --- .../ContentEditing/ContentPropertyBasic.cs | 12 +++- .../Mapping/ContentPropertyBasicMapper.cs | 8 ++- .../Models/Mapping/ContentVariantMapper.cs | 65 ++++++++++++++++--- .../Models/Mapping/MapperContextExtensions.cs | 19 +++++- 4 files changed, 92 insertions(+), 12 deletions(-) diff --git a/src/Umbraco.Web/Models/ContentEditing/ContentPropertyBasic.cs b/src/Umbraco.Web/Models/ContentEditing/ContentPropertyBasic.cs index c5c22484ad..2b70a63035 100644 --- a/src/Umbraco.Web/Models/ContentEditing/ContentPropertyBasic.cs +++ b/src/Umbraco.Web/Models/ContentEditing/ContentPropertyBasic.cs @@ -53,11 +53,21 @@ namespace Umbraco.Web.Models.ContentEditing [ReadOnly(true)] public string Culture { get; set; } + /// + /// The segment of the property + /// + /// + /// The segment value of a property can always be null but can only have a non-null value + /// when the property can be varied by segment. + /// + [DataMember(Name = "segment")] + [ReadOnly(true)] + public string Segment { get; set; } + /// /// Used internally during model mapping /// [IgnoreDataMember] internal IDataEditor PropertyEditor { get; set; } - } } diff --git a/src/Umbraco.Web/Models/Mapping/ContentPropertyBasicMapper.cs b/src/Umbraco.Web/Models/Mapping/ContentPropertyBasicMapper.cs index 36c1b360b2..aacafd2f2a 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentPropertyBasicMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentPropertyBasicMapper.cs @@ -70,8 +70,14 @@ namespace Umbraco.Web.Models.Mapping dest.Culture = culture; + var segment = context.GetSegment(); + + // The current segment can always be null, even if the propertyType *can* be varied by segment + // so the nullcheck like with culture is not performed here. + dest.Segment = segment; + // if no 'IncludeProperties' were specified or this property is set to be included - we will map the value and return. - dest.Value = editor.GetValueEditor().ToEditor(property, DataTypeService, culture); + dest.Value = editor.GetValueEditor().ToEditor(property, DataTypeService, culture, segment); } } } diff --git a/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs b/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs index c279ae2c70..848eaa0594 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs @@ -21,27 +21,35 @@ namespace Umbraco.Web.Models.Mapping public IEnumerable Map(IContent source, MapperContext context) { - var result = new List(); - if (!source.ContentType.VariesByCulture()) + var variants = new List(); + + var variesByCulture = source.ContentType.VariesByCulture(); + var variesBySegment = source.ContentType.VariesBySegment(); + + if (!variesByCulture && !variesBySegment) { //this is invariant so just map the IContent instance to ContentVariationDisplay - result.Add(context.Map(source)); + variants.Add(context.Map(source)); + return variants; } - else + + if (variesByCulture && !variesBySegment) { + // Culture only + var allLanguages = _localizationService.GetAllLanguages().OrderBy(x => x.Id).ToList(); if (allLanguages.Count == 0) return Enumerable.Empty(); //this should never happen var langs = context.MapEnumerable(allLanguages).ToList(); //create a variant for each language, then we'll populate the values - var variants = langs.Select(x => + variants.AddRange(langs.Select(x => { //We need to set the culture in the mapping context since this is needed to ensure that the correct property values //are resolved during the mapping context.SetCulture(x.IsoCode); return context.Map(source); - }).ToList(); + })); for (int i = 0; i < langs.Count; i++) { @@ -63,10 +71,49 @@ namespace Umbraco.Web.Models.Mapping //Insert the default language as the first item variants.Insert(0, defaultLang); - - return variants; } - return result; + else if (variesBySegment && !variesByCulture) + { + // Segment only + throw new NotSupportedException("ContentVariantMapper not implemented for segment only!"); + } + else + { + // Culture and segment + + var allLanguages = _localizationService.GetAllLanguages().OrderBy(x => x.Id).ToList(); + if (allLanguages.Count == 0) return Enumerable.Empty(); //this should never happen + + var langs = context.MapEnumerable(allLanguages).ToList(); + + // All segments, including the unsegmented (= NULL) segment. + // TODO: The NULl segment might have to be changed to be empty string? + var segments = source.Properties + .SelectMany(p => p.Values.Select(v => v.Segment)) + .Distinct(); + + // Add all variants + foreach (var language in langs) + { + foreach (var segment in segments) + { + context.SetCulture(language.IsoCode); + context.SetSegment(segment); + + var variantDisplay = context.Map(source); + + variantDisplay.Language = language; + variantDisplay.Segment = segment; + variantDisplay.Name = source.GetCultureName(language.IsoCode); + + variants.Add(variantDisplay); + } + } + + // TODO: Sorting + } + + return variants; } } } diff --git a/src/Umbraco.Web/Models/Mapping/MapperContextExtensions.cs b/src/Umbraco.Web/Models/Mapping/MapperContextExtensions.cs index 1538f1a987..20a387c679 100644 --- a/src/Umbraco.Web/Models/Mapping/MapperContextExtensions.cs +++ b/src/Umbraco.Web/Models/Mapping/MapperContextExtensions.cs @@ -8,6 +8,7 @@ namespace Umbraco.Web.Models.Mapping internal static class MapperContextExtensions { private const string CultureKey = "Map.Culture"; + private const string SegmentKey = "Map.Segment"; private const string IncludedPropertiesKey = "Map.IncludedProperties"; /// @@ -18,6 +19,14 @@ namespace Umbraco.Web.Models.Mapping return context.HasItems && context.Items.TryGetValue(CultureKey, out var obj) && obj is string s ? s : null; } + /// + /// Gets the context segment. + /// + public static string GetSegment(this MapperContext context) + { + return context.HasItems && context.Items.TryGetValue(SegmentKey, out var obj) && obj is string s ? s : null; + } + /// /// Sets a context culture. /// @@ -26,6 +35,14 @@ namespace Umbraco.Web.Models.Mapping context.Items[CultureKey] = culture; } + /// + /// Sets a context segment. + /// + public static void SetSegment(this MapperContext context, string segment) + { + context.Items[SegmentKey] = segment; + } + /// /// Get included properties. /// @@ -42,4 +59,4 @@ namespace Umbraco.Web.Models.Mapping context.Items[IncludedPropertiesKey] = properties; } } -} \ No newline at end of file +} From 8f0ee6d71154bad2f389bc5aa6a04f19ad183045 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Thu, 29 Aug 2019 14:38:46 +0200 Subject: [PATCH 3/8] Added support for all 4 cases in ContentVariantMapper --- .../Models/Mapping/ContentVariantMapper.cs | 155 ++++++++++-------- 1 file changed, 83 insertions(+), 72 deletions(-) diff --git a/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs b/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs index 848eaa0594..d16f244a0a 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs @@ -21,99 +21,110 @@ namespace Umbraco.Web.Models.Mapping public IEnumerable Map(IContent source, MapperContext context) { - var variants = new List(); - var variesByCulture = source.ContentType.VariesByCulture(); var variesBySegment = source.ContentType.VariesBySegment(); + IList variants = new List(); + if (!variesByCulture && !variesBySegment) { - //this is invariant so just map the IContent instance to ContentVariationDisplay - variants.Add(context.Map(source)); - return variants; + // this is invariant so just map the IContent instance to ContentVariationDisplay + var variantDisplay = context.Map(source); + variants.Add(variantDisplay); } - - if (variesByCulture && !variesBySegment) + else if (variesByCulture && !variesBySegment) { - // Culture only - - var allLanguages = _localizationService.GetAllLanguages().OrderBy(x => x.Id).ToList(); - if (allLanguages.Count == 0) return Enumerable.Empty(); //this should never happen - - var langs = context.MapEnumerable(allLanguages).ToList(); - - //create a variant for each language, then we'll populate the values - variants.AddRange(langs.Select(x => - { - //We need to set the culture in the mapping context since this is needed to ensure that the correct property values - //are resolved during the mapping - context.SetCulture(x.IsoCode); - return context.Map(source); - })); - - for (int i = 0; i < langs.Count; i++) - { - var x = langs[i]; - var variant = variants[i]; - - variant.Language = x; - variant.Name = source.GetCultureName(x.IsoCode); - } - - //Put the default language first in the list & then sort rest by a-z - var defaultLang = variants.SingleOrDefault(x => x.Language.IsDefault); - - //Remove the default language from the list for now - variants.Remove(defaultLang); - - //Sort the remaining languages a-z - variants = variants.OrderBy(x => x.Language.Name).ToList(); - - //Insert the default language as the first item - variants.Insert(0, defaultLang); + var languages = GetLanguages(context); + variants = languages + .Select(language => CreateVariantDisplay(context, source, language, null)) + .ToList(); } else if (variesBySegment && !variesByCulture) { // Segment only - throw new NotSupportedException("ContentVariantMapper not implemented for segment only!"); + var segments = GetSegments(source); + variants = segments + .Select(segment => CreateVariantDisplay(context, source, null, segment)) + .ToList(); } else { // Culture and segment + var languages = GetLanguages(context).ToList(); + var segments = GetSegments(source).ToList(); - var allLanguages = _localizationService.GetAllLanguages().OrderBy(x => x.Id).ToList(); - if (allLanguages.Count == 0) return Enumerable.Empty(); //this should never happen - - var langs = context.MapEnumerable(allLanguages).ToList(); - - // All segments, including the unsegmented (= NULL) segment. - // TODO: The NULl segment might have to be changed to be empty string? - var segments = source.Properties - .SelectMany(p => p.Values.Select(v => v.Segment)) - .Distinct(); - - // Add all variants - foreach (var language in langs) + if (languages.Count == 0 || segments.Count == 0) { - foreach (var segment in segments) - { - context.SetCulture(language.IsoCode); - context.SetSegment(segment); - - var variantDisplay = context.Map(source); - - variantDisplay.Language = language; - variantDisplay.Segment = segment; - variantDisplay.Name = source.GetCultureName(language.IsoCode); - - variants.Add(variantDisplay); - } + // This should not happen + throw new ArgumentException("No languages or segments available"); } - // TODO: Sorting + variants = languages + .SelectMany(language => segments + .Select(segment => CreateVariantDisplay(context, source, language, segment))) + .ToList(); } - return variants; + return SortVariants(variants); + } + + private IList SortVariants(IList variants) + { + if (variants == null || variants.Count <= 1) + { + return variants; + } + + // Default variant first, then order by language, segment. + return variants + .OrderBy(v => IsDefaultLanguage(v) ? 0 : 1) + .ThenBy(v => IsDefaultSegment(v) ? 0 : 1) + .ThenBy(v => v?.Language?.Name) + .ThenBy(v => v.Segment) + .ToList(); + } + + private static bool IsDefaultSegment(ContentVariantDisplay variant) + { + return variant.Segment == null; + } + + private static bool IsDefaultLanguage(ContentVariantDisplay variant) + { + return variant.Language == null || variant.Language.IsDefault; + } + + private IEnumerable GetLanguages(MapperContext context) + { + var allLanguages = _localizationService.GetAllLanguages().OrderBy(x => x.Id).ToList(); + if (allLanguages.Count == 0) + { + // This should never happen + return Enumerable.Empty(); + } + else + { + return context.MapEnumerable(allLanguages).ToList(); + } + } + + private IEnumerable GetSegments(IContent content) + { + return content.Properties.SelectMany(p => p.Values.Select(v => v.Segment)).Distinct(); + } + + private ContentVariantDisplay CreateVariantDisplay(MapperContext context, IContent content, Language language, string segment) + { + context.SetCulture(language?.IsoCode); + context.SetSegment(segment); + + var variantDisplay = context.Map(content); + + variantDisplay.Segment = segment; + variantDisplay.Language = language; + variantDisplay.Name = content.GetCultureName(language?.IsoCode); + + return variantDisplay; } } } From 813998a614c89dbcd9d11e6019aad535cd18009e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Fri, 11 Oct 2019 14:13:49 +0200 Subject: [PATCH 4/8] Support for saving edited segment values --- .../Editors/Binders/ContentItemBinder.cs | 9 +++++--- src/Umbraco.Web/Editors/ContentController.cs | 23 +++++++++++++++---- .../ContentEditing/ContentVariantSave.cs | 6 +++++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs b/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs index 11a876345d..d9d56c584f 100644 --- a/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs +++ b/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs @@ -49,7 +49,7 @@ namespace Umbraco.Web.Editors.Binders { foreach (var variant in model.Variants) { - if (variant.Culture.IsNullOrWhiteSpace()) + if (variant.Culture.IsNullOrWhiteSpace() && variant.Segment.IsNullOrWhiteSpace()) { //map the property dto collection (no culture is passed to the mapping context so it will be invariant) variant.PropertyCollectionDto = Current.Mapper.Map(model.PersistedContent); @@ -59,7 +59,11 @@ namespace Umbraco.Web.Editors.Binders //map the property dto collection with the culture of the current variant variant.PropertyCollectionDto = Current.Mapper.Map( model.PersistedContent, - context => context.SetCulture(variant.Culture)); + context => + { + context.SetCulture(variant.Culture); + context.SetSegment(variant.Segment); + }); } //now map all of the saved values to the dto @@ -87,6 +91,5 @@ namespace Umbraco.Web.Editors.Binders model.ParentId, contentType); } - } } diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 9d5af028e3..bc2b55833e 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -1836,8 +1836,13 @@ namespace Umbraco.Web.Editors /// private void MapValuesForPersistence(ContentItemSave contentSave) { - // inline method to determine if a property type varies - bool Varies(Property property) => property.PropertyType.VariesByCulture(); + // inline method to determine the culture and segment to persist the property + (string culture, string segment) PropertyCultureAndSegment(Property property, ContentVariantSave variant) + { + var culture = property.PropertyType.VariesByCulture() ? variant.Culture : null; + var segment = property.PropertyType.VariesBySegment() ? variant.Segment : null; + return (culture, segment); + } var variantIndex = 0; @@ -1876,8 +1881,18 @@ namespace Umbraco.Web.Editors MapPropertyValuesForPersistence( contentSave, propertyCollection, - (save, property) => Varies(property) ? property.GetValue(variant.Culture) : property.GetValue(), //get prop val - (save, property, v) => { if (Varies(property)) property.SetValue(v, variant.Culture); else property.SetValue(v); }, //set prop val + (save, property) => + { + // Get property value + (var culture, var segment) = PropertyCultureAndSegment(property, variant); + return property.GetValue(culture, segment); + }, + (save, property, v) => + { + // Set property value + (var culture, var segment) = PropertyCultureAndSegment(property, variant); + property.SetValue(v, culture, segment); + }, variant.Culture); variantIndex++; diff --git a/src/Umbraco.Web/Models/ContentEditing/ContentVariantSave.cs b/src/Umbraco.Web/Models/ContentEditing/ContentVariantSave.cs index deadac949a..9a7555ad92 100644 --- a/src/Umbraco.Web/Models/ContentEditing/ContentVariantSave.cs +++ b/src/Umbraco.Web/Models/ContentEditing/ContentVariantSave.cs @@ -29,6 +29,12 @@ namespace Umbraco.Web.Models.ContentEditing [DataMember(Name = "culture")] public string Culture { get; set; } + /// + /// The segment of this variant, if this is invariant than this is null or empty + /// + [DataMember(Name = "segment")] + public string Segment { get; set; } + /// /// Indicates if the variant should be updated /// From 1da29b29b90145c389e9b1569386d62e743ddcb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Fri, 30 Aug 2019 11:37:44 +0200 Subject: [PATCH 5/8] Only set segment on property when property can be varied by segment --- .../Models/Mapping/ContentPropertyBasicMapper.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Web/Models/Mapping/ContentPropertyBasicMapper.cs b/src/Umbraco.Web/Models/Mapping/ContentPropertyBasicMapper.cs index aacafd2f2a..cf1bc3c253 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentPropertyBasicMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentPropertyBasicMapper.cs @@ -70,10 +70,9 @@ namespace Umbraco.Web.Models.Mapping dest.Culture = culture; - var segment = context.GetSegment(); - - // The current segment can always be null, even if the propertyType *can* be varied by segment - // so the nullcheck like with culture is not performed here. + // Get the segment, which is always allowed to be null even if the propertyType *can* be varied by segment. + // There is therefore no need to perform the null check like with culture above. + var segment = !property.PropertyType.VariesBySegment() ? null : context.GetSegment(); dest.Segment = segment; // if no 'IncludeProperties' were specified or this property is set to be included - we will map the value and return. From 24afe3f130e81155af894081d5c5b1a495533cb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Tue, 7 Jan 2020 09:34:12 +0100 Subject: [PATCH 6/8] Changed ArgumentException to InvalidOperationException --- src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs b/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs index d16f244a0a..864928d06d 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs @@ -56,7 +56,7 @@ namespace Umbraco.Web.Models.Mapping if (languages.Count == 0 || segments.Count == 0) { // This should not happen - throw new ArgumentException("No languages or segments available"); + throw new InvalidOperationException("No languages or segments available"); } variants = languages From 71b3aa1cb0d1ab848dc20714a133d42806427d78 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 13 Jan 2020 22:16:19 +1100 Subject: [PATCH 7/8] Removes the need for checking variant when binding content --- .../Editors/Binders/ContentItemBinder.cs | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs b/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs index d9d56c584f..a6cba6ce41 100644 --- a/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs +++ b/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs @@ -49,22 +49,15 @@ namespace Umbraco.Web.Editors.Binders { foreach (var variant in model.Variants) { - if (variant.Culture.IsNullOrWhiteSpace() && variant.Segment.IsNullOrWhiteSpace()) - { - //map the property dto collection (no culture is passed to the mapping context so it will be invariant) - variant.PropertyCollectionDto = Current.Mapper.Map(model.PersistedContent); - } - else - { - //map the property dto collection with the culture of the current variant - variant.PropertyCollectionDto = Current.Mapper.Map( - model.PersistedContent, - context => - { - context.SetCulture(variant.Culture); - context.SetSegment(variant.Segment); - }); - } + //map the property dto collection with the culture of the current variant + variant.PropertyCollectionDto = Current.Mapper.Map( + model.PersistedContent, + context => + { + // either of these may be null and that is ok, if it's invariant they will be null which is what is expected + context.SetCulture(variant.Culture); + context.SetSegment(variant.Segment); + }); //now map all of the saved values to the dto _modelBinderHelper.MapPropertyValuesFromSaved(variant, variant.PropertyCollectionDto); From 63fab6a56b905dc514a0f0358bdf700fd81d34eb Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 14 Jan 2020 11:47:31 +1100 Subject: [PATCH 8/8] adds notes --- src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs b/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs index 864928d06d..5d076812f3 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs @@ -108,6 +108,13 @@ namespace Umbraco.Web.Models.Mapping } } + /// + /// Returns all segments assigned to the content + /// + /// + /// + /// Returns all segments assigned to the content including 'null' values + /// private IEnumerable GetSegments(IContent content) { return content.Properties.SelectMany(p => p.Values.Select(v => v.Segment)).Distinct();