diff --git a/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js b/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js index cbe9b561a0..5e3a1fc0d8 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js @@ -365,7 +365,7 @@ action: action, variants: _.map(displayModel.variants, function(v) { return { - name: v.name, + name: v.name ? v.name : "", //if its null/empty,we must pass up an empty string else we get json converter errors properties: getContentProperties(v.tabs), culture: v.language ? v.language.culture : null, publish: v.publish, diff --git a/src/Umbraco.Web.UI.Client/src/views/content/overlays/publish.controller.js b/src/Umbraco.Web.UI.Client/src/views/content/overlays/publish.controller.js index 3873b7718a..a9070f40c9 100644 --- a/src/Umbraco.Web.UI.Client/src/views/content/overlays/publish.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/content/overlays/publish.controller.js @@ -55,10 +55,13 @@ } function hasAnyData(variant) { - - if(variant.name == null || variant.name.length === 0) { - return false; - } + + //** DO NOT MERGE THIS!!!!!! THIS IS FOR TESTING ***** + //if(variant.name == null || variant.name.length === 0) { + // return false; + //} + + var result = variant.isDirty != null; if(result) return true; diff --git a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml index 8700daff03..1299a14c2f 100644 --- a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml +++ b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml @@ -1409,6 +1409,7 @@ To manage your website, simply open the Umbraco back office and start adding con Invitation has been re-sent to %0% Cannot publish the document since the required '%0%' is not published Validation failed for language '%0%' + Validation failed for language '%0%' and could not be saved Document type was exported to file An error occurred while exporting the document type The release date cannot be in the past diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 12047b628b..aa0f3eb1a3 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -359,7 +359,7 @@ namespace Umbraco.Web.Editors // translate the content type name if applicable mapped.ContentTypeName = Services.TextService.UmbracoDictionaryTranslate(mapped.ContentTypeName); // if your user type doesn't have access to the Settings section it would not get this property mapped - if(mapped.DocumentType != null) + if (mapped.DocumentType != null) mapped.DocumentType.Name = Services.TextService.UmbracoDictionaryTranslate(mapped.DocumentType.Name); //remove the listview app if it exists @@ -595,6 +595,70 @@ namespace Umbraco.Web.Editors return contentItemDisplay; } + /// + /// Validates critical data for persistence and updates the ModelState and result accordingly + /// + /// + /// Returns the total number of variants (will be one if it's an invariant content item) + /// + /// + /// For invariant, the variants collection count will be 1 and this will check if that invariant item has the critical values for persistence (i.e. Name) + /// + /// For variant, each variant will be checked for critical data for persistence and if it's not there then it's flags will be reset and it will not + /// be persisted. However, we also need to deal with the case where all variants don't pass this check and then there is nothing to save. This also deals + /// with removing the Name validation keys based on data annotations validation for items that haven't been marked to be saved. + /// + /// + /// returns false if persistence cannot take place, returns true if persistence can take place even if there are validation errors + /// + private bool ValidateCriticalData(ContentItemSave contentItem, out int variantCount) + { + var variants = contentItem.Variants.ToList(); + variantCount = variants.Count; + var savedCount = 0; + var variantCriticalValidationErrors = new List(); + for (var i = 0; i < variants.Count; i++) + { + var variant = variants[i]; + if (variant.Save) + { + //ensure the variant has all critical required data to be persisted + if (!RequiredForPersistenceAttribute.HasRequiredValuesForPersistence(variant)) + { + variantCriticalValidationErrors.Add(variant.Culture); + //if there's no Name, it cannot be persisted at all reset the flags, this cannot be saved or published + variant.Save = variant.Publish = false; + + //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, "speechBubbles/contentCultureCriticalValidationError"); + else + return false; //It's invariant and is missing critical data, it cannot be saved + } + + savedCount++; + } + else + { + var msKey = $"Variants[{i}].Name"; + if (ModelState.ContainsKey(msKey)) + { + //if it's not being saved, remove the validation key + if (!variant.Save) ModelState.Remove(msKey); + } + } + } + + if (savedCount == variantCriticalValidationErrors.Count) + { + //in this case there can be nothing saved since all variants marked to be saved haven't passed critical validation rules + return false; + } + + return true; + } + private ContentItemDisplay PostSaveInternal(ContentItemSave contentItem, Func saveMethod) { //Recent versions of IE/Edge may send in the full client side file path instead of just the file name. @@ -616,24 +680,8 @@ namespace Umbraco.Web.Editors // * Permissions are valid MapValuesForPersistence(contentItem); - //This a custom check for any variants not being flagged for Saving since we'll need to manually - //remove the ModelState validation for the Name. - //We are also tracking which cultures have an invalid Name - var variantCount = 0; - var variantNameErrors = new List(); - foreach (var variant in contentItem.Variants) - { - var msKey = $"Variants[{variantCount}].Name"; - if (ModelState.ContainsKey(msKey)) - { - if (!variant.Save || IsCreatingAction(contentItem.Action)) - ModelState.Remove(msKey); - else - variantNameErrors.Add(variant.Culture); - } - variantCount++; - } - + var passesCriticalValidationRules = ValidateCriticalData(contentItem, out var variantCount); + //We need to manually check the validation results here because: // * We still need to save the entity even if there are validation value errors // * Depending on if the entity is new, and if there are non property validation errors (i.e. the name is null) @@ -642,30 +690,14 @@ namespace Umbraco.Web.Editors // a message indicating this if (ModelState.IsValid == false) { - //another special case, 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 (variantCount > 1) + //check for critical data validation issues, we can't continue saving if this data is invalid + if (!passesCriticalValidationRules) { - foreach (var c in variantNameErrors) - { - AddCultureValidationError(c, "speechBubbles/contentCultureValidationError"); - } - } - - if (IsCreatingAction(contentItem.Action)) - { - if (!RequiredForPersistenceAttribute.HasRequiredValuesForPersistence(contentItem) - || contentItem.Variants - .Where(x => x.Save) - .Select(RequiredForPersistenceAttribute.HasRequiredValuesForPersistence) - .Any(x => x == false)) - { - //ok, so the absolute mandatory data is invalid and it's new, we cannot actually continue! - // add the model state to the outgoing object and throw a validation message - var forDisplay = MapToDisplay(contentItem.PersistedContent); - forDisplay.Errors = ModelState.ToErrorDictionary(); - throw new HttpResponseException(Request.CreateValidationErrorResponse(forDisplay)); - } + //ok, so the absolute mandatory data is invalid and it's new, we cannot actually continue! + // add the model state to the outgoing object and throw a validation message + var forDisplay = MapToDisplay(contentItem.PersistedContent); + forDisplay.Errors = ModelState.ToErrorDictionary(); + throw new HttpResponseException(Request.CreateValidationErrorResponse(forDisplay)); } //if there's only one variant and the model state is not valid we cannot publish so change it to save @@ -689,7 +721,6 @@ namespace Umbraco.Web.Editors break; } } - } bool wasCancelled; @@ -1336,7 +1367,7 @@ namespace Umbraco.Web.Editors if (publishResult.Success == false) { var notificationModel = new SimpleNotificationModel(); - AddMessageForPublishStatus(new [] { publishResult }, notificationModel); + AddMessageForPublishStatus(new[] { publishResult }, notificationModel); return Request.CreateValidationErrorResponse(notificationModel); } diff --git a/src/Umbraco.Web/Editors/MediaController.cs b/src/Umbraco.Web/Editors/MediaController.cs index 1411e6aca4..8233d540e5 100644 --- a/src/Umbraco.Web/Editors/MediaController.cs +++ b/src/Umbraco.Web/Editors/MediaController.cs @@ -498,6 +498,7 @@ namespace Umbraco.Web.Editors if (ModelState.IsValid == false) { if (!RequiredForPersistenceAttribute.HasRequiredValuesForPersistence(contentItem) + //TODO: Why are we only doing this on SaveNew? If the Name is null, how can we ever save it even if it already exists? && (contentItem.Action == ContentSaveAction.SaveNew)) { //ok, so the absolute mandatory data is invalid and it's new, we cannot actually continue!