diff --git a/docs/release-notes.md b/docs/release-notes.md index 81d20823..51062cc8 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -1,5 +1,14 @@ # Release Notes +## 1.4.6 + +This release has a few fixes that provide better user feedback during registration and while updating the user profile. + +Fix deletion of user in the administrative section. Its associated tokens were somehow set to `NULL` by Hibernate before +being deleted. + +Hide research focus in the user profile if nothing has been entered. + ## 1.4.5 This release resolves an issue with slow SMTP servers causing a proxy timeout by sending emails asynchronously. diff --git a/mkdocs.yml b/mkdocs.yml index ba99edd7..a1ce9ec2 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -12,4 +12,4 @@ nav: plugins: - macros extra: - rdp_version: 1.4.3 + rdp_version: 1.4.6 diff --git a/pom.xml b/pom.xml index 3a256afb..20e57789 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ ubc.pavlab rdp - 1.4.5 + 1.4.6 @@ -175,7 +175,7 @@ 1.8 - 8.5.60 + 8.5.71 diff --git a/src/main/java/ubc/pavlab/rdp/controllers/LoginController.java b/src/main/java/ubc/pavlab/rdp/controllers/LoginController.java index e653da78..4c375e6f 100644 --- a/src/main/java/ubc/pavlab/rdp/controllers/LoginController.java +++ b/src/main/java/ubc/pavlab/rdp/controllers/LoginController.java @@ -67,7 +67,7 @@ public ModelAndView createNewUser( @Validated(User.ValidationUserAccount.class) RedirectAttributes redirectAttributes, Locale locale ) { ModelAndView modelAndView = new ModelAndView( "registration" ); - User userExists = userService.findUserByEmailNoAuth( user.getEmail() ); + User existingUser = userService.findUserByEmailNoAuth( user.getEmail() ); user.setEnabled( false ); @@ -78,9 +78,15 @@ public ModelAndView createNewUser( @Validated(User.ValidationUserAccount.class) userProfile.setHideGenelist( false ); userProfile.setContactEmailVerified( false ); - if ( userExists != null ) { - bindingResult.rejectValue( "email", "error.user", "There is already a user registered this email." ); - log.warn( "Trying to register an already registered email." ); + if ( existingUser != null ) { + if ( existingUser.isEnabled() ) { + bindingResult.rejectValue( "email", "error.user", "There is already a user registered this email." ); + } else { + // maybe the user is attempting to re-register, unaware that the confirmation hasn't been processed + userService.createVerificationTokenForUser( existingUser, locale ); + bindingResult.rejectValue( "email", "error.user", "You have already registered an account with this email. We just sent you a new confirmation email." ); + } + log.warn( "Trying to register an already registered email: " + user.getEmail() + "." ); } if ( bindingResult.hasErrors() ) { diff --git a/src/main/java/ubc/pavlab/rdp/controllers/UserController.java b/src/main/java/ubc/pavlab/rdp/controllers/UserController.java index 348d0205..af00b9ea 100644 --- a/src/main/java/ubc/pavlab/rdp/controllers/UserController.java +++ b/src/main/java/ubc/pavlab/rdp/controllers/UserController.java @@ -5,7 +5,7 @@ import lombok.extern.apachecommons.CommonsLog; import org.hibernate.validator.constraints.NotEmpty; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.ApplicationEventPublisher; +import org.springframework.context.MessageSource; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; @@ -13,6 +13,7 @@ import org.springframework.security.authentication.BadCredentialsException; import org.springframework.stereotype.Controller; import org.springframework.validation.BindingResult; +import org.springframework.validation.FieldError; import org.springframework.web.bind.annotation.*; import org.springframework.web.multipart.MultipartFile; import org.springframework.web.servlet.ModelAndView; @@ -57,6 +58,9 @@ public class UserController { @Autowired private ApplicationSettings applicationSettings; + @Autowired + private MessageSource messageSource; + @GetMapping(value = { "/user/home" }) public ModelAndView userHome() { ModelAndView modelAndView = new ModelAndView( "user/home" ); @@ -243,16 +247,54 @@ public String verifyContactEmail( @RequestParam String token, RedirectAttributes @Data static class ProfileWithOrganUberonIds { @Valid - private Profile profile; - private Set organUberonIds; + private final Profile profile; + private final Set organUberonIds; + } + + @Data + static class ProfileSavedModel { + private final String message; + private final boolean contactEmailVerified; + } + + @Data + static class FieldErrorModel { + private final String field; + private final String message; + private final Object rejectedValue; + + public static FieldErrorModel fromFieldError( FieldError fieldError ) { + return new FieldErrorModel( fieldError.getField(), fieldError.getDefaultMessage(), fieldError.getRejectedValue() ); + } + } + + @Data + static class BindingResultModel { + private final List fieldErrors; + + public static BindingResultModel fromBindingResult( BindingResult bindingResult ) { + return new BindingResultModel( bindingResult.getFieldErrors().stream().map( FieldErrorModel::fromFieldError ).collect( Collectors.toList() ) ); + } } @ResponseBody - @PostMapping(value = "/user/profile", produces = MediaType.TEXT_PLAIN_VALUE) - public String saveProfile( @RequestBody ProfileWithOrganUberonIds profileWithOrganUberonIds, Locale locale ) { + @PostMapping(value = "/user/profile", produces = MediaType.APPLICATION_JSON_VALUE) + public ResponseEntity saveProfile( @Valid @RequestBody ProfileWithOrganUberonIds profileWithOrganUberonIds, BindingResult bindingResult, Locale locale ) { User user = userService.findCurrentUser(); - userService.updateUserProfileAndPublicationsAndOrgans( user, profileWithOrganUberonIds.profile, profileWithOrganUberonIds.profile.getPublications(), profileWithOrganUberonIds.organUberonIds, locale ); - return "Saved."; + if ( bindingResult.hasErrors() ) { + return ResponseEntity.badRequest() + .body( BindingResultModel.fromBindingResult( bindingResult ) ); + } else { + String previousContactEmail = user.getProfile().getContactEmail(); + user = userService.updateUserProfileAndPublicationsAndOrgans( user, profileWithOrganUberonIds.profile, profileWithOrganUberonIds.profile.getPublications(), profileWithOrganUberonIds.organUberonIds, locale ); + String message = messageSource.getMessage( "UserController.profileSaved", new Object[]{ user.getProfile().getContactEmail() }, locale ); + if ( user.getProfile().getContactEmail() != null && + !user.getProfile().getContactEmail().equals( previousContactEmail ) && + !user.getProfile().isContactEmailVerified() ) { + message = messageSource.getMessage( "UserController.profileSavedAndContactEmailUpdated", new String[]{ user.getProfile().getContactEmail() }, locale ); + } + return ResponseEntity.ok( new ProfileSavedModel( message, user.getProfile().isContactEmailVerified() ) ); + } } @ResponseBody diff --git a/src/main/java/ubc/pavlab/rdp/model/AccessToken.java b/src/main/java/ubc/pavlab/rdp/model/AccessToken.java index 56a60ec1..7c9d7159 100644 --- a/src/main/java/ubc/pavlab/rdp/model/AccessToken.java +++ b/src/main/java/ubc/pavlab/rdp/model/AccessToken.java @@ -22,8 +22,8 @@ public class AccessToken extends Token implements UserContent { @GeneratedValue(strategy = GenerationType.AUTO) private Integer id; - @ManyToOne(fetch = FetchType.EAGER) - @JoinColumn(name = "user_id", nullable = false) + @ManyToOne(optional = false) + @JoinColumn(name = "user_id") private User user; @Override diff --git a/src/main/java/ubc/pavlab/rdp/model/PasswordResetToken.java b/src/main/java/ubc/pavlab/rdp/model/PasswordResetToken.java index 8fc60ba8..34e97639 100644 --- a/src/main/java/ubc/pavlab/rdp/model/PasswordResetToken.java +++ b/src/main/java/ubc/pavlab/rdp/model/PasswordResetToken.java @@ -26,8 +26,8 @@ public class PasswordResetToken extends Token implements UserContent { @GeneratedValue(strategy = GenerationType.AUTO) private Integer id; - @OneToOne(fetch = FetchType.EAGER) - @JoinColumn(nullable = false, name = "user_id") + @ManyToOne(optional = false) + @JoinColumn(name = "user_id") private User user; @Override diff --git a/src/main/java/ubc/pavlab/rdp/model/User.java b/src/main/java/ubc/pavlab/rdp/model/User.java index 6e67d13d..ef57dec3 100644 --- a/src/main/java/ubc/pavlab/rdp/model/User.java +++ b/src/main/java/ubc/pavlab/rdp/model/User.java @@ -76,17 +76,17 @@ public interface ValidationServiceAccount { @JsonIgnore private final Set roles = new HashSet<>(); - @OneToMany(cascade = CascadeType.ALL) + @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true) @JoinColumn(name = "user_id") @JsonIgnore private final Set accessTokens = new HashSet<>(); - @OneToMany(cascade = CascadeType.ALL) + @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true) @JoinColumn(name = "user_id") @JsonIgnore private final Set verificationTokens = new HashSet<>(); - @OneToMany(cascade = CascadeType.ALL) + @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true) @JoinColumn(name = "user_id") @JsonIgnore private final Set passwordResetTokens = new HashSet<>(); diff --git a/src/main/java/ubc/pavlab/rdp/model/VerificationToken.java b/src/main/java/ubc/pavlab/rdp/model/VerificationToken.java index dbc1c422..42af71f2 100644 --- a/src/main/java/ubc/pavlab/rdp/model/VerificationToken.java +++ b/src/main/java/ubc/pavlab/rdp/model/VerificationToken.java @@ -27,8 +27,8 @@ public class VerificationToken extends Token implements UserContent { @GeneratedValue(strategy = GenerationType.AUTO) private Integer id; - @OneToOne(fetch = FetchType.EAGER) - @JoinColumn(nullable = false, name = "user_id") + @ManyToOne(optional = false) + @JoinColumn(name = "user_id") private User user; @Email diff --git a/src/main/java/ubc/pavlab/rdp/services/RemoteResourceService.java b/src/main/java/ubc/pavlab/rdp/services/RemoteResourceService.java index e47c639d..240c20fc 100644 --- a/src/main/java/ubc/pavlab/rdp/services/RemoteResourceService.java +++ b/src/main/java/ubc/pavlab/rdp/services/RemoteResourceService.java @@ -1,5 +1,6 @@ package ubc.pavlab.rdp.services; +import ubc.pavlab.rdp.controllers.ApiController; import ubc.pavlab.rdp.exception.RemoteException; import ubc.pavlab.rdp.model.Taxon; import ubc.pavlab.rdp.model.User; @@ -10,6 +11,7 @@ import java.net.URI; import java.util.Collection; +import java.util.Locale; import java.util.Set; import java.util.UUID; @@ -19,15 +21,47 @@ */ public interface RemoteResourceService { + /** + * Get the version of the remote API by reading its OpenAPI specification. + * + * @param remoteHost from which only the authority is used with {@link URI#getAuthority()} + * @return the API version + * @throws RemoteException if any error occured while retrieving the API version + */ String getApiVersion( URI remoteHost ) throws RemoteException; + /** + * Find users by name among all partner registries. + * + * @see ApiController#searchUsersByName(String, Boolean, Set, Set, Set, String, String, Locale) + */ Collection findUsersByLikeName( String nameLike, Boolean prefix, Set researcherPositions, Collection researcherTypes, Collection organUberonIds ); + /** + * Find users by description among all partner registries. + * + * @see ApiController#searchUsersByDescription(String, Set, Set, Set, String, String, Locale) + */ Collection findUsersByDescription( String descriptionLike, Set researcherPositions, Collection researcherTypes, Collection organUberonIds ); + /** + * Find genes by symbol among all partner registries. + * + * @see ApiController#searchUsersByGeneSymbol(String, Integer, Set, Integer, Set, Set, Set, String, String, Locale) + */ Collection findGenesBySymbol( String symbol, Taxon taxon, Set tier, Integer orthologTaxonId, Set researcherPositions, Set researcherTypes, Set organUberonIds ); + /** + * Retrieve a user from a specific registry. + * + * @see ApiController#getUserById(Integer, String, String, Locale) + */ User getRemoteUser( Integer userId, URI remoteHost ) throws RemoteException; + /** + * Retrieve an anonymized user from a specific registry. + * + * @see ApiController#getUserByAnonymousId(UUID, String, String, Locale) + */ User getAnonymizedUser( UUID anonymousId, URI remoteHost ) throws RemoteException; } diff --git a/src/main/java/ubc/pavlab/rdp/services/RemoteResourceServiceImpl.java b/src/main/java/ubc/pavlab/rdp/services/RemoteResourceServiceImpl.java index b52f77f1..2b50f62c 100644 --- a/src/main/java/ubc/pavlab/rdp/services/RemoteResourceServiceImpl.java +++ b/src/main/java/ubc/pavlab/rdp/services/RemoteResourceServiceImpl.java @@ -11,10 +11,7 @@ import org.springframework.stereotype.Service; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import org.springframework.util.concurrent.ListenableFuture; -import org.springframework.util.concurrent.ListenableFutureCallback; import org.springframework.web.client.AsyncRestTemplate; -import org.springframework.web.client.RestClientException; import org.springframework.web.util.UriComponentsBuilder; import ubc.pavlab.rdp.exception.RemoteException; import ubc.pavlab.rdp.model.Taxon; @@ -69,9 +66,11 @@ public String getApiVersion( URI remoteHost ) throws RemoteException { .toUri(); try { OpenAPI openAPI = asyncRestTemplate.getForEntity( uri, OpenAPI.class ).get().getBody(); - // OpenAPI specification was introduced in 1.4, so we assume 1.0.0 for previous versions + // The OpenAPI specification was introduced in 1.4, so we assume 1.0.0 for previous versions if ( openAPI.getInfo() == null ) { return "1.0.0"; + } else if ( openAPI.getInfo().getVersion().equals( "v0" ) ) { + return "1.4.0"; // the version number was missing in early 1.4 } else { return openAPI.getInfo().getVersion(); } @@ -146,14 +145,7 @@ public User getRemoteUser( Integer userId, URI remoteHost ) throws RemoteExcepti .buildAndExpand( Collections.singletonMap( "userId", userId ) ) .toUri(); - try { - ResponseEntity responseEntity = asyncRestTemplate.getForEntity( uri, User.class ).get(); - User user = responseEntity.getBody(); - initUser( user ); - return user; - } catch ( InterruptedException | ExecutionException e ) { - throw new RemoteException( MessageFormat.format( "Unsuccessful response received for {0}.", uri ), e ); - } + return getUserByUri( uri ); } @Override @@ -174,6 +166,10 @@ public User getAnonymizedUser( UUID anonymousId, URI remoteHost ) throws RemoteE .buildAndExpand( Collections.singletonMap( "anonymousId", anonymousId ) ) .toUri(); + return getUserByUri( uri ); + } + + private User getUserByUri( URI uri ) throws RemoteException { try { ResponseEntity responseEntity = asyncRestTemplate.getForEntity( uri, User.class ).get(); User user = responseEntity.getBody(); diff --git a/src/main/java/ubc/pavlab/rdp/util/VersionUtils.java b/src/main/java/ubc/pavlab/rdp/util/VersionUtils.java index 60d506f5..2b3904ea 100644 --- a/src/main/java/ubc/pavlab/rdp/util/VersionUtils.java +++ b/src/main/java/ubc/pavlab/rdp/util/VersionUtils.java @@ -1,17 +1,20 @@ package ubc.pavlab.rdp.util; +import lombok.extern.apachecommons.CommonsLog; + +@CommonsLog public class VersionUtils { private static final int[] FACTORS = new int[]{ 99 * 99 * 99, 99 * 99, 99 }; - private static int parseVersion( String version ) { + private static int parseVersion( String version ) throws IllegalArgumentException { int i = 0; int v = 0; String[] components = version.split( "\\." ); for ( String c : components ) { int ci = Integer.parseInt( c ); if ( ci < 0 || ci > 99 ) { - throw new RuntimeException( "Version component must be within 0 and 99." ); + throw new IllegalArgumentException( "Version component must be within 0 and 99." ); } v += FACTORS[i++] * ci; } diff --git a/src/main/resources/db/migration/common/V1.4.6__make_token_user_id_nullable.sql b/src/main/resources/db/migration/common/V1.4.6__make_token_user_id_nullable.sql new file mode 100644 index 00000000..7139df1e --- /dev/null +++ b/src/main/resources/db/migration/common/V1.4.6__make_token_user_id_nullable.sql @@ -0,0 +1,4 @@ +-- I'm not exactly sure why this is necessary, but Hibernate set this column to NULL before removing it +alter table access_token modify column user_id integer null; +alter table password_reset_token modify column user_id integer null; +alter table verification_token modify column user_id integer null; \ No newline at end of file diff --git a/src/main/resources/messages.properties b/src/main/resources/messages.properties index 948d7085..1fef2ed2 100644 --- a/src/main/resources/messages.properties +++ b/src/main/resources/messages.properties @@ -36,7 +36,7 @@ TierType.TIER1=Tier 1 TierType.TIER2=Tier 2 TierType.TIER3=Tier 3 -AbstractUserDetailsAuthenticationProvider.badCredentials=Username/Password is incorrect. +AbstractUserDetailsAuthenticationProvider.badCredentials=Your username or password is incorrect. AbstractUserDetailsAuthenticationProvider.disabled=Your account is disabled, please confirm your email. Click here to resend confirmation. AbstractUserDetailsAuthenticationProvider.expired=User account has expired. AbstractUserDetailsAuthenticationProvider.locked=User account is locked. @@ -55,6 +55,11 @@ SearchController.errorNoOrthologs=No orthologs of {0} found in # {1} contains the taxon scientific name SearchController.errorNoGene=Unknown gene {0} in taxon {1}. +UserController.profileSaved=Your profile has been saved. +# {0} contains the contact email +UserController.profileSavedAndContactEmailUpdated=Your profile has been saved. Your contact email was updated and an \ +email with a verification link has been sent to {0}. + # {0} contains the site shortname ApiConfig.title={0} RESTful API # {0} contains the site shortname diff --git a/src/main/resources/static/js/profile.js b/src/main/resources/static/js/profile.js index 20d64211..eec6218b 100644 --- a/src/main/resources/static/js/profile.js +++ b/src/main/resources/static/js/profile.js @@ -162,40 +162,45 @@ var profile = collectProfile(); var spinner = $(this).find('.spinner'); - spinner.removeClass("d-none"); + spinner.toggleClass('d-none', false); + + // reset any invalid state + $('.is-invalid').toggleClass('is-invalid', false); + $('.invalid-feedback').remove(); // noinspection JSUnusedLocalSymbols $.ajax({ type: "POST", url: window.location.href, data: JSON.stringify(profile), - contentType: "application/json", - success: function (r) { - $('.success-row').show(); - $('.error-row').hide(); - spinner.addClass("d-none"); - initialProfile = collectProfile(); - $('.new-row').removeClass("new-row"); - $('#saved-button').focus(); - // hide the verification badge - $('.contact-email-verification-badge').toggleClass('d-none', true); - }, - error: function (r) { - var errorMessages = []; - try { - $.each(r.responseJSON.errors, function (idx, item) { - errorMessages.push(item.field + " - " + item.defaultMessage); - }); - } finally { - var message = "Profile Not Saved: " + errorMessages.join(", "); - - $('.success-row').hide(); - var $error = $('.error-row'); - $error.find('div.alert').text(message); - $error.show(); - spinner.addClass("d-none"); - } + contentType: "application/json" + }).done(function (r) { + // update the initial profile to the newly saved one + initialProfile = profile; + $('#profile-success-alert-message').text(r.message); + $('#profile-success-alert').show(); + $('#profile-error-alert').hide(); + // display the verified badge if the email is verified + // hide the not verified badge and resend link if the email is verified + // sorry for the inversed logic here, the d-none class hides the tag + $('#contact-email-verified-badge').toggleClass('d-none', !r.contactEmailVerified); + $('#contact-email-not-verified-badge').toggleClass('d-none', r.contactEmailVerified); + $('#contact-email-resend-verification-email-button').toggleClass('d-none', r.contactEmailVerified); + $('#saved-button').focus(); + }).fail(function (r) { + var message = "Your profile could not be saved."; + if ('fieldErrors' in r.responseJSON) { + r.responseJSON.fieldErrors.forEach(function (fieldError) { + $('[name="' + fieldError.field + '"]') + .toggleClass('is-invalid', true) + .after($('
', {'class': 'invalid-feedback text-danger d-block'}).text(fieldError.message)); + }); } + $('#profile-error-alert-message').html(message); + $('#profile-error-alert').show(); + $('#profile-success-alert').hide(); + }).always(function () { + spinner.toggleClass('d-none', true); }); }); })(); diff --git a/src/main/resources/templates/user/profile.html b/src/main/resources/templates/user/profile.html index d04b557d..af0b5280 100644 --- a/src/main/resources/templates/user/profile.html +++ b/src/main/resources/templates/user/profile.html @@ -27,16 +27,18 @@
-