Refactored CMS Kit and added TODOs.

modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/Blogs/BlogFeatureAdminAppService.cs

@@ -32,7 +32,7 @@ namespace Volo.CmsKit.Admin.Blogs
             EventBus = eventBus;
         }
 
-        [Authorize(CmsKitAdminPermissions.Blogs.Features)]
+        [Authorize(CmsKitAdminPermissions.Blogs.Features)] //TODO: Move to the class level (all methods check the same permission)?
         public virtual async Task<List<BlogFeatureDto>> GetListAsync(Guid blogId)
         {
             var blogFeatures = await BlogFeatureManager.GetListAsync(blogId);

modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/MediaDescriptors/MediaDescriptorAdminAppService.cs

@@ -33,6 +33,7 @@ namespace Volo.CmsKit.Admin.MediaDescriptors
         {
             var definition = await MediaDescriptorDefinitionStore.GetDefinitionAsync(inputStream.EntityType);
 
+            /* TODO: Shouldn't CreatePolicies be a dictionary and we check for inputStream.EntityType? */
             await CheckAnyOfPoliciesAsync(definition.CreatePolicies);
 
             var newId = GuidGenerator.Create();
@@ -53,6 +54,7 @@ namespace Volo.CmsKit.Admin.MediaDescriptors
 
             var definition = await MediaDescriptorDefinitionStore.GetDefinitionAsync(mediaDescriptor.EntityType);
 
+            /* TODO: Shouldn't DeletePolicies be a dictionary and we check for inputStream.EntityType? */
             await CheckAnyOfPoliciesAsync(definition.DeletePolicies);
 
             await MediaContainer.DeleteAsync(id.ToString());

modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/Pages/PageAdminAppService.cs

@@ -48,8 +48,10 @@ namespace Volo.CmsKit.Admin.Pages
         [Authorize(CmsKitAdminPermissions.Pages.Create)]
         public virtual async Task<PageDto> CreateAsync(CreatePageInputDto input)
         {
+            /* TODO: This should be in a domain service, like PageManager.CreateAsync(...)
+                     Otherwise, we can't ensure the slug check is applied.
+                     The same pattern is already done for BlogManager */
             await CheckPageSlugAsync(input.Slug);
-
             var page = new Page(GuidGenerator.Create(), input.Title, input.Slug, input.Content, CurrentTenant.Id);
 
             await PageRepository.InsertAsync(page);
@@ -64,11 +66,12 @@ namespace Volo.CmsKit.Admin.Pages
 
             if (page.Slug != input.Slug)
             {
+                /* TODO: This should be in a domain service, like PageManager.SetSlugAsync(page, input.Slug) */
                 await CheckPageSlugAsync(input.Slug);
+                page.SetSlug(input.Slug);
             }
 
             page.SetTitle(input.Title);
-            page.SetSlug(input.Slug);
             page.SetContent(input.Content);
 
             await PageRepository.UpdateAsync(page);

modules/cms-kit/src/Volo.CmsKit.Domain.Shared/Volo/CmsKit/CmsKitErrorCodes.cs

@@ -8,6 +8,7 @@
             public const string EntityNotTaggable = "CmsKit:Tag:0002";
         }
 
+        // TODO: Unused? Remove, if so.
         public const string ContentAlreadyExist = "CmsKit:0002";
 
         public static class Pages

modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/Blog.cs

@@ -17,7 +17,12 @@ namespace Volo.CmsKit.Blogs
 
         public virtual Guid? TenantId { get; protected set; }
         
-        protected internal Blog(Guid id, [NotNull] string name, [NotNull] string slug, [CanBeNull] Guid? tenantId = null) : base(id)
+        protected internal Blog(
+            Guid id,
+            [NotNull] string name,
+            [NotNull] string slug,
+            [CanBeNull] Guid? tenantId = null
+            ) : base(id)
         {
             SetName(name);
             SetSlug(slug);

modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogFeature.cs

@@ -11,7 +11,7 @@ namespace Volo.CmsKit.Blogs
 
         public string FeatureName { get; protected set; }
 
-        public bool IsEnabled { get; set; } = true;
+        public bool IsEnabled { get; protected internal set; }
 
         public BlogFeature(Guid blogId, [NotNull] string featureName, bool isEnabled = true)
         {
@@ -20,6 +20,12 @@ namespace Volo.CmsKit.Blogs
             IsEnabled = isEnabled;
         }
         
+        /*
+         TODO: Overriding Equals is not a good practice for entities (see https://github.com/abpframework/abp/issues/1728)
+               Also, the implementation is not a true equal implementation. It just special comparison for a specific case
+               (used in BlogFeatureManager.GetListAsync). Remove Equals and just do the logic in-place, or create a static
+               method like BlogFeature.IsSameBlogFeature(BlogFeature other).
+        */
         public bool Equals(BlogFeature other)
         {
             return BlogId == other?.BlogId && FeatureName == other?.FeatureName;

modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogFeatureManager.cs

@@ -2,9 +2,6 @@
 using System.Collections.Generic;
 using System.Threading.Tasks;
 using Volo.Abp.Domain.Services;
-using Volo.Abp.EventBus.Distributed;
-using Volo.Abp.EventBus.Local;
-using Volo.Abp.Uow;
 
 namespace Volo.CmsKit.Blogs
 {
@@ -26,6 +23,11 @@ namespace Volo.CmsKit.Blogs
         {
             var blogFeatures = await BlogFeatureRepository.GetListAsync(blogId);
 
+            /* TODO: Creating transient entities in DefaultBlogFeatureProvider.GetDefaultFeaturesAsync
+                     is not a good idea. Returned list will contain mixed (some in db some in-memory).
+                     For example, if I delete/update one of the BlogFeature comes from in-memory,
+                     I will have an strange behaviour. You should find another way.
+             */
             var defaultFeatures = await DefaultBlogFeatureProvider.GetDefaultFeaturesAsync(blogId);
 
             defaultFeatures.ForEach(x => blogFeatures.AddIfNotContains(x));

modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogPostManager.cs

@@ -34,6 +34,12 @@ namespace Volo.CmsKit.Blogs
             Check.NotNullOrEmpty(title, nameof(title));
             Check.NotNullOrEmpty(slug, nameof(slug));
 
+            /* TODO: BlogRepository.GetAsync already throws the same exception.
+                     So, if you get Blog you don't have to check its existence.
+                     Actually, you SHOULD not check. What if I've created the Blog, but
+                     not saved to database, and I am creating a post inside it in same UOW.
+                     In this case, you throw an unnecessary exception.
+            */
             await CheckBlogExistenceAsync(blog.Id);
 
             var blogPost = new BlogPost(

modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogPostSlugAlreadyExistException.cs

@@ -6,11 +6,13 @@ namespace Volo.CmsKit.Blogs
 {
     public class BlogPostSlugAlreadyExistException : BusinessException
     {
+        //TODO: Delete this constructor, not used
         internal BlogPostSlugAlreadyExistException(string code = null, string message = null, string details = null, Exception innerException = null, Microsoft.Extensions.Logging.LogLevel logLevel = Microsoft.Extensions.Logging.LogLevel.Warning) : base(code, message, details, innerException, logLevel)
         {
         }
 
-        internal BlogPostSlugAlreadyExistException(SerializationInfo serializationInfo, System.Runtime.Serialization.StreamingContext context) : base(serializationInfo, context)
+        public BlogPostSlugAlreadyExistException(SerializationInfo serializationInfo, StreamingContext context)
+            : base(serializationInfo, context)
         {
         }
 

modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogSlugAlreadyExistException.cs

@@ -8,14 +8,13 @@ namespace Volo.CmsKit.Blogs
     public class BlogSlugAlreadyExistException : BusinessException
     {
         public BlogSlugAlreadyExistException(string slug)
+            : base(code: CmsKitErrorCodes.Blogs.SlugAlreadyExists)
         {
-            Code = CmsKitErrorCodes.Blogs.SlugAlreadyExists;
-
             WithData(nameof(Blog.Slug), slug);
         }
         
-        public BlogSlugAlreadyExistException(
-            SerializationInfo serializationInfo, StreamingContext context) : base(serializationInfo, context)
+        public BlogSlugAlreadyExistException(SerializationInfo serializationInfo, StreamingContext context) 
+            : base(serializationInfo, context)
         {
         }
     }

modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/Extensions/SlugExtensions.cs

@@ -6,7 +6,7 @@ namespace Volo.CmsKit.Blogs.Extensions
     public static class SlugExtensions
     {
         static readonly SlugHelper SlugHelper = new ();
-        public static string NormalizeSlug(this string value)
+        public static string NormalizeSlug(this string value) //TODO: Should not be an extension method.
         {
             return SlugHelper.GenerateSlug(value?.Unidecode());
         }

modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/MediaDescriptors/DefaultMediaDescriptorDefinitionStore.cs

@@ -29,17 +29,20 @@ namespace Volo.CmsKit.MediaDescriptors
         {
             Check.NotNullOrWhiteSpace(entityType, nameof(entityType));
 
-            var result = Options.EntityTypes.SingleOrDefault(x => x.EntityType.Equals(entityType, StringComparison.InvariantCultureIgnoreCase)) ??
-                         throw new EntityCantHaveMediaException(entityType);
+            var definition = Options.EntityTypes.SingleOrDefault(
+                x => x.EntityType.Equals(entityType, StringComparison.InvariantCultureIgnoreCase)
+            ) ?? throw new EntityCantHaveMediaException(entityType);
 
-            return Task.FromResult(result);
+            return Task.FromResult(definition);
         }
 
         public virtual Task<bool> IsDefinedAsync([NotNull] string entityType)
         {
             Check.NotNullOrEmpty(entityType, nameof(entityType));
 
-            var isDefined = Options.EntityTypes.Any(a => a.EntityType.Equals(entityType, StringComparison.InvariantCultureIgnoreCase));
+            var isDefined = Options.EntityTypes.Any(
+                a => a.EntityType.Equals(entityType, StringComparison.InvariantCultureIgnoreCase)
+            );
 
             return Task.FromResult(isDefined);
         }

modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/MediaDescriptors/EntityCantHaveMediaException.cs

@@ -9,10 +9,10 @@ namespace Volo.CmsKit.MediaDescriptors
         {
         }
 
-        public EntityCantHaveMediaException(string entityType) 
+        public EntityCantHaveMediaException(string entityType)
+          : base(code: CmsKitErrorCodes.MediaDescriptors.EntityTypeDoesntExist)
         {
             EntityType = entityType;
-            Code = CmsKitErrorCodes.MediaDescriptors.EntityTypeDoesntExist;
             WithData(nameof(entityType), EntityType);
         }
 

modules/cms-kit/src/Volo.CmsKit.EntityFrameworkCore/Volo/CmsKit/Blogs/EfCoreBlogPostRepository.cs

@@ -33,7 +33,7 @@ namespace Volo.CmsKit.Blogs
 
             blogPost.Author = await (await GetDbContextAsync())
                                 .Set<CmsUser>()
-                                .FirstOrDefaultAsync(x =>x.Id == blogPost.AuthorId);
+                                .FirstOrDefaultAsync(x =>x.Id == blogPost.AuthorId, GetCancellationToken(cancellationToken));
 
             return blogPost;
         }