Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DTSRD-2134.Delete User from Professional #1589

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Conversation

SabinaHMCTS
Copy link
Contributor

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

Copy link
Contributor

@lukasz-wolski lukasz-wolski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refer to inlined comments


JsonPath response = professionalApiClient.deleteUserFromOrganisation(userDeletionRequest,OK);

assertThat(response).isNotNull();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Add find call and assert user not found after deletion
  2. Add scenario for deletion failed email addresses from PRD
  3. Add scenario for deletion failed email addresses from UP

public DeleteUserResponse deleteUserForOrganisation(List<String> emails) {
var deleteOrganisationResponse = new DeleteOrganisationResponse();
if (emails.isEmpty()) {
throw new InvalidRequest("Please provide both email addresses");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont understand this message why both email addresses ?

Set<String> userIds = new HashSet<>();
Optional.ofNullable(professionalUserRepository
.findByEmailAddress(RefDataUtil.removeAllSpaces(email)))
.ifPresentOrElse(professionalUser -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete users only when not ADMIN

.findByEmailAddress(RefDataUtil.removeAllSpaces(email)))
.ifPresentOrElse(professionalUser -> {
userIds.add(professionalUser.getUserIdentifier());
userAttributeRepository.deleteByProfessionalUserId(professionalUser.getId());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cascade delete should take care of it

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


Set<String> userIdsToBeDeleted = emails.stream().map(
email -> {
Set<String> userIds = new HashSet<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you dont need this set object, return it from stream itself

Collectors.toSet()));
deleteOrganisationResponse = RefDataUtil
.deleteUserProfilesFromUp(deleteUserProfileRequest, userProfileFeignClient);
if (deleteOrganisationResponse == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@Test
void testDeleteProfessionalUserFromOrganisation() {
DeleteUserResponse deleteUserResponse =
new DeleteUserResponse(STATUS_CODE_204, "The organisation has deleted successfully");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong message you are trying delete user : The organisation has deleted successfully


}


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1)Add scenario for deletion failed email addresses from PRD
2)Add scenario for deletion failed email addresses from UP

import org.springframework.stereotype.Repository;
import uk.gov.hmcts.reform.professionalapi.domain.UserAttribute;

import java.util.UUID;

@Repository
public interface UserAttributeRepository extends JpaRepository<UserAttribute, UUID> {

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need this native query, cascade delete should take care of it

verify(userProfileFeignClient, times(0)).deleteUserProfile(any());

DeleteUserResponse deleteUserResponse =
new DeleteUserResponse(204, "The organisation has deleted successfully");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong message you are trying delete user : The organisation has deleted successfully

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants