Closes #3207: Fix assign role email for collection sent twice

fairchive-webapp/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDataverseCommand.java

@@ -45,15 +45,7 @@ public Dataverse execute(CommandContext ctxt)  {
         }
 
         //Before setting dataverse to released send notifications to users with download file
-        List<RoleAssignment> ras = ctxt.roles().directRoleAssignments(dataverse);
-        for (RoleAssignment ra : ras) {
-            if (ra.getRole().has(Permission.DownloadFile)) {
-                for (AuthenticatedUser au : ctxt.roleAssignees().getExplicitUsers(ctxt.roleAssignees().getRoleAssignee(ra.getAssigneeIdentifier()))) {
-                    ctxt.notifications().sendNotificationWithEmail(au, new Timestamp(new Date().getTime()), NotificationType.ASSIGNROLE,
-                                                                   dataverse.getId(), NotificationObjectType.DATAVERSE);
-                }
-            }
-        }
+        sendAssignRoleNotifications(ctxt);
 
         dataverse.setPublicationDate(new Timestamp(new Date().getTime()));
         dataverse.setReleaseUser((AuthenticatedUser) getUser());
@@ -66,4 +58,20 @@ public Dataverse execute(CommandContext ctxt)  {
         return savedDataverse;
     }
 
+    /**
+     * Sends notifications about assigned roles in a dataverse, but only
+     * when role did not allow to view unpublished dataverse.
+     * Since for this cases we postponed sending the notification
+     * until the dataverse can be accessed and now when dataverse is
+     * published it can be accessed by anyone
+     */
+    private void sendAssignRoleNotifications(CommandContext ctxt) {
+        ctxt.roles().directRoleAssignments(dataverse).stream()
+            .filter(ra -> !ra.getRole().has(Permission.ViewUnpublishedDataverse))
+            .flatMap(ra -> ctxt.roleAssignees().getExplicitUsers(ctxt.roleAssignees().getRoleAssignee(ra.getAssigneeIdentifier())).stream())
+            .distinct()
+            .forEach(au -> ctxt.notifications().sendNotificationWithEmail(
+                                au, new Timestamp(new Date().getTime()), NotificationType.ASSIGNROLE,
+                                dataverse.getId(), NotificationObjectType.DATAVERSE));
+    }
 }

fairchive-webapp/src/main/java/edu/harvard/iq/dataverse/permission/ManagePermissionsService.java

@@ -15,8 +15,8 @@
 import edu.harvard.iq.dataverse.persistence.group.ExplicitGroup;
 import edu.harvard.iq.dataverse.persistence.user.AuthenticatedUser;
 import edu.harvard.iq.dataverse.persistence.user.DataverseRole;
-import edu.harvard.iq.dataverse.persistence.user.DataverseRole.BuiltInRole;
 import edu.harvard.iq.dataverse.persistence.user.NotificationType;
+import edu.harvard.iq.dataverse.persistence.user.Permission;
 import edu.harvard.iq.dataverse.persistence.user.RoleAssignee;
 import edu.harvard.iq.dataverse.persistence.user.RoleAssignment;
 import io.vavr.control.Try;
@@ -69,14 +69,20 @@ public RoleAssignment assignRoleWithNotification(DataverseRole role, RoleAssigne
     }
 
     /***
-     * For FILE_DOWNLOADER role we don't notify user if dataset is unpublished since with FILE_DOWNLOADER role
-     * user is not able to access dataset.
-     * @param role - role to be assigned
-     * @param object - object to which we assign role
-     * @return false if role is FILE_DOWNLOADER and dataverse is unpublished
+     * For unpublished dataverses and datasets we do not send a notification
+     * if the assigned role does not allow to go to the dataverse or dataset page.
+     * If we would send such notification then the user would obtain a notification
+     * with a link that he cannot access.
+     * Sending this notification will be postponed until the object is published.
      */
     private boolean shouldUserBeNotified(DataverseRole role, DvObject object) {
-        return !(role.getAlias().equals(BuiltInRole.FILE_DOWNLOADER.getAlias()) && !object.isReleased());
+        if (object.isReleased()) {
+            return true;
+        }
+        if (object.isInstanceofDataverse()) {
+            return role.has(Permission.ViewUnpublishedDataverse);
+        }
+        return role.has(Permission.ViewUnpublishedDataset);
     }
 
 

fairchive-webapp/src/test/java/edu/harvard/iq/dataverse/permission/ManagePermissionsServiceIT.java

@@ -60,7 +60,7 @@ public void shouldAssignRoleWithNotification() {
         dataverseSession.logIn(authenticationService.findByID(1L));
         String userEmail = dataverseSession.getUser().getDisplayInfo().getEmailAddress();
         Dataverse dataverse = dataverseDao.find(1L);
-        DataverseRole roleToBeAssigned = roleService.findBuiltinRoleByAlias(BuiltInRole.EDITOR);
+        DataverseRole roleToBeAssigned = roleService.findBuiltinRoleByAlias(BuiltInRole.CURATOR);
 
         // when
         managePermissionsService.assignRoleWithNotification(roleToBeAssigned, dataverseSession.getUser(), dataverse);

fairchive-webapp/src/test/java/edu/harvard/iq/dataverse/permission/ManagePermissionsServiceTest.java

@@ -12,6 +12,8 @@
 import edu.harvard.iq.dataverse.persistence.dataverse.Dataverse;
 import edu.harvard.iq.dataverse.persistence.user.AuthenticatedUser;
 import edu.harvard.iq.dataverse.persistence.user.DataverseRole;
+import edu.harvard.iq.dataverse.persistence.user.NotificationType;
+import edu.harvard.iq.dataverse.persistence.user.Permission;
 import edu.harvard.iq.dataverse.persistence.user.RoleAssignment;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -26,9 +28,11 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
 
 @ExtendWith(MockitoExtension.class)
@@ -67,11 +71,27 @@ public void setUp() {
 
     @Test
     public void assignRoleWithNotification() {
+        // given
+        role.addPermission(Permission.ViewUnpublishedDataverse);
+
+        // when
+        RoleAssignment resultAssignment = managePermissionsService.assignRoleWithNotification(role, roleAssignee, dvObject);
+
+        // then
+        verify(commandEngine, times(1)).submit(any(AssignRoleCommand.class));
+        verify(userNotificationService).sendNotificationWithEmail(eq(roleAssignee), any(), eq(NotificationType.ASSIGNROLE),
+                eq(dvObject.getId()), eq(NotificationObjectType.DATAVERSE));
+        assertEquals("testRole", resultAssignment.getRole().getName());
+    }
+
+    @Test
+    public void assignRoleWithNotification__no_notification_for_unpublished_dataverse_and_no_view_unpublished_permission() {
         // given & when
         RoleAssignment resultAssignment = managePermissionsService.assignRoleWithNotification(role, roleAssignee, dvObject);
 
         // then
         verify(commandEngine, times(1)).submit(any(AssignRoleCommand.class));
+        verifyNoInteractions(userNotificationService);
         assertEquals("testRole", resultAssignment.getRole().getName());
     }