Updated service layer for adding user group to project to validate if…

CHANGELOG.md

@@ -3,6 +3,7 @@
 
 ## [23.01.2]
 * [UI]: Fixed bug that caused all metadata fields to be removed when single field was removed from a template. See [PR 1482](https://github.com/phac-nml/irida/pull/1482)
+* [Developer]: Fixed bug which allowed duplicated entries in the user_group_project table which prevented the user group from being removed. Fixed bug which was preventing analyses with `html` file outputs from completing. See [PR 1483](https://github.com/phac-nml/irida/pull/1483)
 
 ## [23.01.1] - 2023/03/21
 * [UI]: Fixed issue where template order was not applied when applying a linelist template. See [PR 1479](https://github.com/phac-nml/irida/pull/1479)

src/main/java/ca/corefacility/bioinformatics/irida/ria/utilities/FileUtilities.java

@@ -249,7 +249,7 @@ public static String getFileExt(Path filepath) {
 				if (isZippedFile(filepath)) {
 					ext = "html-zip";
 				}
-			} catch (IOException e) {
+			} catch (IOException | StorageException e) {
 				logger.error("Could not find file " + filepath.toString());
 			}
 		}

src/main/java/ca/corefacility/bioinformatics/irida/ria/web/ajax/projects/ProjectUserGroupsAjaxController.java

@@ -3,6 +3,7 @@
 import java.util.List;
 import java.util.Locale;
 
+import ca.corefacility.bioinformatics.irida.exceptions.EntityExistsException;
 import ca.corefacility.bioinformatics.irida.ria.web.exceptions.UIConstraintViolationException;
 
 import org.springframework.beans.factory.annotation.Autowired;
@@ -82,7 +83,11 @@ public ResponseEntity<List<UserGroup>> getAvailableUserGroupsForProject(@Request
 	@RequestMapping(value = "/add", method = RequestMethod.POST)
 	public ResponseEntity<String> addUserGroupToProject(@RequestParam Long projectId,
 			@RequestBody NewMemberRequest request, Locale locale) {
-		return ResponseEntity.ok(service.addUserGroupToProject(projectId, request, locale));
+		try {
+			return ResponseEntity.ok(service.addUserGroupToProject(projectId, request, locale));
+		} catch (EntityExistsException e) {
+			return ResponseEntity.status(HttpStatus.CONFLICT).body(e.getMessage());
+		}
 	}
 
 	/**

src/main/java/ca/corefacility/bioinformatics/irida/ria/web/services/UIProjectUserGroupsService.java

@@ -4,6 +4,7 @@
 import java.util.Locale;
 import java.util.stream.Collectors;
 
+import ca.corefacility.bioinformatics.irida.exceptions.EntityExistsException;
 import ca.corefacility.bioinformatics.irida.ria.web.exceptions.UIConstraintViolationException;
 
 import org.springframework.context.MessageSource;
@@ -106,15 +107,21 @@ public List<UserGroup> getAvailableUserGroupsForProject(Long projectId, String q
 	public String addUserGroupToProject(Long projectId, NewMemberRequest request, Locale locale) {
 		Project project = projectService.read(projectId);
 		UserGroup group = userGroupService.read(request.getId());
-		ProjectRole role = ProjectRole.fromString(request.getProjectRole());
-		ProjectMetadataRole metadataRole;
-		if (role.equals(ProjectRole.PROJECT_OWNER)) {
-			metadataRole = ProjectMetadataRole.LEVEL_4;
-		} else {
-			metadataRole = ProjectMetadataRole.fromString(request.getMetadataRole());
+
+		try {
+			ProjectRole role = ProjectRole.fromString(request.getProjectRole());
+			ProjectMetadataRole metadataRole;
+			if (role.equals(ProjectRole.PROJECT_OWNER)) {
+				metadataRole = ProjectMetadataRole.LEVEL_4;
+			} else {
+				metadataRole = ProjectMetadataRole.fromString(request.getMetadataRole());
+			}
+
+			projectService.addUserGroupToProject(project, group, role, metadataRole);
+			return messageSource.getMessage("server.usergroups.add", new Object[] { group.getLabel() }, locale);
+		} catch(EntityExistsException e) {
+			throw new EntityExistsException(messageSource.getMessage("server.usergroups.add.error", new Object[] { group.getLabel() }, locale));
 		}
-		projectService.addUserGroupToProject(project, group, role, metadataRole);
-		return messageSource.getMessage("server.usergroups.add", new Object[] { group.getLabel() }, locale);
 	}
 
 	/**

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

@@ -770,6 +770,15 @@ public Page<Project> findAllProjects(String searchValue, int currentPage, int le
 	@PreAuthorize("hasRole('ROLE_ADMIN') or hasPermission(#project, 'canManageLocalProjectSettings')")
 	public Join<Project, UserGroup> addUserGroupToProject(final Project project, final UserGroup userGroup,
 			final ProjectRole role, ProjectMetadataRole metadataRole) {
+		List<UserGroupProjectJoin> userGroupProjectJoinList = ugpjRepository.findProjectsByUserGroup(userGroup)
+				.stream()
+				.filter(ugpj -> ugpj.getSubject().equals(project))
+				.collect(Collectors.toList());
+
+		if (userGroupProjectJoinList.size() > 0) {
+			throw new EntityExistsException("The user group is already linked to this project");
+		}
+
 		Collection<UserGroupJoin> userGroupJoins = userGroupJoinRepository.findUsersInGroup(userGroup);
 		for (UserGroupJoin userGroupJoin : userGroupJoins) {
 			User user = userGroupJoin.getSubject();

src/main/resources/ca/corefacility/bioinformatics/irida/database/changesets/23.01/add-unique-constraint-to-user-group-project.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<databaseChangeLog xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                   xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+                   xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
+         http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.3.xsd">
+    <changeSet id="add-unique-constraint-to-user-group-project" author="deep">
+        <sql>
+            DELETE ugp1 FROM user_group_project ugp1, user_group_project ugp2 WHERE ugp1.id > ugp2.id AND ugp1.project_id = ugp2.project_id AND ugp1.user_group_id = ugp2.user_group_id;
+        </sql>
+        <addUniqueConstraint columnNames="project_id, user_group_id"
+                             constraintName="UK_USERGROUP_PROJECT"
+                             tableName="user_group_project"
+        />
+    </changeSet>
+</databaseChangeLog>

src/main/resources/ca/corefacility/bioinformatics/irida/database/changesets/23.01/all-changes.xml

@@ -3,4 +3,5 @@
 <databaseChangeLog xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://www.liquibase.org/xml/ns/dbchangelog" xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
          http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.1.xsd">
     <include file="metadata-field-add-label-constraint.xml" relativeToChangelogFile="true" />
+    <include file="add-unique-constraint-to-user-group-project.xml" relativeToChangelogFile="true" />
 </databaseChangeLog>

src/main/resources/i18n/messages.properties

@@ -450,6 +450,7 @@ AddGroupButton.group.label-help=Search by user group name
 AddGroupButton.group.role=Project Role
 AddGroupButton.group.okText=Add Group to Project
 server.usergroups.add={0} has been added to this project.
+server.usergroups.add.error={0} has already been added to this project.
 server.usergroups.update.success=Role updated successfully.
 
 server.update.metadataRole.success={0} metadata role successfully updated to {1}

src/main/webapp/resources/js/apis/projects/user-groups.js

@@ -34,7 +34,6 @@ export async function getAvailableGroupsForProject({ projectId, query }) {
       `${BASE_URL}/available?${params.toString()}`
     );
     return Promise.resolve(data);
-    s;
   } catch (e) {
     return Promise.reject(e.response.data);
   }
@@ -101,17 +100,19 @@ export async function updateUserGroupProjectRole({
  * @returns {Promise<AxiosResponse<any>>}
  */
 export async function updateUserGroupProjectMetadataRole({
- projectId,
- id,
- metadataRole = "",
+  projectId,
+  id,
+  metadataRole = "",
 }) {
   const params = new URLSearchParams({
     projectId,
     id,
     metadataRole,
   });
   try {
-    const { data } = await axios.put(`${BASE_URL}/metadata-role?${params.toString()}`);
+    const { data } = await axios.put(
+      `${BASE_URL}/metadata-role?${params.toString()}`
+    );
     return Promise.resolve(data);
   } catch (e) {
     return Promise.reject(e.response.data);

src/main/webapp/resources/js/components/project-user-groups/AddGroupButton.jsx

@@ -36,6 +36,7 @@ export function AddGroupButton({
 }) {
   const { roles: metadataRoles } = useMetadataRoles();
   const [metadataRole, setMetadataRole] = useState("LEVEL_1");
+
   /*
   Required a reference to the user select input so that focus can be set
   to it when the window opens.
@@ -87,7 +88,6 @@ export function AddGroupButton({
     form,
     visible,
   });
-
   /*
   Watch for changes to the debounced entered value for the user search.
   Once it changes send a request for filtered users.
@@ -123,12 +123,20 @@ export function AddGroupButton({
       groupId,
       role,
       metadataRole,
-    }).then((message) => {
-      onGroupAdded();
-      notification.success({ message });
-      form.resetFields();
-      setVisible(false);
-    });
+    })
+      .then((message) => {
+        onGroupAdded();
+        notification.success({ message });
+        setVisible(false);
+        setQuery("");
+        setGroupId(undefined);
+        setRole(defaultRole);
+        setMetadataRole("LEVEL_1");
+        form.resetFields();
+      })
+      .catch((message) => {
+        notification.error({ message });
+      });
   };
 
   /*
@@ -155,12 +163,16 @@ export function AddGroupButton({
         <Form
           form={form}
           layout="vertical"
-          initialValues={{ role, metadataRole }}
+          initialValues={{
+            groupId,
+            role,
+            metadataRole,
+          }}
         >
           <Form.Item
             label={i18n("AddGroupButton.group.label")}
             help={i18n("AddGroupButton.group.label-help")}
-            name="user"
+            name="groupId"
           >
             <Select
               ref={groupRef}

src/test/java/ca/corefacility/bioinformatics/irida/ria/unit/utilities/FileUtilitiesTest.java

@@ -1,20 +1,20 @@
 package ca.corefacility.bioinformatics.irida.ria.unit.utilities;
 
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
 import java.io.IOException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import ca.corefacility.bioinformatics.irida.exceptions.StorageException;
 import ca.corefacility.bioinformatics.irida.repositories.filesystem.IridaFileStorageLocalUtilityImpl;
 import ca.corefacility.bioinformatics.irida.repositories.filesystem.IridaFileStorageUtility;
 import ca.corefacility.bioinformatics.irida.ria.utilities.FileUtilities;
 import ca.corefacility.bioinformatics.irida.util.IridaFiles;
 
+import static org.junit.jupiter.api.Assertions.*;
+
 public class FileUtilitiesTest {
 	private IridaFileStorageUtility iridaFileStorageUtility;
 
@@ -34,5 +34,28 @@ public void testIsZippedFile() throws IOException {
 
 		isZipped = FileUtilities.isZippedFile(zippedSnpTreePath);
 		assertTrue(isZipped, "snp_tree.tree.zip is zipped");
+
+		// File doesn't exist on local disk
+		Path htmlFile = Paths.get("src/test/resources/files/test_html.html");
+
+		assertThrows(StorageException.class, () -> {
+			FileUtilities.isZippedFile(htmlFile);
+		});
+	}
+
+	@Test
+	public void testGetFileExt()  {
+		// File doesn't exist on local disk
+		Path fastqFile = Paths.get("src/test/resources/files/test_fastq.fastq");
+		// File doesn't exist on local disk
+		Path htmlFile = Paths.get("src/test/resources/files/test_html.html");
+		// File exists on local disk
+		Path htmlFileZipped = Paths.get("src/test/resources/files/test_html_zipped.html.zip");
+
+		// Even though the file doesn't exist on disk we should still get the correct extension
+		assertEquals("fastq", FileUtilities.getFileExt(fastqFile), "Extension should be fastq");
+		assertEquals("html", FileUtilities.getFileExt(htmlFile), "Extension should be html");
+
+		assertEquals("html-zip", FileUtilities.getFileExt(htmlFileZipped), "Extension should be html-zip");
 	}
 }

src/test/java/ca/corefacility/bioinformatics/irida/service/impl/unit/ProjectServiceImplTest.java

@@ -80,6 +80,7 @@ public class ProjectServiceImplTest {
 	private ProjectSubscriptionService projectSubscriptionService;
 	private UserGroupJoinRepository userGroupJoinRepository;
 
+
 	private Validator validator;
 
 	@BeforeEach
@@ -493,4 +494,25 @@ public void testGetProjectsForUser() {
 		assertTrue(projects.stream().anyMatch(p -> p.getSubject().equals(p1)), "Should have found user project join.");
 		assertTrue(projects.stream().anyMatch(p -> p.getSubject().equals(p2)), "Should have found group project join.");
 	}
+
+	@Test
+	public void testAddUserGroupToProject() {
+		final Project p1 = new Project("p1");
+		final Project p2 = new Project("p2");
+		final UserGroup ug1 = new UserGroup("group");
+		final UserGroup ug2 = new UserGroup("group");
+
+		UserGroupProjectJoin groupProjectJoin = new UserGroupProjectJoin(p1, ug1, ProjectRole.PROJECT_OWNER,
+				ProjectMetadataRole.LEVEL_4);
+
+		when(ugpjRepository.findProjectsByUserGroup(ug1)).thenReturn(ImmutableList.of(groupProjectJoin));
+
+		// Throw exception when linking user group to project which is already linked
+		assertThrows(EntityExistsException.class, () -> {
+			projectService.addUserGroupToProject(p1, ug1, ProjectRole.PROJECT_OWNER, ProjectMetadataRole.LEVEL_4);
+		});
+
+		// Throw no exception when linking user group to project which is not already linked
+		assertDoesNotThrow(() -> projectService.addUserGroupToProject(p2, ug2, ProjectRole.PROJECT_OWNER, ProjectMetadataRole.LEVEL_4));
+	}
 }