From 6a584016eedd40428f0f8c00342226cca8e91508 Mon Sep 17 00:00:00 2001 From: rmmayo Date: Thu, 7 Nov 2024 19:51:10 +0000 Subject: [PATCH] #2943 - fixed bug where unexpected number of rows returned when deleting a project/quiz from admin group, or an admin group w/ a project/quiz in it, and a member of the group was still a member of the project/quiz from more than one other group --- .../skills/storage/repos/UserRoleRepo.groovy | 6 +- .../AdminGroupDefManagementSpecs.groovy | 84 +++++++++++++++++++ 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/service/src/main/java/skills/storage/repos/UserRoleRepo.groovy b/service/src/main/java/skills/storage/repos/UserRoleRepo.groovy index 0966c5a58..bdf65420c 100644 --- a/service/src/main/java/skills/storage/repos/UserRoleRepo.groovy +++ b/service/src/main/java/skills/storage/repos/UserRoleRepo.groovy @@ -43,15 +43,15 @@ interface UserRoleRepo extends CrudRepository { UserRole findByUserIdAndRoleNameAndAdminGroupId(String userId, RoleName roleName, String adminGroupId) @Nullable - @Query('''SELECT 'true' from UserRole ur where ur.userId = ?1 and ur.roleName = 'ROLE_QUIZ_ADMIN' and ur.quizId = ?2 and ur.adminGroupId is not null''') + @Query('''SELECT DISTINCT 'true' from UserRole ur where ur.userId = ?1 and ur.roleName = 'ROLE_QUIZ_ADMIN' and ur.quizId = ?2 and ur.adminGroupId is not null''') Boolean isUserQuizGroupAdmin(String userId, String quizId) @Nullable - @Query('''SELECT 'true' from UserRole ur where ur.userId = ?1 and ur.roleName = 'ROLE_PROJECT_ADMIN' and ur.projectId = ?2 and ur.adminGroupId is not null''') + @Query('''SELECT DISTINCT 'true' from UserRole ur where ur.userId = ?1 and ur.roleName = 'ROLE_PROJECT_ADMIN' and ur.projectId = ?2 and ur.adminGroupId is not null''') Boolean isUserProjectGroupAdmin(String userId, String projectId) @Nullable - @Query(value = '''SELECT 'true' FROM user_roles ur WHERE ur.user_id = ?1 and ur.role_name in ('ROLE_ADMIN_GROUP_MEMBER','ROLE_ADMIN_GROUP_OWNER') and ur.admin_group_id = ?2 ''', nativeQuery = true) + @Query(value = '''SELECT DISTINCT 'true' FROM user_roles ur WHERE ur.user_id = ?1 and ur.role_name in ('ROLE_ADMIN_GROUP_MEMBER','ROLE_ADMIN_GROUP_OWNER') and ur.admin_group_id = ?2 ''', nativeQuery = true) Boolean isUserGroupAdminMemberOrOwner(String userId, String adminGroupId) void deleteByQuizIdAndAdminGroupIdAndRoleName(String quizId, String adminGroupId, RoleName roleName) diff --git a/service/src/test/java/skills/intTests/adminGroups/AdminGroupDefManagementSpecs.groovy b/service/src/test/java/skills/intTests/adminGroups/AdminGroupDefManagementSpecs.groovy index 3992b1c48..daf57b923 100644 --- a/service/src/test/java/skills/intTests/adminGroups/AdminGroupDefManagementSpecs.groovy +++ b/service/src/test/java/skills/intTests/adminGroups/AdminGroupDefManagementSpecs.groovy @@ -510,4 +510,88 @@ class AdminGroupDefManagementSpecs extends DefaultIntSpec { adminGroupMembers2.find {it.userId == otherUserId && it.roleName == RoleName.ROLE_ADMIN_GROUP_MEMBER.toString()} !adminGroupMembers2.find {it.userId == otherUserId && it.roleName == RoleName.ROLE_ADMIN_GROUP_OWNER.toString()} } + + def "remove admin group with project, members are removed from project for this group but remain if also assigned to the same project in other groups"() { + def otherUserId = getRandomUsers(1, true, ['skills@skills.org', DEFAULT_ROOT_USER_ID])[0] + createService(otherUserId) + def adminGroup1 = createAdminGroup(1) + def adminGroup2 = createAdminGroup(2) + def adminGroup3 = createAdminGroup(3) + + def proj = createProject(1) + def subj = createSubject(1, 1) + def skill = createSkill(1, 1) + skillsService.createProjectAndSubjectAndSkills(proj, subj, [skill]) + + def quiz = QuizDefFactory.createQuiz(1, "Fancy Description") + skillsService.createQuizDef(quiz) + when: + + skillsService.createAdminGroupDef(adminGroup1) + skillsService.addAdminGroupMember(adminGroup1.adminGroupId, otherUserId) + skillsService.addQuizToAdminGroup(adminGroup1.adminGroupId, quiz.quizId) + skillsService.addProjectToAdminGroup(adminGroup1.adminGroupId, proj.projectId) + + skillsService.createAdminGroupDef(adminGroup2) + skillsService.addAdminGroupMember(adminGroup2.adminGroupId, otherUserId) + skillsService.addQuizToAdminGroup(adminGroup2.adminGroupId, quiz.quizId) + skillsService.addProjectToAdminGroup(adminGroup2.adminGroupId, proj.projectId) + + skillsService.createAdminGroupDef(adminGroup3) + skillsService.addAdminGroupMember(adminGroup3.adminGroupId, otherUserId) + skillsService.addQuizToAdminGroup(adminGroup3.adminGroupId, quiz.quizId) + skillsService.addProjectToAdminGroup(adminGroup3.adminGroupId, proj.projectId) + + def projectAdminsBeforeRemove = userRoleRepository.findAllByProjectIdIgnoreCase(proj.projectId) + def quizAdminsBeforeRemove = userRoleRepository.findAllByQuizIdIgnoreCase(quiz.quizId) + def adminGroupRolesBeforeRemove = userRoleRepository.findAllByAdminGroupIdIgnoreCase(adminGroup1.adminGroupId) + def adminGroupDefsBeforeRemove = skillsService.getAdminGroupDefs() + + skillsService.removeAdminGroupDef(adminGroup1.adminGroupId) + + def projectAdminsAfterRemove = userRoleRepository.findAllByProjectIdIgnoreCase(proj.projectId) + def quizAdminsAfterRemove = userRoleRepository.findAllByQuizIdIgnoreCase(quiz.quizId) + def adminGroupRolesAfterRemove = userRoleRepository.findAllByAdminGroupIdIgnoreCase(adminGroup1.adminGroupId) + def adminGroupDefsAfterRemove = skillsService.getAdminGroupDefs() + + then: + adminGroupDefsBeforeRemove && adminGroupDefsBeforeRemove.size() == 3 && + adminGroupDefsBeforeRemove.find { it.adminGroupId == adminGroup1.adminGroupId } && + adminGroupDefsBeforeRemove.find { it.adminGroupId == adminGroup2.adminGroupId } && + adminGroupDefsBeforeRemove.find { it.adminGroupId == adminGroup3.adminGroupId } + projectAdminsBeforeRemove.size() == 6 + projectAdminsBeforeRemove.find { it.userId == skillsService.currentUser.userId && it.roleName == RoleName.ROLE_PROJECT_ADMIN && it.adminGroupId == adminGroup1.adminGroupId } + projectAdminsBeforeRemove.find { it.userId == otherUserId && it.roleName == RoleName.ROLE_PROJECT_ADMIN && it.adminGroupId == adminGroup1.adminGroupId } + projectAdminsBeforeRemove.find { it.userId == skillsService.currentUser.userId && it.roleName == RoleName.ROLE_PROJECT_ADMIN && it.adminGroupId == adminGroup2.adminGroupId } + projectAdminsBeforeRemove.find { it.userId == otherUserId && it.roleName == RoleName.ROLE_PROJECT_ADMIN && it.adminGroupId == adminGroup2.adminGroupId } + projectAdminsBeforeRemove.find { it.userId == skillsService.currentUser.userId && it.roleName == RoleName.ROLE_PROJECT_ADMIN && it.adminGroupId == adminGroup3.adminGroupId } + projectAdminsBeforeRemove.find { it.userId == otherUserId && it.roleName == RoleName.ROLE_PROJECT_ADMIN && it.adminGroupId == adminGroup3.adminGroupId } + quizAdminsBeforeRemove.size() == 6 + quizAdminsBeforeRemove.find { it.userId == skillsService.currentUser.userId && it.roleName == RoleName.ROLE_QUIZ_ADMIN && it.adminGroupId == adminGroup1.adminGroupId } + quizAdminsBeforeRemove.find { it.userId == otherUserId && it.roleName == RoleName.ROLE_QUIZ_ADMIN && it.adminGroupId == adminGroup1.adminGroupId } + quizAdminsBeforeRemove.find { it.userId == skillsService.currentUser.userId && it.roleName == RoleName.ROLE_QUIZ_ADMIN && it.adminGroupId == adminGroup2.adminGroupId } + quizAdminsBeforeRemove.find { it.userId == otherUserId && it.roleName == RoleName.ROLE_QUIZ_ADMIN && it.adminGroupId == adminGroup2.adminGroupId } + quizAdminsBeforeRemove.find { it.userId == skillsService.currentUser.userId && it.roleName == RoleName.ROLE_QUIZ_ADMIN && it.adminGroupId == adminGroup3.adminGroupId } + quizAdminsBeforeRemove.find { it.userId == otherUserId && it.roleName == RoleName.ROLE_QUIZ_ADMIN && it.adminGroupId == adminGroup3.adminGroupId } + adminGroupRolesBeforeRemove.size() == 6 + adminGroupRolesBeforeRemove.find { it.userId == skillsService.currentUser.userId && it.roleName == RoleName.ROLE_ADMIN_GROUP_OWNER && it.adminGroupId == adminGroup1.adminGroupId } + adminGroupRolesBeforeRemove.find { it.userId == otherUserId && it.roleName == RoleName.ROLE_ADMIN_GROUP_MEMBER && it.adminGroupId == adminGroup1.adminGroupId } + + + projectAdminsAfterRemove.size() == 4 + projectAdminsAfterRemove.find { it.userId == skillsService.currentUser.userId && it.roleName == RoleName.ROLE_PROJECT_ADMIN && it.adminGroupId == adminGroup2.adminGroupId } + projectAdminsAfterRemove.find { it.userId == otherUserId && it.roleName == RoleName.ROLE_PROJECT_ADMIN && it.adminGroupId == adminGroup2.adminGroupId } + projectAdminsAfterRemove.find { it.userId == skillsService.currentUser.userId && it.roleName == RoleName.ROLE_PROJECT_ADMIN && it.adminGroupId == adminGroup3.adminGroupId } + projectAdminsAfterRemove.find { it.userId == otherUserId && it.roleName == RoleName.ROLE_PROJECT_ADMIN && it.adminGroupId == adminGroup3.adminGroupId } + quizAdminsAfterRemove.size() == 4 + quizAdminsAfterRemove.find { it.userId == skillsService.currentUser.userId && it.roleName == RoleName.ROLE_QUIZ_ADMIN && it.adminGroupId == adminGroup2.adminGroupId } + quizAdminsAfterRemove.find { it.userId == otherUserId && it.roleName == RoleName.ROLE_QUIZ_ADMIN && it.adminGroupId == adminGroup2.adminGroupId } + adminGroupRolesAfterRemove.size() == 0 + quizAdminsAfterRemove.find { it.userId == skillsService.currentUser.userId && it.roleName == RoleName.ROLE_QUIZ_ADMIN && it.adminGroupId == adminGroup2.adminGroupId } + quizAdminsAfterRemove.find { it.userId == otherUserId && it.roleName == RoleName.ROLE_QUIZ_ADMIN && it.adminGroupId == adminGroup2.adminGroupId } + quizAdminsAfterRemove.find { it.userId == skillsService.currentUser.userId && it.roleName == RoleName.ROLE_QUIZ_ADMIN && it.adminGroupId == adminGroup3.adminGroupId } + quizAdminsAfterRemove.find { it.userId == otherUserId && it.roleName == RoleName.ROLE_QUIZ_ADMIN && it.adminGroupId == adminGroup3.adminGroupId } + + adminGroupDefsAfterRemove && adminGroupDefsAfterRemove.size() == 2 && adminGroupDefsAfterRemove.find { it.adminGroupId == adminGroup2.adminGroupId } && adminGroupDefsAfterRemove.find { it.adminGroupId == adminGroup2.adminGroupId} + } }