HOTFIX: bug preventing usergroup manager from adding members to group linked to project

CHANGELOG.md

@@ -1,6 +1,9 @@
 
 # Changelog
 
+## [22.09.5] - Unreleased
+* [Developer/UI]: Fixed bug preventing a manager of a user group from adding new members when this manager is a collaborator on one of these users projects. Also, fixed issue with a user group member added with an owner role for a group was set with a member role. See [PR 1431](https://github.com/phac-nml/irida/pull/1431)
+
 ## [22.09.4] - 2022/11/14
 * [REST]: Fixed issue with project/samples api response missing samples when a sample has a default sequencing object. See [PR 1413](https://github.com/phac-nml/irida/pull/1413)
 

src/main/java/ca/corefacility/bioinformatics/irida/repositories/user/UserGroupRepository.java

@@ -12,7 +12,8 @@
  * A repository for {@link UserGroup}.
  *
  */
-public interface UserGroupRepository extends IridaJpaRepository<UserGroup, Long> {
+public interface
+UserGroupRepository extends IridaJpaRepository<UserGroup, Long> {
 
 	/**
 	 * Find a collection of {@link UserGroup} not already attached to the

src/main/java/ca/corefacility/bioinformatics/irida/service/ProjectSubscriptionService.java

@@ -8,6 +8,7 @@
 import ca.corefacility.bioinformatics.irida.model.project.Project;
 import ca.corefacility.bioinformatics.irida.model.subscription.ProjectSubscription;
 import ca.corefacility.bioinformatics.irida.model.user.User;
+import ca.corefacility.bioinformatics.irida.model.user.group.UserGroup;
 
 /**
  * A specialized service layer for project subscriptions.
@@ -57,4 +58,23 @@ public interface ProjectSubscriptionService extends CRUDService<Long, ProjectSub
 	 */
 	public void removeProjectSubscriptionForProjectAndUser(Project project, User user);
 
+	/**
+	 * Create a new project subscription associated with a {@link Project} and {@link User} in a {@link UserGroup}.
+	 *
+	 * @param project   the {@link Project} to add the subscription for
+	 * @param user      the {@link User} to add the subscription for
+	 * @param userGroup the {@link UserGroup} to which the user belongs
+	 * @return The newly created project subscription
+	 */
+	public ProjectSubscription addProjectSubscriptionsForUserInUserGroup(Project project, User user,
+			UserGroup userGroup);
+
+	/**
+	 * Remove the project subscription associated with a {@link Project} and {@link User} in a {@link UserGroup}.
+	 *
+	 * @param project   the {@link Project} to add the subscription for
+	 * @param user      the {@link User} to add the subscription for
+	 * @param userGroup the {@link UserGroup} to which the user belongs
+	 */
+	public void removeProjectSubscriptionsForUserInUserGroup(Project project, User user, UserGroup userGroup);
 }

src/main/java/ca/corefacility/bioinformatics/irida/service/impl/ProjectServiceImpl.java

@@ -23,7 +23,6 @@
 import org.springframework.security.access.prepost.PostFilter;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.security.core.context.SecurityContextHolder;
-import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 import org.springframework.util.StringUtils;
@@ -279,7 +278,7 @@ public void removeUserGroupFromProject(Project project, UserGroup userGroup) thr
 		Collection<UserGroupJoin> userGroupJoins = userGroupJoinRepository.findUsersInGroup(userGroup);
 		for (UserGroupJoin userGroupJoin : userGroupJoins) {
 			User user = userGroupJoin.getSubject();
-			projectSubscriptionService.removeProjectSubscriptionForProjectAndUser(project, user);
+			projectSubscriptionService.removeProjectSubscriptionsForUserInUserGroup(project, user, userGroup);
 		}
 		final UserGroupProjectJoin j = ugpjRepository.findByProjectAndUserGroup(project, userGroup);
 		if (!allowRoleChange(project, j.getProjectRole())) {
@@ -760,7 +759,7 @@ public Join<Project, UserGroup> addUserGroupToProject(final Project project, fin
 		Collection<UserGroupJoin> userGroupJoins = userGroupJoinRepository.findUsersInGroup(userGroup);
 		for (UserGroupJoin userGroupJoin : userGroupJoins) {
 			User user = userGroupJoin.getSubject();
-			projectSubscriptionService.addProjectSubscriptionForProjectAndUser(project, user);
+			projectSubscriptionService.addProjectSubscriptionsForUserInUserGroup(project, user, userGroup);
 		}
 		return ugpjRepository.save(new UserGroupProjectJoin(project, userGroup, role, metadataRole));
 

src/main/java/ca/corefacility/bioinformatics/irida/service/impl/ProjectSubscriptionServiceImpl.java

@@ -17,6 +17,7 @@
 import ca.corefacility.bioinformatics.irida.model.project.Project;
 import ca.corefacility.bioinformatics.irida.model.subscription.ProjectSubscription;
 import ca.corefacility.bioinformatics.irida.model.user.User;
+import ca.corefacility.bioinformatics.irida.model.user.group.UserGroup;
 import ca.corefacility.bioinformatics.irida.model.user.group.UserGroupProjectJoin;
 import ca.corefacility.bioinformatics.irida.repositories.ProjectSubscriptionRepository;
 import ca.corefacility.bioinformatics.irida.repositories.joins.project.ProjectUserJoinRepository;
@@ -104,8 +105,50 @@ public List<Project> getProjectsForUserWithEmailSubscriptions(User user) {
 	 */
 	@Override
 	@PreAuthorize(
-			"hasRole('ROLE_ADMIN') or hasPermission(#project, 'canManageLocalProjectSettings') or hasPermission(#userGroup, 'canUpdateUserGroup')")
+			"hasRole('ROLE_ADMIN') or hasPermission(#project, 'canManageLocalProjectSettings')")
 	public ProjectSubscription addProjectSubscriptionForProjectAndUser(Project project, User user) {
+		return addProjectSubscription(project, user);
+	}
+
+	/**
+	 * {@inheritDoc}
+	 */
+	@Override
+	@PreAuthorize(
+			"hasRole('ROLE_ADMIN') or hasPermission(#project, 'canManageLocalProjectSettings')")
+	public void removeProjectSubscriptionForProjectAndUser(Project project, User user) {
+		removeProjectSubscription(project, user);
+	}
+
+	/**
+	 * {@inheritDoc}
+	 */
+	@Override
+	@PreAuthorize(
+			"hasRole('ROLE_ADMIN') or hasPermission(#project, 'canManageLocalProjectSettings') or hasPermission(#userGroup, 'canUpdateUserGroup')")
+	public ProjectSubscription addProjectSubscriptionsForUserInUserGroup(Project project, User user,
+			UserGroup userGroup) {
+		return addProjectSubscription(project, user);
+	}
+
+	/**
+	 * {@inheritDoc}
+	 */
+	@Override
+	@PreAuthorize(
+			"hasRole('ROLE_ADMIN') or hasPermission(#project, 'canManageLocalProjectSettings') or hasPermission(#userGroup, 'canUpdateUserGroup')")
+	public void removeProjectSubscriptionsForUserInUserGroup(Project project, User user, UserGroup userGroup) {
+		removeProjectSubscription(project, user);
+	}
+
+	/**
+	 * Add a project subscription for the user
+	 *
+	 * @param project The {@link Project}
+	 * @param user    The {@link User}
+	 * @return the {@link ProjectSubscription} created for the user
+	 */
+	private ProjectSubscription addProjectSubscription(Project project, User user) {
 		ProjectSubscription newProjectSubscription = null;
 		ProjectSubscription projectSubscription = projectSubscriptionRepository.findProjectSubscriptionByUserAndProject(
 				user, project);
@@ -117,12 +160,12 @@ public ProjectSubscription addProjectSubscriptionForProjectAndUser(Project proje
 	}
 
 	/**
-	 * {@inheritDoc}
+	 * Remove project subscription for the user
+	 *
+	 * @param project The {@link Project}
+	 * @param user    The {@link User}
 	 */
-	@Override
-	@PreAuthorize(
-			"hasRole('ROLE_ADMIN') or hasPermission(#project, 'canManageLocalProjectSettings') or hasPermission(#userGroup, 'canUpdateUserGroup')")
-	public void removeProjectSubscriptionForProjectAndUser(Project project, User user) {
+	private void removeProjectSubscription(Project project, User user) {
 		ProjectUserJoin projectUserjoin = pujRepository.getProjectJoinForUser(project, user);
 		Collection<UserGroupProjectJoin> userGroupProjects = ugpjRepository.findByProjectAndUser(project, user);
 

src/main/java/ca/corefacility/bioinformatics/irida/service/impl/user/UserGroupServiceImpl.java

@@ -228,7 +228,7 @@ public UserGroupJoin addUserToGroup(final User user, final UserGroup userGroup,
 				userGroup);
 		for (UserGroupProjectJoin userGroupProjectJoin : userGroupProjectJoins) {
 			Project project = userGroupProjectJoin.getSubject();
-			projectSubscriptionService.addProjectSubscriptionForProjectAndUser(project, user);
+			projectSubscriptionService.addProjectSubscriptionsForUserInUserGroup(project, user, userGroup);
 		}
 
 		final UserGroupJoin join = new UserGroupJoin(user, userGroup, role);
@@ -270,7 +270,7 @@ public void removeUserFromGroup(final User user, final UserGroup userGroup) thro
 				userGroup);
 		for (UserGroupProjectJoin userGroupProjectJoin : userGroupProjectJoins) {
 			Project project = userGroupProjectJoin.getSubject();
-			projectSubscriptionService.removeProjectSubscriptionForProjectAndUser(project, user);
+			projectSubscriptionService.removeProjectSubscriptionsForUserInUserGroup(project, user, userGroup);
 		}
 
 		userGroupJoinRepository.delete(join);

src/main/webapp/resources/js/components/roles/GroupRole.jsx

@@ -35,6 +35,7 @@ export function GroupRole({ item, canManage, updateRoleFn }) {
       style={{ width: "100%" }}
       loading={loading}
       disabled={loading}
+      className="member-group-role"
     >
       {roles.map((role) => (
         <Select.Option value={role.value} key={role.value}>

src/main/webapp/resources/js/pages/admin/components/user-groups/AddUserToGroupButton.jsx

@@ -43,7 +43,7 @@ export function AddUserToGroupButton({
   /*
   The value of the currently selected role from the role input
    */
-  const [projectRole, setProjectRole] = useState("GROUP_MEMBER");
+  const [groupRole, setGroupRole] = useState("GROUP_MEMBER");
 
   /*
   Value to send to the server to query for a list of potential users.
@@ -94,7 +94,7 @@ export function AddUserToGroupButton({
   }, [visible]);
 
   const addMember = () => {
-    addMemberFn({ id: userId, projectRole })
+    addMemberFn({ id: userId, role: groupRole })
       .then((message) => {
         addMemberSuccessFn();
         notification.success({ message });
@@ -133,11 +133,7 @@ export function AddUserToGroupButton({
         onOk={addMember}
         okText={i18n("AddMemberButton.modal.okText")}
       >
-        <Form
-          layout="vertical"
-          form={form}
-          initialValues={{ role: projectRole }}
-        >
+        <Form layout="vertical" form={form} initialValues={{ role: groupRole }}>
           <Form.Item
             label={i18n("AddMemberButton.modal.user-label")}
             help={i18n("AddMemberButton.modal.user-help")}
@@ -158,15 +154,19 @@ export function AddUserToGroupButton({
           </Form.Item>
           <Form.Item
             label={i18n("UserGroupMembersTable.role")}
-            name="projectRole"
-            initialValue={projectRole}
+            name="groupRole"
+            initialValue={groupRole}
           >
             <Radio.Group
               style={{ display: "flex" }}
-              onChange={(e) => setProjectRole(e.target.value)}
+              onChange={(e) => setGroupRole(e.target.value)}
             >
               {userGroupRoles.map((role) => (
-                <Radio.Button key={role.value} value={role.value}>
+                <Radio.Button
+                  key={role.value}
+                  value={role.value}
+                  className={`t-group-role-${role.label.toLowerCase()}`}
+                >
                   {role.label}
                 </Radio.Button>
               ))}

src/test/java/ca/corefacility/bioinformatics/irida/ria/integration/components/AddMemberButton.java

@@ -47,4 +47,28 @@ public void addMember(WebDriver driver, String name) {
 		modalOkBtn.click();
 		wait.until(ExpectedConditions.invisibilityOf(addMemberModal));
 	}
+
+	public void addMember(WebDriver driver, String name, String role) {
+		wait.until(ExpectedConditions.elementToBeClickable(addMemberBtn));
+		addMemberBtn.click();
+		wait.until(ExpectedConditions.visibilityOf(addMemberModal));
+		AbstractPage.waitForTime(100);
+		WebElement input = driver.switchTo().activeElement();
+		input.sendKeys(name);
+		wait.until(ExpectedConditions.visibilityOf(newMemberList.get(0)));
+		newMemberList.get(0).click();
+		if(role.equals("GROUP_OWNER")) {
+			WebElement element = driver.findElements(By.className("t-group-role-owner")).get(0);
+			wait.until(ExpectedConditions.elementToBeClickable(element));
+			element.click();
+		} else {
+			WebElement element = driver.findElements(By.className("t-group-role-member")).get(0);
+			wait.until(ExpectedConditions.elementToBeClickable(element));
+			element.click();
+		}
+		WebElement modalOkBtn = addMemberModal.findElement(By.cssSelector(".ant-btn.ant-btn-primary"));
+		wait.until(ExpectedConditions.elementToBeClickable(modalOkBtn));
+		modalOkBtn.click();
+		wait.until(ExpectedConditions.invisibilityOf(addMemberModal));
+	}
 }

src/test/java/ca/corefacility/bioinformatics/irida/ria/integration/groups/UserGroupsIT.java

@@ -5,8 +5,10 @@
 
 import ca.corefacility.bioinformatics.irida.ria.integration.AbstractIridaUIITChromeDriver;
 import ca.corefacility.bioinformatics.irida.ria.integration.pages.LoginPage;
+import ca.corefacility.bioinformatics.irida.ria.integration.pages.ProjectMembersPage;
 import ca.corefacility.bioinformatics.irida.ria.integration.pages.groups.UserGroupsDetailsPage;
 import ca.corefacility.bioinformatics.irida.ria.integration.pages.groups.UserGroupsListingPage;
+import ca.corefacility.bioinformatics.irida.ria.integration.pages.projects.ProjectUserGroupsPage;
 
 import com.github.springtestdbunit.annotation.DatabaseSetup;
 
@@ -100,4 +102,77 @@ public void testUserGroupsAsAdmin() {
 		assertTrue(driver().getCurrentUrl().endsWith("/admin/groups"), "Redirects user to admin panel user groups page");
 		assertEquals(2, listingPage.getNumberOfExistingUserGroups(), "Should have 2 user groups");
 	}
+
+	@Test
+	public void testAddGroupMemberWhenManagerOnMemberProjectAsCollaborator() {
+		Long PROJECT_ID = 9L;
+		// Login as a user and add manager as collaborator on project
+		LoginPage.loginAsUser(driver());
+		ProjectMembersPage projectMembersPage = ProjectMembersPage.goToRemoteProject(driver(), PROJECT_ID);
+		// Add manager as collaborator on project
+		projectMembersPage.addUserToProject("mrtest");
+		LoginPage.logout(driver());
+
+		// Login as manager and create a new group
+		LoginPage.loginAsManager(driver());
+		// Test listing user groups
+		UserGroupsListingPage listingPage = UserGroupsListingPage.initPage(driver());
+		listingPage.gotoPage();
+		assertEquals(2, listingPage.getNumberOfExistingUserGroups(), "Should have 2 user groups");
+		// Test creating a new group
+		final String GROUP_NAME = "NEW_GROUP";
+		final String PRE_CREATION_URL = driver().getCurrentUrl();
+		listingPage.createNewUserGroup(GROUP_NAME);
+		listingPage.validateRouteChange(PRE_CREATION_URL);
+		assertTrue(driver().getCurrentUrl().contains("/groups"), "Redirects user to main app user details page");
+		String currUrl = driver().getCurrentUrl();
+		int NEW_GROUP_ID = Integer.parseInt(driver().getCurrentUrl().substring(currUrl.lastIndexOf("/") + 1));
+		LoginPage.logout(driver());
+
+		// Login as user and add this new group to the project
+		LoginPage.loginAsUser(driver());
+		ProjectUserGroupsPage projectUserGroupsPage = ProjectUserGroupsPage.goToPage(driver(), PROJECT_ID);
+		projectUserGroupsPage.addUserGroup(GROUP_NAME);
+		LoginPage.logout(driver());
+
+		// Login as manager and add user to the new group
+		LoginPage.loginAsManager(driver());
+		UserGroupsDetailsPage detailsPage = UserGroupsDetailsPage.initPage(driver());
+		detailsPage.gotoPage(NEW_GROUP_ID);
+		assertEquals(GROUP_NAME, detailsPage.getUserGroupName(), "Should be on the new user groups page");
+		assertEquals(1, detailsPage.getNumberOfMembers(), "Should be 1 group member");
+		detailsPage.addGroupMember("testUser", "GROUP_MEMBER");
+		assertEquals(2, detailsPage.getNumberOfMembers(), "Should be 2 group members");
+	}
+
+	@Test
+	public void testAddUserGroupMemberWithSelectedRole() {
+		LoginPage.loginAsManager(driver());
+
+		UserGroupsListingPage listingPage = UserGroupsListingPage.initPage(driver());
+		listingPage.gotoPage();
+		assertEquals(2, listingPage.getNumberOfExistingUserGroups(), "Should have 2 user groups");
+
+		final String GROUP_NAME = "NEW_GROUP";
+		final String PRE_CREATION_URL = driver().getCurrentUrl();
+		listingPage.createNewUserGroup(GROUP_NAME);
+		listingPage.validateRouteChange(PRE_CREATION_URL);
+		assertFalse(driver().getCurrentUrl().contains("/admin/groups"), "Does not redirect to admin panel user details page");
+		assertTrue(driver().getCurrentUrl().contains("/groups"), "Redirects user to main app user details page");
+
+		UserGroupsDetailsPage detailsPage = UserGroupsDetailsPage.initPage(driver());
+		assertEquals(GROUP_NAME, detailsPage.getUserGroupName(), "Should be on the new user groups page");
+		assertEquals(1, detailsPage.getNumberOfMembers(), "Should be 1 group member");
+		assertEquals("Owner", detailsPage.getUserGroupMemberRole(0));
+
+		// Add as group user
+		detailsPage.addGroupMember("testUser", "GROUP_MEMBER");
+		assertEquals(2, detailsPage.getNumberOfMembers(), "Should be 2 group members");
+		assertEquals("Member", detailsPage.getUserGroupMemberRole(1));
+
+		// Add as group owner
+		detailsPage.addGroupMember("testUser2", "GROUP_OWNER");
+		assertEquals(3, detailsPage.getNumberOfMembers(), "Should be 3 group members");
+		assertEquals("Owner", detailsPage.getUserGroupMemberRole(2));
+	}
 }

src/test/java/ca/corefacility/bioinformatics/irida/ria/integration/pages/groups/UserGroupsDetailsPage.java

@@ -1,7 +1,9 @@
 package ca.corefacility.bioinformatics.irida.ria.integration.pages.groups;
 
 import java.time.Duration;
+import java.util.List;
 
+import org.openqa.selenium.By;
 import org.openqa.selenium.Keys;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.WebElement;
@@ -74,7 +76,7 @@ public int getNumberOfMembers() {
 	}
 
 	public void addGroupMember(String searchTerm, String role) {
-		addMemberButton.addMember(driver, searchTerm);
+		addMemberButton.addMember(driver, searchTerm, role);
 	}
 
 	public void deleteGroup() {
@@ -86,4 +88,9 @@ public void deleteGroup() {
 		wait.until(ExpectedConditions.elementToBeClickable(deleteConfirmBtn));
 		deleteConfirmBtn.click();
 	}
+
+	public String getUserGroupMemberRole(int row) {
+		List<WebElement> memberSelectedRoles = driver.findElements(By.className("ant-select-selection-item"));
+		return memberSelectedRoles.get(row).getText();
+	}
 }