Fixes how critical data is validated

This commit is contained in:
Shannon
2019-03-12 16:10:35 +11:00
parent 95d1ceb529
commit 5ae6a68078
5 changed files with 85 additions and 49 deletions

View File

@@ -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,

View File

@@ -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;

View File

@@ -1409,6 +1409,7 @@ To manage your website, simply open the Umbraco back office and start adding con
<key alias="resendInviteSuccess">Invitation has been re-sent to %0%</key>
<key alias="contentReqCulturePublishError">Cannot publish the document since the required '%0%' is not published</key>
<key alias="contentCultureValidationError">Validation failed for language '%0%'</key>
<key alias="contentCultureCriticalValidationError">Validation failed for language '%0%' and could not be saved</key>
<key alias="documentTypeExportedSuccess">Document type was exported to file</key>
<key alias="documentTypeExportedError">An error occurred while exporting the document type</key>
<key alias="scheduleErrReleaseDate1">The release date cannot be in the past</key>

View File

@@ -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;
}
/// <summary>
/// Validates critical data for persistence and updates the ModelState and result accordingly
/// </summary>
/// <param name="contentItem"></param>
/// <param name="variantCount">Returns the total number of variants (will be one if it's an invariant content item)</param>
/// <returns></returns>
/// <remarks>
/// 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.
/// </remarks>
/// <returns>
/// returns false if persistence cannot take place, returns true if persistence can take place even if there are validation errors
/// </returns>
private bool ValidateCriticalData(ContentItemSave contentItem, out int variantCount)
{
var variants = contentItem.Variants.ToList();
variantCount = variants.Count;
var savedCount = 0;
var variantCriticalValidationErrors = new List<string>();
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<IContent, OperationResult> 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<string>();
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);
}

View File

@@ -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!