From 02e1220dfea171ea1fa0c4cdd7bab6c65f3942ef Mon Sep 17 00:00:00 2001 From: amontenegro Date: Wed, 1 Nov 2023 14:46:09 -0600 Subject: [PATCH] Working on remider emails, refactoring code so we get the primary email information along with the email and orcid id --- .../orcid/core/utils/VerifyEmailUtils.java | 19 ++-- .../resources/i18n/email_common_en.properties | 3 + .../i18n/email_subject_en.properties | 2 +- .../resources/i18n/email_verify_en.properties | 15 +--- .../i18n/email_welcome_en.properties | 6 +- .../how_do_i_verify_my_email_address_html.ftl | 36 ++++++++ .../template/verification_email_html_v2.ftl | 89 ++++++------------- .../core/template/welcome_email_html_v2.ftl | 65 ++------------ .../orcid/core/template/welcome_email_v2.ftl | 2 +- .../org/orcid/persistence/dao/ProfileDao.java | 6 +- .../persistence/dao/impl/ProfileDaoImpl.java | 47 ++-------- .../orcid/persistence/dao/ProfileDaoTest.java | 6 +- .../cli/manager/EmailMessageSenderImpl.java | 50 ++++------- .../frontend/email/RecordEmailSender.java | 16 ++-- .../web/controllers/AdminController.java | 7 +- .../controllers/ManageProfileController.java | 26 +++--- .../controllers/PasswordResetController.java | 3 +- .../frontend/email/RecordEmailSenderTest.java | 2 +- .../ManageProfileControllerTest.java | 16 ++-- 19 files changed, 143 insertions(+), 273 deletions(-) create mode 100644 orcid-core/src/main/resources/org/orcid/core/template/how_do_i_verify_my_email_address_html.ftl diff --git a/orcid-core/src/main/java/org/orcid/core/utils/VerifyEmailUtils.java b/orcid-core/src/main/java/org/orcid/core/utils/VerifyEmailUtils.java index 523900c3f62..fa7572d75db 100644 --- a/orcid-core/src/main/java/org/orcid/core/utils/VerifyEmailUtils.java +++ b/orcid-core/src/main/java/org/orcid/core/utils/VerifyEmailUtils.java @@ -29,21 +29,14 @@ public class VerifyEmailUtils { @Resource private EncryptionManager encryptionManager; - public Map createParamsForVerificationEmail(String emailFriendlyName, String orcid, String email, String primaryEmail, Locale locale) { + public Map createParamsForVerificationEmail(String emailFriendlyName, String orcid, String email, boolean isPrimary, Locale locale) { Map templateParams = new HashMap(); - - String subject = getSubject(primaryEmail.equalsIgnoreCase(email) ? "email.subject.verify_reminder_primary" : "email.subject.verify_reminder", locale); - - templateParams.put("primaryEmail", primaryEmail); - templateParams.put("isPrimary", primaryEmail.equalsIgnoreCase(email)); - templateParams.put("emailName", emailFriendlyName); - String verificationUrl = createVerificationUrl(email, orcidUrlManager.getBaseUrl()); - templateParams.put("verificationUrl", verificationUrl); + templateParams.put("isPrimary", isPrimary); + templateParams.put("userName", emailFriendlyName); + templateParams.put("verificationUrl", createVerificationUrl(email, orcidUrlManager.getBaseUrl())); templateParams.put("orcid", orcid); - templateParams.put("email", email); - templateParams.put("subject", subject); - templateParams.put("baseUri", orcidUrlManager.getBaseUrl()); - templateParams.put("baseUriHttp", orcidUrlManager.getBaseUriHttp()); + templateParams.put("subject", getSubject((isPrimary ? "email.subject.verify_reminder_primary" : "email.subject.verify_reminder"), locale)); + templateParams.put("baseUri", orcidUrlManager.getBaseUrl()); addMessageParams(templateParams, locale); return templateParams; } diff --git a/orcid-core/src/main/resources/i18n/email_common_en.properties b/orcid-core/src/main/resources/i18n/email_common_en.properties index 4f684ba6c2f..f53bb0cd213 100644 --- a/orcid-core/src/main/resources/i18n/email_common_en.properties +++ b/orcid-core/src/main/resources/i18n/email_common_en.properties @@ -66,3 +66,6 @@ email.common.privacy_policy=privacy policy email.common.address1=ORCID, Inc. email.common.address2=10411 Motor City Drive, Suite 750, Bethesda, MD 20817, USA email.common.received_email_as_service=You have received this email as a service announcement related to your ORCID Account. + +email.welcome.your_id.id=Your ORCID iD: +email.welcome.your_id.link=Your ORCID record is diff --git a/orcid-core/src/main/resources/i18n/email_subject_en.properties b/orcid-core/src/main/resources/i18n/email_subject_en.properties index c47b6f21991..fe7cf273f76 100644 --- a/orcid-core/src/main/resources/i18n/email_subject_en.properties +++ b/orcid-core/src/main/resources/i18n/email_subject_en.properties @@ -2,7 +2,7 @@ # NotificaionManager.java email.subject.verify_reminder=[ORCID] Verify your email address -email.subject.verify_reminder_primary=[ORCID] Reminder to verify your primary email address +email.subject.verify_reminder_primary=[ORCID] Don't forget to verify your email address email.subject.reset=[ORCID] About your password reset request email.subject.forgotten_id=[ORCID] Your ORCID iD email.subject.reset_not_found=[ORCID] Email address not found diff --git a/orcid-core/src/main/resources/i18n/email_verify_en.properties b/orcid-core/src/main/resources/i18n/email_verify_en.properties index 89ccf28065e..330dbba7088 100644 --- a/orcid-core/src/main/resources/i18n/email_verify_en.properties +++ b/orcid-core/src/main/resources/i18n/email_verify_en.properties @@ -3,15 +3,6 @@ ## Used on: # verification_email -email.verify.primary_reminder_v2=This is a reminder that you need to verify your primary email address before you can begin adding information such as your affiliation, biography, or keywords manually to your ORCID record. -email.verify.primary_reminder=This is a reminder that you need to verify your primary email address before you can begin adding information manually to your ORCID record. -email.verify.thank_you=To verify your email address, click the following link and sign into your ORCID record. If you can't click the link, copy and paste it into your browser's address bar: -email.verify.thank_you_button=To verify your email address, click the following button and sign into your ORCID record. If you can't click the button, copy and paste the link below into your browser's address bar: -#${verificationUrl} -email.verify.1=Your 16-digit ORCID identifier is -#${orcid} -email.verify.2=, and your full ORCID iD and the link to your public record is -#${pubBaseUri}/${orcid} -email.verify.primary_email_1=(primary email address: -email.verify.primary_email_2=) -email.verify.if_you_did_not=If you did not add this email address to your ORCID record, please contact us immediately by replying to this email. +email.verify.hi=Hi +email.verify.primary.reminder=This is a friendly reminder to verify your email address in your ORCID record so that you can unlock advanced editing features in your ORCID record. Until then, you will only be able to manage your names and email addresses in your ORCID record. +email.verify.alternate.reminder=You need to verify your email address. \ No newline at end of file diff --git a/orcid-core/src/main/resources/i18n/email_welcome_en.properties b/orcid-core/src/main/resources/i18n/email_welcome_en.properties index 58b30c52d9f..3967f2ae41e 100644 --- a/orcid-core/src/main/resources/i18n/email_welcome_en.properties +++ b/orcid-core/src/main/resources/i18n/email_welcome_en.properties @@ -36,12 +36,10 @@ email.button=Verify your email address email.welcome=Welcome to ORCID email.welcome.congrats=Congratulations on creating your new ORCID iD! This persistent digital identifier, that you own and control, will distinguish you from every other researcher and reduce your burden when you use it in manuscript and grant submission systems. email.welcome.for_more_information=for more information on how to get the most out of your ORCID record. -email.welcome.please_visit_your=Please visit our +email.welcome.please_visit_our=Please visit our email.welcome.researcher_homepage=researcher homepage email.welcome.verify.1=Verifying your email address unlocks advanced editing features in your ORCID record. Until then you will only be able to manage your names and email addresses in your ORCID record. email.welcome.verify.2=How do I verify my email address? email.welcome.verify.3=Simply click the button below to sign into your ORCID record and complete verification. email.welcome.verify.4=Or, copy and paste the link below into your browser's address bar: -email.welcome.visit=Please visit our researcher homepage for more information on how to get the most out of your ORCID record. -email.welcome.your_id.id=Your ORCID iD: -email.welcome.your_id.link=Your ORCID record is +email.welcome.visit=Please visit our researcher homepage for more information on how to get the most out of your ORCID record. \ No newline at end of file diff --git a/orcid-core/src/main/resources/org/orcid/core/template/how_do_i_verify_my_email_address_html.ftl b/orcid-core/src/main/resources/org/orcid/core/template/how_do_i_verify_my_email_address_html.ftl new file mode 100644 index 00000000000..cee91dc8194 --- /dev/null +++ b/orcid-core/src/main/resources/org/orcid/core/template/how_do_i_verify_my_email_address_html.ftl @@ -0,0 +1,36 @@ +

+ <@emailMacros.msg "email.welcome.verify.1" /> +

+

+ <@emailMacros.msg "email.welcome.verify.2" />
+ <@emailMacros.msg "email.welcome.verify.3" /> + + + + + + +
+ <@emailMacros.msg "email.button" /> +
+

+

+ <@emailMacros.msg "email.welcome.verify.4" /> + + + + + + +
+

+ ${verificationUrl}?lang=${locale} +

+
+

\ No newline at end of file diff --git a/orcid-core/src/main/resources/org/orcid/core/template/verification_email_html_v2.ftl b/orcid-core/src/main/resources/org/orcid/core/template/verification_email_html_v2.ftl index 0bfbce81fb9..5bf21902452 100644 --- a/orcid-core/src/main/resources/org/orcid/core/template/verification_email_html_v2.ftl +++ b/orcid-core/src/main/resources/org/orcid/core/template/verification_email_html_v2.ftl @@ -20,70 +20,31 @@ <#escape x as x?html> - - ${subject} - - -
- ORCID.org -
-

- <#if isPrimary?? && isPrimary> - <@emailMacros.msg "email.verify.primary_reminder_v2" /><@emailMacros.space /> - - <@emailMacros.msg "email.verify.thank_you_button" /> + + ${subject} + + +

+ ORCID.org +
+

+ <@emailMacros.msg "email.welcome.your_id.id" /><@emailMacros.space />${orcidId}
+ <@emailMacros.msg "email.welcome.your_id.link" /><@emailMacros.space />${baseUri}/${orcidId}

- - - - - - -
- <@emailMacros.msg "email.button" /> -
- - - - - - - -
-

- ${verificationUrl}?lang=${locale} -

-
- -

- <@emailMacros.msg "email.verify.1" /><@emailMacros.space />${orcid}<@emailMacros.msg "email.verify.2" /><@emailMacros.space />${baseUri}/${orcid}<@emailMacros.space /><@emailMacros.msg "email.verify.primary_email_1" /><@emailMacros.space />${primaryEmail}<@emailMacros.msg "email.verify.primary_email_2" />. -

- <#include "email_footer_html.ftl"/> -
- +
+

+ <@emailMacros.msg "email.verify.hi" /><@emailMacros.space />${userName},
+ <#if isPrimary?? && isPrimary><@emailMacros.msg "email.verify.primary.reminder" /><#elseif><@emailMacros.msg "email.verify.alternate.reminder" /> +

+ <#include "how_do_i_verify_my_email_address_html.ftl"/> +
+ <#if isPrimary?? && isPrimary> +

+ <@emailMacros.msg "email.welcome.please_visit_our" /><@emailMacros.space /><@emailMacros.msg "email.welcome.researcher_homepage" /><@emailMacros.space /><@emailMacros.msg "email.welcome.for_more_information" /> +

+ + <#include "email_footer_html.ftl"/> +
+ diff --git a/orcid-core/src/main/resources/org/orcid/core/template/welcome_email_html_v2.ftl b/orcid-core/src/main/resources/org/orcid/core/template/welcome_email_html_v2.ftl index d3aefd6e42d..e3ba3ed8192 100644 --- a/orcid-core/src/main/resources/org/orcid/core/template/welcome_email_html_v2.ftl +++ b/orcid-core/src/main/resources/org/orcid/core/template/welcome_email_html_v2.ftl @@ -27,74 +27,19 @@
ORCID.org
-

+

<@emailMacros.msg "email.welcome.your_id.id" /><@emailMacros.space />${orcidId}
<@emailMacros.msg "email.welcome.your_id.link" /><@emailMacros.space />${baseUri}/${orcidId}

-

+

<@emailMacros.msg "email.welcome" /><@emailMacros.space />${userName},
<@emailMacros.msg "email.welcome.congrats" />

-

- <@emailMacros.msg "email.welcome.verify.1" /> -

- -

- <@emailMacros.msg "email.welcome.verify.2" />
- <@emailMacros.msg "email.welcome.verify.3" /> - - - - - - -
- <@emailMacros.msg "email.button" /> -
-

- -

- <@emailMacros.msg "email.welcome.verify.4" /> - - - - - - -
-

- ${verificationUrl}?lang=${locale} -

-
-

+ <#include "how_do_i_verify_my_email_address_html.ftl"/>
-

- <@emailMacros.msg "email.welcome.please_visit_your" /><@emailMacros.space /><@emailMacros.msg "email.welcome.researcher_homepage" /><@emailMacros.space /><@emailMacros.msg "email.welcome.for_more_information" /> +

+ <@emailMacros.msg "email.welcome.please_visit_our" /><@emailMacros.space /><@emailMacros.msg "email.welcome.researcher_homepage" /><@emailMacros.space /><@emailMacros.msg "email.welcome.for_more_information" />

<#include "email_footer_html.ftl"/>
diff --git a/orcid-core/src/main/resources/org/orcid/core/template/welcome_email_v2.ftl b/orcid-core/src/main/resources/org/orcid/core/template/welcome_email_v2.ftl index 36c0ba7456b..8ee76ac1a85 100644 --- a/orcid-core/src/main/resources/org/orcid/core/template/welcome_email_v2.ftl +++ b/orcid-core/src/main/resources/org/orcid/core/template/welcome_email_v2.ftl @@ -36,6 +36,6 @@ ${verificationUrl}?lang=${locale} -<@emailMacros.msg "email.welcome.please_visit_your" /><@emailMacros.space /><@emailMacros.msg "email.welcome.researcher_homepage" /><@emailMacros.space /><@emailMacros.msg "email.welcome.for_more_information" /> +<@emailMacros.msg "email.welcome.please_visit_our" /><@emailMacros.space /><@emailMacros.msg "email.welcome.researcher_homepage" /><@emailMacros.space /><@emailMacros.msg "email.welcome.for_more_information" /> <#include "email_footer.ftl"/> diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileDao.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileDao.java index 4a9626f4148..1737562b61a 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileDao.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileDao.java @@ -4,7 +4,7 @@ import java.util.Date; import java.util.List; -import org.apache.commons.lang3.tuple.Pair; +import org.apache.commons.lang3.tuple.Triple; import org.orcid.persistence.jpa.entities.EmailEventType; import org.orcid.persistence.jpa.entities.IndexingStatus; import org.orcid.persistence.jpa.entities.OrcidGrantedAuthority; @@ -76,10 +76,8 @@ public interface ProfileDao extends GenericDao { void updateLastModifiedDateAndIndexingStatusWithoutResult(String orcid, Date lastModified, IndexingStatus indexingStatus); - public List> findEmailsUnverfiedDays(int daysUnverified, int maxResults); + public List> findEmailsUnverfiedDays(int daysUnverified, int maxResults); - public List> findEmailsUnverifiedDaysAndEventType(int daysUnverified, int maxResults, List eventTypes); - String retrieveOrcidType(String orcid); List findInfoForDecryptionAnalysis(); diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileDaoImpl.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileDaoImpl.java index 2d97a19c212..baf0d02de86 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileDaoImpl.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileDaoImpl.java @@ -5,18 +5,14 @@ import java.util.Collections; import java.util.Date; import java.util.List; -import java.util.Objects; -import java.util.Optional; -import java.util.stream.Collectors; import javax.persistence.NoResultException; import javax.persistence.Query; import javax.persistence.TypedQuery; -import org.apache.commons.lang3.tuple.Pair; +import org.apache.commons.lang3.tuple.Triple; import org.orcid.persistence.aop.UpdateProfileLastModifiedAndIndexingStatus; import org.orcid.persistence.dao.ProfileDao; -import org.orcid.persistence.jpa.entities.EmailEventType; import org.orcid.persistence.jpa.entities.IndexingStatus; import org.orcid.persistence.jpa.entities.OrcidGrantedAuthority; import org.orcid.persistence.jpa.entities.ProfileEntity; @@ -24,7 +20,6 @@ import org.orcid.persistence.jpa.entities.ProfileEventType; import org.springframework.beans.factory.annotation.Value; import org.springframework.transaction.annotation.Transactional; -import org.springframework.util.StringUtils; public class ProfileDaoImpl extends GenericDaoImpl implements ProfileDao { @@ -136,8 +131,8 @@ public List findUnclaimedNotIndexedAfterWaitPeriod(int waitPeriodDays, i @SuppressWarnings("unchecked") @Override - public List> findEmailsUnverfiedDays(int daysUnverified, int maxResults) { - StringBuilder queryString = new StringBuilder("SELECT e.email, e.date_created FROM email e "); + public List> findEmailsUnverfiedDays(int daysUnverified, int maxResults) { + StringBuilder queryString = new StringBuilder("SELECT e.email, e.is_primary, e.date_created FROM email e "); queryString.append("LEFT JOIN email_event ev ON e.email = ev.email "); queryString.append("JOIN profile p on p.orcid = e.orcid and p.claimed = true "); queryString.append("AND p.deprecated_date is null AND p.profile_deactivation_date is null AND p.account_expiry is null "); @@ -149,43 +144,13 @@ public List> findEmailsUnverfiedDays(int daysUnverified, int Query query = entityManager.createNativeQuery(queryString.toString()); query.setMaxResults(maxResults); List dbInfo = query.getResultList(); - List> results = new ArrayList>(); + List> results = new ArrayList>(); dbInfo.stream().forEach(element -> { - Pair pair = Pair.of((String) element[0], (Date) element[1]); + Triple pair = Triple.of((String) element[0], (Boolean) element[1], (Date) element[2]); results.add(pair); }); return results; - } - - @SuppressWarnings("unchecked") - @Override - public List> findEmailsUnverifiedDaysAndEventType(int daysUnverified, int maxResults, List eventTypes) { - List stringList = eventTypes.stream().map(Optional::ofNullable) //Stream> - .map(opt -> opt.orElse(null)) - .map(Objects::toString) - .map(opt -> "'"+opt+"'") - .collect(Collectors.toList()); - - String eventsTypeStr = StringUtils.collectionToCommaDelimitedString(stringList); - StringBuilder queryString = new StringBuilder("SELECT e.email, e.date_created FROM email e "); - queryString.append("LEFT JOIN email_event ev ON e.email = ev.email "); - queryString.append("JOIN profile p on p.orcid = e.orcid and p.claimed = true "); - queryString.append("AND p.deprecated_date is null AND p.profile_deactivation_date is null AND p.account_expiry is null "); - queryString.append("where (ev.email IS NULL or email_event_type NOT IN ("+ eventsTypeStr+"))" + "and e.is_verified = false "); - queryString.append("and e.date_created < (now() - CAST('").append(daysUnverified).append("' AS INTERVAL DAY)) "); - queryString.append("and (e.source_id = e.orcid OR e.source_id is null)"); - queryString.append(" GROUP BY e.email, e.date_created, e.last_modified"); - queryString.append(" ORDER BY e.last_modified"); - Query query = entityManager.createNativeQuery(queryString.toString()); - query.setMaxResults(maxResults); - List dbInfo = query.getResultList(); - List> results = new ArrayList>(); - dbInfo.stream().forEach(element -> { - Pair pair = Pair.of((String) element[0], (Date) element[1]); - results.add(pair); - }); - return results; - } + } @SuppressWarnings("unchecked") @Override diff --git a/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileDaoTest.java b/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileDaoTest.java index 6a86cede3e0..96e6551b8e9 100644 --- a/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileDaoTest.java +++ b/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileDaoTest.java @@ -17,7 +17,7 @@ import javax.persistence.EntityManager; import javax.persistence.Query; -import org.apache.commons.lang3.tuple.Pair; +import org.apache.commons.lang3.tuple.Triple; import org.dbunit.dataset.DataSetException; import org.joda.time.LocalDateTime; import org.junit.AfterClass; @@ -405,13 +405,13 @@ public void findEmailsUnverfiedDaysTest() throws IllegalAccessException { // Created 15 days ago and verified assertEquals(1, insertEmailWithDateCreated("verified_2@test.orcid.org", "896dea808bbf69bde1b177f27800e84d17763860bffde1dfd8ef200e79ff9971", orcid, true, LocalDateTime.now().minusDays(15).toDate())); - List> results = profileDao.findEmailsUnverfiedDays(7, 100); + List> results = profileDao.findEmailsUnverfiedDays(7, 100); assertNotNull(results); assertEquals(2, results.size()); boolean found1 = false, found2 = false; - for(Pair element : results) { + for(Triple element : results) { assertNotNull(element.getRight()); if(element.getLeft().equals("unverified_2@test.orcid.org")) { found1 = true; diff --git a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/email/cli/manager/EmailMessageSenderImpl.java b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/email/cli/manager/EmailMessageSenderImpl.java index 526e7812c7d..c4deda62955 100644 --- a/orcid-scheduler-web/src/main/java/org/orcid/scheduler/email/cli/manager/EmailMessageSenderImpl.java +++ b/orcid-scheduler-web/src/main/java/org/orcid/scheduler/email/cli/manager/EmailMessageSenderImpl.java @@ -24,7 +24,7 @@ import org.apache.commons.codec.binary.Base64; import org.apache.commons.lang3.LocaleUtils; import org.apache.commons.lang3.time.DurationFormatUtils; -import org.apache.commons.lang3.tuple.Pair; +import org.apache.commons.lang3.tuple.Triple; import org.joda.time.LocalDateTime; import org.orcid.core.constants.EmailConstants; import org.orcid.core.locale.LocaleManager; @@ -571,7 +571,7 @@ private String getHtmlBody(NotificationAdministrative notificationAdministrative @Override synchronized public void processUnverifiedEmails2Days() { LOGGER.info("About to process unverIfied emails for 2 days reminder"); - List> elements = Collections.> emptyList(); + List> elements = Collections.> emptyList(); do { elements = profileDaoReadOnly.findEmailsUnverfiedDays(verifyReminderAfterTwoDays, 100); LOGGER.info("Got batch of {} profiles with unverified emails for 2 days reminder", elements.size()); @@ -581,9 +581,9 @@ synchronized public void processUnverifiedEmails2Days() { if(Features.SEND_ALL_VERIFICATION_EMAILS.isActive()) { tooOld = now.minusDays(emailTooOld).toDate(); } - for (Pair element : elements) { + for (Triple element : elements) { if(element.getRight() == null || element.getRight().after(tooOld)) { - processUnverifiedEmails2DaysInTransaction(element.getLeft()); + processUnverifiedEmailsInTransaction(element.getLeft(), element.getMiddle(), EmailEventType.VERIFY_EMAIL_2_DAYS_SENT, EmailEventType.VERIFY_EMAIL_2_DAYS_SENT_SKIPPED); } else { // Mark is as too old to send the verification email markUnverifiedEmailAsTooOld(element.getLeft()); @@ -596,15 +596,15 @@ synchronized public void processUnverifiedEmails2Days() { synchronized public void processUnverifiedEmails7Days() { if(Features.SEND_ALL_VERIFICATION_EMAILS.isActive()) { LOGGER.info("About to process unverIfied emails for 7 days reminder"); - List> elements = Collections.> emptyList(); + List> elements = Collections.> emptyList(); do { elements = profileDaoReadOnly.findEmailsUnverfiedDays(verifyReminderAfterSevenDays, 100); LOGGER.info("Got batch of {} profiles with unverified emails for 7 days reminder", elements.size()); LocalDateTime now = LocalDateTime.now(); Date tooOld = now.minusDays(emailTooOld).toDate(); - for (Pair element : elements) { + for (Triple element : elements) { if(element.getRight() == null || element.getRight().after(tooOld)) { - processUnverifiedEmailsInTransaction(element.getLeft(), EmailEventType.VERIFY_EMAIL_7_DAYS_SENT, EmailEventType.VERIFY_EMAIL_7_DAYS_SENT_SKIPPED); + processUnverifiedEmailsInTransaction(element.getLeft(), element.getMiddle(), EmailEventType.VERIFY_EMAIL_7_DAYS_SENT, EmailEventType.VERIFY_EMAIL_7_DAYS_SENT_SKIPPED); } else { // Mark is as too old to send the verification email markUnverifiedEmailAsTooOld(element.getLeft()); @@ -618,15 +618,15 @@ synchronized public void processUnverifiedEmails7Days() { synchronized public void processUnverifiedEmails28Days() { if(Features.SEND_ALL_VERIFICATION_EMAILS.isActive()) { LOGGER.info("About to process unverIfied emails for 28 days reminder"); - List> elements = Collections.> emptyList(); + List> elements = Collections.> emptyList(); do { elements = profileDaoReadOnly.findEmailsUnverfiedDays(verifyReminderAfterTwentyEightDays, 100); LOGGER.info("Got batch of {} profiles with unverified emails for 28 days reminder", elements.size()); LocalDateTime now = LocalDateTime.now(); Date tooOld = now.minusDays(emailTooOld).toDate(); - for (Pair element : elements) { + for (Triple element : elements) { if(element.getRight() == null || element.getRight().after(tooOld)) { - processUnverifiedEmailsInTransaction(element.getLeft(),EmailEventType.VERIFY_EMAIL_28_DAYS_SENT, EmailEventType.VERIFY_EMAIL_28_DAYS_SENT_SKIPPED); + processUnverifiedEmailsInTransaction(element.getLeft(), element.getMiddle(), EmailEventType.VERIFY_EMAIL_28_DAYS_SENT, EmailEventType.VERIFY_EMAIL_28_DAYS_SENT_SKIPPED); } else { // Mark is as too old to send the verification email markUnverifiedEmailAsTooOld(element.getLeft()); @@ -636,14 +636,14 @@ synchronized public void processUnverifiedEmails28Days() { } } - private void processUnverifiedEmailsInTransaction(final String email, EmailEventType eventSent, EmailEventType eventSkipped) { + private void processUnverifiedEmailsInTransaction(final String email, final Boolean isPrimaryEmail, EmailEventType eventSent, EmailEventType eventSkipped) { transactionTemplate.execute(new TransactionCallbackWithoutResult() { @Override @Transactional protected void doInTransactionWithoutResult(TransactionStatus status) { try { String userOrcid = emailManagerReadOnly.findOrcidIdByEmail(email); - sendVerificationReminderEmail(userOrcid, email); + sendVerificationReminderEmail(userOrcid, email, isPrimaryEmail); emailEventDao.persist(new EmailEventEntity(email, eventSent)); emailEventDao.flush(); } catch (Exception e) { @@ -653,26 +653,7 @@ protected void doInTransactionWithoutResult(TransactionStatus status) { } } }); - } - - private void processUnverifiedEmails2DaysInTransaction(final String email) { - transactionTemplate.execute(new TransactionCallbackWithoutResult() { - @Override - @Transactional - protected void doInTransactionWithoutResult(TransactionStatus status) { - try { - String userOrcid = emailManagerReadOnly.findOrcidIdByEmail(email); - sendVerificationReminderEmail(userOrcid, email); - emailEventDao.persist(new EmailEventEntity(email, EmailEventType.VERIFY_EMAIL_2_DAYS_SENT)); - emailEventDao.flush(); - } catch (Exception e) { - LOGGER.error("Unable to send unverified email reminder to email: " + email, e); - emailEventDao.persist(new EmailEventEntity(email, EmailEventType.VERIFY_EMAIL_2_DAYS_SENT_SKIPPED)); - emailEventDao.flush(); - } - } - }); - } + } private void markUnverifiedEmailAsTooOld(final String email) { transactionTemplate.execute(new TransactionCallbackWithoutResult() { @@ -685,13 +666,12 @@ protected void doInTransactionWithoutResult(TransactionStatus status) { }); } - private void sendVerificationReminderEmail(String userOrcid, String email) { + private void sendVerificationReminderEmail(String userOrcid, String email, Boolean isPrimaryEmail) { ProfileEntity profile = profileEntityCacheManager.retrieve(userOrcid); Locale locale = getUserLocaleFromProfileEntity(profile); - String primaryEmail = emailManagerReadOnly.findPrimaryEmail(userOrcid).getEmail(); String emailFriendlyName = recordNameManagerV3.deriveEmailFriendlyName(userOrcid); - Map templateParams = verifyEmailUtils.createParamsForVerificationEmail(emailFriendlyName, userOrcid, email, primaryEmail, locale); + Map templateParams = verifyEmailUtils.createParamsForVerificationEmail(emailFriendlyName, userOrcid, email, isPrimaryEmail, locale); String subject = (String) templateParams.get("subject"); templateParams.put("isReminder", true); // Generate body from template diff --git a/orcid-web/src/main/java/org/orcid/frontend/email/RecordEmailSender.java b/orcid-web/src/main/java/org/orcid/frontend/email/RecordEmailSender.java index ec3333351b0..564c8fb71bb 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/email/RecordEmailSender.java +++ b/orcid-web/src/main/java/org/orcid/frontend/email/RecordEmailSender.java @@ -379,23 +379,21 @@ public void sendForgottenIdEmailNotFoundEmail(String email, Locale locale) { public void sendVerificationEmailToNonPrimaryEmails(String userOrcid) { emailManager.getEmails(userOrcid).getEmails().stream().filter(e -> !e.isPrimary()).map(e -> e.getEmail()).forEach(e -> { - sendVerificationEmail(userOrcid, e); + sendVerificationEmail(userOrcid, e, false); }); } - public void sendVerificationEmail(String userOrcid, String email) { - processVerificationEmail(userOrcid, email, false); + public void sendVerificationEmail(String userOrcid, String email, Boolean isPrimaryEmail) { + processVerificationEmail(userOrcid, email, isPrimaryEmail); } - private void processVerificationEmail(String userOrcid, String email, boolean isVerificationReminder) { + private void processVerificationEmail(String userOrcid, String email, boolean isPrimaryEmail) { ProfileEntity profile = profileEntityCacheManager.retrieve(userOrcid); Locale locale = getUserLocaleFromProfileEntity(profile); - - String primaryEmail = emailManager.findPrimaryEmail(userOrcid).getEmail(); + String emailFriendlyName = recordNameManager.deriveEmailFriendlyName(userOrcid); - Map templateParams = verifyEmailUtils.createParamsForVerificationEmail(emailFriendlyName, userOrcid, email, primaryEmail, locale); - String subject = (String) templateParams.get("subject"); - templateParams.put("isReminder", isVerificationReminder); + Map templateParams = verifyEmailUtils.createParamsForVerificationEmail(emailFriendlyName, userOrcid, email, isPrimaryEmail, locale); + String subject = (String) templateParams.get("subject"); // Generate body from template String body = templateManager.processTemplate("verification_email_v2.ftl", templateParams); String htmlBody = templateManager.processTemplate("verification_email_html_v2.ftl", templateParams); diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/AdminController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/AdminController.java index e92a67ca7a9..2badc25731a 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/AdminController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/AdminController.java @@ -26,6 +26,8 @@ import org.orcid.core.manager.v3.read_only.RecordNameManagerReadOnly; import org.orcid.core.togglz.Features; import org.orcid.core.utils.OrcidStringUtils; +import org.orcid.frontend.email.RecordEmailSender; +import org.orcid.frontend.web.util.PasswordConstants; import org.orcid.jaxb.model.clientgroup.ClientType; import org.orcid.jaxb.model.clientgroup.MemberType; import org.orcid.jaxb.model.common.OrcidType; @@ -33,8 +35,6 @@ import org.orcid.jaxb.model.v3.release.record.Email; import org.orcid.jaxb.model.v3.release.record.Emails; import org.orcid.jaxb.model.v3.release.record.Name; -import org.orcid.frontend.email.RecordEmailSender; -import org.orcid.frontend.web.util.PasswordConstants; import org.orcid.persistence.jpa.entities.ClientDetailsEntity; import org.orcid.persistence.jpa.entities.ProfileEntity; import org.orcid.pojo.AdminChangePassword; @@ -332,7 +332,8 @@ else if (PojoUtil.isEmpty(email) || !validateEmailAddress(email)) // Notify any new email address if (!emailsToNotify.isEmpty()) { for (String emailToNotify : emailsToNotify) { - recordEmailSender.sendVerificationEmail(orcid, emailToNotify); + boolean isPrimaryEmail = emailManager.isPrimaryEmail(orcid, emailToNotify); + recordEmailSender.sendVerificationEmail(orcid, emailToNotify, isPrimaryEmail); } } profileDetails.setStatus(getMessage("admin.success")); diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/ManageProfileController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/ManageProfileController.java index 4fa8de4c1f6..1579ef03bb1 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/ManageProfileController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/ManageProfileController.java @@ -62,7 +62,6 @@ import org.orcid.pojo.ajaxForm.Email; import org.orcid.pojo.ajaxForm.Errors; import org.orcid.pojo.ajaxForm.NamesForm; -import org.orcid.pojo.ajaxForm.OtherNamesForm; import org.orcid.pojo.ajaxForm.PojoUtil; import org.orcid.pojo.ajaxForm.Text; import org.orcid.pojo.ajaxForm.Visibility; @@ -79,8 +78,6 @@ import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.mvc.support.RedirectAttributes; -import com.fasterxml.jackson.databind.JsonNode; - /** * @author Declan Newman (declan) Date: 22/02/2012 */ @@ -500,16 +497,18 @@ public ModelAndView confirmDeactivateOrcidAccount(HttpServletRequest request, Ht @RequestMapping(value = "/verifyEmail.json", method = RequestMethod.GET) public @ResponseBody Errors verifyEmail(HttpServletRequest request, @RequestParam("email") String email) { String currentUserOrcid = getCurrentUserOrcid(); - String primaryEmail = emailManager.findPrimaryEmail(currentUserOrcid).getEmail(); - if (primaryEmail.equals(email)) - request.getSession().setAttribute(EmailConstants.CHECK_EMAIL_VALIDATED, false); - - String emailOwner = emailManagerReadOnly.findOrcidIdByEmail(email); + + String emailOwner = emailManagerReadOnly.findOrcidIdByEmail(email); if(!currentUserOrcid.equals(emailOwner)) { - throw new IllegalArgumentException("Invalid email address provided"); + throw new IllegalArgumentException("Invalid email address provided"); + } + + boolean isPrimaryEmail = emailManagerReadOnly.isPrimaryEmail(currentUserOrcid, email); + if (isPrimaryEmail) { + request.getSession().setAttribute(EmailConstants.CHECK_EMAIL_VALIDATED, false); } - recordEmailSender.sendVerificationEmail(currentUserOrcid, email); + recordEmailSender.sendVerificationEmail(currentUserOrcid, email, isPrimaryEmail); return new Errors(); } @@ -649,7 +648,7 @@ public ModelAndView confirmDeactivateOrcidAccount(HttpServletRequest request, Ht request.getSession().setAttribute(EmailConstants.CHECK_EMAIL_VALIDATED, false); recordEmailSender.sendEmailAddressChangedNotification(currentUserOrcid, keys.get("new"), keys.get("old")); } - recordEmailSender.sendVerificationEmail(currentUserOrcid, OrcidStringUtils.filterEmailAddress(email.getValue())); + recordEmailSender.sendVerificationEmail(currentUserOrcid, OrcidStringUtils.filterEmailAddress(email.getValue()), email.isPrimary()); } else { email.setErrors(errors); } @@ -745,7 +744,7 @@ public ModelAndView confirmDeactivateOrcidAccount(HttpServletRequest request, Ht String oldPrimary = keys.get("old"); recordEmailSender.sendEmailAddressChangedNotification(orcid, newPrimary, oldPrimary); if(keys.containsKey("sendVerification")) { - recordEmailSender.sendVerificationEmail(orcid, newPrimary); + recordEmailSender.sendVerificationEmail(orcid, newPrimary, true); request.getSession().setAttribute(EmailConstants.CHECK_EMAIL_VALIDATED, false); } } @@ -795,7 +794,8 @@ public ModelAndView confirmDeactivateOrcidAccount(HttpServletRequest request, Ht recordEmailSender.sendEmailAddressChangedNotification(orcid, newPrimary, oldPrimary); } String verifyAddress = keys.get("verifyAddress"); - recordEmailSender.sendVerificationEmail(orcid, verifyAddress); + boolean isPrimaryEmail = keys.containsKey("new") ? true : false; + recordEmailSender.sendVerificationEmail(orcid, verifyAddress, isPrimaryEmail); } else { editEmail.setErrors(errors); } diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PasswordResetController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PasswordResetController.java index 2ebc5e67a8f..58dd41d5e43 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PasswordResetController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PasswordResetController.java @@ -535,7 +535,8 @@ private void reactivateAndLogUserIn(HttpServletRequest request, HttpServletRespo // Notify any new email address if (!emailsToNotify.isEmpty()) { for (String emailToNotify : emailsToNotify) { - recordEmailSender.sendVerificationEmail(orcid, emailToNotify); + boolean isPrimaryEmail = emailManager.isPrimaryEmail(orcid, emailToNotify); + recordEmailSender.sendVerificationEmail(orcid, emailToNotify, isPrimaryEmail); } } diff --git a/orcid-web/src/test/java/org/orcid/frontend/email/RecordEmailSenderTest.java b/orcid-web/src/test/java/org/orcid/frontend/email/RecordEmailSenderTest.java index 395525982bd..a8fccba27aa 100644 --- a/orcid-web/src/test/java/org/orcid/frontend/email/RecordEmailSenderTest.java +++ b/orcid-web/src/test/java/org/orcid/frontend/email/RecordEmailSenderTest.java @@ -134,7 +134,7 @@ public void testSendVerificationEmail() throws JAXBException, IOException, URISy email.setEmail("josiah_carberry@brown.edu"); when(mockEmailManager.findPrimaryEmail(anyString())).thenReturn(email); - recordEmailSender.sendVerificationEmail("4444-4444-4444-4446", "josiah_carberry@brown.edu"); + recordEmailSender.sendVerificationEmail("4444-4444-4444-4446", "josiah_carberry@brown.edu", true); } @Test diff --git a/orcid-web/src/test/java/org/orcid/frontend/web/controllers/ManageProfileControllerTest.java b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/ManageProfileControllerTest.java index b6e3b48be85..3323573e4bf 100644 --- a/orcid-web/src/test/java/org/orcid/frontend/web/controllers/ManageProfileControllerTest.java +++ b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/ManageProfileControllerTest.java @@ -964,7 +964,7 @@ public void testVerifyEmail() { mockRequest.setSession(mockSession); controller.verifyEmail(mockRequest, "email@email.com"); - verify(mockRecordEmailSender, Mockito.times(1)).sendVerificationEmail(eq(USER_ORCID), eq("email@email.com")); + verify(mockRecordEmailSender, Mockito.times(1)).sendVerificationEmail(eq(USER_ORCID), eq("email@email.com"), eq(false)); } @Test @@ -987,7 +987,7 @@ public void testAddEmail_noPrimaryEmailChange() { controller.addEmails(mockRequest, newEmail); verify(mockRecordEmailSender, Mockito.never()).sendEmailAddressChangedNotification(any(), any(), any()); - verify(mockRecordEmailSender, Mockito.times(1)).sendVerificationEmail(eq(USER_ORCID), eq("new@email.com")); + verify(mockRecordEmailSender, Mockito.times(1)).sendVerificationEmail(eq(USER_ORCID), eq("new@email.com"), eq(false)); } @Test @@ -1010,7 +1010,7 @@ public void testAddEmail_primaryEmailChange() { controller.addEmails(mockRequest, newEmail); verify(mockRecordEmailSender, Mockito.times(1)).sendEmailAddressChangedNotification(eq(USER_ORCID), eq("new@email.com"), eq("old@email.com")); - verify(mockRecordEmailSender, Mockito.times(1)).sendVerificationEmail(eq(USER_ORCID), eq("new@email.com")); + verify(mockRecordEmailSender, Mockito.times(1)).sendVerificationEmail(eq(USER_ORCID), eq("new@email.com"), eq(false)); } @Test @@ -1031,7 +1031,7 @@ public void testSetPrimary_nothingChange() { controller.setPrimary(mockRequest, email); verify(mockRecordEmailSender, Mockito.never()).sendEmailAddressChangedNotification(any(), any(), any()); - verify(mockRecordEmailSender, Mockito.never()).sendVerificationEmail(any(), any()); + verify(mockRecordEmailSender, Mockito.never()).sendVerificationEmail(any(), any(), any()); } @Test @@ -1052,7 +1052,7 @@ public void testSetPrimary_primaryEmailChange() { controller.setPrimary(mockRequest, email); verify(mockRecordEmailSender, Mockito.times(1)).sendEmailAddressChangedNotification(eq(USER_ORCID), eq("email@orcid.org"), eq("old@orcid.org")); - verify(mockRecordEmailSender, Mockito.never()).sendVerificationEmail(any(), any()); + verify(mockRecordEmailSender, Mockito.never()).sendVerificationEmail(any(), any(), any()); } @Test @@ -1076,7 +1076,7 @@ public void testSetPrimary_primaryEmailChangeAndPrimaryIsNotVerified() { controller.setPrimary(mockRequest, email); verify(mockRecordEmailSender, Mockito.times(1)).sendEmailAddressChangedNotification(eq(USER_ORCID), eq("email@orcid.org"), eq("old@orcid.org")); - verify(mockRecordEmailSender, Mockito.times(1)).sendVerificationEmail(eq(USER_ORCID), eq("email@orcid.org")); + verify(mockRecordEmailSender, Mockito.times(1)).sendVerificationEmail(eq(USER_ORCID), eq("email@orcid.org"), eq(true)); } @Test @@ -1100,7 +1100,7 @@ public void testEditEmail_noPrimaryChange() { controller.editEmail(mockRequest, email); verify(mockRecordEmailSender, Mockito.never()).sendEmailAddressChangedNotification(any(), any(), any()); - verify(mockRecordEmailSender, Mockito.times(1)).sendVerificationEmail(eq(USER_ORCID), eq("email@orcid.org")); + verify(mockRecordEmailSender, Mockito.times(1)).sendVerificationEmail(eq(USER_ORCID), eq("email@orcid.org"), eq(false)); } @Test @@ -1121,7 +1121,7 @@ public void testEditEmail_primaryEmailChange() { controller.editEmail(mockRequest, email); verify(mockRecordEmailSender, Mockito.times(1)).sendEmailAddressChangedNotification(eq(USER_ORCID), eq("email@orcid.org"), eq("old@orcid.org")); - verify(mockRecordEmailSender, Mockito.times(1)).sendVerificationEmail(eq(USER_ORCID), eq("email@orcid.org")); + verify(mockRecordEmailSender, Mockito.times(1)).sendVerificationEmail(eq(USER_ORCID), eq("email@orcid.org"), eq(true)); } protected Authentication getAuthentication(String orcid) {