From 5a5291d149f87cade1fbd1779049f50f1a5e384b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Knippers?= Date: Tue, 18 Feb 2020 11:46:31 +0100 Subject: [PATCH] V8/feature/ab4550 segments ui variant picker (#7676) * Fixes incorrect property inheritance logic * Fixes crash in canVariantPublish when variant.language is null * Adds variant display name in dropdown * Logic for invariant properties updated to also support segment invariance * Properties varied by segment only now properly saved when multiple variants are saved/published * Logic for disabling property editors moved to function and corrected for all cases of culture/segment properties * Fixes syntax error in less file * Fixes empty variants returned from GetEmpty() for a ContentType set to vary by segment * Replaced _.each with _.find to prevent having to loop through all variants and/or somehow open multiple. It is not possible to break out of _.each using a return statement, it simply returns that current function but _.each will continue calling the others after that. * Added a null check on Culture prop which is now possibly null due to segments * Makes sure segments are not completely removed when their value is null. During save/publish, Umbraco first deletes all property data of a specific version and then adds property values again. However, any segments that were created but had an empty value would not be added again which meant the segments were entirely gone afterwards. * GetSegments() updated to always return the default segment, not only when there are no segments at all. This makes sure that even if there is no property data for the default segment in the database but only for some segments, the default segment will still be returned here. --- .../Persistence/Factories/PropertyFactory.cs | 9 +++++---- .../umbvariantcontenteditors.directive.js | 12 ++++++----- src/Umbraco.Web/Editors/ContentController.cs | 8 +++++++- .../Models/Mapping/ContentVariantMapper.cs | 20 +++++++------------ 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs b/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs index 33dabe1b2435..92d397520e2c 100644 --- a/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs @@ -134,15 +134,16 @@ public static IEnumerable BuildDtos(ContentVariation contentVar // publishing = deal with edit and published values foreach (var propertyValue in property.Values) { - var isInvariantValue = propertyValue.Culture == null; - var isCultureValue = propertyValue.Culture != null && propertyValue.Segment == null; + var isInvariantValue = propertyValue.Culture == null && propertyValue.Segment == null; + var isCultureValue = propertyValue.Culture != null; + var isSegmentValue = propertyValue.Segment != null; // deal with published value - if (propertyValue.PublishedValue != null && publishedVersionId > 0) + if ((propertyValue.PublishedValue != null || isSegmentValue) && publishedVersionId > 0) propertyDataDtos.Add(BuildDto(publishedVersionId, property, languageRepository.GetIdByIsoCode(propertyValue.Culture), propertyValue.Segment, propertyValue.PublishedValue)); // deal with edit value - if (propertyValue.EditedValue != null) + if (propertyValue.EditedValue != null || isSegmentValue) propertyDataDtos.Add(BuildDto(currentVersionId, property, languageRepository.GetIdByIsoCode(propertyValue.Culture), propertyValue.Segment, propertyValue.EditedValue)); // property.Values will contain ALL of it's values, both variant and invariant which will be populated if the diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbvariantcontenteditors.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbvariantcontenteditors.directive.js index 697e980823be..15df67def399 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbvariantcontenteditors.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbvariantcontenteditors.directive.js @@ -172,12 +172,14 @@ function requestSplitView(args) { var culture = args.culture; var segment = args.segment; - _.each(vm.content.variants, function (v) { - if ((!v.language || v.language.culture === culture) && v.segment === segment) { - openSplitView(v); - return; - } + + var variant = _.find(vm.content.variants, function (v) { + return (!v.language || v.language.culture === culture) && v.segment === segment; }); + + if (variant != null) { + openSplitView(variant); + } } /** Closes the split view */ diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 0e8787a99c25..74240bbf6c49 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -897,7 +897,13 @@ private void SaveAndNotify(ContentItemSave contentItem, Func 1) { var cultureErrors = ModelState.GetCulturesWithErrors(Services.LocalizationService, cultureForInvariantErrors); - foreach (var c in contentItem.Variants.Where(x => x.Save && !cultureErrors.Contains(x.Culture)).Select(x => x.Culture).ToArray()) + + var savedWithoutErrors = contentItem.Variants + .Where(x => x.Save && !cultureErrors.Contains(x.Culture) && x.Culture != null) + .Select(x => x.Culture) + .ToArray(); + + foreach (var c in savedWithoutErrors) { AddSuccessNotification(notifications, c, Services.TextService.Localize("speechBubbles/editContentSavedHeader"), diff --git a/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs b/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs index c5f7e1a37acb..0ed8ffb80ee9 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentVariantMapper.cs @@ -117,21 +117,15 @@ private IEnumerable GetLanguages(MapperContext context) /// private IEnumerable GetSegments(IContent content) { - // The current segments of a content item are determined - // entirely on the current property values of the content. - var segments = content.Properties - .SelectMany(p => p.Values.Select(v => v.Segment)) - .Distinct() - .ToList(); + // The default segment (null) is always there, + // even when there is no property data at all yet + var segments = new List { null }; - if(segments.Count == 0) - { - // The default segment is always there, - // even when there is no property data at all yet - segments.Add(null); - } + // Add actual segments based on the property values + segments.AddRange(content.Properties.SelectMany(p => p.Values.Select(v => v.Segment))); - return segments; + // Do not return a segment more than once + return segments.Distinct(); } private ContentVariantDisplay CreateVariantDisplay(MapperContext context, IContent content, Language language, string segment)