Refactor IdentityUserManager & fix unit tests.

modules/identity/src/Volo.Abp.Identity.Application.Contracts/Volo/Abp/Identity/IdentityErrorCodes.cs → modules/identity/src/Volo.Abp.Identity.Domain.Shared/Volo/Abp/Identity/IdentityErrorCodes.cs

@@ -1,11 +1,8 @@
-using System;
+namespace Volo.Abp.Identity
-using System.Collections.Generic;
-using System.Text;
-
-namespace Volo.Abp.Identity
 {
     public static class IdentityErrorCodes
     {
         public const string UserSelfDeletion = "Volo.Abp.Identity:010001";
+        public const string MaxAllowedOuMembership = "Volo.Abp.Identity:010002";
     }
 }

modules/identity/src/Volo.Abp.Identity.Domain.Shared/Volo/Abp/Identity/Localization/en.json

@@ -101,6 +101,7 @@
     "Description:Abp.Identity.SignIn.EnablePhoneNumberConfirmation": "Whether the phoneNumber can be confirmed by the user.",
     "Description:Abp.Identity.SignIn.RequireConfirmedPhoneNumber": "Whether a confirmed telephone number is required to sign in.",
     "Description:Abp.Identity.User.IsUserNameUpdateEnabled": "Whether the username can be updated by the user.",
-    "Description:Abp.Identity.User.IsEmailUpdateEnabled": "Whether the email can be updated by the user."
+    "Description:Abp.Identity.User.IsEmailUpdateEnabled": "Whether the email can be updated by the user.",
+    "Volo.Abp.Identity:010002": "Can not set more than {MaxUserMembershipCount} organization unit for a user!"
   }
 }

modules/identity/src/Volo.Abp.Identity.Domain.Shared/Volo/Abp/Identity/Localization/tr.json

@@ -72,6 +72,7 @@
     "Permission:Delete": "Silme",
     "Permission:ChangePermissions": "İzinleri değiştirme",
     "Permission:UserManagement": "Kullanıcı yönetimi",
-    "Permission:UserLookup": "Kullanıcı sorgulama"
+    "Permission:UserLookup": "Kullanıcı sorgulama",
+    "Volo.Abp.Identity:010002": "Bir kullanıcı en fazla {MaxUserMembershipCount} organizasyon birimine üye olabilir!"
   }
 }

modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityUserManager.cs

@@ -100,47 +100,42 @@ namespace Volo.Abp.Identity
 
         public virtual async Task<bool> IsInOrganizationUnitAsync(Guid userId, Guid ouId)
         {
-            return await IsInOrganizationUnitAsync(
+            var user = await IdentityUserRepository.GetAsync(userId, cancellationToken: CancellationToken);
-                await GetByIdAsync(userId),
+            return user.IsInOrganizationUnit(ouId);
-                await OrganizationUnitRepository.GetAsync(ouId)
-                );
         }
 
-        public virtual Task<bool> IsInOrganizationUnitAsync(IdentityUser user, OrganizationUnit ou)
+        public virtual async Task<bool> IsInOrganizationUnitAsync(IdentityUser user, OrganizationUnit ou)
         {
-            return Task.FromResult(user.IsInOrganizationUnit(ou.Id));
+            await IdentityUserRepository.EnsureCollectionLoadedAsync(user, u => u.OrganizationUnits, CancellationTokenProvider.Token);
+            return user.IsInOrganizationUnit(ou.Id);
         }
 
         public virtual async Task AddToOrganizationUnitAsync(Guid userId, Guid ouId)
         {
             await AddToOrganizationUnitAsync(
-                await IdentityUserRepository.GetAsync(userId, true),
+                await IdentityUserRepository.GetAsync(userId, cancellationToken: CancellationToken),
-                await OrganizationUnitRepository.GetAsync(ouId)
+                await OrganizationUnitRepository.GetAsync(ouId, cancellationToken: CancellationToken)
                 );
         }
 
         public virtual async Task AddToOrganizationUnitAsync(IdentityUser user, OrganizationUnit ou)
         {
             await IdentityUserRepository.EnsureCollectionLoadedAsync(user, u => u.OrganizationUnits, CancellationTokenProvider.Token);
-            
-            var currentOus = user.OrganizationUnits;
 
-            if (currentOus.Any(cou => cou.OrganizationUnitId == ou.Id && cou.UserId == user.Id))
+            if (user.OrganizationUnits.Any(cou => cou.OrganizationUnitId == ou.Id))
             {
                 return;
             }
 
-            await CheckMaxUserOrganizationUnitMembershipCountAsync(user.TenantId, currentOus.Count + 1);
+            await CheckMaxUserOrganizationUnitMembershipCountAsync(user.OrganizationUnits.Count + 1);
 
             user.AddOrganizationUnit(ou.Id);
         }
 
         public virtual async Task RemoveFromOrganizationUnitAsync(Guid userId, Guid ouId)
         {
-            await RemoveFromOrganizationUnitAsync(
+            var user = await IdentityUserRepository.GetAsync(userId, cancellationToken: CancellationToken);
-                await IdentityUserRepository.GetAsync(userId, true),
+            user.RemoveOrganizationUnit(ouId);
-                await OrganizationUnitRepository.GetAsync(ouId)
-                );
         }
 
         public virtual async Task RemoveFromOrganizationUnitAsync(IdentityUser user, OrganizationUnit ou)
@@ -153,9 +148,9 @@ namespace Volo.Abp.Identity
         public virtual async Task SetOrganizationUnitsAsync(Guid userId, params Guid[] organizationUnitIds)
         {
             await SetOrganizationUnitsAsync(
-                await IdentityUserRepository.GetAsync(userId, true),
+                await IdentityUserRepository.GetAsync(userId, cancellationToken: CancellationToken),
                 organizationUnitIds
-                );
+            );
         }
 
         public virtual async Task SetOrganizationUnitsAsync(IdentityUser user, params Guid[] organizationUnitIds)
@@ -163,38 +158,36 @@ namespace Volo.Abp.Identity
             Check.NotNull(user, nameof(user));
             Check.NotNull(organizationUnitIds, nameof(organizationUnitIds));
 
-            await CheckMaxUserOrganizationUnitMembershipCountAsync(user.TenantId, organizationUnitIds.Length);
+            await CheckMaxUserOrganizationUnitMembershipCountAsync(organizationUnitIds.Length);
 
-            var currentOus = await IdentityUserRepository.GetOrganizationUnitsAsync(user.Id);
+            await IdentityUserRepository.EnsureCollectionLoadedAsync(user, u => u.OrganizationUnits, CancellationTokenProvider.Token);
 
             //Remove from removed OUs
-            foreach (var currentOu in currentOus)
+            foreach (var ouId in user.OrganizationUnits.Select(uou => uou.OrganizationUnitId).ToArray())
             {
-                if (!organizationUnitIds.Contains(currentOu.Id))
+                if (!organizationUnitIds.Contains(ouId))
                 {
-                    await RemoveFromOrganizationUnitAsync(user.Id, currentOu.Id);
+                    user.RemoveOrganizationUnit(ouId);
                 }
             }
 
             //Add to added OUs
             foreach (var organizationUnitId in organizationUnitIds)
             {
-                if (currentOus.All(ou => ou.Id != organizationUnitId))
+                if (!user.IsInOrganizationUnit(organizationUnitId))
                 {
-                    await AddToOrganizationUnitAsync(
+                    user.AddOrganizationUnit(organizationUnitId);
-                        user,
-                        await OrganizationUnitRepository.GetAsync(organizationUnitId)
-                        );
                 }
             }
         }
 
-        private async Task CheckMaxUserOrganizationUnitMembershipCountAsync(Guid? tenantId, int requestedCount)
+        private async Task CheckMaxUserOrganizationUnitMembershipCountAsync(int requestedCount)
         {
             var maxCount = await SettingProvider.GetAsync<int>(IdentitySettingNames.OrganizationUnit.MaxUserMembershipCount);
             if (requestedCount > maxCount)
             {
-                throw new AbpException(string.Format("Can not set more than {0} organization unit for a user!", maxCount));
+                throw new BusinessException(IdentityErrorCodes.MaxAllowedOuMembership)
+                    .WithData("MaxUserMembershipCount", maxCount);
             }
         }
 
@@ -203,33 +196,33 @@ namespace Volo.Abp.Identity
         {
             await IdentityUserRepository.EnsureCollectionLoadedAsync(user, u => u.OrganizationUnits, CancellationTokenProvider.Token);
 
-            var ouOfUser = user.OrganizationUnits;
+            return await OrganizationUnitRepository.GetListAsync(
-
+                user.OrganizationUnits.Select(t => t.OrganizationUnitId),
-            return await OrganizationUnitRepository.GetListAsync(ouOfUser.Select(t => t.OrganizationUnitId));
+                CancellationToken
+            );
         }
 
         [UnitOfWork]
-        public virtual async Task<List<IdentityUser>> GetUsersInOrganizationUnitAsync(OrganizationUnit organizationUnit,
+        public virtual async Task<List<IdentityUser>> GetUsersInOrganizationUnitAsync(
+            OrganizationUnit organizationUnit,
             bool includeChildren = false)
         {
             if (includeChildren)
             {
                 return await IdentityUserRepository
-                    .GetUsersInOrganizationUnitWithChildrenAsync(organizationUnit.Code)
+                    .GetUsersInOrganizationUnitWithChildrenAsync(organizationUnit.Code, CancellationToken);
-                    ;
             }
             else
             {
                 return await IdentityUserRepository
-                    .GetUsersInOrganizationUnitAsync(organizationUnit.Id)
+                    .GetUsersInOrganizationUnitAsync(organizationUnit.Id, CancellationToken);
-                    ;
             }
         }
 
         public virtual async Task<IdentityResult> AddDefaultRolesAsync([NotNull] IdentityUser user)
         {
             await UserRepository.EnsureCollectionLoadedAsync(user, u => u.Roles, CancellationToken);
-            
+
             foreach (var role in await RoleRepository.GetDefaultOnesAsync(cancellationToken: CancellationToken))
             {
                 if (!user.IsInRole(role.Id))
@@ -237,9 +230,8 @@ namespace Volo.Abp.Identity
                     user.AddRole(role.Id);
                 }
             }
-            
-            return await UpdateUserAsync(user);
 
+            return await UpdateUserAsync(user);
         }
     }
 }

modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/Organizations/IOrganizationUnitRepository.cs

@@ -17,6 +17,5 @@ namespace Volo.Abp.Identity.Organizations
         Task<List<OrganizationUnit>> GetListAsync(bool includeDetails = true, CancellationToken cancellationToken = default);
 
         Task<OrganizationUnit> GetOrganizationUnitAsync(string displayName, bool includeDetails = false, CancellationToken cancellationToken = default);
-
     }
 }

modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/OrganizationUnitManager_Tests.cs

@@ -6,6 +6,7 @@ using System.Linq;
 using System.Text;
 using System.Threading.Tasks;
 using Volo.Abp.Identity.Organizations;
+using Volo.Abp.Uow;
 using Xunit;
 
 namespace Volo.Abp.Identity
@@ -17,6 +18,7 @@ namespace Volo.Abp.Identity
         private readonly IdentityTestData _testData;
         private readonly IIdentityRoleRepository _identityRoleRepository;
         private readonly ILookupNormalizer _lookupNormalizer;
+        private readonly IUnitOfWorkManager _unitOfWorkManager;
         public OrganizationUnitManager_Tests()
         {
             _organizationUnitManager = GetRequiredService<OrganizationUnitManager>();
@@ -24,6 +26,7 @@ namespace Volo.Abp.Identity
             _identityRoleRepository = GetRequiredService<IIdentityRoleRepository>();
             _lookupNormalizer = GetRequiredService<ILookupNormalizer>();
             _testData = GetRequiredService<IdentityTestData>();
+            _unitOfWorkManager = GetRequiredService<IUnitOfWorkManager>();
         }
 
         [Fact]
@@ -87,13 +90,21 @@ namespace Volo.Abp.Identity
         [Fact]
         public async Task AddRoleToOrganizationUnitAsync()
         {
-            var ou = await _organizationUnitRepository.GetOrganizationUnitAsync("OU1", true);
+            OrganizationUnit ou = null;
-            var adminRole = await _identityRoleRepository.FindByNormalizedNameAsync(_lookupNormalizer.NormalizeName("admin"));
+            IdentityRole adminRole = null;
-            await _organizationUnitManager.AddRoleToOrganizationUnitAsync(adminRole, ou);
+
-            
+            using (var uow = _unitOfWorkManager.Begin())
-            //TODO: This method has a bug: add role not work
+            {
+                ou = await _organizationUnitRepository.GetOrganizationUnitAsync("OU1", true);
+                adminRole = await _identityRoleRepository.FindByNormalizedNameAsync(_lookupNormalizer.NormalizeName("admin"));
+                await _organizationUnitManager.AddRoleToOrganizationUnitAsync(adminRole, ou);
+                await _organizationUnitRepository.UpdateAsync(ou);
+
+                await uow.CompleteAsync();
+            }
+
             ou = await _organizationUnitRepository.GetOrganizationUnitAsync("OU1", includeDetails: true);
-            ou.Roles.FirstOrDefault().RoleId.ShouldBe(adminRole.Id);
+            ou.Roles.First().RoleId.ShouldBe(adminRole.Id);
         }
 
         [Fact]