From 8e0912cbf1f6270d2dcccdb653b57237b4dcf234 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Tue, 1 Apr 2025 15:49:49 +0200 Subject: [PATCH] Only prevent the unpublish or delete of a related item when configured to do so if it is related as a child, not as a parent (#18886) * Only prevent the unpubkish or delete of a related item when configured to do so if it is related as a child, not as a parent. * Fixed incorect parameter names. * Fixed failing integration tests. * Use using variable instead to reduce nesting * Applied suggestions from code review. * Used simple using statement throughout RelationService for consistency. * Applied XML header comments consistently. --------- Co-authored-by: mole --- .../Models/RelationDirectionFilter.cs | 12 + .../Services/ContentEditingServiceBase.cs | 4 +- .../Services/ContentPublishingService.cs | 2 +- src/Umbraco.Core/Services/ContentService.cs | 2 +- src/Umbraco.Core/Services/IRelationService.cs | 12 + src/Umbraco.Core/Services/RelationService.cs | 472 ++++++++---------- .../ContentEditingServiceTests.Delete.cs | 40 +- ...entEditingServiceTests.MoveToRecycleBin.cs | 32 +- .../Services/ContentEditingServiceTests.cs | 8 + ...ContentPublishingServiceTests.Unpublish.cs | 44 +- .../Services/ContentPublishingServiceTests.cs | 4 +- 11 files changed, 343 insertions(+), 289 deletions(-) create mode 100644 src/Umbraco.Core/Models/RelationDirectionFilter.cs diff --git a/src/Umbraco.Core/Models/RelationDirectionFilter.cs b/src/Umbraco.Core/Models/RelationDirectionFilter.cs new file mode 100644 index 0000000000..1a71f8e070 --- /dev/null +++ b/src/Umbraco.Core/Models/RelationDirectionFilter.cs @@ -0,0 +1,12 @@ +namespace Umbraco.Cms.Core.Models; + +/// +/// Definition of relation directions used as a filter when requesting if a given item has relations. +/// +[Flags] +public enum RelationDirectionFilter +{ + Parent = 1, + Child = 2, + Any = Parent | Child +} diff --git a/src/Umbraco.Core/Services/ContentEditingServiceBase.cs b/src/Umbraco.Core/Services/ContentEditingServiceBase.cs index 63232f7262..08d43e7235 100644 --- a/src/Umbraco.Core/Services/ContentEditingServiceBase.cs +++ b/src/Umbraco.Core/Services/ContentEditingServiceBase.cs @@ -1,4 +1,4 @@ -using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Models; @@ -198,7 +198,7 @@ internal abstract class ContentEditingServiceBase(status, content)); } - if (disabledWhenReferenced && _relationService.IsRelated(content.Id)) + if (disabledWhenReferenced && _relationService.IsRelated(content.Id, RelationDirectionFilter.Child)) { return Attempt.FailWithStatus(referenceFailStatus, content); } diff --git a/src/Umbraco.Core/Services/ContentPublishingService.cs b/src/Umbraco.Core/Services/ContentPublishingService.cs index 4b81f8df49..f5c4a11cb5 100644 --- a/src/Umbraco.Core/Services/ContentPublishingService.cs +++ b/src/Umbraco.Core/Services/ContentPublishingService.cs @@ -311,7 +311,7 @@ internal sealed class ContentPublishingService : IContentPublishingService return Attempt.Fail(ContentPublishingOperationStatus.ContentNotFound); } - if (_contentSettings.DisableUnpublishWhenReferenced && _relationService.IsRelated(content.Id)) + if (_contentSettings.DisableUnpublishWhenReferenced && _relationService.IsRelated(content.Id, RelationDirectionFilter.Child)) { scope.Complete(); return Attempt.Fail(ContentPublishingOperationStatus.CannotUnpublishWhenReferenced); diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index 9b42428938..f20c3df4af 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -2767,7 +2767,7 @@ public class ContentService : RepositoryService, IContentService { foreach (IContent content in contents) { - if (_contentSettings.DisableDeleteWhenReferenced && _relationService.IsRelated(content.Id)) + if (_contentSettings.DisableDeleteWhenReferenced && _relationService.IsRelated(content.Id, RelationDirectionFilter.Child)) { continue; } diff --git a/src/Umbraco.Core/Services/IRelationService.cs b/src/Umbraco.Core/Services/IRelationService.cs index 676e7ea17c..e556b2c3e4 100644 --- a/src/Umbraco.Core/Services/IRelationService.cs +++ b/src/Umbraco.Core/Services/IRelationService.cs @@ -297,8 +297,20 @@ public interface IRelationService : IService /// /// Id of an object to check relations for /// Returns True if any relations exists with the given Id, otherwise False + [Obsolete("Please use the overload taking a RelationDirectionFilter parameter. Scheduled for removal in Umbraco 17.")] bool IsRelated(int id); + /// + /// Checks whether any relations exists for the passed in Id and direction. + /// + /// Id of an object to check relations for + /// Indicates whether to check for relations as parent, child or in either direction. + /// Returns True if any relations exists with the given Id, otherwise False + bool IsRelated(int id, RelationDirectionFilter directionFilter) +#pragma warning disable CS0618 // Type or member is obsolete + => IsRelated(id); +#pragma warning restore CS0618 // Type or member is obsolete + /// /// Checks whether two items are related /// diff --git a/src/Umbraco.Core/Services/RelationService.cs b/src/Umbraco.Core/Services/RelationService.cs index 6c54ee6e54..ad419ade86 100644 --- a/src/Umbraco.Core/Services/RelationService.cs +++ b/src/Umbraco.Core/Services/RelationService.cs @@ -63,28 +63,22 @@ public class RelationService : RepositoryService, IRelationService /// public IRelation? GetById(int id) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - return _relationRepository.Get(id); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + return _relationRepository.Get(id); } /// public IRelationType? GetRelationTypeById(int id) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - return _relationTypeRepository.Get(id); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + return _relationTypeRepository.Get(id); } /// public IRelationType? GetRelationTypeById(Guid id) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - return _relationTypeRepository.Get(id); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + return _relationTypeRepository.Get(id); } /// @@ -93,10 +87,8 @@ public class RelationService : RepositoryService, IRelationService /// public IEnumerable GetAllRelations(params int[] ids) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - return _relationRepository.GetMany(ids); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + return _relationRepository.GetMany(ids); } /// @@ -106,20 +98,16 @@ public class RelationService : RepositoryService, IRelationService /// public IEnumerable GetAllRelationsByRelationType(int relationTypeId) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.RelationTypeId == relationTypeId); - return _relationRepository.Get(query); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + IQuery query = Query().Where(x => x.RelationTypeId == relationTypeId); + return _relationRepository.Get(query); } /// public IEnumerable GetAllRelationTypes(params int[] ids) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - return _relationTypeRepository.GetMany(ids); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + return _relationTypeRepository.GetMany(ids); } /// @@ -144,10 +132,8 @@ public class RelationService : RepositoryService, IRelationService .Take(take)); } - public int CountRelationTypes() - { - return _relationTypeRepository.Count(null); - } + /// + public int CountRelationTypes() => _relationTypeRepository.Count(null); /// public IEnumerable GetByParentId(int id) => GetByParentId(id, null); @@ -155,24 +141,22 @@ public class RelationService : RepositoryService, IRelationService /// public IEnumerable GetByParentId(int id, string? relationTypeAlias) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + if (relationTypeAlias.IsNullOrWhiteSpace()) { - if (relationTypeAlias.IsNullOrWhiteSpace()) - { - IQuery qry1 = Query().Where(x => x.ParentId == id); - return _relationRepository.Get(qry1); - } - - IRelationType? relationType = GetRelationType(relationTypeAlias!); - if (relationType == null) - { - return Enumerable.Empty(); - } - - IQuery qry2 = - Query().Where(x => x.ParentId == id && x.RelationTypeId == relationType.Id); - return _relationRepository.Get(qry2); + IQuery qry1 = Query().Where(x => x.ParentId == id); + return _relationRepository.Get(qry1); } + + IRelationType? relationType = GetRelationType(relationTypeAlias!); + if (relationType == null) + { + return Enumerable.Empty(); + } + + IQuery qry2 = + Query().Where(x => x.ParentId == id && x.RelationTypeId == relationType.Id); + return _relationRepository.Get(qry2); } /// @@ -188,24 +172,22 @@ public class RelationService : RepositoryService, IRelationService /// public IEnumerable GetByChildId(int id, string? relationTypeAlias) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + if (relationTypeAlias.IsNullOrWhiteSpace()) { - if (relationTypeAlias.IsNullOrWhiteSpace()) - { - IQuery qry1 = Query().Where(x => x.ChildId == id); - return _relationRepository.Get(qry1); - } - - IRelationType? relationType = GetRelationType(relationTypeAlias!); - if (relationType == null) - { - return Enumerable.Empty(); - } - - IQuery qry2 = - Query().Where(x => x.ChildId == id && x.RelationTypeId == relationType.Id); - return _relationRepository.Get(qry2); + IQuery qry1 = Query().Where(x => x.ChildId == id); + return _relationRepository.Get(qry1); } + + IRelationType? relationType = GetRelationType(relationTypeAlias!); + if (relationType == null) + { + return Enumerable.Empty(); + } + + IQuery qry2 = + Query().Where(x => x.ChildId == id && x.RelationTypeId == relationType.Id); + return _relationRepository.Get(qry2); } /// @@ -218,39 +200,34 @@ public class RelationService : RepositoryService, IRelationService /// public IEnumerable GetByParentOrChildId(int id) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.ChildId == id || x.ParentId == id); - return _relationRepository.Get(query); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + IQuery query = Query().Where(x => x.ChildId == id || x.ParentId == id); + return _relationRepository.Get(query); } + /// public IEnumerable GetByParentOrChildId(int id, string relationTypeAlias) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + IRelationType? relationType = GetRelationType(relationTypeAlias); + if (relationType == null) { - IRelationType? relationType = GetRelationType(relationTypeAlias); - if (relationType == null) - { - return Enumerable.Empty(); - } - - IQuery query = Query().Where(x => - (x.ChildId == id || x.ParentId == id) && x.RelationTypeId == relationType.Id); - return _relationRepository.Get(query); + return Enumerable.Empty(); } + + IQuery query = Query().Where(x => + (x.ChildId == id || x.ParentId == id) && x.RelationTypeId == relationType.Id); + return _relationRepository.Get(query); } /// public IRelation? GetByParentAndChildId(int parentId, int childId, IRelationType relationType) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.ParentId == parentId && - x.ChildId == childId && - x.RelationTypeId == relationType.Id); - return _relationRepository.Get(query).FirstOrDefault(); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + IQuery query = Query().Where(x => x.ParentId == parentId && + x.ChildId == childId && + x.RelationTypeId == relationType.Id); + return _relationRepository.Get(query).FirstOrDefault(); } /// @@ -283,47 +260,41 @@ public class RelationService : RepositoryService, IRelationService /// public IEnumerable GetByRelationTypeId(int relationTypeId) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.RelationTypeId == relationTypeId); - return _relationRepository.Get(query); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + IQuery query = Query().Where(x => x.RelationTypeId == relationTypeId); + return _relationRepository.Get(query); } /// public IEnumerable GetPagedByRelationTypeId(int relationTypeId, long pageIndex, int pageSize, out long totalRecords, Ordering? ordering = null) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery? query = Query().Where(x => x.RelationTypeId == relationTypeId); - return _relationRepository.GetPagedRelationsByQuery(query, pageIndex, pageSize, out totalRecords, ordering); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + IQuery? query = Query().Where(x => x.RelationTypeId == relationTypeId); + return _relationRepository.GetPagedRelationsByQuery(query, pageIndex, pageSize, out totalRecords, ordering); } + /// public async Task> GetPagedByChildKeyAsync(Guid childKey, int skip, int take, string? relationTypeAlias) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - return await _relationRepository.GetPagedByChildKeyAsync(childKey, skip, take, relationTypeAlias); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + return await _relationRepository.GetPagedByChildKeyAsync(childKey, skip, take, relationTypeAlias); } + /// public async Task, RelationOperationStatus>> GetPagedByRelationTypeKeyAsync(Guid key, int skip, int take, Ordering? ordering = null) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + IRelationType? relationType = _relationTypeRepository.Get(key); + if (relationType is null) { - IRelationType? relationType = _relationTypeRepository.Get(key); - if (relationType is null) - { - return await Task.FromResult(Attempt.FailWithStatus, RelationOperationStatus>(RelationOperationStatus.RelationTypeNotFound, null!)); - } - - PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageNumber, out var pageSize); - - IQuery query = Query().Where(x => x.RelationTypeId == relationType.Id); - IEnumerable relations = _relationRepository.GetPagedRelationsByQuery(query, pageNumber, pageSize, out var totalRecords, ordering); - return await Task.FromResult(Attempt.SucceedWithStatus(RelationOperationStatus.Success, new PagedModel(totalRecords, relations))); + return await Task.FromResult(Attempt.FailWithStatus, RelationOperationStatus>(RelationOperationStatus.RelationTypeNotFound, null!)); } + + PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageNumber, out var pageSize); + + IQuery query = Query().Where(x => x.RelationTypeId == relationType.Id); + IEnumerable relations = _relationRepository.GetPagedRelationsByQuery(query, pageNumber, pageSize, out var totalRecords, ordering); + return await Task.FromResult(Attempt.SucceedWithStatus(RelationOperationStatus.Success, new PagedModel(totalRecords, relations))); } /// @@ -394,19 +365,15 @@ public class RelationService : RepositoryService, IRelationService /// public IEnumerable GetPagedParentEntitiesByChildId(int id, long pageIndex, int pageSize, out long totalChildren, params UmbracoObjectTypes[] entityTypes) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - return _relationRepository.GetPagedParentEntitiesByChildId(id, pageIndex, pageSize, out totalChildren, entityTypes.Select(x => x.GetGuid()).ToArray()); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + return _relationRepository.GetPagedParentEntitiesByChildId(id, pageIndex, pageSize, out totalChildren, entityTypes.Select(x => x.GetGuid()).ToArray()); } /// public IEnumerable GetPagedChildEntitiesByParentId(int id, long pageIndex, int pageSize, out long totalChildren, params UmbracoObjectTypes[] entityTypes) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - return _relationRepository.GetPagedChildEntitiesByParentId(id, pageIndex, pageSize, out totalChildren, entityTypes.Select(x => x.GetGuid()).ToArray()); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + return _relationRepository.GetPagedChildEntitiesByParentId(id, pageIndex, pageSize, out totalChildren, entityTypes.Select(x => x.GetGuid()).ToArray()); } /// @@ -440,22 +407,20 @@ public class RelationService : RepositoryService, IRelationService // TODO: We don't check if this exists first, it will throw some sort of data integrity exception if it already exists, is that ok? var relation = new Relation(parentId, childId, relationType); - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + EventMessages eventMessages = EventMessagesFactory.Get(); + var savingNotification = new RelationSavingNotification(relation, eventMessages); + if (scope.Notifications.PublishCancelable(savingNotification)) { - EventMessages eventMessages = EventMessagesFactory.Get(); - var savingNotification = new RelationSavingNotification(relation, eventMessages); - if (scope.Notifications.PublishCancelable(savingNotification)) - { - scope.Complete(); - return relation; // TODO: returning sth that does not exist here?! - } - - _relationRepository.Save(relation); - scope.Notifications.Publish( - new RelationSavedNotification(relation, eventMessages).WithStateFrom(savingNotification)); scope.Complete(); - return relation; + return relation; // TODO: returning sth that does not exist here?! } + + _relationRepository.Save(relation); + scope.Notifications.Publish( + new RelationSavedNotification(relation, eventMessages).WithStateFrom(savingNotification)); + scope.Complete(); + return relation; } /// @@ -491,31 +456,37 @@ public class RelationService : RepositoryService, IRelationService /// public bool HasRelations(IRelationType relationType) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.RelationTypeId == relationType.Id); - return _relationRepository.Get(query).Any(); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + IQuery query = Query().Where(x => x.RelationTypeId == relationType.Id); + return _relationRepository.Get(query).Any(); } /// - public bool IsRelated(int id) + public bool IsRelated(int id) => IsRelated(id, RelationDirectionFilter.Any); + + /// + public bool IsRelated(int id, RelationDirectionFilter directionFilter) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + IQuery query = Query(); + + query = directionFilter switch { - IQuery query = Query().Where(x => x.ParentId == id || x.ChildId == id); - return _relationRepository.Get(query).Any(); - } + RelationDirectionFilter.Parent => query.Where(x => x.ParentId == id), + RelationDirectionFilter.Child => query.Where(x => x.ChildId == id), + RelationDirectionFilter.Any => query.Where(x => x.ParentId == id || x.ChildId == id), + _ => throw new ArgumentOutOfRangeException(nameof(directionFilter)), + }; + + return _relationRepository.Get(query).Any(); } /// public bool AreRelated(int parentId, int childId) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.ParentId == parentId && x.ChildId == childId); - return _relationRepository.Get(query).Any(); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + IQuery query = Query().Where(x => x.ParentId == parentId && x.ChildId == childId); + return _relationRepository.Get(query).Any(); } /// @@ -540,65 +511,61 @@ public class RelationService : RepositoryService, IRelationService /// public void Save(IRelation relation) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + EventMessages eventMessages = EventMessagesFactory.Get(); + var savingNotification = new RelationSavingNotification(relation, eventMessages); + if (scope.Notifications.PublishCancelable(savingNotification)) { - EventMessages eventMessages = EventMessagesFactory.Get(); - var savingNotification = new RelationSavingNotification(relation, eventMessages); - if (scope.Notifications.PublishCancelable(savingNotification)) - { - scope.Complete(); - return; - } - - _relationRepository.Save(relation); scope.Complete(); - scope.Notifications.Publish( - new RelationSavedNotification(relation, eventMessages).WithStateFrom(savingNotification)); + return; } + + _relationRepository.Save(relation); + scope.Complete(); + scope.Notifications.Publish( + new RelationSavedNotification(relation, eventMessages).WithStateFrom(savingNotification)); } + /// public void Save(IEnumerable relations) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + IRelation[] relationsA = relations.ToArray(); + + EventMessages messages = EventMessagesFactory.Get(); + var savingNotification = new RelationSavingNotification(relationsA, messages); + if (scope.Notifications.PublishCancelable(savingNotification)) { - IRelation[] relationsA = relations.ToArray(); - - EventMessages messages = EventMessagesFactory.Get(); - var savingNotification = new RelationSavingNotification(relationsA, messages); - if (scope.Notifications.PublishCancelable(savingNotification)) - { - scope.Complete(); - return; - } - - _relationRepository.Save(relationsA); scope.Complete(); - scope.Notifications.Publish( - new RelationSavedNotification(relationsA, messages).WithStateFrom(savingNotification)); + return; } + + _relationRepository.Save(relationsA); + scope.Complete(); + scope.Notifications.Publish( + new RelationSavedNotification(relationsA, messages).WithStateFrom(savingNotification)); } /// public void Save(IRelationType relationType) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + EventMessages eventMessages = EventMessagesFactory.Get(); + var savingNotification = new RelationTypeSavingNotification(relationType, eventMessages); + if (scope.Notifications.PublishCancelable(savingNotification)) { - EventMessages eventMessages = EventMessagesFactory.Get(); - var savingNotification = new RelationTypeSavingNotification(relationType, eventMessages); - if (scope.Notifications.PublishCancelable(savingNotification)) - { - scope.Complete(); - return; - } - - _relationTypeRepository.Save(relationType); - Audit(AuditType.Save, Constants.Security.SuperUserId, relationType.Id, $"Saved relation type: {relationType.Name}"); scope.Complete(); - scope.Notifications.Publish( - new RelationTypeSavedNotification(relationType, eventMessages).WithStateFrom(savingNotification)); + return; } + + _relationTypeRepository.Save(relationType); + Audit(AuditType.Save, Constants.Security.SuperUserId, relationType.Id, $"Saved relation type: {relationType.Name}"); + scope.Complete(); + scope.Notifications.Publish( + new RelationTypeSavedNotification(relationType, eventMessages).WithStateFrom(savingNotification)); } + /// public async Task> CreateAsync(IRelationType relationType, Guid userKey) { if (relationType.Id != 0) @@ -614,6 +581,7 @@ public class RelationService : RepositoryService, IRelationService userKey); } + /// public async Task> UpdateAsync(IRelationType relationType, Guid userKey) => await SaveAsync( relationType, @@ -669,105 +637,97 @@ public class RelationService : RepositoryService, IRelationService /// public void Delete(IRelation relation) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + EventMessages eventMessages = EventMessagesFactory.Get(); + var deletingNotification = new RelationDeletingNotification(relation, eventMessages); + if (scope.Notifications.PublishCancelable(deletingNotification)) { - EventMessages eventMessages = EventMessagesFactory.Get(); - var deletingNotification = new RelationDeletingNotification(relation, eventMessages); - if (scope.Notifications.PublishCancelable(deletingNotification)) - { - scope.Complete(); - return; - } - - _relationRepository.Delete(relation); scope.Complete(); - scope.Notifications.Publish( - new RelationDeletedNotification(relation, eventMessages).WithStateFrom(deletingNotification)); + return; } + + _relationRepository.Delete(relation); + scope.Complete(); + scope.Notifications.Publish( + new RelationDeletedNotification(relation, eventMessages).WithStateFrom(deletingNotification)); } /// public void Delete(IRelationType relationType) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + EventMessages eventMessages = EventMessagesFactory.Get(); + var deletingNotification = new RelationTypeDeletingNotification(relationType, eventMessages); + if (scope.Notifications.PublishCancelable(deletingNotification)) { - EventMessages eventMessages = EventMessagesFactory.Get(); - var deletingNotification = new RelationTypeDeletingNotification(relationType, eventMessages); - if (scope.Notifications.PublishCancelable(deletingNotification)) - { - scope.Complete(); - return; - } - - _relationTypeRepository.Delete(relationType); scope.Complete(); - scope.Notifications.Publish( - new RelationTypeDeletedNotification(relationType, eventMessages).WithStateFrom(deletingNotification)); + return; } + + _relationTypeRepository.Delete(relationType); + scope.Complete(); + scope.Notifications.Publish( + new RelationTypeDeletedNotification(relationType, eventMessages).WithStateFrom(deletingNotification)); } + /// public async Task> DeleteAsync(Guid key, Guid userKey) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + IRelationType? relationType = _relationTypeRepository.Get(key); + if (relationType is null) { - IRelationType? relationType = _relationTypeRepository.Get(key); - if (relationType is null) - { - return Attempt.FailWithStatus(RelationTypeOperationStatus.NotFound, null); - } - - EventMessages eventMessages = EventMessagesFactory.Get(); - var deletingNotification = new RelationTypeDeletingNotification(relationType, eventMessages); - if (scope.Notifications.PublishCancelable(deletingNotification)) - { - scope.Complete(); - return Attempt.FailWithStatus(RelationTypeOperationStatus.CancelledByNotification, null); - } - - _relationTypeRepository.Delete(relationType); - var currentUser = await _userIdKeyResolver.GetAsync(userKey); - Audit(AuditType.Delete, currentUser, relationType.Id, "Deleted relation type"); - scope.Notifications.Publish(new RelationTypeDeletedNotification(relationType, eventMessages).WithStateFrom(deletingNotification)); - scope.Complete(); - return await Task.FromResult(Attempt.SucceedWithStatus(RelationTypeOperationStatus.Success, relationType)); + return Attempt.FailWithStatus(RelationTypeOperationStatus.NotFound, null); } + + EventMessages eventMessages = EventMessagesFactory.Get(); + var deletingNotification = new RelationTypeDeletingNotification(relationType, eventMessages); + if (scope.Notifications.PublishCancelable(deletingNotification)) + { + scope.Complete(); + return Attempt.FailWithStatus(RelationTypeOperationStatus.CancelledByNotification, null); + } + + _relationTypeRepository.Delete(relationType); + var currentUser = await _userIdKeyResolver.GetAsync(userKey); + Audit(AuditType.Delete, currentUser, relationType.Id, "Deleted relation type"); + scope.Notifications.Publish(new RelationTypeDeletedNotification(relationType, eventMessages).WithStateFrom(deletingNotification)); + scope.Complete(); + return await Task.FromResult(Attempt.SucceedWithStatus(RelationTypeOperationStatus.Success, relationType)); } /// public void DeleteRelationsOfType(IRelationType relationType) { var relations = new List(); - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + IQuery query = Query().Where(x => x.RelationTypeId == relationType.Id); + var allRelations = _relationRepository.Get(query).ToList(); + relations.AddRange(allRelations); + + // TODO: N+1, we should be able to do this in a single call + foreach (IRelation relation in relations) { - IQuery query = Query().Where(x => x.RelationTypeId == relationType.Id); - var allRelations = _relationRepository.Get(query).ToList(); - relations.AddRange(allRelations); - - // TODO: N+1, we should be able to do this in a single call - foreach (IRelation relation in relations) - { - _relationRepository.Delete(relation); - } - - scope.Complete(); - - scope.Notifications.Publish(new RelationDeletedNotification(relations, EventMessagesFactory.Get())); + _relationRepository.Delete(relation); } + + scope.Complete(); + + scope.Notifications.Publish(new RelationDeletedNotification(relations, EventMessagesFactory.Get())); } + /// public bool AreRelated(int parentId, int childId, IRelationType relationType) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => - x.ParentId == parentId && x.ChildId == childId && x.RelationTypeId == relationType.Id); - return _relationRepository.Get(query).Any(); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + IQuery query = Query().Where(x => + x.ParentId == parentId && x.ChildId == childId && x.RelationTypeId == relationType.Id); + return _relationRepository.Get(query).Any(); } + /// public IEnumerable GetAllowedObjectTypes() => - new[] - { + [ UmbracoObjectTypes.Document, UmbracoObjectTypes.Media, UmbracoObjectTypes.Member, @@ -778,17 +738,15 @@ public class RelationService : RepositoryService, IRelationService UmbracoObjectTypes.MemberGroup, UmbracoObjectTypes.ROOT, UmbracoObjectTypes.RecycleBin, - }; + ]; #region Private Methods private IRelationType? GetRelationType(string relationTypeAlias) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.Alias == relationTypeAlias); - return _relationTypeRepository.Get(query).FirstOrDefault(); - } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); + IQuery query = Query().Where(x => x.Alias == relationTypeAlias); + return _relationTypeRepository.Get(query).FirstOrDefault(); } private IEnumerable GetRelationsByListOfTypeIds(IEnumerable relationTypeIds) diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.Delete.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.Delete.cs index 93395a768e..dd4a2452e4 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.Delete.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.Delete.cs @@ -1,8 +1,7 @@ -using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection; using NUnit.Framework; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Configuration.Models; -using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.OperationStatus; using Umbraco.Cms.Tests.Integration.Attributes; @@ -12,29 +11,20 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; public partial class ContentEditingServiceTests { protected IRelationService RelationService => GetRequiredService(); + public static void ConfigureDisableDeleteWhenReferenced(IUmbracoBuilder builder) - { - builder.Services.Configure(config => + => builder.Services.Configure(config => config.DisableDeleteWhenReferenced = true); - } - - public void Relate(IContent child, IContent parent) - { - var relatedContentRelType = RelationService.GetRelationTypeByAlias(Constants.Conventions.RelationTypes.RelatedDocumentAlias); - - var relation = RelationService.Relate(child.Id, parent.Id, relatedContentRelType); - RelationService.Save(relation); - } - [Test] [ConfigureBuilder(ActionName = nameof(ConfigureDisableDeleteWhenReferenced))] - public async Task Cannot_Delete_Referenced_Content() + public async Task Cannot_Delete_When_Content_Is_Related_As_A_Child_And_Configured_To_Disable_When_Related() { var moveAttempt = await ContentEditingService.MoveToRecycleBinAsync(Subpage.Key, Constants.Security.SuperUserKey); Assert.IsTrue(moveAttempt.Success); - Relate(Subpage, Subpage2); + // Setup a relation where the page being deleted is related to another page as a child (e.g. the other page has a picker and has selected this page). + Relate(Subpage2, Subpage); var result = await ContentEditingService.DeleteFromRecycleBinAsync(Subpage.Key, Constants.Security.SuperUserKey); Assert.IsFalse(result.Success); Assert.AreEqual(ContentEditingOperationStatus.CannotDeleteWhenReferenced, result.Status); @@ -44,6 +34,24 @@ public partial class ContentEditingServiceTests Assert.IsNotNull(subpage); } + [Test] + [ConfigureBuilder(ActionName = nameof(ConfigureDisableDeleteWhenReferenced))] + public async Task Can_Delete_When_Content_Is_Related_As_A_Parent_And_Configured_To_Disable_When_Related() + { + var moveAttempt = await ContentEditingService.MoveToRecycleBinAsync(Subpage.Key, Constants.Security.SuperUserKey); + Assert.IsTrue(moveAttempt.Success); + + // Setup a relation where the page being deleted is related to another page as a child (e.g. the other page has a picker and has selected this page). + Relate(Subpage, Subpage2); + var result = await ContentEditingService.DeleteFromRecycleBinAsync(Subpage.Key, Constants.Security.SuperUserKey); + Assert.IsTrue(result.Success); + Assert.AreEqual(ContentEditingOperationStatus.Success, result.Status); + + // re-get and verify deleted + var subpage = await ContentEditingService.GetAsync(Subpage.Key); + Assert.IsNull(subpage); + } + [TestCase(true)] [TestCase(false)] public async Task Can_Delete_FromRecycleBin(bool variant) diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.MoveToRecycleBin.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.MoveToRecycleBin.cs index ac7700131f..35a5b3244f 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.MoveToRecycleBin.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.MoveToRecycleBin.cs @@ -1,4 +1,4 @@ -using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection; using NUnit.Framework; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Configuration.Models; @@ -9,12 +9,9 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; public partial class ContentEditingServiceTests { - - public static void ConfigureDisableDelete(IUmbracoBuilder builder) - { - builder.Services.Configure(config => + public static new void ConfigureDisableUnpublishWhenReferencedTrue(IUmbracoBuilder builder) + => builder.Services.Configure(config => config.DisableUnpublishWhenReferenced = true); - } [TestCase(true)] [TestCase(false)] @@ -33,10 +30,11 @@ public partial class ContentEditingServiceTests } [Test] - [ConfigureBuilder(ActionName = nameof(ConfigureDisableDelete))] - public async Task Cannot_Move_To_Recycle_Bin_If_Referenced() + [ConfigureBuilder(ActionName = nameof(ConfigureDisableUnpublishWhenReferencedTrue))] + public async Task Cannot_Move_To_Recycle_Bin_When_Content_Is_Related_As_A_Child_And_Configured_To_Disable_When_Related() { - Relate(Subpage, Subpage2); + // Setup a relation where the page being deleted is related to another page as a child (e.g. the other page has a picker and has selected this page). + Relate(Subpage2, Subpage); var moveAttempt = await ContentEditingService.MoveToRecycleBinAsync(Subpage.Key, Constants.Security.SuperUserKey); Assert.IsFalse(moveAttempt.Success); Assert.AreEqual(ContentEditingOperationStatus.CannotMoveToRecycleBinWhenReferenced, moveAttempt.Status); @@ -47,6 +45,22 @@ public partial class ContentEditingServiceTests Assert.IsFalse(content.Trashed); } + [Test] + [ConfigureBuilder(ActionName = nameof(ConfigureDisableUnpublishWhenReferencedTrue))] + public async Task Can_Move_To_Recycle_Bin_When_Content_Is_Related_As_A_Parent_And_Configured_To_Disable_When_Related() + { + // Setup a relation where the page being deleted is related to another page as a child (e.g. the other page has a picker and has selected this page). + Relate(Subpage, Subpage2); + var moveAttempt = await ContentEditingService.MoveToRecycleBinAsync(Subpage.Key, Constants.Security.SuperUserKey); + Assert.IsTrue(moveAttempt.Success); + Assert.AreEqual(ContentEditingOperationStatus.Success, moveAttempt.Status); + + // re-get and verify moved + var content = await ContentEditingService.GetAsync(Subpage.Key); + Assert.IsNotNull(content); + Assert.IsTrue(content.Trashed); + } + [Test] public async Task Cannot_Move_Non_Existing_To_Recycle_Bin() { diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.cs index 68daf63c8d..67daf08ae5 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.cs @@ -16,6 +16,14 @@ public partial class ContentEditingServiceTests : ContentEditingServiceTestsBase [SetUp] public void Setup() => ContentRepositoryBase.ThrowOnWarning = true; + public void Relate(IContent parent, IContent child) + { + var relatedContentRelType = RelationService.GetRelationTypeByAlias(Constants.Conventions.RelationTypes.RelatedDocumentAlias); + + var relation = RelationService.Relate(parent.Id, child.Id, relatedContentRelType); + RelationService.Save(relation); + } + protected override void CustomTestSetup(IUmbracoBuilder builder) => builder.AddNotificationHandler(); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Unpublish.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Unpublish.cs index 38c3c03829..c292727785 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Unpublish.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.Unpublish.cs @@ -1,14 +1,22 @@ +using Microsoft.Extensions.DependencyInjection; using NUnit.Framework; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.OperationStatus; using Umbraco.Cms.Tests.Common.Builders; using Umbraco.Cms.Tests.Common.Builders.Extensions; +using Umbraco.Cms.Tests.Integration.Attributes; namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; public partial class ContentPublishingServiceTests { + public static new void ConfigureDisableUnpublishWhenReferencedTrue(IUmbracoBuilder builder) + => builder.Services.Configure(config => + config.DisableUnpublishWhenReferenced = true); + [Test] public async Task Can_Unpublish_Root() { @@ -92,6 +100,40 @@ public partial class ContentPublishingServiceTests VerifyIsPublished(Textpage.Key); } + [Test] + [ConfigureBuilder(ActionName = nameof(ConfigureDisableUnpublishWhenReferencedTrue))] + public async Task Cannot_Unpublish_When_Content_Is_Related_As_A_Child_And_Configured_To_Disable_When_Related() + { + await ContentPublishingService.PublishAsync(Textpage.Key, MakeModel(_allCultures), Constants.Security.SuperUserKey); + VerifyIsPublished(Textpage.Key); + + // Setup a relation where the page being unpublished is related to another page as a child (e.g. the other page has a picker and has selected this page). + RelationService.Relate(Subpage, Textpage, Constants.Conventions.RelationTypes.RelatedDocumentAlias); + + var result = await ContentPublishingService.UnpublishAsync(Textpage.Key, null, Constants.Security.SuperUserKey); + + Assert.IsFalse(result.Success); + Assert.AreEqual(ContentPublishingOperationStatus.CannotUnpublishWhenReferenced, result.Result); + VerifyIsPublished(Textpage.Key); + } + + [Test] + [ConfigureBuilder(ActionName = nameof(ConfigureDisableUnpublishWhenReferencedTrue))] + public async Task Can_Unpublish_When_Content_Is_Related_As_A_Parent_And_Configured_To_Disable_When_Related() + { + await ContentPublishingService.PublishAsync(Textpage.Key, MakeModel(_allCultures), Constants.Security.SuperUserKey); + VerifyIsPublished(Textpage.Key); + + // Setup a relation where the page being unpublished is related to another page as a parent (e.g. this page has a picker and has selected the other page). + RelationService.Relate(Textpage, Subpage, Constants.Conventions.RelationTypes.RelatedDocumentAlias); + + var result = await ContentPublishingService.UnpublishAsync(Textpage.Key, null, Constants.Security.SuperUserKey); + + Assert.IsTrue(result.Success); + Assert.AreEqual(ContentPublishingOperationStatus.Success, result.Result); + VerifyIsNotPublished(Textpage.Key); + } + [Test] public async Task Can_Unpublish_Single_Culture() { @@ -229,8 +271,6 @@ public partial class ContentPublishingServiceTests Assert.AreEqual(0, content.PublishedCultures.Count()); } - - [Test] public async Task Can_Unpublish_Non_Mandatory_Cultures() { diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.cs index 6e5cf55fc7..fcfbd65b1b 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentPublishingServiceTests.cs @@ -1,4 +1,4 @@ -using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection; using NUnit.Framework; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Configuration.Models; @@ -20,6 +20,8 @@ public partial class ContentPublishingServiceTests : UmbracoIntegrationTestWithC { private IContentPublishingService ContentPublishingService => GetRequiredService(); + private IRelationService RelationService => GetRequiredService(); + private static readonly ISet _allCultures = new HashSet(){ "*" }; private static CultureAndScheduleModel MakeModel(ISet cultures) => new CultureAndScheduleModel()