diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index f1220c721f..8bf57f4270 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -186,7 +186,7 @@ namespace Umbraco.Core.Models if (_cultureInfos == null) _cultureInfos = new Dictionary(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(); } - + + /// + /// Validates the content item's properties for the provided culture/segment + /// + /// + /// + /// + /// + /// This will not perform validation for properties that do not match the required ContentVariation based on the culture/segment values provided + /// 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) diff --git a/src/Umbraco.Core/Models/IContent.cs b/src/Umbraco.Core/Models/IContent.cs index 59ff548e16..fca50c328c 100644 --- a/src/Umbraco.Core/Models/IContent.cs +++ b/src/Umbraco.Core/Models/IContent.cs @@ -167,7 +167,7 @@ namespace Umbraco.Core.Models /// A value indicating whether the values could be published. /// /// The document must then be published via the content service. - /// Values are not published if they are not valie. + /// Values are not published if they are not valid. /// //fixme return an Attempt with some error results if it doesn't work bool TryPublishAllValues(); diff --git a/src/Umbraco.Tests/Models/VariationTests.cs b/src/Umbraco.Tests/Models/VariationTests.cs index 1b9000617e..9f124b65fc 100644 --- a/src/Umbraco.Tests/Models/VariationTests.cs +++ b/src/Umbraco.Tests/Models/VariationTests.cs @@ -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); 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 a375631709..252d63b530 100644 --- a/src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml +++ b/src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml @@ -1416,7 +1416,8 @@ To manage your website, simply open the Umbraco back office and start adding con Member was exported to file An error occurred while exporting the member Cannot publish the document since the required '%0%' is not published - Validation failed for language '%0%' + Validation failed for language '%0%' + Unexpected validation failed for language '%0%' Uses CSS syntax ex: h1, .redHeader, .blueTex diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index c32daaf784..3490e7e8fa 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -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>(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 } } } + + /// + /// This will call TryPublishValues on the content item for each culture that needs to be published including the invariant culture + /// + /// + /// + /// + /// + private bool TryPublishValues(ContentItemSave contentItem, IEnumerable otherVariantsToValidate, IDictionary allLangs) + { + var culturesToPublish = new List { 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; + } /// /// Publishes a document with a given ID diff --git a/src/Umbraco.Web/Models/Mapping/PropertyTypeVariationsResolver.cs b/src/Umbraco.Web/Models/Mapping/PropertyTypeVariationsResolver.cs index 772caed964..6c0fa9915e 100644 --- a/src/Umbraco.Web/Models/Mapping/PropertyTypeVariationsResolver.cs +++ b/src/Umbraco.Web/Models/Mapping/PropertyTypeVariationsResolver.cs @@ -5,18 +5,18 @@ using ContentVariation = Umbraco.Core.Models.ContentVariation; namespace Umbraco.Web.Models.Mapping { + /// + /// Returns the for a + /// internal class PropertyTypeVariationsResolver: IValueResolver { 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; } } diff --git a/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs b/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs index 6863a267b1..cc8523b107 100644 --- a/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs +++ b/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs @@ -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.");