yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

FIxes issue where an invariant property type was being validated with a culture when that would always fail. Fixes issue when saving a content type to ensure that a property type is never both variant and invariant. Adds test.

Browse Source
This commit is contained in:
Shannon
2018-05-10 18:01:41 +10:00
parent 8be861809a
commit f2b78c06ef
7 changed files with 115 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
src/Umbraco.Core/Models/ContentBase.cs
View File

@@ -186,7 +186,7 @@ namespace Umbraco.Core.Models
if (_cultureInfos == null)
_cultureInfos = new Dictionary<string, (string Name, DateTime Date)>(StringComparer.OrdinalIgnoreCase);
_cultureInfos[culture] = (name, DateTime.Now) ;
_cultureInfos[culture] = (name, DateTime.Now);
OnPropertyChanged(Ps.Value.NamesSelector);
}
@@ -311,10 +311,30 @@ namespace Umbraco.Core.Models
{
return Properties.Where(x => !x.IsAllValid()).ToArray();
}
/// <summary>
/// Validates the content item's properties for the provided culture/segment
/// </summary>
/// <param name="culture"></param>
/// <param name="segment"></param>
/// <returns></returns>
/// <remarks>
/// This will not perform validation for properties that do not match the required ContentVariation based on the culture/segment values provided
/// </remarks>
public virtual Property[] Validate(string culture = null, string segment = null)
{
return Properties.Where(x => !x.IsValid(culture, segment)).ToArray();
return Properties.Where(x =>
{
if (!culture.IsNullOrWhiteSpace() && !x.PropertyType.Variations.HasAny(ContentVariation.CultureNeutral | ContentVariation.CultureSegment))
return false; //has a culture, this prop is only culture invariant, ignore
if (culture.IsNullOrWhiteSpace() && !x.PropertyType.Variations.HasAny(ContentVariation.InvariantNeutral | ContentVariation.InvariantSegment))
return false; //no culture, this prop is only culture variant, ignore
if (!segment.IsNullOrWhiteSpace() && !x.PropertyType.Variations.HasAny(ContentVariation.InvariantSegment | ContentVariation.CultureSegment))
return false; //has segment, this prop is only segment neutral, ignore
if (segment.IsNullOrWhiteSpace() && !x.PropertyType.Variations.HasAny(ContentVariation.InvariantNeutral | ContentVariation.CultureNeutral))
return false; //no segment, this prop is only non segment neutral, ignore
return !x.IsValid(culture, segment);
}).ToArray();
}
public virtual Property[] ValidateCulture(string culture = null)

2
src/Umbraco.Core/Models/IContent.cs
View File

@@ -167,7 +167,7 @@ namespace Umbraco.Core.Models
/// <returns>A value indicating whether the values could be published.</returns>
/// <remarks>
/// <para>The document must then be published via the content service.</para>
/// <para>Values are not published if they are not valie.</para>
/// <para>Values are not published if they are not valid.</para>
/// </remarks>
//fixme return an Attempt with some error results if it doesn't work
bool TryPublishAllValues();

41
src/Umbraco.Tests/Models/VariationTests.cs
View File

@@ -215,7 +215,7 @@ namespace Umbraco.Tests.Models
// can publish value
// and get edited and published values
content.TryPublishValues();
Assert.IsTrue(content.TryPublishValues());
Assert.AreEqual("a", content.GetValue("prop"));
Assert.AreEqual("a", content.GetValue("prop", published: true));
@@ -246,7 +246,7 @@ namespace Umbraco.Tests.Models
// and get edited and published values
Assert.IsFalse(content.TryPublishValues(langFr)); // no name
content.SetName("name-fr", langFr);
content.TryPublishValues(langFr);
Assert.IsTrue(content.TryPublishValues(langFr));
Assert.AreEqual("b", content.GetValue("prop"));
Assert.IsNull(content.GetValue("prop", published: true));
Assert.AreEqual("c", content.GetValue("prop", langFr));
@@ -260,7 +260,7 @@ namespace Umbraco.Tests.Models
Assert.IsNull(content.GetValue("prop", langFr, published: true));
// can publish all
content.TryPublishAllValues();
Assert.IsTrue(content.TryPublishAllValues());
Assert.AreEqual("b", content.GetValue("prop"));
Assert.AreEqual("b", content.GetValue("prop", published: true));
Assert.AreEqual("c", content.GetValue("prop", langFr));
@@ -300,6 +300,39 @@ namespace Umbraco.Tests.Models
Assert.AreEqual("c", content.GetValue("prop", langFr, published: true));
}
[Test]
public void ContentPublishValuesWithMixedPropertyTypeVariations()
{
const string langFr = "fr-FR";
var contentType = new ContentType(-1) { Alias = "contentType" };
contentType.Variations |= ContentVariation.CultureNeutral; //supports both variant/invariant
//In real life, a property cannot be both invariant + variant and be mandatory. If this happens validation will always fail when doing TryPublishValues since the invariant value will always be empty.
//so here we are only setting properties to one or the other, not both
var variantPropType = new PropertyType("editor", ValueStorageType.Nvarchar) { Alias = "prop1", Variations = ContentVariation.CultureNeutral, Mandatory = true };
var invariantPropType = new PropertyType("editor", ValueStorageType.Nvarchar) { Alias = "prop2", Variations = ContentVariation.InvariantNeutral, Mandatory = true};
contentType.AddPropertyType(variantPropType);
contentType.AddPropertyType(invariantPropType);
var content = new Content("content", -1, contentType) { Id = 1, VersionId = 1 };
content.SetName("hello", langFr);
Assert.IsFalse(content.TryPublishValues(langFr)); //will fail because prop1 is mandatory
content.SetValue("prop1", "a", langFr);
Assert.IsTrue(content.TryPublishValues(langFr));
Assert.AreEqual("a", content.GetValue("prop1", langFr, published: true));
//this will be null because we tried to publish values for a specific culture but this property is invariant
Assert.IsNull(content.GetValue("prop2", published: true));
Assert.IsFalse(content.TryPublishValues()); //will fail because prop2 is mandatory
content.SetValue("prop2", "b");
Assert.IsTrue(content.TryPublishValues());
Assert.AreEqual("b", content.GetValue("prop2", published: true));
}
[Test]
public void ContentPublishVariations()
{
@@ -327,7 +360,7 @@ namespace Umbraco.Tests.Models
// works with a name
// and then FR is available, and published
content.SetName("name-fr", langFr);
content.TryPublishValues(langFr);
Assert.IsTrue(content.TryPublishValues(langFr));
// now UK is available too
content.SetName("name-uk", langUk);

3
src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml
View File

@@ -1416,7 +1416,8 @@ To manage your website, simply open the Umbraco back office and start adding con
<key alias="memberExportedSuccess">Member was exported to file</key>
<key alias="memberExportedError">An error occurred while exporting the member</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="contentCultureValidationError">Validation failed for language '%0%'</key>
<key alias="contentCultureUnexpectedValidationError">Unexpected validation failed for language '%0%'</key>
</area>
<area alias="stylesheet">
<key alias="aliasHelp">Uses CSS syntax ex: h1, .redHeader, .blueTex</key>

55
src/Umbraco.Web/Editors/ContentController.cs
View File

@@ -716,11 +716,11 @@ namespace Umbraco.Web.Editors
//check if we are publishing other variants and validate them
var allLangs = Services.LocalizationService.GetAllLanguages().ToDictionary(x => x.IsoCode, x => x, StringComparer.InvariantCultureIgnoreCase);
var variantsToValidate = contentItem.PublishVariations.Where(x => !x.Culture.InvariantEquals(contentItem.Culture)).ToList();
var otherVariantsToValidate = contentItem.PublishVariations.Where(x => !x.Culture.InvariantEquals(contentItem.Culture)).ToList();
//validate any mandatory variants that are not in the list
var mandatoryLangs = Mapper.Map<IEnumerable<ILanguage>, IEnumerable<Language>>(allLangs.Values)
.Where(x => variantsToValidate.All(v => !v.Culture.InvariantEquals(x.IsoCode))) //don't include variants above
.Where(x => otherVariantsToValidate.All(v => !v.Culture.InvariantEquals(x.IsoCode))) //don't include variants above
.Where(x => !x.IsoCode.InvariantEquals(contentItem.Culture)) //don't include the current variant
.Where(x => x.Mandatory);
foreach (var lang in mandatoryLangs)
@@ -736,11 +736,13 @@ namespace Umbraco.Web.Editors
if (canPublish)
{
//validate all variants to be published
foreach (var publishVariation in variantsToValidate)
//validate all other variants to be published
foreach (var publishVariation in otherVariantsToValidate)
{
var invalid = contentItem.PersistedContent.Validate(publishVariation.Culture).Any();
if (invalid)
//validate the culture property values, we don't need to validate any invariant property values here because they will have
//been validated in the post.
var invalidProperties = contentItem.PersistedContent.Validate(publishVariation.Culture);
if (invalidProperties.Length > 0)
{
var errMsg = Services.TextService.Localize("speechBubbles/contentCultureValidationError", new[] { allLangs[publishVariation.Culture].CultureName });
ModelState.AddModelError("publish_variant_" + publishVariation.Culture + "_", errMsg);
@@ -751,14 +753,13 @@ namespace Umbraco.Web.Editors
if (canPublish)
{
//set all publish values for the variant we are publishing and those variants flagged for publishing
//we are not checking for a return value here because we've already pre-validated the property values.
contentItem.PersistedContent.TryPublishValues(contentItem.Culture);
foreach (var publishVariation in variantsToValidate)
{
contentItem.PersistedContent.TryPublishValues(publishVariation.Culture);
}
//try to publish all the values on the model
canPublish = TryPublishValues(contentItem, otherVariantsToValidate, allLangs);
}
if (canPublish)
{
//proceed to publish if all validation still succeeds
publishStatus = Services.ContentService.SaveAndPublish(contentItem.PersistedContent, Security.CurrentUser.Id);
wasCancelled = publishStatus.Result == PublishResultType.FailedCancelledByEvent;
}
@@ -771,6 +772,34 @@ namespace Umbraco.Web.Editors
}
}
}
/// <summary>
/// This will call TryPublishValues on the content item for each culture that needs to be published including the invariant culture
/// </summary>
/// <param name="contentItem"></param>
/// <param name="otherVariantsToValidate"></param>
/// <param name="allLangs"></param>
/// <returns></returns>
private bool TryPublishValues(ContentItemSave contentItem, IEnumerable<ContentVariationPublish> otherVariantsToValidate, IDictionary<string, ILanguage> allLangs)
{
var culturesToPublish = new List<string> { contentItem.Culture };
if (!contentItem.Culture.IsNullOrWhiteSpace())
culturesToPublish.Add(null); //we need to publish the invariant values if culture is specified, so we can pass in null
culturesToPublish.AddRange(otherVariantsToValidate.Select(x => x.Culture));
foreach(var culture in culturesToPublish)
{
var valid = contentItem.PersistedContent.TryPublishValues(culture);
if (!valid)
{
var errMsg = Services.TextService.Localize("speechBubbles/contentCultureUnexpectedValidationError", new[] { allLangs[culture].CultureName });
ModelState.AddModelError("publish_variant_" + culture + "_", errMsg);
return false;
}
}
return true;
}
/// <summary>
/// Publishes a document with a given ID

16
src/Umbraco.Web/Models/Mapping/PropertyTypeVariationsResolver.cs
View File

@@ -5,18 +5,18 @@ using ContentVariation = Umbraco.Core.Models.ContentVariation;
namespace Umbraco.Web.Models.Mapping
{
/// <summary>
/// Returns the <see cref="ContentVariation"/> for a <see cref="PropertyType"/>
/// </summary>
internal class PropertyTypeVariationsResolver: IValueResolver<PropertyTypeBasic, PropertyType, ContentVariation>
{
public ContentVariation Resolve(PropertyTypeBasic source, PropertyType destination, ContentVariation destMember, ResolutionContext context)
{
//this will always be the case, a content type will always be allowed to be invariant
var result = ContentVariation.InvariantNeutral;
if (source.AllowCultureVariant)
{
result |= ContentVariation.CultureNeutral;
}
//A property type should only be one type of culture variation.
//If a property type allows both variant and invariant then it generally won't be able to save because validation
//occurs when performing something like IContent.TryPublishAllValues and it will result in validation problems because
//the invariant value will not be set since in the UI only the variant values are edited if it supports it.
var result = source.AllowCultureVariant ? ContentVariation.CultureNeutral : ContentVariation.InvariantNeutral;
return result;
}
}

3
src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs
View File

@@ -58,7 +58,8 @@ namespace Umbraco.Web.WebApi.Binders
protected override bool ValidateCultureVariant(ContentItemSave postedItem, HttpActionContext actionContext)
{
var contentType = postedItem.PersistedContent.GetContentType();
if (contentType.Variations.Has(Core.Models.ContentVariation.CultureNeutral) && postedItem.Culture.IsNullOrWhiteSpace())
if (contentType.Variations.HasAny(Core.Models.ContentVariation.CultureNeutral | Core.Models.ContentVariation.CultureSegment)
&& postedItem.Culture.IsNullOrWhiteSpace())
{
//we cannot save a content item that is culture variant if no culture was specified in the request!
actionContext.Response = actionContext.Request.CreateValidationErrorResponse($"No 'Culture' found in request. Cannot save a content item that is of a {Core.Models.ContentVariation.CultureNeutral} content type without a specified culture.");