improve readability, add transactions, cleanup

ORCID · Aug 8, 2024 · 3dd0985 · 3dd0985
1 parent 9464c87
commit 3dd0985
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 9 deletions.
diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/EmailManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/EmailManagerImpl.java
@@ -67,6 +67,7 @@ public void removeEmail(String orcid, String email) {
     }
 
     @Override
+    @Transactional
     @UpdateProfileLastModifiedAndIndexingStatus
     public boolean verifyEmail(String orcid, String email) {
         boolean result = emailDao.verifyEmail(email);
@@ -113,6 +114,7 @@ public boolean moveEmailToOtherAccount(String email, String origin, String desti
     }
 
     @Override
+    @Transactional
     public boolean verifySetCurrentAndPrimary(String orcid, String email) {
         if (PojoUtil.isEmpty(orcid) || PojoUtil.isEmpty(email)) {
             throw new IllegalArgumentException("orcid or email param is empty or null");

diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileEmailDomainManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileEmailDomainManagerImpl.java
@@ -3,12 +3,13 @@
 
 import org.orcid.core.manager.v3.ProfileEmailDomainManager;
 import org.orcid.core.manager.v3.read_only.impl.ProfileEmailDomainManagerReadOnlyImpl;
+import org.orcid.jaxb.model.v3.release.common.Visibility;
 import org.orcid.persistence.dao.EmailDao;
 import org.orcid.persistence.dao.EmailDomainDao;
 import org.orcid.persistence.dao.ProfileEmailDomainDao;
 import org.orcid.persistence.jpa.entities.EmailDomainEntity;
-import org.orcid.persistence.jpa.entities.EmailEntity;
 import org.orcid.persistence.jpa.entities.ProfileEmailDomainEntity;
+import org.springframework.transaction.annotation.Transactional;
 
 import javax.annotation.Resource;
 import java.util.*;
@@ -28,6 +29,9 @@ public class ProfileEmailDomainManagerImpl extends ProfileEmailDomainManagerRead
     @Resource
     protected EmailDao emailDao;
 
+    private static final String DEFAULT_DOMAIN_VISIBILITY = Visibility.PRIVATE.toString();
+
+    @Transactional
     public void updateEmailDomains(String orcid, org.orcid.pojo.ajaxForm.Emails newEmails) {
         List<ProfileEmailDomainEntity> existingEmailDomains = profileEmailDomainDao.findByOrcid(orcid);
 
@@ -37,7 +41,6 @@ public void updateEmailDomains(String orcid, org.orcid.pojo.ajaxForm.Emails newE
                 for (ProfileEmailDomainEntity existingEmailDomain : existingEmailDomains) {
                     if (existingEmailDomain.getEmailDomain().equals(emailDomain.getValue())) {
                         if (!existingEmailDomain.getVisibility().equals(emailDomain.getVisibility())) {
-                            // TODO: add visibility check to see that you can't make it more restrictive than the email?
                             profileEmailDomainDao.updateVisibility(orcid, emailDomain.getValue(), emailDomain.getVisibility());
                         }
                     }
@@ -61,16 +64,14 @@ public void updateEmailDomains(String orcid, org.orcid.pojo.ajaxForm.Emails newE
     }
 
     public void processDomain(String orcid, String email) {
-        // TODO: QUESTION FOR ANGEL: if something fails here, should we prevent the verification from being completed?
-        //  Verification is the only way for this to be triggered, so if this fails, but the email gets verified then it becomes a bit messy
         String domain = email.split("@")[1];
         EmailDomainEntity domainInfo = emailDomainDao.findByEmailDomain(domain);
         // Check if email is professional
-        if (domainInfo != null && domainInfo.getCategory().toString().equals("PROFESSIONAL")) {
+        if (domainInfo != null && domainInfo.getCategory().equals(EmailDomainEntity.DomainCategory.PROFESSIONAL)) {
             ProfileEmailDomainEntity existingDomain = profileEmailDomainDao.findByEmailDomain(orcid, domain);
             // ADD NEW DOMAIN IF ONE DOESN'T EXIST
             if (existingDomain == null) {
-                profileEmailDomainDao.addEmailDomain(orcid, domain, "PRIVATE");
+                profileEmailDomainDao.addEmailDomain(orcid, domain, DEFAULT_DOMAIN_VISIBILITY);
             }
         }
     }

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
@@ -532,7 +532,6 @@ public ModelAndView confirmDeactivateOrcidAccount(HttpServletRequest request, Ht
     public @ResponseBody org.orcid.pojo.ajaxForm.Emails getEmails(HttpServletRequest request) {                                
         Emails v2Emails = emailManager.getEmails(getCurrentUserOrcid());
 
-        // TODO: Note to Angel - feels hacky, not sure how to better implement a togglz where the arg count changes (updatedDomains gets added to valueOf() below)
         List<ProfileEmailDomainEntity> emailDomains = null;
         if (Features.EMAIL_DOMAINS.isActive()) {
             emailDomains = profileEmailDomainManagerReadOnly.getEmailDomains(getCurrentUserOrcid());
@@ -606,8 +605,6 @@ public ModelAndView confirmDeactivateOrcidAccount(HttpServletRequest request, Ht
         Emails updatedSet = emailManager.getEmails(getCurrentUserOrcid());
         List<ProfileEmailDomainEntity> updatedDomains = null;
         if (Features.EMAIL_DOMAINS.isActive()) {
-            // TODO: note to Angel - ideally this should be 1 call? I.e. updateEmailDomains() should return
-            //  a list of List<ProfileEmailDomainEntity>?
             profileEmailDomainManager.updateEmailDomains(orcid, newEmailSet);
             updatedDomains = profileEmailDomainManagerReadOnly.getEmailDomains(getCurrentUserOrcid());
         }