V14: Fix member mapping (#16106)

* convert key to name before saving roles

* Rework MemberEditingService to convert keys

* Rename variable to groups

* Extract to variable

Author: Zeegaan

src/Umbraco.Cms.Api.Management/Factories/MemberPresentationFactory.cs

@@ -18,26 +18,31 @@ internal sealed class MemberPresentationFactory : IMemberPresentationFactory
     private readonly IMemberService _memberService;
     private readonly IMemberTypeService _memberTypeService;
     private readonly ITwoFactorLoginService _twoFactorLoginService;
+    private readonly IMemberGroupService _memberGroupService;
 
     public MemberPresentationFactory(
         IUmbracoMapper umbracoMapper,
         IMemberService memberService,
         IMemberTypeService memberTypeService,
-        ITwoFactorLoginService twoFactorLoginService)
+        ITwoFactorLoginService twoFactorLoginService,
+        IMemberGroupService memberGroupService)
     {
         _umbracoMapper = umbracoMapper;
         _memberService = memberService;
         _memberTypeService = memberTypeService;
         _twoFactorLoginService = twoFactorLoginService;
+        _memberGroupService = memberGroupService;
     }
 
     public async Task<MemberResponseModel> CreateResponseModelAsync(IMember member, IUser currentUser)
     {
         MemberResponseModel responseModel = _umbracoMapper.Map<MemberResponseModel>(member)!;
 
         responseModel.IsTwoFactorEnabled = await _twoFactorLoginService.IsTwoFactorEnabledAsync(member.Key);
-        responseModel.Groups = _memberService.GetAllRoles(member.Username);
+        IEnumerable<string> roles = _memberService.GetAllRoles(member.Username);
 
+        // Get the member groups per role, so we can return the group keys
+        responseModel.Groups = roles.Select(x => _memberGroupService.GetByName(x)).WhereNotNull().Select(x => x.Key).ToArray();
         return currentUser.HasAccessToSensitiveData()
             ? responseModel
             : await RemoveSensitiveDataAsync(member, responseModel);

src/Umbraco.Cms.Api.Management/ViewModels/Member/CreateMemberRequestModel.cs

@@ -12,7 +12,7 @@ public class CreateMemberRequestModel : CreateContentRequestModelBase<MemberValu
 
     public required ReferenceByIdModel MemberType { get; set; }
 
-    public IEnumerable<string>? Groups { get; set; }
+    public IEnumerable<Guid>? Groups { get; set; }
 
     public bool IsApproved { get; set; }
 }

src/Umbraco.Cms.Api.Management/ViewModels/Member/MemberResponseModel.cs

@@ -25,5 +25,5 @@ public class MemberResponseModel : ContentResponseModelBase<MemberValueModel, Me
 
     public DateTime? LastPasswordChangeDate { get; set; }
 
-    public IEnumerable<string> Groups { get; set; } = [];
+    public IEnumerable<Guid> Groups { get; set; } = [];
 }

src/Umbraco.Cms.Api.Management/ViewModels/Member/UpdateMemberRequestModel.cs

@@ -12,7 +12,7 @@ public class UpdateMemberRequestModel : UpdateContentRequestModelBase<MemberValu
 
     public string? NewPassword { get; set; }
 
-    public IEnumerable<string>? Groups { get; set; }
+    public IEnumerable<Guid>? Groups { get; set; }
 
     public bool IsApproved { get; set; }
 

src/Umbraco.Core/Models/ContentEditing/MemberEditingModelBase.cs

@@ -4,7 +4,7 @@ public abstract class MemberEditingModelBase : ContentEditingModelBase
 {
     public bool IsApproved { get; set; }
 
-    public IEnumerable<string>? Roles { get; set; }
+    public IEnumerable<Guid>? Roles { get; set; }
 
     public string Email { get; set; } = string.Empty;
 

src/Umbraco.Infrastructure/Services/MemberEditingService.cs

@@ -18,6 +18,7 @@ internal sealed class MemberEditingService : IMemberEditingService
     private readonly ITwoFactorLoginService _twoFactorLoginService;
     private readonly IPasswordChanger<MemberIdentityUser> _passwordChanger;
     private readonly ILogger<MemberEditingService> _logger;
+    private readonly IMemberGroupService _memberGroupService;
 
     public MemberEditingService(
         IMemberService memberService,
@@ -26,7 +27,8 @@ public MemberEditingService(
         IMemberManager memberManager,
         ITwoFactorLoginService twoFactorLoginService,
         IPasswordChanger<MemberIdentityUser> passwordChanger,
-        ILogger<MemberEditingService> logger)
+        ILogger<MemberEditingService> logger,
+        IMemberGroupService memberGroupService)
     {
         _memberService = memberService;
         _memberTypeService = memberTypeService;
@@ -35,6 +37,7 @@ public MemberEditingService(
         _twoFactorLoginService = twoFactorLoginService;
         _passwordChanger = passwordChanger;
         _logger = logger;
+        _memberGroupService = memberGroupService;
     }
 
     public async Task<IMember?> GetAsync(Guid key)
@@ -246,16 +249,29 @@ private async Task<MemberEditingOperationStatus> ValidateMemberDataAsync(MemberE
         return MemberEditingOperationStatus.Success;
     }
 
-    private async Task<bool> UpdateRoles(IEnumerable<string>? roles, MemberIdentityUser identityMember)
+    private async Task<bool> UpdateRoles(IEnumerable<Guid>? roles, MemberIdentityUser identityMember)
     {
+        // We have to convert the GUIDS to names here, as roles on a member are stored by name, not key.
+        var memberGroups = new List<IMemberGroup>();
+        foreach (Guid key in roles ?? Enumerable.Empty<Guid>())
+        {
+
+            IMemberGroup? group = await _memberGroupService.GetAsync(key);
+            if (group is not null)
+            {
+                memberGroups.Add(group);
+            }
+        }
+
         // We're gonna look up the current roles now because the below code can cause
         // events to be raised and developers could be manually adding roles to members in
         // their handlers. If we don't look this up now there's a chance we'll just end up
         // removing the roles they've assigned.
         IEnumerable<string> currentRoles = (await _memberManager.GetRolesAsync(identityMember)).ToList();
 
         // find the ones to remove and remove them
-        var rolesToRemove = currentRoles.Except(roles ?? Enumerable.Empty<string>()).ToArray();
+        IEnumerable<string> memberGroupNames = memberGroups.Select(x => x.Name).WhereNotNull().ToArray();
+        var rolesToRemove = currentRoles.Except(memberGroupNames).ToArray();
 
         // Now let's do the role provider stuff - now that we've saved the content item (that is important since
         // if we are changing the username, it must be persisted before looking up the member roles).
@@ -270,8 +286,8 @@ private async Task<bool> UpdateRoles(IEnumerable<string>? roles, MemberIdentityU
         }
 
         // find the ones to add and add them
-        var rolesToAdd = roles?.Except(currentRoles).ToArray();
-        if (rolesToAdd?.Any() is true)
+        var rolesToAdd = memberGroupNames.Except(currentRoles).ToArray();
+        if (rolesToAdd.Any())
         {
             // add the ones submitted
             IdentityResult identityResult = await _memberManager.AddToRolesAsync(identityMember, rolesToAdd);

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MemberEditingServiceTests.cs

@@ -25,6 +25,8 @@ public class MemberEditingServiceTests : UmbracoIntegrationTest
 
     private IUserService UserService => GetRequiredService<IUserService>();
 
+    private IMemberGroupService MemberGroupService => GetRequiredService<IMemberGroupService>();
+
     [Test]
     public async Task Can_Create_Member()
     {
@@ -129,6 +131,7 @@ public async Task Can_Change_Member_Roles()
     {
         MemberService.AddRole("RoleTwo");
         MemberService.AddRole("RoleThree");
+        var groups = new[] { MemberGroupService.GetByName("RoleTwo"), MemberGroupService.GetByName("RoleThree") };
 
         var member = await CreateMemberAsync();
 
@@ -138,7 +141,7 @@ public async Task Can_Change_Member_Roles()
             Username = member.Username,
             IsApproved = true,
             InvariantName = member.Name,
-            Roles = new [] { "RoleTwo", "RoleThree" }
+            Roles = groups.Select(x => x.Key),
         };
 
         var result = await MemberEditingService.UpdateAsync(member.Key, updateModel, SuperUser());
@@ -432,6 +435,7 @@ private async Task<IMember> CreateMemberAsync(Guid? key = null, bool titleIsSens
         memberType.SetIsSensitiveProperty("title", titleIsSensitive);
         MemberTypeService.Save(memberType);
         MemberService.AddRole("RoleOne");
+        var group = MemberGroupService.GetByName("RoleOne");
 
         var createModel = new MemberCreateModel
         {
@@ -441,7 +445,7 @@ private async Task<IMember> CreateMemberAsync(Guid? key = null, bool titleIsSens
             Password = "SuperSecret123",
             IsApproved = true,
             ContentTypeKey = memberType.Key,
-            Roles = new [] { "RoleOne" },
+            Roles = new [] { group.Key },
             InvariantName = "T. Est",
             InvariantProperties = new[]
             {