V14: Fix member mapping (#16106)

* convert key to name before saving roles

* Rework MemberEditingService to convert keys

* Rename variable to groups

* Extract to variable

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

@@ -18,17 +18,20 @@ 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)
@@ -36,8 +39,10 @@ internal sealed class MemberPresentationFactory : IMemberPresentationFactory
         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 @@ internal sealed class MemberEditingService : IMemberEditingService
         IMemberManager memberManager,
         ITwoFactorLoginService twoFactorLoginService,
         IPasswordChanger<MemberIdentityUser> passwordChanger,
-        ILogger<MemberEditingService> logger)
+        ILogger<MemberEditingService> logger,
+        IMemberGroupService memberGroupService)
     {
         _memberService = memberService;
         _memberTypeService = memberTypeService;
@@ -35,6 +37,7 @@ internal sealed class MemberEditingService : IMemberEditingService
         _twoFactorLoginService = twoFactorLoginService;
         _passwordChanger = passwordChanger;
         _logger = logger;
+        _memberGroupService = memberGroupService;
     }
 
     public async Task<IMember?> GetAsync(Guid key)
@@ -246,8 +249,20 @@ internal sealed class MemberEditingService : IMemberEditingService
         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
@@ -255,7 +270,8 @@ internal sealed class MemberEditingService : IMemberEditingService
         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 @@ internal sealed class MemberEditingService : IMemberEditingService
         }
 
         // 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);