Skip to content

Commit

Permalink
Properly handle null researcher categories when updating a profile
Browse files Browse the repository at this point in the history 
Pass it separately so it can be annotated with @nullable.
  • Loading branch information
@arteymix
arteymix committed Oct 8, 2024
1 parent 9052a22 commit fbeb4a9
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 25 deletions.
8 changes: 7 additions & 1 deletion src/main/java/ubc/pavlab/rdp/controllers/UserController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,13 @@ public ResponseEntity<?> saveProfile( @Valid @RequestBody ProfileWithOrganUberon
.body( BindingResultModel.fromBindingResult( bindingResult ) );
} else {
String previousContactEmail = user.getProfile().getContactEmail();
user = userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profileWithOrganUberonIdsAndOntologyTerms.profile, profileWithOrganUberonIdsAndOntologyTerms.profile.getPublications(), profileWithOrganUberonIdsAndOntologyTerms.organUberonIds, profileWithOrganUberonIdsAndOntologyTerms.ontologyTermIds, locale );
user = userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user,
profileWithOrganUberonIdsAndOntologyTerms.profile,
profileWithOrganUberonIdsAndOntologyTerms.profile.getResearcherCategories(),
profileWithOrganUberonIdsAndOntologyTerms.profile.getPublications(),
profileWithOrganUberonIdsAndOntologyTerms.organUberonIds,
profileWithOrganUberonIdsAndOntologyTerms.ontologyTermIds,
locale );
String message = messageSource.getMessage( "UserController.profileSaved", new Object[]{ user.getProfile().getContactEmail() }, locale );
if ( user.getProfile().getContactEmail() != null &&
!user.getProfile().getContactEmail().equals( previousContactEmail ) &&
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/ubc/pavlab/rdp/services/UserService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ User updateTermsAndGenesInTaxon( User user,

User updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( User user,
Profile profile,
Set<Publication> publications,
@Nullable Set<ResearcherCategory> researcherCategories,
@Nullable Set<Publication> publications,
@Nullable Set<String> organUberonIds,
@Nullable Set<Integer> ontologyTermIds,
Locale locale );
Expand Down
16 changes: 9 additions & 7 deletions src/main/java/ubc/pavlab/rdp/services/UserServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,7 @@ public void sendGeneAccessRequest( User requestingUser, UserGene userGene, Strin
@Transactional
@Override
@PreAuthorize("hasPermission(#user, 'update')")
public User updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( User user, Profile profile, @Nullable Set<Publication> publications, @Nullable Set<String> organUberonIds, @Nullable Set<Integer> termIdsByOntologyId, Locale locale ) {
public User updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( User user, Profile profile, @Nullable Set<ResearcherCategory> researcherCategories, @Nullable Set<Publication> publications, @Nullable Set<String> organUberonIds, @Nullable Set<Integer> termIdsByOntologyId, Locale locale ) {
user.getProfile().setDepartment( profile.getDepartment() );
user.getProfile().setDescription( profile.getDescription() );
user.getProfile().setLastName( profile.getLastName() );
Expand All @@ -762,12 +762,14 @@ public User updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( User user
user.getProfile().setResearcherPosition( profile.getResearcherPosition() );
user.getProfile().setOrganization( profile.getOrganization() );

if ( applicationSettings.getProfile().getEnabledResearcherCategories().containsAll( profile.getResearcherCategories() ) ) {
user.getProfile().getResearcherCategories().retainAll( profile.getResearcherCategories() );
user.getProfile().getResearcherCategories().addAll( profile.getResearcherCategories() );
} else {
log.warn( MessageFormat.format( "User {0} attempted to set user {1} researcher type to an unknown value {2}.",
findCurrentUser(), user, profile.getResearcherCategories() ) );
if ( researcherCategories != null ) {
if ( applicationSettings.getProfile().getEnabledResearcherCategories().containsAll( researcherCategories ) ) {
user.getProfile().getResearcherCategories().retainAll( researcherCategories );
user.getProfile().getResearcherCategories().addAll( researcherCategories );
} else {
log.warn( MessageFormat.format( "User {0} attempted to set user {1} researcher type to an unknown value {2}.",
findCurrentUser(), user, researcherCategories ) );
}
}

if ( user.getProfile().getContactEmail() == null ||
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/ubc/pavlab/rdp/controllers/UserControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public class UserControllerTest {
@BeforeEach
public void setUp() {
when( taxonService.findById( any() ) ).then( i -> createTaxon( i.getArgument( 0, Integer.class ) ) );
when( userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( any(), any(), any(), any(), any(), any() ) ).thenAnswer( arg -> arg.getArgument( 0, User.class ) );
when( userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( any(), any(), any(), any(), any(), any(), any() ) ).thenAnswer( arg -> arg.getArgument( 0, User.class ) );
}

@Test
Expand Down Expand Up @@ -635,7 +635,7 @@ public void givenLoggedIn_whenSaveProfile_thenReturnSucceed()
expectedProfile.setWebsite( "http://test.com" );
expectedProfile.setPrivacyLevel( PrivacyLevelType.PRIVATE );

verify( userService ).updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, expectedProfile, expectedProfile.getPublications(), null, null, Locale.ENGLISH );
verify( userService ).updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, expectedProfile, expectedProfile.getResearcherCategories(), expectedProfile.getPublications(), null, null, Locale.ENGLISH );
}

@Test
Expand Down Expand Up @@ -684,7 +684,7 @@ public void givenLoggedIn_whenSaveProfileWithNewPrivacyLevel_thenReturn200()

Profile profile = user.getProfile();
profile.setPrivacyLevel( PrivacyLevelType.SHARED );
verify( userService ).updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, profile.getPublications(), null, null, Locale.ENGLISH );
verify( userService ).updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, profile.getResearcherCategories(), profile.getPublications(), null, null, Locale.ENGLISH );
}

@Test
Expand Down Expand Up @@ -713,7 +713,7 @@ public void givenLoggedIn_whenSaveProfileWithUberonOrganIds_thenReturnSuccess()
.locale( Locale.ENGLISH ) )
.andExpect( status().isOk() );

verify( userService ).updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, user.getProfile(), user.getProfile().getPublications(), Sets.newSet( organ.getUberonId() ), null, Locale.ENGLISH );
verify( userService ).updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, user.getProfile(), user.getProfile().getResearcherCategories(), user.getProfile().getPublications(), Sets.newSet( organ.getUberonId() ), null, Locale.ENGLISH );
}

@Test
Expand Down
24 changes: 12 additions & 12 deletions src/test/java/ubc/pavlab/rdp/services/UserServiceImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ public void updateUserProfileAndPublications_whenPublications_thenReplaceAll() {
}
).collect( Collectors.toSet() );

User updatedUser = userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, user.getProfile(), newPublications, null, null, Locale.getDefault() );
User updatedUser = userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, user.getProfile(), null, newPublications, null, null, Locale.getDefault() );

assertThat( updatedUser.getProfile().getPublications() ).containsExactlyElementsOf( newPublications );

Expand All @@ -589,7 +589,7 @@ public void updateUserProfileAndPublication_whenOrgansIsEnabled_thenSaveOrgans()
.map( id -> createOrgan( id, null, null ) )
.collect( Collectors.toSet() ) );
Set<String> organUberonIds = Sets.newSet( "UBERON:00001", "UBERON:00002" );
user = userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, user.getProfile(), null, organUberonIds, null, Locale.getDefault() );
user = userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, user.getProfile(), null, null, organUberonIds, null, Locale.getDefault() );
verify( organInfoService ).findByUberonIdIn( organUberonIds );
assertThat( user.getUserOrgans().values() )
.extracting( "uberonId" )
Expand All @@ -601,7 +601,7 @@ public void updateUserProfileAndPublication_whenOrgansIsNotEnabled_thenIgnoreOrg
when( applicationSettings.getOrgans().isEnabled() ).thenReturn( false );
User user = createUser( 1 );
Set<String> organUberonIds = Sets.newSet( "UBERON:00001", "UBERON:00002" );
user = userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, user.getProfile(), null, organUberonIds, null, Locale.getDefault() );
user = userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, user.getProfile(), null, null, organUberonIds, null, Locale.getDefault() );
verifyNoInteractions( organInfoService );
assertThat( user.getUserOrgans() ).isEmpty();
}
Expand All @@ -614,7 +614,7 @@ public void updateUserProfileAndPublicationsAndOrgans_whenPrivacyLevelChangeAndC

Profile profile = new Profile();
profile.setPrivacyLevel( PrivacyLevelType.SHARED );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, Locale.getDefault() );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, null, Locale.getDefault() );

assertThat( user.getProfile() ).hasFieldOrPropertyWithValue( "privacyLevel", PrivacyLevelType.PUBLIC );
}
Expand All @@ -630,7 +630,7 @@ public void updateUserProfileAndPublicationsAndOrgans_whenPrivacyLevelChange_the

Profile profile = new Profile();
profile.setPrivacyLevel( PrivacyLevelType.SHARED );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, Locale.getDefault() );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, null, Locale.getDefault() );

assertThat( user.getProfile() )
.hasFieldOrPropertyWithValue( "privacyLevel", PrivacyLevelType.SHARED );
Expand All @@ -644,12 +644,12 @@ public void updateUserProfileAndPublicationsAndOrgans_whenContactEmailIsSet_then
Profile profile = new Profile();
profile.setPrivacyLevel( PrivacyLevelType.PUBLIC );
profile.setContactEmail( "[email protected]" );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, Locale.getDefault() );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, null, Locale.getDefault() );
verify( userListener ).onContactEmailUpdate( any( OnContactEmailUpdateEvent.class ) );
assertThat( user.getProfile().isContactEmailVerified() ).isFalse();

// make sure that if user update its profile later on, he doesn't get spammed
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, Locale.getDefault() );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, null, Locale.getDefault() );
verifyNoMoreInteractions( userListener );
assertThat( user.getProfile().isContactEmailVerified() ).isFalse();
}
Expand All @@ -660,7 +660,7 @@ public void updateUserProfileAndPublicationsAndOrgans_whenContactEmailIsEqualToU
Profile profile = new Profile();
profile.setPrivacyLevel( PrivacyLevelType.PUBLIC );
profile.setContactEmail( user.getEmail() );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, Locale.getDefault() );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, null, Locale.getDefault() );
assertThat( user.getProfile().isContactEmailVerified() ).isTrue();
verifyNoInteractions( userListener );
}
Expand All @@ -674,7 +674,7 @@ public void updateUserProfileAndPublicationsAndOrgans_whenContactEmailIsNull_the
Profile profile = new Profile();
profile.setPrivacyLevel( PrivacyLevelType.PUBLIC );
profile.setContactEmail( null );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, Locale.getDefault() );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, null, Locale.getDefault() );
assertThat( user.getProfile().getContactEmail() ).isNull();
assertThat( user.getProfile().isContactEmailVerified() ).isFalse();
assertThat( user.getProfile().getContactEmailVerifiedAt() ).isNull();
Expand All @@ -690,7 +690,7 @@ public void updateUserProfileAndPublicationsAndOrgans_whenContactEmailIsEmpty_th
Profile profile = new Profile();
profile.setPrivacyLevel( PrivacyLevelType.PUBLIC );
profile.setContactEmail( "" );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, Locale.getDefault() );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, null, Locale.getDefault() );
assertThat( user.getProfile().getContactEmail() ).isEmpty();
assertThat( user.getProfile().isContactEmailVerified() ).isFalse();
assertThat( user.getProfile().getContactEmailVerifiedAt() ).isNull();
Expand All @@ -704,7 +704,7 @@ public void updateUserProfileAndPublicationsAndOrgans_whenResearcherPositionIsSe
Profile profile = new Profile();
profile.setPrivacyLevel( PrivacyLevelType.PUBLIC );
profile.setResearcherPosition( ResearcherPosition.PRINCIPAL_INVESTIGATOR );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, Locale.getDefault() );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, null, Locale.getDefault() );
assertThat( user.getProfile().getResearcherPosition() ).isEqualTo( ResearcherPosition.PRINCIPAL_INVESTIGATOR );
verify( profileSettings ).getEnabledResearcherPositions();
verify( userRepository ).save( user );
Expand All @@ -717,7 +717,7 @@ public void updateUserProfileAndPublicationsAndOrgans_whenResearcherCategoriesAr
Profile profile = new Profile();
profile.setPrivacyLevel( PrivacyLevelType.PUBLIC );
profile.getResearcherCategories().add( ResearcherCategory.IN_SILICO );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, null, null, null, Locale.getDefault() );
userService.updateUserProfileAndPublicationsAndOrgansAndOntologyTerms( user, profile, profile.getResearcherCategories(), null, null, null, Locale.getDefault() );
assertThat( user.getProfile().getResearcherCategories() ).containsExactly( ResearcherCategory.IN_SILICO );
verify( profileSettings ).getEnabledResearcherCategories();
verify( userRepository ).save( user );
Expand Down

0 comments on commit fbeb4a9

Please sign in to comment.