add installation brand name to subject of verify email message #3941#3945
add installation brand name to subject of verify email message #3941#3945
Conversation
bsilverstein95
left a comment
bsilverstein95
left a comment
There was a problem hiding this comment.
Fixes broken email branding issue for verification emails sent by the app upon user email change 👍 also consolidates another piece of mail into MailUtil.java
| mailService.sendSystemEmail(toAddress, subject, messageBody); | ||
| } | ||
| } catch (Exception e) { | ||
| logger.info("The root dataverse is not present. Don't send a notification to dataverseAdmin."); |
There was a problem hiding this comment.
Does this warrant more than info? I'd at least put this at warn
There was a problem hiding this comment.
Meh, as we discussed in person, anything higher than "fine" is shown. I think "info" is high enough.
| Dataverse rootDataverse = dataverseService.findRootDataverse(); | ||
| if (rootDataverse != null) { | ||
| String rootDataverseName = rootDataverse.getName(); | ||
| // FIXME: consider refactoring this into MailServiceBean.sendNotificationEmail. Then we might need |
There was a problem hiding this comment.
might need what?
Sorry, the developer died while writing that comment. 😉
I just finished off the comment in 5b75860. Thanks for noticing, @oscardssmith !
| case CHECKSUMIMPORT: | ||
| return BundleUtil.getStringFromBundle("notification.email.import.checksum.subject", rootDvNameAsList); | ||
| case CONFIRMEMAIL: | ||
| return BundleUtil.getStringFromBundle("notification.email.verifyEmail.subject", rootDvNameAsList); |
There was a problem hiding this comment.
As I just mentioned to @sekmiller some day it might be nice to associate these bundle keys with the UserNotification object. Maybe you just call userNotification.getEmailSubject() or something. Out of scope for now but something to consider for the future.
|
Looks good. Passing to QA. As Phil noted there are some things worth thinking about when we do a more comprehensive rewrite of notifications. |
6 participants
New Contributors
Welcome! New contributors should at least glance at CONTRIBUTING.md, especially the section on pull requests where we encourage you to reach out to other developers before you start coding. Also, please note that we measure code coverage and prefer you write unit tests. Pull requests can still be reviewed without tests or completion of the checklist outlined below. Thanks!
Related Issues
Pull Request Checklist