diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/UserGroupRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/UserGroupRepository.cs index 3935027ada..3b247950e4 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/UserGroupRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/UserGroupRepository.cs @@ -411,8 +411,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement return; //now the user association - RemoveAllUsersFromGroup(entity.UserGroup.Id); - AddUsersToGroup(entity.UserGroup.Id, entity.UserIds); + RefreshUsersInGroup(entity.UserGroup.Id, entity.UserIds); } protected override void PersistUpdatedItem(UserGroupWithUsers entity) @@ -424,8 +423,18 @@ namespace Umbraco.Core.Persistence.Repositories.Implement return; //now the user association - RemoveAllUsersFromGroup(entity.UserGroup.Id); - AddUsersToGroup(entity.UserGroup.Id, entity.UserIds); + RefreshUsersInGroup(entity.UserGroup.Id, entity.UserIds); + } + + /// + /// Adds a set of users to a group, first removing any that exist + /// + /// Id of group + /// Ids of users + private void RefreshUsersInGroup(int groupId, int[] userIds) + { + RemoveAllUsersFromGroup(groupId); + AddUsersToGroup(groupId, userIds); } /// @@ -444,7 +453,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement /// Ids of users private void AddUsersToGroup(int groupId, int[] userIds) { - // TODO: Check if the user exists? foreach (var userId in userIds) { var dto = new User2UserGroupDto diff --git a/src/Umbraco.Web/Editors/UserGroupsController.cs b/src/Umbraco.Web/Editors/UserGroupsController.cs index e79cfd625c..1b64722735 100644 --- a/src/Umbraco.Web/Editors/UserGroupsController.cs +++ b/src/Umbraco.Web/Editors/UserGroupsController.cs @@ -50,13 +50,8 @@ namespace Umbraco.Web.Editors if (isAuthorized == false) throw new HttpResponseException(Request.CreateResponse(HttpStatusCode.Unauthorized, isAuthorized.Result)); - //current user needs to be added to a new group if not an admin (possibly only if no other users are added?) to avoid a 401 - if(!Security.CurrentUser.IsAdmin() && (userGroupSave.Id == null || Convert.ToInt32(userGroupSave.Id) >= 0)/* && !userGroupSave.Users.Any() */) - { - var userIds = userGroupSave.Users.ToList(); - userIds.Add(Security.CurrentUser.Id); - userGroupSave.Users = userIds; - } + //need to ensure current user is in a group if not an admin to avoid a 401 + EnsureNonAdminUserIsInSavedUserGroup(userGroupSave); //save the group Services.UserService.Save(userGroupSave.PersistedUserGroup, userGroupSave.Users.ToArray()); @@ -87,6 +82,23 @@ namespace Umbraco.Web.Editors return display; } + private void EnsureNonAdminUserIsInSavedUserGroup(UserGroupSave userGroupSave) + { + if (Security.CurrentUser.IsAdmin()) + { + return; + } + + var userIds = userGroupSave.Users.ToList(); + if (userIds.Contains(Security.CurrentUser.Id)) + { + return; + } + + userIds.Add(Security.CurrentUser.Id); + userGroupSave.Users = userIds; + } + /// /// Returns the scaffold for creating a new user group ///