From b08571ac2c6bdf9ed4d8b1522a79102fe7edc6fd Mon Sep 17 00:00:00 2001 From: ggmaleva Date: Fri, 16 Apr 2021 16:09:29 +0300 Subject: [PATCH 1/2] deactivate role while remove role and fix get member list query --- .../orgmanager/entity/MemberRoleEntity.java | 7 ++--- .../repository/MemberRepository.java | 9 +++--- .../orgmanager/service/MemberRoleService.java | 3 ++ .../service/MemberRoleServiceImpl.java | 8 +++++ .../service/OrganizationService.java | 10 +++--- .../OrganizationServiceIntegrationTest.java | 31 ++++++++++++++++--- 6 files changed, 52 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/rbkmoney/orgmanager/entity/MemberRoleEntity.java b/src/main/java/com/rbkmoney/orgmanager/entity/MemberRoleEntity.java index bb5aa4d..fd4cb8a 100644 --- a/src/main/java/com/rbkmoney/orgmanager/entity/MemberRoleEntity.java +++ b/src/main/java/com/rbkmoney/orgmanager/entity/MemberRoleEntity.java @@ -1,9 +1,6 @@ package com.rbkmoney.orgmanager.entity; -import lombok.AllArgsConstructor; -import lombok.Builder; -import lombok.Data; -import lombok.NoArgsConstructor; +import lombok.*; import javax.persistence.Entity; import javax.persistence.Id; @@ -17,9 +14,11 @@ import java.io.Serializable; @AllArgsConstructor @NoArgsConstructor @Table(name = "member_role") +@EqualsAndHashCode(onlyExplicitlyIncluded = true) public class MemberRoleEntity implements Serializable { @Id + @EqualsAndHashCode.Include private String id; private String organizationId; private String roleId; diff --git a/src/main/java/com/rbkmoney/orgmanager/repository/MemberRepository.java b/src/main/java/com/rbkmoney/orgmanager/repository/MemberRepository.java index fa7f95c..b937d73 100644 --- a/src/main/java/com/rbkmoney/orgmanager/repository/MemberRepository.java +++ b/src/main/java/com/rbkmoney/orgmanager/repository/MemberRepository.java @@ -20,14 +20,15 @@ public interface MemberRepository extends JpaRepository { " mr.scope_id as scopeId, " + " mr.resource_id as resourceId" + " FROM org_manager.member_to_member_role mtmr, " + - " org_manager.organization o, " + + " org_manager.member_to_organization mto, " + " org_manager.member_role mr, " + " org_manager.member m " + " WHERE " + - " o.id = ?1 " + + " mto.organization_id = ?1 " + + " AND mto .member_id = m.id " + " AND mr.active = 'true' " + - " AND mr.id = mtmr.member_role_id " + - " AND mr.organization_id = o.id " + + " AND mr.id = mtmr.member_role_id " + + " AND mr.organization_id = mto.organization_id " + " AND m.id = mtmr.member_id ", nativeQuery = true) List getOrgMemberList(String orgId); diff --git a/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleService.java b/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleService.java index 2afcec6..5be6f37 100644 --- a/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleService.java +++ b/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleService.java @@ -1,10 +1,13 @@ package com.rbkmoney.orgmanager.service; +import com.rbkmoney.orgmanager.entity.MemberRoleEntity; import com.rbkmoney.swag.organizations.model.MemberRole; public interface MemberRoleService { MemberRole findById(String id); + MemberRoleEntity getById(String id); + } diff --git a/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleServiceImpl.java b/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleServiceImpl.java index 2acbaa1..cc4ff97 100644 --- a/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleServiceImpl.java +++ b/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleServiceImpl.java @@ -1,6 +1,7 @@ package com.rbkmoney.orgmanager.service; import com.rbkmoney.orgmanager.converter.MemberRoleConverter; +import com.rbkmoney.orgmanager.entity.MemberRoleEntity; import com.rbkmoney.orgmanager.exception.ResourceNotFoundException; import com.rbkmoney.orgmanager.repository.MemberRoleRepository; import com.rbkmoney.swag.organizations.model.MemberRole; @@ -23,4 +24,11 @@ public class MemberRoleServiceImpl implements MemberRoleService { .orElseThrow(ResourceNotFoundException::new); } + @Override + @Transactional(readOnly = true) + public MemberRoleEntity getById(String id) { + return repository.findById(id) + .orElseThrow(ResourceNotFoundException::new); + } + } diff --git a/src/main/java/com/rbkmoney/orgmanager/service/OrganizationService.java b/src/main/java/com/rbkmoney/orgmanager/service/OrganizationService.java index 7113708..cb32b2f 100644 --- a/src/main/java/com/rbkmoney/orgmanager/service/OrganizationService.java +++ b/src/main/java/com/rbkmoney/orgmanager/service/OrganizationService.java @@ -41,6 +41,7 @@ public class OrganizationService { private final MemberRoleConverter memberRoleConverter; private final MemberRepository memberRepository; private final InvitationRepository invitationRepository; + private final MemberRoleService memberRoleService; // TODO [a.romanov]: idempotency public ResponseEntity create( @@ -115,13 +116,13 @@ public class OrganizationService { public void expelOrgMember(String orgId, String userId) { OrganizationEntity organization = findById(orgId); MemberEntity member = getMember(userId, organization); - deactivateOrgMemberRole(orgId, member); + deactivateOrgMemberRoles(orgId, member); member.getRoles() .removeIf(memberRoleEntity -> memberRoleEntity.getOrganizationId().equals(orgId)); organization.getMembers().remove(member); } - private void deactivateOrgMemberRole(String orgId, MemberEntity member) { + private void deactivateOrgMemberRoles(String orgId, MemberEntity member) { member.getRoles() .stream() .filter(memberRoleEntity -> memberRoleEntity.getOrganizationId().equals(orgId)) @@ -132,8 +133,9 @@ public class OrganizationService { public void removeMemberRole(String orgId, String userId, String memberRoleId) { OrganizationEntity organization = findById(orgId); MemberEntity member = getMember(userId, organization); - member.getRoles() - .removeIf(memberRoleEntity -> memberRoleEntity.getId().equals(memberRoleId)); + MemberRoleEntity roleToRemove = memberRoleService.getById(memberRoleId); + roleToRemove.setActive(Boolean.FALSE); + member.getRoles().remove(roleToRemove); } @Transactional(readOnly = true) diff --git a/src/test/java/com/rbkmoney/orgmanager/service/OrganizationServiceIntegrationTest.java b/src/test/java/com/rbkmoney/orgmanager/service/OrganizationServiceIntegrationTest.java index 455a5f8..60c5454 100644 --- a/src/test/java/com/rbkmoney/orgmanager/service/OrganizationServiceIntegrationTest.java +++ b/src/test/java/com/rbkmoney/orgmanager/service/OrganizationServiceIntegrationTest.java @@ -145,7 +145,9 @@ public class OrganizationServiceIntegrationTest extends AbstractRepositoryTest { void shouldGetListOrgMembers() { MemberEntity member1 = TestObjectFactory.testMemberEntity(TestObjectFactory.randomString()); MemberEntity member2 = TestObjectFactory.testMemberEntity(TestObjectFactory.randomString()); + MemberEntity anotherMember = TestObjectFactory.testMemberEntity(TestObjectFactory.randomString()); OrganizationEntity organization = TestObjectFactory.buildOrganization(Set.of(member1, member2)); + OrganizationEntity anotherOrganization = TestObjectFactory.buildOrganization(Set.of(anotherMember)); MemberRoleEntity activeMember1RoleInOrg = buildMemberRole(RoleId.ACCOUNTANT, organization.getId()); activeMember1RoleInOrg.setActive(Boolean.TRUE); MemberRoleEntity savedMember1Role = memberRoleRepository.save(activeMember1RoleInOrg); @@ -157,12 +159,16 @@ public class OrganizationServiceIntegrationTest extends AbstractRepositoryTest { nonActiveMember2RoleInOrg.setActive(Boolean.FALSE); MemberRoleEntity savedNonActiveMember2Role = memberRoleRepository.save(nonActiveMember2RoleInOrg); member2.setRoles(Set.of(savedActiveMember2Role, savedNonActiveMember2Role)); - memberRepository.saveAll(Set.of(member1, member2)); - OrganizationEntity savedOrganization = organizationRepository.save(organization); + MemberRoleEntity activeAnotherMemberRoleInOrg = buildMemberRole(RoleId.ACCOUNTANT, anotherOrganization.getId()); + activeAnotherMemberRoleInOrg.setActive(Boolean.TRUE); + MemberRoleEntity savedAnotherRole = memberRoleRepository.save(activeAnotherMemberRoleInOrg); + anotherMember.setRoles(Set.of(savedAnotherRole)); + memberRepository.saveAll(Set.of(member1, member2, anotherMember)); + organizationRepository.saveAll(Set.of(organization, anotherOrganization)); - MemberOrgListResult memberOrgListResult = organizationService.listMembers(savedOrganization.getId()); + MemberOrgListResult memberOrgListResult = organizationService.listMembers(organization.getId()); - assertEquals(savedOrganization.getMembers().size(), memberOrgListResult.getResult().size()); + assertEquals(organization.getMembers().size(), memberOrgListResult.getResult().size()); List roles = memberOrgListResult.getResult().stream() .map(Member::getRoles) @@ -173,8 +179,25 @@ public class OrganizationServiceIntegrationTest extends AbstractRepositoryTest { assertThat(roles, hasItem(activeMember1RoleInOrg.getId())); assertThat(roles, hasItem(activeMember2RoleInOrg.getId())); assertThat(roles, not(hasItem(nonActiveMember2RoleInOrg.getId()))); + assertThat(roles, not(hasItem(activeAnotherMemberRoleInOrg.getId()))); + } + + @Test + @Transactional + void removeMemberRoleTest() { + MemberEntity memberEntity = TestObjectFactory.testMemberEntity(TestObjectFactory.randomString()); + OrganizationEntity organization = TestObjectFactory.buildOrganization(memberEntity); + MemberRoleEntity savedMemberRole = + memberRoleRepository.save(TestObjectFactory.buildMemberRole(RoleId.ACCOUNTANT, organization.getId())); + memberEntity.setRoles(Set.of(savedMemberRole)); + MemberEntity savedMember = memberRepository.save(memberEntity); + OrganizationEntity savedOrganization = organizationRepository.save(organization); + + organizationService.removeMemberRole(savedOrganization.getId(), savedMember.getId(), savedMemberRole.getId()); + assertThat(memberRepository.findById(savedMember.getId()).get().getRoles(), not(hasItem(savedMemberRole))); + assertFalse(memberRoleRepository.findById(savedMemberRole.getId()).get().isActive()); } From c371bb27499e43e9784e137ba11b9a3df4324229 Mon Sep 17 00:00:00 2001 From: ggmaleva Date: Fri, 16 Apr 2021 16:54:20 +0300 Subject: [PATCH 2/2] fix after review --- .../java/com/rbkmoney/orgmanager/service/MemberRoleService.java | 2 +- .../com/rbkmoney/orgmanager/service/MemberRoleServiceImpl.java | 2 +- .../com/rbkmoney/orgmanager/service/OrganizationService.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleService.java b/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleService.java index 5be6f37..b79541e 100644 --- a/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleService.java +++ b/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleService.java @@ -7,7 +7,7 @@ public interface MemberRoleService { MemberRole findById(String id); - MemberRoleEntity getById(String id); + MemberRoleEntity findEntityById(String id); } diff --git a/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleServiceImpl.java b/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleServiceImpl.java index cc4ff97..5d24a23 100644 --- a/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleServiceImpl.java +++ b/src/main/java/com/rbkmoney/orgmanager/service/MemberRoleServiceImpl.java @@ -26,7 +26,7 @@ public class MemberRoleServiceImpl implements MemberRoleService { @Override @Transactional(readOnly = true) - public MemberRoleEntity getById(String id) { + public MemberRoleEntity findEntityById(String id) { return repository.findById(id) .orElseThrow(ResourceNotFoundException::new); } diff --git a/src/main/java/com/rbkmoney/orgmanager/service/OrganizationService.java b/src/main/java/com/rbkmoney/orgmanager/service/OrganizationService.java index cb32b2f..14e242b 100644 --- a/src/main/java/com/rbkmoney/orgmanager/service/OrganizationService.java +++ b/src/main/java/com/rbkmoney/orgmanager/service/OrganizationService.java @@ -133,7 +133,7 @@ public class OrganizationService { public void removeMemberRole(String orgId, String userId, String memberRoleId) { OrganizationEntity organization = findById(orgId); MemberEntity member = getMember(userId, organization); - MemberRoleEntity roleToRemove = memberRoleService.getById(memberRoleId); + MemberRoleEntity roleToRemove = memberRoleService.findEntityById(memberRoleId); roleToRemove.setActive(Boolean.FALSE); member.getRoles().remove(roleToRemove); }