diff --git a/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs b/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs index d77867152a..0279f82f42 100644 --- a/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs @@ -489,7 +489,7 @@ namespace Umbraco.Tests.Web.Controllers var display = JsonConvert.DeserializeObject(response.Item2); Assert.AreEqual(2, display.Errors.Count()); Assert.IsTrue(display.Errors.ContainsKey("Variants[0].Name")); - Assert.IsTrue(display.Errors.ContainsKey("_content_variant_en-US_")); + Assert.IsTrue(display.Errors.ContainsKey("_content_variant_en-US_null_")); } // TODO: There are SOOOOO many more tests we should write - a lot of them to do with validation diff --git a/src/Umbraco.Tests/Web/ModelStateExtensionsTests.cs b/src/Umbraco.Tests/Web/ModelStateExtensionsTests.cs index 8d285e0375..7b25e60b5a 100644 --- a/src/Umbraco.Tests/Web/ModelStateExtensionsTests.cs +++ b/src/Umbraco.Tests/Web/ModelStateExtensionsTests.cs @@ -22,19 +22,19 @@ namespace Umbraco.Tests.Web ms.AddPropertyError(new ValidationResult("no header image"), "headerImage", null); //invariant property ms.AddPropertyError(new ValidationResult("title missing"), "title", "en-US"); //variant property - var result = ms.GetCulturesWithErrors(localizationService.Object, "en-US"); + var result = ms.GetVariantsWithErrors("en-US"); //even though there are 2 errors, they are both for en-US since that is the default language and one of the errors is for an invariant property Assert.AreEqual(1, result.Count); - Assert.AreEqual("en-US", result[0]); + Assert.AreEqual("en-US", result[0].culture); ms = new ModelStateDictionary(); - ms.AddCultureValidationError("en-US", "generic culture error"); + ms.AddVariantValidationError("en-US", null, "generic culture error"); - result = ms.GetCulturesWithErrors(localizationService.Object, "en-US"); + result = ms.GetVariantsWithErrors("en-US"); Assert.AreEqual(1, result.Count); - Assert.AreEqual("en-US", result[0]); + Assert.AreEqual("en-US", result[0].culture); } [Test] @@ -47,11 +47,11 @@ namespace Umbraco.Tests.Web ms.AddPropertyError(new ValidationResult("no header image"), "headerImage", null); //invariant property ms.AddPropertyError(new ValidationResult("title missing"), "title", "en-US"); //variant property - var result = ms.GetCulturesWithPropertyErrors(localizationService.Object, "en-US"); + var result = ms.GetVariantsWithPropertyErrors("en-US"); //even though there are 2 errors, they are both for en-US since that is the default language and one of the errors is for an invariant property Assert.AreEqual(1, result.Count); - Assert.AreEqual("en-US", result[0]); + Assert.AreEqual("en-US", result[0].culture); } [Test] diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js index 0b137a5fbe..c07bb9bc83 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js @@ -210,7 +210,7 @@ function appendRuntimeData() { $scope.content.variants.forEach((variant) => { variant.compositeId = contentEditingHelper.buildCompositeVariantId(variant); - variant.htmlId = "_content_variant_" + variant.compositeId; + variant.htmlId = "_content_variant_" + variant.compositeId + "_"; }); } diff --git a/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js index 8f2aa1d22b..bfcc0d536e 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js @@ -328,9 +328,11 @@ function contentEditingHelper(fileManager, $q, $location, $routeParams, editorSt * * @description * Returns a id for the variant that is unique between all variants on the content + * Note "invariant" is used for the invariant culture, + * "null" is used for the NULL segment */ buildCompositeVariantId: function (variant) { - return (variant.language ? variant.language.culture : "invariant") + "_" + (variant.segment ? variant.segment : ""); + return (variant.language ? variant.language.culture : "invariant") + "_" + (variant.segment ? variant.segment : "null"); }, diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 691d6afa3a..565eebf80e 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -37,6 +37,7 @@ using Umbraco.Core.Models.Entities; using Umbraco.Core.Persistence; using Umbraco.Core.Security; using Umbraco.Web.Routing; +using Umbraco.Core.Collections; namespace Umbraco.Web.Editors { @@ -706,12 +707,19 @@ namespace Umbraco.Web.Editors { if (variantCount > 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 variantErrors = ModelState.GetVariantsWithErrors(cultureForInvariantErrors); + + var validVariants = contentItem.Variants + .Where(x => x.Save && !variantErrors.Contains((x.Culture, x.Segment))) + .Select(x => (culture: x.Culture, segment: x.Segment)); + + foreach (var (culture, segment) in validVariants) { - AddSuccessNotification(notifications, c, + var variantName = GetVariantName(culture, segment); + + AddSuccessNotification(notifications, culture, segment, Services.TextService.Localize("speechBubbles/editContentSendToPublish"), - Services.TextService.Localize("speechBubbles/editVariantSendToPublishText", new[] { _allLangs.Value[c].CultureName })); + Services.TextService.Localize("speechBubbles/editVariantSendToPublishText", new[] { variantName })); } } else if (ModelState.IsValid) @@ -842,7 +850,7 @@ namespace Umbraco.Web.Editors //if there's more than 1 variant, then we need to add the culture specific error //messages based on the variants in error so that the messages show in the publish/save dialog if (variants.Count > 1) - AddCultureValidationError(variant.Culture, "publish/contentPublishedFailedByMissingName"); + AddVariantValidationError(variant.Culture, variant.Segment, "publish/contentPublishedFailedByMissingName"); else return false; //It's invariant and is missing critical data, it cannot be saved } @@ -896,18 +904,19 @@ namespace Umbraco.Web.Editors { if (variantCount > 1) { - var cultureErrors = ModelState.GetCulturesWithErrors(Services.LocalizationService, cultureForInvariantErrors); + var variantErrors = ModelState.GetVariantsWithErrors(cultureForInvariantErrors); 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) + .Where(x => x.Save && !variantErrors.Contains((x.Culture, x.Segment))) + .Select(x => (culture: x.Culture, segment: x.Segment)); + + foreach (var (culture, segment) in savedWithoutErrors) { - AddSuccessNotification(notifications, c, + var variantName = GetVariantName(culture, segment); + + AddSuccessNotification(notifications, culture, segment, Services.TextService.Localize("speechBubbles/editContentSavedHeader"), - Services.TextService.Localize(variantSavedLocalizationKey, new[] { _allLangs.Value[c].CultureName })); + Services.TextService.Localize(variantSavedLocalizationKey, new[] { variantName })); } } else if (ModelState.IsValid) @@ -1040,14 +1049,16 @@ namespace Umbraco.Web.Editors if (!isPublished && releaseDates.Count == 0) { //can't continue, a mandatory variant is not published and not scheduled for publishing - AddCultureValidationError(culture, "speechBubbles/scheduleErrReleaseDate2"); + // TODO: Add segment + AddVariantValidationError(culture, null, "speechBubbles/scheduleErrReleaseDate2"); isValid = false; continue; } if (!isPublished && releaseDates.Any(x => nonMandatoryVariantReleaseDates.Any(r => x.Date > r.Date))) { //can't continue, a mandatory variant is not published and it's scheduled for publishing after a non-mandatory - AddCultureValidationError(culture, "speechBubbles/scheduleErrReleaseDate3"); + // TODO: Add segment + AddVariantValidationError(culture, null, "speechBubbles/scheduleErrReleaseDate3"); isValid = false; continue; } @@ -1061,7 +1072,7 @@ namespace Umbraco.Web.Editors //1) release date cannot be less than now if (variant.ReleaseDate.HasValue && variant.ReleaseDate < DateTime.Now) { - AddCultureValidationError(variant.Culture, "speechBubbles/scheduleErrReleaseDate1"); + AddVariantValidationError(variant.Culture, variant.Segment, "speechBubbles/scheduleErrReleaseDate1"); isValid = false; continue; } @@ -1069,7 +1080,7 @@ namespace Umbraco.Web.Editors //2) expire date cannot be less than now if (variant.ExpireDate.HasValue && variant.ExpireDate < DateTime.Now) { - AddCultureValidationError(variant.Culture, "speechBubbles/scheduleErrExpireDate1"); + AddVariantValidationError(variant.Culture, variant.Segment, "speechBubbles/scheduleErrExpireDate1"); isValid = false; continue; } @@ -1077,7 +1088,7 @@ namespace Umbraco.Web.Editors //3) expire date cannot be less than release date if (variant.ExpireDate.HasValue && variant.ReleaseDate.HasValue && variant.ExpireDate <= variant.ReleaseDate) { - AddCultureValidationError(variant.Culture, "speechBubbles/scheduleErrExpireDate2"); + AddVariantValidationError(variant.Culture, variant.Segment, "speechBubbles/scheduleErrExpireDate2"); isValid = false; continue; } @@ -1102,12 +1113,13 @@ namespace Umbraco.Web.Editors /// global notifications will be shown if all variant processing is successful and the save/publish dialog is closed, otherwise /// variant specific notifications are used to show success messages in the save/publish dialog. /// - private static void AddSuccessNotification(IDictionary notifications, string culture, string header, string msg) + private static void AddSuccessNotification(IDictionary notifications, string culture, string segment, string header, string msg) { //add the global notification (which will display globally if all variants are successfully processed) notifications[string.Empty].AddSuccessNotification(header, msg); //add the variant specific notification (which will display in the dialog if all variants are not successfully processed) - notifications.GetOrCreate(culture).AddSuccessNotification(header, msg); + var key = culture + "_" + segment; + notifications.GetOrCreate(key).AddSuccessNotification(header, msg); } /// @@ -1157,17 +1169,16 @@ namespace Umbraco.Web.Editors return publishStatus; } - //All variants in this collection should have a culture if we get here! but we'll double check and filter here - var cultureVariants = contentItem.Variants.Where(x => !x.Culture.IsNullOrWhiteSpace()).ToList(); - var mandatoryCultures = _allLangs.Value.Values.Where(x => x.IsMandatory).Select(x => x.IsoCode).ToList(); - var cultureErrors = ModelState.GetCulturesWithErrors(Services.LocalizationService, cultureForInvariantErrors); + var variantErrors = ModelState.GetVariantsWithErrors(cultureForInvariantErrors); + + var variants = contentItem.Variants.ToList(); //validate if we can publish based on the mandatory language requirements var canPublish = ValidatePublishingMandatoryLanguages( - cultureErrors, - contentItem, cultureVariants, mandatoryCultures, + variantErrors, + contentItem, variants, mandatoryCultures, mandatoryVariant => mandatoryVariant.Publish); //Now check if there are validation errors on each variant. @@ -1177,11 +1188,11 @@ namespace Umbraco.Web.Editors foreach (var variant in contentItem.Variants) { - if (cultureErrors.Contains(variant.Culture)) + if (variantErrors.Contains((variant.Culture, variant.Segment))) variant.Publish = false; } - var culturesToPublish = cultureVariants.Where(x => x.Publish).Select(x => x.Culture).ToArray(); + var culturesToPublish = variants.Where(x => x.Publish).Select(x => x.Culture).ToArray(); if (canPublish) { @@ -1229,17 +1240,16 @@ namespace Umbraco.Web.Editors return publishStatus; } - //All variants in this collection should have a culture if we get here! but we'll double check and filter here - var cultureVariants = contentItem.Variants.Where(x => !x.Culture.IsNullOrWhiteSpace()).ToList(); - var mandatoryCultures = _allLangs.Value.Values.Where(x => x.IsMandatory).Select(x => x.IsoCode).ToList(); - var cultureErrors = ModelState.GetCulturesWithErrors(Services.LocalizationService, cultureForInvariantErrors); + var variantErrors = ModelState.GetVariantsWithErrors(cultureForInvariantErrors); + + var variants = contentItem.Variants.ToList(); //validate if we can publish based on the mandatory languages selected var canPublish = ValidatePublishingMandatoryLanguages( - cultureErrors, - contentItem, cultureVariants, mandatoryCultures, + variantErrors, + contentItem, variants, mandatoryCultures, mandatoryVariant => mandatoryVariant.Publish); //if none are published and there are validation errors for mandatory cultures, then we can't publish anything @@ -1251,19 +1261,19 @@ namespace Umbraco.Web.Editors //It is a requirement that this is performed AFTER ValidatePublishingMandatoryLanguages. foreach (var variant in contentItem.Variants) { - if (cultureErrors.Contains(variant.Culture)) + if (variantErrors.Contains((variant.Culture, variant.Segment))) variant.Publish = false; } //At this stage all variants might have failed validation which means there are no cultures flagged for publishing! - var culturesToPublish = cultureVariants.Where(x => x.Publish).Select(x => x.Culture).ToArray(); + var culturesToPublish = variants.Where(x => x.Publish).Select(x => x.Culture).ToArray(); canPublish = canPublish && culturesToPublish.Length > 0; if (canPublish) { //try to publish all the values on the model - this will generally only fail if someone is tampering with the request //since there's no reason variant rules would be violated in normal cases. - canPublish = PublishCulture(contentItem.PersistedContent, cultureVariants, defaultCulture); + canPublish = PublishCulture(contentItem.PersistedContent, variants, defaultCulture); } if (canPublish) @@ -1288,16 +1298,16 @@ namespace Umbraco.Web.Editors /// /// Validate if publishing is possible based on the mandatory language requirements /// - /// + /// /// - /// + /// /// /// /// private bool ValidatePublishingMandatoryLanguages( - IReadOnlyCollection culturesWithValidationErrors, + IReadOnlyCollection<(string culture, string segment)> variantsWithValidationErrors, ContentItemSave contentItem, - IReadOnlyCollection cultureVariants, + IReadOnlyCollection variants, IReadOnlyList mandatoryCultures, Func publishingCheck) { @@ -1308,11 +1318,11 @@ namespace Umbraco.Web.Editors { //Check if a mandatory language is missing from being published - var mandatoryVariant = cultureVariants.First(x => x.Culture.InvariantEquals(culture)); + var mandatoryVariant = variants.First(x => x.Culture.InvariantEquals(culture)); var isPublished = contentItem.PersistedContent.Published && contentItem.PersistedContent.IsCulturePublished(culture); var isPublishing = isPublished || publishingCheck(mandatoryVariant); - var isValid = !culturesWithValidationErrors.InvariantContains(culture); + var isValid = !variantsWithValidationErrors.Select(v => v.culture).InvariantContains(culture); result.Add((mandatoryVariant, isPublished || isPublishing, isValid)); } @@ -1327,19 +1337,19 @@ namespace Umbraco.Web.Editors if (r.publishing && !r.isValid) { //flagged for publishing but the mandatory culture is invalid - AddCultureValidationError(r.model.Culture, "publish/contentPublishedFailedReqCultureValidationError"); + AddVariantValidationError(r.model.Culture, r.model.Segment, "publish/contentPublishedFailedReqCultureValidationError"); canPublish = false; } else if (r.publishing && r.isValid && firstInvalidMandatoryCulture != null) { //in this case this culture also cannot be published because another mandatory culture is invalid - AddCultureValidationError(r.model.Culture, "publish/contentPublishedFailedReqCultureValidationError", firstInvalidMandatoryCulture); + AddVariantValidationError(r.model.Culture, r.model.Segment, "publish/contentPublishedFailedReqCultureValidationError", firstInvalidMandatoryCulture); canPublish = false; } else if (!r.publishing) { //cannot continue publishing since a required culture that is not currently being published isn't published - AddCultureValidationError(r.model.Culture, "speechBubbles/contentReqCulturePublishError"); + AddVariantValidationError(r.model.Culture, r.model.Segment, "speechBubbles/contentReqCulturePublishError"); canPublish = false; } } @@ -1364,7 +1374,7 @@ namespace Umbraco.Web.Editors var valid = persistentContent.PublishCulture(CultureImpact.Explicit(variant.Culture, defaultCulture.InvariantEquals(variant.Culture))); if (!valid) { - AddCultureValidationError(variant.Culture, "speechBubbles/contentCultureValidationError"); + AddVariantValidationError(variant.Culture, variant.Segment, "speechBubbles/contentCultureValidationError"); return false; } } @@ -1376,14 +1386,40 @@ namespace Umbraco.Web.Editors /// Adds a generic culture error for use in displaying the culture validation error in the save/publish/etc... dialogs /// /// Culture to assign the error to + /// Segment to assign the error to /// /// /// The culture used in the localization message, null by default which means will be used. /// - private void AddCultureValidationError(string culture, string localizationKey, string cultureToken = null) + private void AddVariantValidationError(string culture, string segment, string localizationKey, string cultureToken = null) { - var errMsg = Services.TextService.Localize(localizationKey, new[] { cultureToken == null ? _allLangs.Value[culture].CultureName : _allLangs.Value[cultureToken].CultureName }); - ModelState.AddCultureValidationError(culture, errMsg); + var cultureToUse = cultureToken ?? culture; + var variantName = GetVariantName(cultureToUse, segment); + + var errMsg = Services.TextService.Localize(localizationKey, new[] { variantName }); + + ModelState.AddVariantValidationError(culture, segment, errMsg); + } + + /// + /// Creates the human readable variant name based on culture and segment + /// + /// Culture + /// Segment + /// + private string GetVariantName(string culture, string segment) + { + if(culture.IsNullOrWhiteSpace() && segment.IsNullOrWhiteSpace()) + { + // TODO: Get name for default variant from somewhere? + return "Default"; + } + + var cultureName = culture == null ? null : _allLangs.Value[culture].CultureName; + var variantName = string.Join(" — ", new[] { segment, cultureName }.Where(x => !x.IsNullOrWhiteSpace())); + + // Format: [—] + return variantName; } /// @@ -1824,11 +1860,11 @@ namespace Umbraco.Web.Editors if (!ModelState.IsValid && display.Variants.Count() > 1) { //Add any culture specific errors here - var cultureErrors = ModelState.GetCulturesWithErrors(Services.LocalizationService, cultureForInvariantErrors); + var variantErrors = ModelState.GetVariantsWithErrors(cultureForInvariantErrors); - foreach (var cultureError in cultureErrors) + foreach (var (culture, segment) in variantErrors) { - AddCultureValidationError(cultureError, "speechBubbles/contentCultureValidationError"); + AddVariantValidationError(culture, segment, "speechBubbles/contentCultureValidationError"); } } diff --git a/src/Umbraco.Web/ModelStateExtensions.cs b/src/Umbraco.Web/ModelStateExtensions.cs index 6144d6f9c9..10b1b5dadd 100644 --- a/src/Umbraco.Web/ModelStateExtensions.cs +++ b/src/Umbraco.Web/ModelStateExtensions.cs @@ -65,11 +65,12 @@ namespace Umbraco.Web /// /// /// + /// /// - internal static void AddCultureValidationError(this System.Web.Http.ModelBinding.ModelStateDictionary modelState, - string culture, string errMsg) + internal static void AddVariantValidationError(this System.Web.Http.ModelBinding.ModelStateDictionary modelState, + string culture, string segment, string errMsg) { - var key = "_content_variant_" + culture + "_"; + var key = "_content_variant_" + (culture.IsNullOrWhiteSpace() ? "invariant" : culture) + "_" + (segment.IsNullOrWhiteSpace() ? "null" : segment) + "_"; if (modelState.ContainsKey(key)) return; modelState.AddModelError(key, errMsg); } @@ -83,23 +84,28 @@ namespace Umbraco.Web /// /// A list of cultures that have property validation errors. The default culture will be returned for any invariant property errors. /// - internal static IReadOnlyList GetCulturesWithPropertyErrors(this System.Web.Http.ModelBinding.ModelStateDictionary modelState, - ILocalizationService localizationService, string cultureForInvariantErrors) + internal static IReadOnlyList<(string culture, string segment)> GetVariantsWithPropertyErrors(this System.Web.Http.ModelBinding.ModelStateDictionary modelState, + string cultureForInvariantErrors) { - //Add any culture specific errors here - var cultureErrors = modelState.Keys + //Add any variant specific errors here + var variantErrors = modelState.Keys + .Where(key => key.StartsWith("_Properties.")) //only choose _Properties errors .Select(x => x.Split('.')) //split into parts - .Where(x => x.Length >= 3 && x[0] == "_Properties") //only choose _Properties errors - .Select(x => x[2]) //select the culture part - .Where(x => !x.IsNullOrWhiteSpace()) //if it has a value - //if it's marked "invariant" than return the default language, this is because we can only edit invariant properties on the default language + .Where(x => x.Length >= 4 && !x[2].IsNullOrWhiteSpace() && !x[3].IsNullOrWhiteSpace()) + .Select(x => (culture: x[2], segment: x[3])) + //if the culture is marked "invariant" than return the default language, this is because we can only edit invariant properties on the default language //so errors for those must show up under the default lang. - .Select(x => x == "invariant" ? cultureForInvariantErrors : x) - .WhereNotNull() + //if the segment is marked "null" then return an actual null + .Select(x => + { + var culture = x.culture == "invariant" ? cultureForInvariantErrors : x.culture; + var segment = x.segment == "null" ? null : x.segment; + return (culture, segment); + }) .Distinct() .ToList(); - return cultureErrors; + return variantErrors; } /// @@ -111,23 +117,33 @@ namespace Umbraco.Web /// /// A list of cultures that have validation errors. The default culture will be returned for any invariant errors. /// - internal static IReadOnlyList GetCulturesWithErrors(this System.Web.Http.ModelBinding.ModelStateDictionary modelState, - ILocalizationService localizationService, string cultureForInvariantErrors) + internal static IReadOnlyList<(string culture, string segment)> GetVariantsWithErrors(this System.Web.Http.ModelBinding.ModelStateDictionary modelState, string cultureForInvariantErrors) { - var propertyCultureErrors = modelState.GetCulturesWithPropertyErrors(localizationService, cultureForInvariantErrors); + var propertyVariantErrors = modelState.GetVariantsWithPropertyErrors(cultureForInvariantErrors); - //now check the other special culture errors that are - var genericCultureErrors = modelState.Keys + //now check the other special variant errors that are + var genericVariantErrors = modelState.Keys .Where(x => x.StartsWith("_content_variant_") && x.EndsWith("_")) - .Select(x => x.TrimStart("_content_variant_").TrimEnd("_")) - .Where(x => !x.IsNullOrWhiteSpace()) + .Select(x => x.TrimStart("_content_variant_").TrimEnd("_")) + .Select(x => + { + // Format "_" + var cs = x.Split(new[] { '_' }); + return (culture: cs[0], segment: cs[1]); + }) + .Where(x => !x.culture.IsNullOrWhiteSpace()) //if it's marked "invariant" than return the default language, this is because we can only edit invariant properties on the default language //so errors for those must show up under the default lang. - .Select(x => x == "invariant" ? cultureForInvariantErrors : x) - .WhereNotNull() + //if the segment is marked "null" then return an actual null + .Select(x => + { + var culture = x.culture == "invariant" ? cultureForInvariantErrors : x.culture; + var segment = x.segment == "null" ? null : x.segment; + return (culture, segment); + }) .Distinct(); - return propertyCultureErrors.Union(genericCultureErrors).ToList(); + return propertyVariantErrors.Union(genericVariantErrors).Distinct().ToList(); } ///