diff --git a/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs index 8f57842f34..c0e4cd7032 100644 --- a/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs +++ b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs @@ -167,17 +167,13 @@ namespace Umbraco.Core.Models } /// - /// This will set the publishing values for names and properties for the content/culture + /// Sets the publishing values for names and properties. /// /// /// - /// - /// A boolean if it's possible to publish the values for the provided culture. This may fail required names are not set. - /// - /// - /// This does not validation property data - /// - public static bool PublishCulture(this IContent content,string culture = "*") + /// A value indicating whether it was possible to publish the names and values for the specified + /// culture(s). The method may fail if required names are not set, but it does NOT validate property data + public static bool PublishCulture(this IContent content, string culture = "*") { culture = culture.NullOrWhiteSpaceAsNull(); diff --git a/src/Umbraco.Core/Models/ContentType.cs b/src/Umbraco.Core/Models/ContentType.cs index 63c758611a..453670253e 100644 --- a/src/Umbraco.Core/Models/ContentType.cs +++ b/src/Umbraco.Core/Models/ContentType.cs @@ -13,7 +13,7 @@ namespace Umbraco.Core.Models [DataContract(IsReference = true)] public class ContentType : ContentTypeCompositionBase, IContentType { - public const bool IsPublishingConst = true; + public const bool SupportsPublishingConst = true; private int _defaultTemplate; private IEnumerable _allowedTemplates; @@ -45,7 +45,7 @@ namespace Umbraco.Core.Models public override ISimpleContentType ToSimple() => new SimpleContentType(this); /// - public override bool IsPublishing => IsPublishingConst; + public override bool SupportsPublishing => SupportsPublishingConst; //Custom comparer for enumerable private static readonly DelegateEqualityComparer> TemplateComparer = new DelegateEqualityComparer>( diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 320f667e07..ed8a098299 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -40,7 +40,7 @@ namespace Umbraco.Core.Models // actually OK as IsPublishing is constant // ReSharper disable once VirtualMemberCallInConstructor - _noGroupPropertyTypes = new PropertyTypeCollection(IsPublishing); + _noGroupPropertyTypes = new PropertyTypeCollection(SupportsPublishing); _noGroupPropertyTypes.CollectionChanged += PropertyTypesChanged; _variations = ContentVariation.Nothing; @@ -61,7 +61,7 @@ namespace Umbraco.Core.Models // actually OK as IsPublishing is constant // ReSharper disable once VirtualMemberCallInConstructor - _noGroupPropertyTypes = new PropertyTypeCollection(IsPublishing); + _noGroupPropertyTypes = new PropertyTypeCollection(SupportsPublishing); _noGroupPropertyTypes.CollectionChanged += PropertyTypesChanged; _variations = ContentVariation.Nothing; @@ -70,7 +70,7 @@ namespace Umbraco.Core.Models public abstract ISimpleContentType ToSimple(); /// - /// Gets a value indicating whether the content type is publishing. + /// Gets a value indicating whether the content type supports publishing. /// /// /// A publishing content type supports draft and published values for properties. @@ -80,7 +80,7 @@ namespace Umbraco.Core.Models /// the draft or published value of a property returns the same thing, and publishing /// a value property has no effect. /// - public abstract bool IsPublishing { get; } + public abstract bool SupportsPublishing { get; } //Custom comparer for enumerable private static readonly DelegateEqualityComparer> ContentTypeSortComparer = @@ -257,7 +257,7 @@ namespace Umbraco.Core.Models { if (_noGroupPropertyTypes != null) _noGroupPropertyTypes.CollectionChanged -= PropertyTypesChanged; - _noGroupPropertyTypes = new PropertyTypeCollection(IsPublishing, value); + _noGroupPropertyTypes = new PropertyTypeCollection(SupportsPublishing, value); _noGroupPropertyTypes.CollectionChanged += PropertyTypesChanged; PropertyTypesChanged(_noGroupPropertyTypes, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); } diff --git a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs index ec6f803107..7497c100bc 100644 --- a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs @@ -226,7 +226,7 @@ namespace Umbraco.Core.Models return null; // create the new group - var group = new PropertyGroup(IsPublishing) { Name = name, SortOrder = 0 }; + var group = new PropertyGroup(SupportsPublishing) { Name = name, SortOrder = 0 }; // check if it is inherited - there might be more than 1 but we want the 1st, to // reuse its sort order - if there are more than 1 and they have different sort diff --git a/src/Umbraco.Core/Models/MediaType.cs b/src/Umbraco.Core/Models/MediaType.cs index f7598b4965..2f087979f7 100644 --- a/src/Umbraco.Core/Models/MediaType.cs +++ b/src/Umbraco.Core/Models/MediaType.cs @@ -10,7 +10,7 @@ namespace Umbraco.Core.Models [DataContract(IsReference = true)] public class MediaType : ContentTypeCompositionBase, IMediaType { - public const bool IsPublishingConst = false; + public const bool SupportsPublishingConst = false; /// /// Constuctor for creating a MediaType with the parent's id. @@ -45,7 +45,7 @@ namespace Umbraco.Core.Models public override ISimpleContentType ToSimple() => new SimpleContentType(this); /// - public override bool IsPublishing => IsPublishingConst; + public override bool SupportsPublishing => SupportsPublishingConst; /// IMediaType IMediaType.DeepCloneWithResetIdentities(string newAlias) => (IMediaType)DeepCloneWithResetIdentities(newAlias); diff --git a/src/Umbraco.Core/Models/MemberType.cs b/src/Umbraco.Core/Models/MemberType.cs index e85aa0e15e..4633775a43 100644 --- a/src/Umbraco.Core/Models/MemberType.cs +++ b/src/Umbraco.Core/Models/MemberType.cs @@ -11,7 +11,7 @@ namespace Umbraco.Core.Models [DataContract(IsReference = true)] public class MemberType : ContentTypeCompositionBase, IMemberType { - public const bool IsPublishingConst = false; + public const bool SupportsPublishingConst = false; //Dictionary is divided into string: PropertyTypeAlias, Tuple: MemberCanEdit, VisibleOnProfile, PropertyTypeId private string _alias; @@ -35,7 +35,7 @@ namespace Umbraco.Core.Models public override ISimpleContentType ToSimple() => new SimpleContentType(this); /// - public override bool IsPublishing => IsPublishingConst; + public override bool SupportsPublishing => SupportsPublishingConst; public override ContentVariation Variations { diff --git a/src/Umbraco.Core/Models/Property.cs b/src/Umbraco.Core/Models/Property.cs index 803b8eb5a5..76349823ac 100644 --- a/src/Umbraco.Core/Models/Property.cs +++ b/src/Umbraco.Core/Models/Property.cs @@ -184,7 +184,7 @@ namespace Umbraco.Core.Models { if (pvalue == null) return null; - return PropertyType.IsPublishing + return PropertyType.SupportsPublishing ? (published ? pvalue.PublishedValue : pvalue.EditedValue) : pvalue.EditedValue; } @@ -244,7 +244,7 @@ namespace Umbraco.Core.Models { if (pvalue == null) return; - if (!PropertyType.IsPublishing) + if (!PropertyType.SupportsPublishing) throw new NotSupportedException("Property type does not support publishing."); var origValue = pvalue.PublishedValue; pvalue.PublishedValue = PropertyType.ConvertAssignedValue(pvalue.EditedValue); @@ -255,7 +255,7 @@ namespace Umbraco.Core.Models { if (pvalue == null) return; - if (!PropertyType.IsPublishing) + if (!PropertyType.SupportsPublishing) throw new NotSupportedException("Property type does not support publishing."); var origValue = pvalue.PublishedValue; pvalue.PublishedValue = PropertyType.ConvertAssignedValue(null); @@ -288,7 +288,7 @@ namespace Umbraco.Core.Models { var (pvalue, _) = GetPValue(culture, segment, true); - if (published && PropertyType.IsPublishing) + if (published && PropertyType.SupportsPublishing) pvalue.PublishedValue = value; else pvalue.EditedValue = value; diff --git a/src/Umbraco.Core/Models/PropertyType.cs b/src/Umbraco.Core/Models/PropertyType.cs index e1da631868..1e950a876c 100644 --- a/src/Umbraco.Core/Models/PropertyType.cs +++ b/src/Umbraco.Core/Models/PropertyType.cs @@ -85,14 +85,19 @@ namespace Umbraco.Core.Models /// Gets a value indicating whether the content type owning this property type is publishing. /// /// - /// When set to true the Property of this Property Type will return it's published value if it has one. - /// This also affects the validation behavior of the Property when assigning values since Members and Media don't support publishing. - /// For those entity types, this will be false which means if a value is attempted to be published on a property when it's not supported - /// by the PropertyType, an exception will be thrown. - /// - /// TODO: Should we rename this and other instances of "IsPublishing" (including parameters) to be more clear? Like SupportsPublishing ? + /// A publishing content type supports draft and published values for properties. + /// It is possible to retrieve either the draft (default) or published value of a property. + /// Setting the value always sets the draft value, which then needs to be published. + /// A non-publishing content type only supports one value for properties. Getting + /// the draft or published value of a property returns the same thing, and publishing + /// a value property has no effect. + /// When true, getting the property value returns the edited value by default, but + /// it is possible to get the published value using the appropriate 'published' method + /// parameter. + /// When false, getting the property value always return the edited value, + /// regardless of the 'published' method parameter. /// - public bool IsPublishing { get; internal set; } + public bool SupportsPublishing { get; internal set; } /// /// Gets of sets the name of the property type. diff --git a/src/Umbraco.Core/Models/PropertyTypeCollection.cs b/src/Umbraco.Core/Models/PropertyTypeCollection.cs index e79015d828..6181ee078b 100644 --- a/src/Umbraco.Core/Models/PropertyTypeCollection.cs +++ b/src/Umbraco.Core/Models/PropertyTypeCollection.cs @@ -23,18 +23,18 @@ namespace Umbraco.Core.Models [IgnoreDataMember] internal Action OnAdd; - internal PropertyTypeCollection(bool isPublishing) + internal PropertyTypeCollection(bool supportsPublishing) { - IsPublishing = isPublishing; + SupportsPublishing = supportsPublishing; } - public PropertyTypeCollection(bool isPublishing, IEnumerable properties) - : this(isPublishing) + public PropertyTypeCollection(bool supportsPublishing, IEnumerable properties) + : this(supportsPublishing) { Reset(properties); } - public bool IsPublishing { get; } + public bool SupportsPublishing { get; } /// /// Resets the collection to only contain the instances referenced in the parameter. @@ -51,7 +51,7 @@ namespace Umbraco.Core.Models protected override void SetItem(int index, PropertyType item) { - item.IsPublishing = IsPublishing; + item.SupportsPublishing = SupportsPublishing; base.SetItem(index, item); OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, index)); } @@ -65,7 +65,7 @@ namespace Umbraco.Core.Models protected override void InsertItem(int index, PropertyType item) { - item.IsPublishing = IsPublishing; + item.SupportsPublishing = SupportsPublishing; base.InsertItem(index, item); OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item)); } @@ -79,7 +79,7 @@ namespace Umbraco.Core.Models // TODO: Instead of 'new' this should explicitly implement one of the collection interfaces members internal new void Add(PropertyType item) { - item.IsPublishing = IsPublishing; + item.SupportsPublishing = SupportsPublishing; // TODO: this is not pretty and should be refactored try @@ -155,7 +155,7 @@ namespace Umbraco.Core.Models public object DeepClone() { - var clone = new PropertyTypeCollection(IsPublishing); + var clone = new PropertyTypeCollection(SupportsPublishing); foreach (var propertyType in this) clone.Add((PropertyType) propertyType.DeepClone()); return clone; diff --git a/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs b/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs index 8e3c7db3b0..c7ce98a89c 100644 --- a/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs @@ -78,7 +78,7 @@ namespace Umbraco.Core.Persistence.Factories var propertyGroups = new PropertyGroupCollection(); foreach (var groupDto in dto.PropertyTypeGroups.Where(x => x.Id.HasValue)) { - var group = new PropertyGroup(MemberType.IsPublishingConst); + var group = new PropertyGroup(MemberType.SupportsPublishingConst); // if the group is defined on the current member type, // assign its identifier, else it will be zero @@ -93,7 +93,7 @@ namespace Umbraco.Core.Persistence.Factories group.Key = groupDto.UniqueId; group.Name = groupDto.Text; group.SortOrder = groupDto.SortOrder; - group.PropertyTypes = new PropertyTypeCollection(MemberType.IsPublishingConst); + group.PropertyTypes = new PropertyTypeCollection(MemberType.SupportsPublishingConst); //Because we are likely to have a group with no PropertyTypes we need to ensure that these are excluded var localGroupDto = groupDto; diff --git a/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs b/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs index d83650d798..f1473b5888 100644 --- a/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs @@ -121,7 +121,7 @@ namespace Umbraco.Core.Persistence.Factories foreach (var property in properties) { - if (property.PropertyType.IsPublishing) + if (property.PropertyType.SupportsPublishing) { //create the resulting hashset if it's not created and the entity varies by culture if (entityVariesByCulture && editedCultures == null) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs index a9bb097346..095c77eb42 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs @@ -25,7 +25,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement _templateRepository = templateRepository; } - protected override bool IsPublishing => ContentType.IsPublishingConst; + protected override bool SupportsPublishing => ContentType.SupportsPublishingConst; protected override IRepositoryCachePolicy CreateCachePolicy() { @@ -60,11 +60,11 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (ids.Any()) { //NOTE: This logic should never be executed according to our cache policy - return ContentTypeQueryMapper.GetContentTypes(Database, SqlSyntax, IsPublishing, this, _templateRepository) + return ContentTypeQueryMapper.GetContentTypes(Database, SqlSyntax, SupportsPublishing, this, _templateRepository) .Where(x => ids.Contains(x.Id)); } - return ContentTypeQueryMapper.GetContentTypes(Database, SqlSyntax, IsPublishing, this, _templateRepository); + return ContentTypeQueryMapper.GetContentTypes(Database, SqlSyntax, SupportsPublishing, this, _templateRepository); } protected override IEnumerable PerformGetAll(params Guid[] ids) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 7f41d2a456..e9b952f11d 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -32,7 +32,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement : base(scopeAccessor, cache, logger) { } - protected abstract bool IsPublishing { get; } + protected abstract bool SupportsPublishing { get; } public IEnumerable> Move(TEntity moving, EntityContainer container) { @@ -1021,7 +1021,7 @@ AND umbracoNode.id <> @id", var dtos = Database .Fetch(sql); - var propertyGroups = PropertyGroupFactory.BuildEntity(dtos, IsPublishing, id, createDate, updateDate,CreatePropertyType); + var propertyGroups = PropertyGroupFactory.BuildEntity(dtos, SupportsPublishing, id, createDate, updateDate,CreatePropertyType); return new PropertyGroupCollection(propertyGroups); } @@ -1057,7 +1057,7 @@ AND umbracoNode.id <> @id", //Reset dirty properties Parallel.ForEach(list, currentFile => currentFile.ResetDirtyProperties(false)); - return new PropertyTypeCollection(IsPublishing, list); + return new PropertyTypeCollection(SupportsPublishing, list); } protected void ValidateAlias(PropertyType pt) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/MediaTypeRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/MediaTypeRepository.cs index 5287af353a..a6f39bdb71 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/MediaTypeRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/MediaTypeRepository.cs @@ -20,7 +20,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement : base(scopeAccessor, cache, logger) { } - protected override bool IsPublishing => MediaType.IsPublishingConst; + protected override bool SupportsPublishing => MediaType.SupportsPublishingConst; protected override IRepositoryCachePolicy CreateCachePolicy() { @@ -55,11 +55,11 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (ids.Any()) { //NOTE: This logic should never be executed according to our cache policy - return ContentTypeQueryMapper.GetMediaTypes(Database, SqlSyntax, IsPublishing, this) + return ContentTypeQueryMapper.GetMediaTypes(Database, SqlSyntax, SupportsPublishing, this) .Where(x => ids.Contains(x.Id)); } - return ContentTypeQueryMapper.GetMediaTypes(Database, SqlSyntax, IsPublishing, this); + return ContentTypeQueryMapper.GetMediaTypes(Database, SqlSyntax, SupportsPublishing, this); } protected override IEnumerable PerformGetAll(params Guid[] ids) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/MemberTypeRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/MemberTypeRepository.cs index 22a5059be7..afb6ac8b43 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/MemberTypeRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/MemberTypeRepository.cs @@ -21,7 +21,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement : base(scopeAccessor, cache, logger) { } - protected override bool IsPublishing => MemberType.IsPublishingConst; + protected override bool SupportsPublishing => MemberType.SupportsPublishingConst; protected override IRepositoryCachePolicy CreateCachePolicy() { diff --git a/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs b/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs index 096b80de1f..c4380f032c 100644 --- a/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs +++ b/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs @@ -290,7 +290,7 @@ namespace Umbraco.Core.PropertyEditors /// public IEnumerable ConvertDbToXml(Property property, IDataTypeService dataTypeService, ILocalizationService localizationService, bool published) { - published &= property.PropertyType.IsPublishing; + published &= property.PropertyType.SupportsPublishing; var nodeName = property.PropertyType.Alias.ToSafeAlias(); diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 412f2a1160..5bdc0959da 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -1029,7 +1029,7 @@ namespace Umbraco.Core.Services.Implement /// This MUST NOT be called from within this service, this used to be a public API and must only be used outside of this service. /// Internally in this service, calls must be made to CommitDocumentChangesInternal /// - /// + /// /// This is the underlying logic for both publishing and unpublishing any document /// Pending publishing/unpublishing changes on a document are made with calls to and /// . @@ -1068,7 +1068,7 @@ namespace Umbraco.Core.Services.Implement if (saveEventArgs == null) throw new ArgumentNullException(nameof(saveEventArgs)); var evtMsgs = saveEventArgs.Messages; - + PublishResult publishResult = null; PublishResult unpublishResult = null; @@ -1094,7 +1094,6 @@ namespace Umbraco.Core.Services.Implement var changeType = isNew ? TreeChangeTypes.RefreshNode : TreeChangeTypes.RefreshBranch; var previouslyPublished = content.HasIdentity && content.Published; - if (publishing) { culturesUnpublishing = content.GetCulturesUnpublishing(); @@ -1130,7 +1129,7 @@ namespace Umbraco.Core.Services.Implement // but: (a) it means we don't reproduce the PublishState logic here and (b) setting the // PublishState to anything other than Publishing or Unpublishing - which is precisely // what we want to do here - throws - content.Published = content.Published; + content.Published = content.Published; } } @@ -1420,6 +1419,7 @@ namespace Umbraco.Core.Services.Implement } } + // utility 'PublishCultures' func used by SaveAndPublishBranch private bool SaveAndPublishBranch_PublishCultures(IContent content, HashSet culturesToPublish) { //TODO: This does not support being able to return invalid property details to bubble up to the UI @@ -1431,7 +1431,8 @@ namespace Umbraco.Core.Services.Implement : content.PublishCulture() && _propertyValidationService.Value.IsPropertyDataValid(content, out _); } - private HashSet SaveAndPublishBranch_ShouldPublish3(ref HashSet cultures, string c, bool published, bool edited, bool isRoot, bool force) + // utility 'ShouldPublish' func used by SaveAndPublishBranch + private HashSet SaveAndPublishBranch_ShouldPublish(ref HashSet cultures, string c, bool published, bool edited, bool isRoot, bool force) { // if published, republish if (published) @@ -1449,7 +1450,6 @@ namespace Umbraco.Core.Services.Implement return cultures; } - /// public IEnumerable SaveAndPublishBranch(IContent content, bool force, string culture = "*", int userId = Constants.Security.SuperUserId) { @@ -1468,10 +1468,10 @@ namespace Umbraco.Core.Services.Implement HashSet culturesToPublish = null; if (!c.ContentType.VariesByCulture()) // invariant content type - return SaveAndPublishBranch_ShouldPublish3(ref culturesToPublish, "*", c.Published, c.Edited, isRoot, force); + return SaveAndPublishBranch_ShouldPublish(ref culturesToPublish, "*", c.Published, c.Edited, isRoot, force); if (culture != "*") // variant content type, specific culture - return SaveAndPublishBranch_ShouldPublish3(ref culturesToPublish, culture, c.IsCulturePublished(culture), c.IsCultureEdited(culture), isRoot, force); + return SaveAndPublishBranch_ShouldPublish(ref culturesToPublish, culture, c.IsCulturePublished(culture), c.IsCultureEdited(culture), isRoot, force); // variant content type, all cultures if (c.Published) @@ -1479,7 +1479,7 @@ namespace Umbraco.Core.Services.Implement // then some (and maybe all) cultures will be 'already published' (unless forcing), // others will have to 'republish this culture' foreach (var x in c.AvailableCultures) - SaveAndPublishBranch_ShouldPublish3(ref culturesToPublish, x, c.IsCulturePublished(x), c.IsCultureEdited(x), isRoot, force); + SaveAndPublishBranch_ShouldPublish(ref culturesToPublish, x, c.IsCulturePublished(x), c.IsCultureEdited(x), isRoot, force); return culturesToPublish; } @@ -1508,7 +1508,7 @@ namespace Umbraco.Core.Services.Implement HashSet culturesToPublish = null; if (!c.ContentType.VariesByCulture()) // invariant content type - return SaveAndPublishBranch_ShouldPublish3(ref culturesToPublish, "*", c.Published, c.Edited, isRoot, force); + return SaveAndPublishBranch_ShouldPublish(ref culturesToPublish, "*", c.Published, c.Edited, isRoot, force); // variant content type, specific cultures if (c.Published) @@ -1516,7 +1516,7 @@ namespace Umbraco.Core.Services.Implement // then some (and maybe all) cultures will be 'already published' (unless forcing), // others will have to 'republish this culture' foreach (var x in cultures) - SaveAndPublishBranch_ShouldPublish3(ref culturesToPublish, x, c.IsCulturePublished(x), c.IsCultureEdited(x), isRoot, force); + SaveAndPublishBranch_ShouldPublish(ref culturesToPublish, x, c.IsCulturePublished(x), c.IsCultureEdited(x), isRoot, force); return culturesToPublish; } @@ -1529,7 +1529,6 @@ namespace Umbraco.Core.Services.Implement return SaveAndPublishBranch(content, force, ShouldPublish, SaveAndPublishBranch_PublishCultures, userId); } - /// internal IEnumerable SaveAndPublishBranch(IContent document, bool force, Func> shouldPublish, Func, bool> publishCultures, @@ -1554,7 +1553,7 @@ namespace Umbraco.Core.Services.Implement throw new InvalidOperationException("Cannot mix PublishCulture and SaveAndPublishBranch."); // deal with the branch root - if it fails, abort - var result = SaveAndPublishBranchOne(scope, document, shouldPublish, publishCultures, true, publishedDocuments, evtMsgs, userId); + var result = SaveAndPublishBranchItem(scope, document, shouldPublish, publishCultures, true, publishedDocuments, evtMsgs, userId); if (result != null) { results.Add(result); @@ -1585,7 +1584,7 @@ namespace Umbraco.Core.Services.Implement } // no need to check path here, parent has to be published here - result = SaveAndPublishBranchOne(scope, d, shouldPublish, publishCultures, false, publishedDocuments, evtMsgs, userId); + result = SaveAndPublishBranchItem(scope, d, shouldPublish, publishCultures, false, publishedDocuments, evtMsgs, userId); if (result != null) { results.Add(result); @@ -1615,7 +1614,7 @@ namespace Umbraco.Core.Services.Implement // shouldPublish: a function determining whether the document has changes that need to be published // note - 'force' is handled by 'editing' // publishValues: a function publishing values (using the appropriate PublishCulture calls) - private PublishResult SaveAndPublishBranchOne(IScope scope, IContent document, + private PublishResult SaveAndPublishBranchItem(IScope scope, IContent document, Func> shouldPublish, Func, bool> publishCultures, bool isRoot, @@ -1637,7 +1636,7 @@ namespace Umbraco.Core.Services.Implement { //TODO: Based on this callback behavior there is no way to know which properties may have been invalid if this failed, see other results of FailedPublishContentInvalid return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, document); - } + } var result = CommitDocumentChangesInternal(scope, document, saveEventArgs, userId, branchOne: true, branchRoot: isRoot); if (result.Success) @@ -2614,7 +2613,6 @@ namespace Umbraco.Core.Services.Implement return new PublishResult(PublishResultType.SuccessPublishCulture, evtMsgs, content); } - Logger.Info("Document {ContentName} (id={ContentId}) has been published.", content.Name, content.Id); return new PublishResult(evtMsgs, content); } diff --git a/src/Umbraco.Core/Services/Implement/MemberService.cs b/src/Umbraco.Core/Services/Implement/MemberService.cs index 45a6dc7dc1..8c69664712 100644 --- a/src/Umbraco.Core/Services/Implement/MemberService.cs +++ b/src/Umbraco.Core/Services/Implement/MemberService.cs @@ -1175,7 +1175,7 @@ namespace Umbraco.Core.Services.Implement var identity = int.MaxValue; var memType = new MemberType(-1); - var propGroup = new PropertyGroup(MemberType.IsPublishingConst) + var propGroup = new PropertyGroup(MemberType.SupportsPublishingConst) { Name = "Membership", Id = --identity diff --git a/src/Umbraco.Core/Services/PropertyValidationService.cs b/src/Umbraco.Core/Services/PropertyValidationService.cs index 538ec323e9..146173dd5c 100644 --- a/src/Umbraco.Core/Services/PropertyValidationService.cs +++ b/src/Umbraco.Core/Services/PropertyValidationService.cs @@ -76,6 +76,7 @@ namespace Umbraco.Core.Services // (including ensuring that the value exists, if mandatory) if ((culture == null || culture == "*") && (segment == null || segment == "*") && property.PropertyType.SupportsVariation(null, null)) { + // validate pvalue (which is the invariant value) pvalue = property.Values.FirstOrDefault(x => x.Culture == null && x.Segment == null); if (!IsValidPropertyValue(property, pvalue?.EditedValue)) return false; @@ -92,17 +93,18 @@ namespace Umbraco.Core.Services // for anything else, validate the existing values (including mandatory), // but we cannot validate mandatory globally (we don't know the possible cultures and segments) - var vvalues = property.Values.Count > (pvalue == null ? 0 : 1) - ? property.Values.Where(x => x != pvalue).ToDictionary(x => new CompositeNStringNStringKey(x.Culture, x.Segment), x => x) - : null; + // validate vvalues (which are the variant values) - if (vvalues == null) return culture == "*" || IsValidPropertyValue(property,null); + // if we don't have vvalues (property.Values is empty or only contains pvalue), validate null + if (property.Values.Count == (pvalue == null ? 0 : 1)) + return culture == "*" || IsValidPropertyValue(property, null); - var pvalues = vvalues.Where(x => - property.PropertyType.SupportsVariation(x.Value.Culture, x.Value.Segment, true) && // the value variation is ok - (culture == "*" || x.Value.Culture.InvariantEquals(culture)) && // the culture matches - (segment == "*" || x.Value.Segment.InvariantEquals(segment))) // the segment matches - .Select(x => x.Value) + // else validate vvalues (but don't revalidate pvalue) + var pvalues = property.Values.Where(x => + x != pvalue && // don't revalidate pvalue + property.PropertyType.SupportsVariation(x.Culture, x.Segment, true) && // the value variation is ok + (culture == "*" || x.Culture.InvariantEquals(culture)) && // the culture matches + (segment == "*" || x.Segment.InvariantEquals(segment))) // the segment matches .ToList(); return pvalues.Count == 0 || pvalues.All(x => IsValidPropertyValue(property, x.EditedValue)); diff --git a/src/Umbraco.Tests/Models/VariationTests.cs b/src/Umbraco.Tests/Models/VariationTests.cs index 798c2f1cf3..5569ecfa60 100644 --- a/src/Umbraco.Tests/Models/VariationTests.cs +++ b/src/Umbraco.Tests/Models/VariationTests.cs @@ -135,7 +135,7 @@ namespace Umbraco.Tests.Models Assert.Throws(() => prop.PublishValues()); // change - propertyType.IsPublishing = true; + propertyType.SupportsPublishing = true; // can get value // and now published value is null @@ -384,14 +384,15 @@ namespace Umbraco.Tests.Models content.SetCultureName("hello", langFr); - Assert.IsTrue(content.PublishCulture(langFr)); + Assert.IsTrue(content.PublishCulture(langFr)); // succeeds because names are ok (not validating properties here) Assert.IsFalse(propertyValidationService.IsPropertyDataValid(content, out _, langFr));// fails because prop1 is mandatory content.SetValue("prop1", "a", langFr); - Assert.IsTrue(content.PublishCulture(langFr)); + Assert.IsTrue(content.PublishCulture(langFr)); // succeeds because names are ok (not validating properties here) Assert.IsFalse(propertyValidationService.IsPropertyDataValid(content, out _, langFr));// fails because prop2 is mandatory and invariant content.SetValue("prop2", "x"); - Assert.IsTrue(content.PublishCulture(langFr)); // now it's ok + Assert.IsTrue(content.PublishCulture(langFr)); // still ok... + Assert.IsTrue(propertyValidationService.IsPropertyDataValid(content, out _, langFr));// now it's ok Assert.AreEqual("a", content.GetValue("prop1", langFr, published: true)); Assert.AreEqual("x", content.GetValue("prop2", published: true)); @@ -484,7 +485,7 @@ namespace Umbraco.Tests.Models [Test] public void ValidationTests() { - var propertyType = new PropertyType("editor", ValueStorageType.Nvarchar) { Alias = "prop", IsPublishing = true }; + var propertyType = new PropertyType("editor", ValueStorageType.Nvarchar) { Alias = "prop", SupportsPublishing = true }; var prop = new Property(propertyType); prop.SetValue("a"); diff --git a/src/Umbraco.Tests/Services/ContentServiceEventTests.cs b/src/Umbraco.Tests/Services/ContentServiceEventTests.cs index 7342665379..0cb301fcad 100644 --- a/src/Umbraco.Tests/Services/ContentServiceEventTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceEventTests.cs @@ -279,10 +279,8 @@ namespace Umbraco.Tests.Services Assert.IsFalse(result.Success); Assert.AreEqual("title", result.InvalidProperties.First().Alias); - //TODO: The ContentService doesn't reset the document's PublishedState so since the above fails, if we then try to do - // a SaveAndPublish again, we will get an exception: "Cannot save-and-publish (un)publishing content, use the dedicated CommitDocumentChanges method." - // but this exception is misleading and is caused because the document's PublishedState wasn't reset. - // So instead, we'll just re-create it. + // when a service operation fails, the object is dirty and should not be re-used, + // re-create it document = new Content("content", -1, contentType); void OnSaving(IContentService sender, ContentSavingEventArgs e) diff --git a/src/Umbraco.Web/Models/Mapping/ContentTypeMapperProfile.cs b/src/Umbraco.Web/Models/Mapping/ContentTypeMapperProfile.cs index 6653877d24..cc2c7b0cb9 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentTypeMapperProfile.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentTypeMapperProfile.cs @@ -163,7 +163,7 @@ namespace Umbraco.Web.Models.Mapping .IgnoreEntityCommonProperties() - .ForMember(dest => dest.IsPublishing, opt => opt.Ignore()) + .ForMember(dest => dest.SupportsPublishing, opt => opt.Ignore()) // see note above - have to do this here? .ForMember(dest => dest.PropertyEditorAlias, opt => opt.Ignore())