diff --git a/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/Blogs/BlogFeatureAdminAppService.cs b/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/Blogs/BlogFeatureAdminAppService.cs index d074d1f455..3ffaeca4a6 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/Blogs/BlogFeatureAdminAppService.cs +++ b/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> GetListAsync(Guid blogId) { var blogFeatures = await BlogFeatureManager.GetListAsync(blogId); diff --git a/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/MediaDescriptors/MediaDescriptorAdminAppService.cs b/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/MediaDescriptors/MediaDescriptorAdminAppService.cs index 45774ca1f2..fca41500bf 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/MediaDescriptors/MediaDescriptorAdminAppService.cs +++ b/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()); diff --git a/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/Pages/PageAdminAppService.cs b/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/Pages/PageAdminAppService.cs index 102b2a6afd..18e57f0f23 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/Pages/PageAdminAppService.cs +++ b/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 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); diff --git a/modules/cms-kit/src/Volo.CmsKit.Domain.Shared/Volo/CmsKit/CmsKitErrorCodes.cs b/modules/cms-kit/src/Volo.CmsKit.Domain.Shared/Volo/CmsKit/CmsKitErrorCodes.cs index 064c504fb4..8ceeb7c924 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Domain.Shared/Volo/CmsKit/CmsKitErrorCodes.cs +++ b/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 diff --git a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/Blog.cs b/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/Blog.cs index 313b282a1f..7a63e7dbf7 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/Blog.cs +++ b/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); diff --git a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogFeature.cs b/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogFeature.cs index 8ec02a1875..ff73a798cb 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogFeature.cs +++ b/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; diff --git a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogFeatureManager.cs b/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogFeatureManager.cs index 809a7ab9d2..f9f4efd37a 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogFeatureManager.cs +++ b/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)); diff --git a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogPostManager.cs b/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogPostManager.cs index 3e15b91117..9a4f627da7 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogPostManager.cs +++ b/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( diff --git a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogPostSlugAlreadyExistException.cs b/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogPostSlugAlreadyExistException.cs index a2cdf68f39..c880b9e5da 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogPostSlugAlreadyExistException.cs +++ b/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) { } diff --git a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogSlugAlreadyExistException.cs b/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogSlugAlreadyExistException.cs index e50f2f623c..1adb95ac33 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/BlogSlugAlreadyExistException.cs +++ b/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) { } } diff --git a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/Extensions/SlugExtensions.cs b/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/Extensions/SlugExtensions.cs index a625d4687f..4934dc56de 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/Blogs/Extensions/SlugExtensions.cs +++ b/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()); } diff --git a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/MediaDescriptors/DefaultMediaDescriptorDefinitionStore.cs b/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/MediaDescriptors/DefaultMediaDescriptorDefinitionStore.cs index 752f921da7..e3caed9904 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/MediaDescriptors/DefaultMediaDescriptorDefinitionStore.cs +++ b/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 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); } diff --git a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/MediaDescriptors/EntityCantHaveMediaException.cs b/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/MediaDescriptors/EntityCantHaveMediaException.cs index 29d60126a1..911e8d2366 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Domain/Volo/CmsKit/MediaDescriptors/EntityCantHaveMediaException.cs +++ b/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); } diff --git a/modules/cms-kit/src/Volo.CmsKit.EntityFrameworkCore/Volo/CmsKit/Blogs/EfCoreBlogPostRepository.cs b/modules/cms-kit/src/Volo.CmsKit.EntityFrameworkCore/Volo/CmsKit/Blogs/EfCoreBlogPostRepository.cs index 81cc527800..e6e35720f6 100644 --- a/modules/cms-kit/src/Volo.CmsKit.EntityFrameworkCore/Volo/CmsKit/Blogs/EfCoreBlogPostRepository.cs +++ b/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() - .FirstOrDefaultAsync(x =>x.Id == blogPost.AuthorId); + .FirstOrDefaultAsync(x =>x.Id == blogPost.AuthorId, GetCancellationToken(cancellationToken)); return blogPost; }