Changes controller to be explicit about which culture will be affiliated with invariant property issues

This commit is contained in:
Shannon
2019-04-04 21:57:49 +11:00
parent 389ee7ec84
commit 92b64b2ca6
5 changed files with 99 additions and 40 deletions

View File

@@ -1,4 +1,5 @@
using System;
using System.Linq;
namespace Umbraco.Core.Models
{
@@ -11,6 +12,31 @@ namespace Umbraco.Core.Models
/// </remarks>
internal class CultureImpact
{
/// <summary>
/// Utility method to return the culture used for invariant property errors based on what cultures are being actively saved,
/// the default culture and the state of the current content item
/// </summary>
/// <param name="content"></param>
/// <param name="savingCultures"></param>
/// <param name="defaultCulture"></param>
/// <returns></returns>
public static string GetCultureForInvariantErrors(IContent content, string[] savingCultures, string defaultCulture)
{
if (content == null) throw new ArgumentNullException(nameof(content));
if (savingCultures == null) throw new ArgumentNullException(nameof(savingCultures));
if (savingCultures.Length == 0) throw new ArgumentException(nameof(savingCultures));
if (defaultCulture == null) throw new ArgumentNullException(nameof(defaultCulture));
var cultureForInvariantErrors = savingCultures.Any(x => x.InvariantEquals(defaultCulture))
//the default culture is being flagged for saving so use it
? defaultCulture
//If the content has no published version, we need to affiliate validation with the first variant being saved.
//If the content has a published version we will not affiliate the validation with any culture (null)
: !content.Published ? savingCultures[0] : null;
return cultureForInvariantErrors;
}
/// <summary>
/// Initializes a new instance of the <see cref="CultureImpact"/> class.
/// </summary>

View File

@@ -1,4 +1,5 @@
using NUnit.Framework;
using Moq;
using NUnit.Framework;
using Umbraco.Core.Models;
namespace Umbraco.Tests.Models
@@ -6,6 +7,29 @@ namespace Umbraco.Tests.Models
[TestFixture]
public class CultureImpactTests
{
[Test]
public void Get_Culture_For_Invariant_Errors()
{
var result = CultureImpact.GetCultureForInvariantErrors(
Mock.Of<IContent>(x => x.Published == true),
new[] { "en-US", "fr-FR" },
"en-US");
Assert.AreEqual("en-US", result); //default culture is being saved so use it
result = CultureImpact.GetCultureForInvariantErrors(
Mock.Of<IContent>(x => x.Published == false),
new[] { "fr-FR" },
"en-US");
Assert.AreEqual("fr-FR", result); //default culture not being saved with not published version, use the first culture being saved
result = CultureImpact.GetCultureForInvariantErrors(
Mock.Of<IContent>(x => x.Published == true),
new[] { "fr-FR" },
"en-US");
Assert.AreEqual(null, result); //default culture not being saved with published version, use null
}
[Test]
public void All_Cultures()
{

View File

@@ -22,7 +22,7 @@ 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);
var result = ms.GetCulturesWithErrors(localizationService.Object, "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);
@@ -31,7 +31,7 @@ namespace Umbraco.Tests.Web
ms = new ModelStateDictionary();
ms.AddCultureValidationError("en-US", "generic culture error");
result = ms.GetCulturesWithErrors(localizationService.Object);
result = ms.GetCulturesWithErrors(localizationService.Object, "en-US");
Assert.AreEqual(1, result.Count);
Assert.AreEqual("en-US", result[0]);
@@ -47,7 +47,7 @@ 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);
var result = ms.GetCulturesWithPropertyErrors(localizationService.Object, "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);

View File

@@ -664,11 +664,19 @@ namespace Umbraco.Web.Editors
[string.Empty] = globalNotifications
};
//The default validation language will be either: The default languauge, else if the content is brand new and the default culture is
// not marked to be saved, it will be the first culture in the list marked for saving.
var defaultCulture = _allLangs.Value.First(x => x.Value.IsDefault).Key;
var cultureForInvariantErrors = CultureImpact.GetCultureForInvariantErrors(
contentItem.PersistedContent,
contentItem.Variants.Where(x => x.Save).Select(x => x.Culture).ToArray(),
defaultCulture);
switch (contentItem.Action)
{
case ContentSaveAction.Save:
case ContentSaveAction.SaveNew:
SaveAndNotify(contentItem, saveMethod, variantCount, notifications, globalNotifications, "editContentSavedText", "editVariantSavedText", out wasCancelled);
SaveAndNotify(contentItem, saveMethod, variantCount, notifications, globalNotifications, "editContentSavedText", "editVariantSavedText", cultureForInvariantErrors, out wasCancelled);
break;
case ContentSaveAction.Schedule:
case ContentSaveAction.ScheduleNew:
@@ -678,7 +686,7 @@ namespace Umbraco.Web.Editors
wasCancelled = false;
break;
}
SaveAndNotify(contentItem, saveMethod, variantCount, notifications, globalNotifications, "editContentScheduledSavedText", "editVariantSavedText", out wasCancelled);
SaveAndNotify(contentItem, saveMethod, variantCount, notifications, globalNotifications, "editContentScheduledSavedText", "editVariantSavedText", cultureForInvariantErrors, out wasCancelled);
break;
case ContentSaveAction.SendPublish:
@@ -689,7 +697,7 @@ namespace Umbraco.Web.Editors
{
if (variantCount > 1)
{
var cultureErrors = ModelState.GetCulturesWithErrors(Services.LocalizationService);
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())
{
AddSuccessNotification(notifications, c,
@@ -708,7 +716,7 @@ namespace Umbraco.Web.Editors
case ContentSaveAction.Publish:
case ContentSaveAction.PublishNew:
{
var publishStatus = PublishInternal(contentItem, out wasCancelled, out var successfulCultures);
var publishStatus = PublishInternal(contentItem, defaultCulture, cultureForInvariantErrors, out wasCancelled, out var successfulCultures);
//global notifications
AddMessageForPublishStatus(new[] { publishStatus }, globalNotifications, successfulCultures);
//variant specific notifications
@@ -728,7 +736,7 @@ namespace Umbraco.Web.Editors
break;
}
var publishStatus = PublishBranchInternal(contentItem, false, out wasCancelled, out var successfulCultures).ToList();
var publishStatus = PublishBranchInternal(contentItem, false, cultureForInvariantErrors, out wasCancelled, out var successfulCultures).ToList();
//global notifications
AddMessageForPublishStatus(publishStatus, globalNotifications, successfulCultures);
@@ -749,7 +757,7 @@ namespace Umbraco.Web.Editors
break;
}
var publishStatus = PublishBranchInternal(contentItem, true, out wasCancelled, out var successfulCultures).ToList();
var publishStatus = PublishBranchInternal(contentItem, true, cultureForInvariantErrors, out wasCancelled, out var successfulCultures).ToList();
//global notifications
AddMessageForPublishStatus(publishStatus, globalNotifications, successfulCultures);
@@ -774,7 +782,7 @@ namespace Umbraco.Web.Editors
}
//lastly, if it is not valid, add the model state to the outgoing object and throw a 400
HandleInvalidModelState(display);
HandleInvalidModelState(display, cultureForInvariantErrors);
if (wasCancelled)
{
@@ -857,6 +865,8 @@ namespace Umbraco.Web.Editors
return true;
}
/// <summary>
/// Helper method to perform the saving of the content and add the notifications to the result
/// </summary>
@@ -873,7 +883,7 @@ namespace Umbraco.Web.Editors
/// </remarks>
private void SaveAndNotify(ContentItemSave contentItem, Func<IContent, OperationResult> saveMethod, int variantCount,
Dictionary<string, SimpleNotificationModel> notifications, SimpleNotificationModel globalNotifications,
string invariantSavedLocalizationKey, string variantSavedLocalizationKey,
string invariantSavedLocalizationKey, string variantSavedLocalizationKey, string cultureForInvariantErrors,
out bool wasCancelled)
{
var saveResult = saveMethod(contentItem.PersistedContent);
@@ -882,7 +892,7 @@ namespace Umbraco.Web.Editors
{
if (variantCount > 1)
{
var cultureErrors = ModelState.GetCulturesWithErrors(Services.LocalizationService);
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())
{
AddSuccessNotification(notifications, c,
@@ -1124,7 +1134,7 @@ namespace Umbraco.Web.Editors
return denied.Count == 0;
}
private IEnumerable<PublishResult> PublishBranchInternal(ContentItemSave contentItem, bool force,
private IEnumerable<PublishResult> PublishBranchInternal(ContentItemSave contentItem, bool force, string cultureForInvariantErrors,
out bool wasCancelled, out string[] successfulCultures)
{
if (!contentItem.PersistedContent.ContentType.VariesByCulture())
@@ -1142,7 +1152,7 @@ namespace Umbraco.Web.Editors
var mandatoryCultures = _allLangs.Value.Values.Where(x => x.IsMandatory).Select(x => x.IsoCode).ToList();
var cultureErrors = ModelState.GetCulturesWithErrors(Services.LocalizationService);
var cultureErrors = ModelState.GetCulturesWithErrors(Services.LocalizationService, cultureForInvariantErrors);
//validate if we can publish based on the mandatory language requirements
var canPublish = ValidatePublishingMandatoryLanguages(
@@ -1198,7 +1208,7 @@ namespace Umbraco.Web.Editors
/// <remarks>
/// If this is a culture variant than we need to do some validation, if it's not we'll publish as normal
/// </remarks>
private PublishResult PublishInternal(ContentItemSave contentItem, out bool wasCancelled, out string[] successfulCultures)
private PublishResult PublishInternal(ContentItemSave contentItem, string defaultCulture, string cultureForInvariantErrors, out bool wasCancelled, out string[] successfulCultures)
{
if (!contentItem.PersistedContent.ContentType.VariesByCulture())
{
@@ -1214,7 +1224,7 @@ namespace Umbraco.Web.Editors
var mandatoryCultures = _allLangs.Value.Values.Where(x => x.IsMandatory).Select(x => x.IsoCode).ToList();
var cultureErrors = ModelState.GetCulturesWithErrors(Services.LocalizationService);
var cultureErrors = ModelState.GetCulturesWithErrors(Services.LocalizationService, cultureForInvariantErrors);
//validate if we can publish based on the mandatory languages selected
var canPublish = ValidatePublishingMandatoryLanguages(
@@ -1243,7 +1253,7 @@ namespace Umbraco.Web.Editors
{
//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);
canPublish = PublishCulture(contentItem.PersistedContent, cultureVariants, defaultCulture);
}
if (canPublish)
@@ -1336,12 +1346,12 @@ namespace Umbraco.Web.Editors
/// <remarks>
/// This would generally never fail unless someone is tampering with the request
/// </remarks>
private bool PublishCulture(IContent persistentContent, IEnumerable<ContentVariantSave> cultureVariants)
private bool PublishCulture(IContent persistentContent, IEnumerable<ContentVariantSave> cultureVariants, string defaultCulture)
{
foreach (var variant in cultureVariants.Where(x => x.Publish))
{
// publishing any culture, implies the invariant culture
var valid = persistentContent.PublishCulture(CultureImpact.Explicit(variant.Culture, IsDefaultCulture(variant.Culture)));
var valid = persistentContent.PublishCulture(CultureImpact.Explicit(variant.Culture, defaultCulture.InvariantEquals(variant.Culture)));
if (!valid)
{
AddCultureValidationError(variant.Culture, "speechBubbles/contentCultureValidationError");
@@ -1786,12 +1796,12 @@ namespace Umbraco.Web.Editors
/// <remarks>
/// This is required to wire up the validation in the save/publish dialog
/// </remarks>
private void HandleInvalidModelState(ContentItemDisplay display)
private void HandleInvalidModelState(ContentItemDisplay display, string cultureForInvariantErrors)
{
if (!ModelState.IsValid && display.Variants.Count() > 1)
{
//Add any culture specific errors here
var cultureErrors = ModelState.GetCulturesWithErrors(Services.LocalizationService);
var cultureErrors = ModelState.GetCulturesWithErrors(Services.LocalizationService, cultureForInvariantErrors);
foreach (var cultureError in cultureErrors)
{
@@ -2125,16 +2135,6 @@ namespace Umbraco.Web.Editors
}
}
/// <summary>
/// Returns true if the culture specified is the default culture
/// </summary>
/// <param name="culture"></param>
/// <returns></returns>
private bool IsDefaultCulture(string culture)
{
return _allLangs.Value.Any(x => x.Value.IsDefault && x.Key.InvariantEquals(culture));
}
/// <summary>
/// Used to map an <see cref="IContent"/> instance to a <see cref="ContentItemDisplay"/> and ensuring a language is present if required
/// </summary>

View File

@@ -4,6 +4,7 @@ using System.ComponentModel.DataAnnotations;
using System.Linq;
using System.Web.Mvc;
using Umbraco.Core;
using Umbraco.Core.Models;
using Umbraco.Core.Services;
namespace Umbraco.Web
@@ -76,9 +77,12 @@ namespace Umbraco.Web
/// </summary>
/// <param name="modelState"></param>
/// <param name="localizationService"></param>
/// <returns></returns>
/// <param name="cultureForInvariantErrors">The culture to affiliate invariant errors with</param>
/// <returns>
/// A list of cultures that have property validation errors. The default culture will be returned for any invariant property errors.
/// </returns>
internal static IReadOnlyList<string> GetCulturesWithPropertyErrors(this System.Web.Http.ModelBinding.ModelStateDictionary modelState,
ILocalizationService localizationService)
ILocalizationService localizationService, string cultureForInvariantErrors)
{
//Add any culture specific errors here
var cultureErrors = modelState.Keys
@@ -88,7 +92,8 @@ namespace Umbraco.Web
.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
//so errors for those must show up under the default lang.
.Select(x => x == "invariant" ? localizationService.GetDefaultLanguageIsoCode() : x)
.Select(x => x == "invariant" ? cultureForInvariantErrors : x)
.WhereNotNull()
.Distinct()
.ToList();
@@ -100,11 +105,14 @@ namespace Umbraco.Web
/// </summary>
/// <param name="modelState"></param>
/// <param name="localizationService"></param>
/// <returns></returns>
/// <param name="cultureForInvariantErrors">The culture to affiliate invariant errors with</param>
/// <returns>
/// A list of cultures that have validation errors. The default culture will be returned for any invariant errors.
/// </returns>
internal static IReadOnlyList<string> GetCulturesWithErrors(this System.Web.Http.ModelBinding.ModelStateDictionary modelState,
ILocalizationService localizationService)
ILocalizationService localizationService, string cultureForInvariantErrors)
{
var propertyCultureErrors = modelState.GetCulturesWithPropertyErrors(localizationService);
var propertyCultureErrors = modelState.GetCulturesWithPropertyErrors(localizationService, cultureForInvariantErrors);
//now check the other special culture errors that are
var genericCultureErrors = modelState.Keys
@@ -113,12 +121,13 @@ namespace Umbraco.Web
.Where(x => !x.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" ? localizationService.GetDefaultLanguageIsoCode() : x)
.Select(x => x == "invariant" ? cultureForInvariantErrors : x)
.WhereNotNull()
.Distinct();
return propertyCultureErrors.Union(genericCultureErrors).ToList();
}
/// <summary>
/// Adds the error to model state correctly for a property so we can use it on the client side.
/// </summary>