DTSRD-2133. Delete organisation and user from UP even if he is in active status #1590

SabinaHMCTS · 2024-04-25T16:12:51Z

Jira link (if applicable)

Change description

Checklist

commit messages are meaningful and follow good commit message guidelines
README and other documentation has been updated / added (if needed)
tests have been updated / new tests has been added (if needed)
Does this PR introduce a breaking change

lukasz-wolski

Please see inlined comment

src/main/java/uk/gov/hmcts/reform/professionalapi/service/impl/OrganisationServiceImpl.java

lukasz-wolski

Very nice usage of forEach() 👍

rajkatla-hmcts · 2024-05-19T21:53:13Z

src/main/java/uk/gov/hmcts/reform/professionalapi/service/impl/OrganisationServiceImpl.java

+    @Transactional
+    public DeleteUserResponse deleteUserForOrganisation(List<String> emails) {
+        if (emails.isEmpty()) {
+            throw new InvalidRequest("Please provide both email addresses");


dont understand this message why both email addresses ?

rajkatla-hmcts · 2024-05-19T22:30:55Z

src/main/java/uk/gov/hmcts/reform/professionalapi/service/impl/OrganisationServiceImpl.java

+                .findByEmailAddress(RefDataUtil.removeAllSpaces(email)));
+            if (!professionalUser.isEmpty()) {
+                userIdsToBeDeleted.add(professionalUser.get().getUserIdentifier());
+                userAttributeRepository.deleteByProfessionalUserId(professionalUser.get().getId());


cascade delete should take care of it

you also need to delete user from user account map table explicitly as its not there in professional user class

rajkatla-hmcts · 2024-05-19T22:35:17Z

src/main/java/uk/gov/hmcts/reform/professionalapi/service/impl/OrganisationServiceImpl.java

+            }
+
+        });
+        DeleteUserProfilesRequest deleteUserRequest = new DeleteUserProfilesRequest(userIdsToBeDeleted);


what if user is not found in professional user table ? this call will make with empty userids set

rajkatla-hmcts · 2024-05-19T22:39:32Z

src/main/java/uk/gov/hmcts/reform/professionalapi/service/impl/OrganisationServiceImpl.java

+        DeleteUserProfilesRequest deleteUserRequest = new DeleteUserProfilesRequest(userIdsToBeDeleted);
+        Optional<DeleteOrganisationResponse> deleteOrganisationResponse = Optional.ofNullable(RefDataUtil
+            .deleteUserProfilesFromUp(deleteUserRequest, userProfileFeignClient));
+        if (deleteOrganisationResponse.isEmpty()) {


this line is not required, response will contain either success or error status or it wont come back if any FeignException

rajkatla-hmcts · 2024-05-19T22:52:25Z

src/main/java/uk/gov/hmcts/reform/professionalapi/repository/UserAttributeRepository.java

@@ -9,4 +11,9 @@
 @Repository
 public interface UserAttributeRepository extends JpaRepository<UserAttribute, UUID> {

+    @Modifying
+    @Query(value = "delete from dbrefdata.user_attribute ua where ua.professional_user_id = :profUserId ",


do you need this native, try cascade delete

rajkatla-hmcts · 2024-05-20T08:36:33Z

src/main/java/uk/gov/hmcts/reform/professionalapi/service/impl/OrganisationServiceImpl.java

+        emails.forEach(email -> {
+            Optional<ProfessionalUser> professionalUser = Optional.ofNullable(professionalUserRepository
+                .findByEmailAddress(RefDataUtil.removeAllSpaces(email)));
+            if (!professionalUser.isEmpty()) {


Delete users only when user is not ADMIN

rajkatla-hmcts · 2024-05-20T08:59:36Z

src/main/java/uk/gov/hmcts/reform/professionalapi/service/impl/OrganisationServiceImpl.java

+        Optional<DeleteOrganisationResponse> deleteOrganisationResponse = Optional.ofNullable(RefDataUtil
+            .deleteUserProfilesFromUp(deleteUserRequest, userProfileFeignClient));
+        if (deleteOrganisationResponse.isEmpty()) {
+            throw new InvalidRequest(ERROR_MESSAGE_UP_FAILED);


this exception will not process remaining email ids, you need to collect only failed email ids and include them in the error message

rajkatla-hmcts · 2024-05-20T09:05:26Z

src/main/java/uk/gov/hmcts/reform/professionalapi/service/impl/OrganisationServiceImpl.java

+        if (emails.isEmpty()) {
+            throw new InvalidRequest("Please provide both email addresses");
+        }
+        Set<String> userIdsToBeDeleted = new HashSet<>();


you dont need this set object return set from stream itself

rajkatla-hmcts · 2024-05-20T09:06:10Z

src/main/java/uk/gov/hmcts/reform/professionalapi/service/impl/OrganisationServiceImpl.java

+            Optional<ProfessionalUser> professionalUser = Optional.ofNullable(professionalUserRepository
+                .findByEmailAddress(RefDataUtil.removeAllSpaces(email)));
+            if (!professionalUser.isEmpty()) {
+                userIdsToBeDeleted.add(professionalUser.get().getUserIdentifier());


dont need this, return ids from stream map and collect as set

rajkatla-hmcts · 2024-08-28T22:37:24Z

src/integrationTest/java/uk/gov/hmcts/reform/professionalapi/DeleteOrganisationIntTest.java

@@ -132,7 +132,7 @@ void returns_400_when_delete_active_organisation_with_more_than_one__user_profil

        Map<String, Object> deleteResponse =
                professionalReferenceDataClient.deleteOrganisation(hmctsAdmin, orgIdentifier);
-        assertThat(deleteResponse.get("http_status")).isEqualTo("400");
+        assertThat(deleteResponse.get("http_status")).isEqualTo(204);


missing functional tests

rajkatla-hmcts · 2024-08-28T22:46:43Z

src/main/java/uk/gov/hmcts/reform/professionalapi/service/impl/OrganisationServiceImpl.java

-            } else if (!IdamStatus.ACTIVE.name().equalsIgnoreCase(newUserResponse.getIdamStatus())) {
-                // If user is not active in the up will send the request to delete
+            } else {
+                // user will be deleted even if he is in active state
                var userIds = new HashSet<String>();
                userIds.add(user.getUserIdentifier());
                DeleteUserProfilesRequest deleteUserRequest = new DeleteUserProfilesRequest(userIds);
                deleteOrganisationResponse = RefDataUtil


All users who belong to an organization should be deleted, not just one user

i can remove the condition to check fro more than one user and delete all users for the organisation but i

just want to be sure and reconfirm as this is an existing functionality

confirmed with Raj that correct logic will be to delete allassociated users witht the organisation when it is being fetched, will inform kiran to inform business users

SabinaHMCTS added 22 commits April 24, 2024 19:02

Upgrading perftest branch to V16 and adding subnet_suffix

c06c1a6

Delete organisation and user even if he is active

443246f

Delete User

2937807

Delete User and User Profile

ed8ec38

Delete User and User Profile

ee25438

Delete User and User Profile

d67d971

Delete User and User Profile

f9b6715

Delete User and User Profile

510e09d

Delete User and User Profile

0df25e2

Fix build

3dd7caf

Fix build

76c03eb

Fix build

10b6973

Fix build

bf2049e

Fix build

1924b8b

Fix build

0132dbc

Fix build

b1c7ca4

Fix build

5fe76bb

Fix build

42df77c

Fix build

57fb69d

Fix build

3a20702

Fix build

af226cb

Fix build

9d27d99

hmcts-jenkins-j-to-z bot deployed to preview April 29, 2024 16:23 View deployment

hmcts-jenkins-j-to-z bot added ns:rd prd:rd rel:rd-professional-api-pr-1590 labels Apr 29, 2024

SabinaHMCTS added 2 commits April 29, 2024 17:47

Fix build

1ebfb97

Fix build

7b24fe2

hmcts-jenkins-j-to-z bot deployed to preview April 29, 2024 18:02 View deployment

Fix build

02cc226

Fix build

5c6303e

hmcts-jenkins-j-to-z bot deployed to preview April 29, 2024 18:43 View deployment

lukasz-wolski requested changes Apr 30, 2024

View reviewed changes

src/main/java/uk/gov/hmcts/reform/professionalapi/service/impl/OrganisationServiceImpl.java Outdated Show resolved Hide resolved

Merge branch 'master' into DTSRD-2133

f6a7fd4

hmcts-jenkins-j-to-z bot deployed to preview May 13, 2024 09:07 View deployment

SabinaHMCTS added 3 commits May 13, 2024 10:53

fixing review comments

f07b996

fixing review comments

d08920f

fixing review comments

3806151

hmcts-jenkins-j-to-z bot deployed to preview May 13, 2024 11:32 View deployment

lukasz-wolski previously approved these changes May 13, 2024

View reviewed changes

rajkatla-hmcts requested changes May 19, 2024

View reviewed changes

rajkatla-hmcts requested changes May 20, 2024

View reviewed changes

SabinaHMCTS dismissed lukasz-wolski’s stale review via 5fcc971 June 4, 2024 11:18

SabinaHMCTS and others added 7 commits June 4, 2024 12:19

removed teh extra code for DTSRD-2134

5fcc971

removed teh extra code for DTSRD-2134

c82455f

removed teh extra code for DTSRD-2134

47e5a12

removed teh extra code for DTSRD-2134

725c733

Merge branch 'master' into DTSRD-2133

a293465

removed teh extra code for DTSRD-2134

3f0f0e2

removed teh extra code for DTSRD-2134

2045d1e

hmcts-jenkins-j-to-z bot deployed to preview June 4, 2024 11:52 View deployment

Merge branch 'master' into DTSRD-2133

b4ef9dc

hmcts-jenkins-j-to-z bot deployed to preview August 29, 2024 15:23 View deployment

hmcts-jenkins-j-to-z bot deployed to preview August 30, 2024 12:22 View deployment

rajkatla-hmcts requested changes Aug 30, 2024

View reviewed changes

SabinaHMCTS added 3 commits August 31, 2024 10:44

merging DTSRD-2132 and DTSRD-2136

c4da7af

endpoint to save SraId

c1c2d81

endpoint to save SraId

83caf02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DTSRD-2133. Delete organisation and user from UP even if he is in active status #1590

DTSRD-2133. Delete organisation and user from UP even if he is in active status #1590

SabinaHMCTS commented Apr 25, 2024

lukasz-wolski left a comment

lukasz-wolski left a comment

rajkatla-hmcts May 19, 2024

SabinaHMCTS Aug 29, 2024

rajkatla-hmcts May 19, 2024 •

edited

Loading

rajkatla-hmcts May 19, 2024

rajkatla-hmcts May 19, 2024

rajkatla-hmcts May 19, 2024

rajkatla-hmcts May 20, 2024

rajkatla-hmcts May 20, 2024

rajkatla-hmcts May 20, 2024

rajkatla-hmcts May 20, 2024

rajkatla-hmcts Aug 28, 2024

rajkatla-hmcts Aug 28, 2024

SabinaHMCTS Aug 30, 2024 •

edited

Loading

SabinaHMCTS Sep 3, 2024

DTSRD-2133. Delete organisation and user from UP even if he is in active status #1590

Are you sure you want to change the base?

DTSRD-2133. Delete organisation and user from UP even if he is in active status #1590

Conversation

SabinaHMCTS commented Apr 25, 2024

Jira link (if applicable)

Change description

Checklist

lukasz-wolski left a comment

Choose a reason for hiding this comment

lukasz-wolski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rajkatla-hmcts May 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SabinaHMCTS Aug 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rajkatla-hmcts May 19, 2024 •

edited

Loading

SabinaHMCTS Aug 30, 2024 •

edited

Loading