Skip to content
Snippets Groups Projects
Commit b04c0228 authored by Ruben Backx's avatar Ruben Backx :coffee:
Browse files

Merge branch '34-importing-groups' into 'development'

Resolve "Importing groups"

Closes #34

See merge request !221
parents dca9b4e1 38c7d557
No related branches found
No related tags found
2 merge requests!229Version 2.2.1,!221Resolve "Importing groups"
Showing
with 202 additions and 5 deletions
...@@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ...@@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased] ## [Unreleased]
### Added ### Added
- Ability to import groups from csv file (@dsavvidi)
### Changed ### Changed
### Fixed ### Fixed
......
...@@ -264,6 +264,9 @@ dependencies { ...@@ -264,6 +264,9 @@ dependencies {
// Guava // Guava
implementation("com.google.guava:guava:11.0.2") implementation("com.google.guava:guava:11.0.2")
// Apache Commons
implementation("org.apache.commons:commons-csv:1.10.0")
implementation("com.github.ulisesbocchio:spring-boot-security-saml:$samlVersion") implementation("com.github.ulisesbocchio:spring-boot-security-saml:$samlVersion")
// Hibernate for database-entity mapping and EntityManagers // Hibernate for database-entity mapping and EntityManagers
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
*/ */
package nl.tudelft.submit.controller; package nl.tudelft.submit.controller;
import java.io.IOException;
import java.time.LocalDateTime; import java.time.LocalDateTime;
import java.util.*; import java.util.*;
import java.util.function.Function; import java.util.function.Function;
...@@ -30,8 +31,11 @@ import org.springframework.security.access.prepost.PreAuthorize; ...@@ -30,8 +31,11 @@ import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.stereotype.Controller; import org.springframework.stereotype.Controller;
import org.springframework.ui.Model; import org.springframework.ui.Model;
import org.springframework.web.bind.annotation.*; import org.springframework.web.bind.annotation.*;
import org.springframework.web.multipart.MultipartFile;
import org.springframework.web.servlet.mvc.support.RedirectAttributes; import org.springframework.web.servlet.mvc.support.RedirectAttributes;
import com.opencsv.exceptions.CsvValidationException;
import nl.tudelft.labracore.api.dto.*; import nl.tudelft.labracore.api.dto.*;
import nl.tudelft.labracore.lib.security.user.AuthenticatedPerson; import nl.tudelft.labracore.lib.security.user.AuthenticatedPerson;
import nl.tudelft.labracore.lib.security.user.Person; import nl.tudelft.labracore.lib.security.user.Person;
...@@ -229,6 +233,21 @@ public class StudentGroupController { ...@@ -229,6 +233,21 @@ public class StudentGroupController {
return "redirect:/module/" + moduleId + "/groups"; return "redirect:/module/" + moduleId + "/groups";
} }
/**
* Imports student groups from a csv file.
*
* @param moduleId the id of the module to which these groups will belong
* @param file the csv file containing the groups
* @return a redirect to getGroupsPerModule()
*/
@PostMapping("{moduleId}/import")
@PreAuthorize("@authorizationService.canImportStudentGroups(#moduleId)")
public String importStudentGroups(@PathVariable Long moduleId,
@RequestParam("file") MultipartFile file) throws IOException, CsvValidationException {
groupService.importStudentGroups(moduleId, file);
return "redirect:/module/" + moduleId + "/groups";
}
/** /**
* Edits group capacity for a module. * Edits group capacity for a module.
* *
......
...@@ -17,9 +17,7 @@ ...@@ -17,9 +17,7 @@
*/ */
package nl.tudelft.submit.csv; package nl.tudelft.submit.csv;
import java.io.FileWriter; import java.io.*;
import java.io.IOException;
import java.io.Reader;
import java.net.MalformedURLException; import java.net.MalformedURLException;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;
import java.nio.file.Files; import java.nio.file.Files;
......
...@@ -1124,12 +1124,22 @@ public class AuthorizationService { ...@@ -1124,12 +1124,22 @@ public class AuthorizationService {
* Checks whether the authenticated user can create a group in a module. * Checks whether the authenticated user can create a group in a module.
* *
* @param moduleId The id of the module * @param moduleId The id of the module
* @return True iff the user can view a group in the module. * @return True iff the user can create a group in the module.
*/ */
public boolean canCreateGroup(Long moduleId) { public boolean canCreateGroup(Long moduleId) {
return isAtLeastHeadTAInModule(moduleId); return isAtLeastHeadTAInModule(moduleId);
} }
/**
* Checks whether the authenticated user can import groups of students.
*
* @param moduleId The id of the module
* @return True, iff the user can import groups of students.
*/
public boolean canImportStudentGroups(Long moduleId) {
return isAtLeastHeadTAInModule(moduleId);
}
/** /**
* Checks whether the authenticated user can edit a group's note. * Checks whether the authenticated user can edit a group's note.
* *
......
...@@ -20,6 +20,9 @@ package nl.tudelft.submit.service; ...@@ -20,6 +20,9 @@ package nl.tudelft.submit.service;
import static nl.tudelft.submit.enums.SwitchAction.JOIN; import static nl.tudelft.submit.enums.SwitchAction.JOIN;
import static nl.tudelft.submit.enums.SwitchAction.LEAVE; import static nl.tudelft.submit.enums.SwitchAction.LEAVE;
import java.io.IOException;
import java.io.InputStreamReader;
import java.nio.charset.StandardCharsets;
import java.util.*; import java.util.*;
import java.util.regex.Matcher; import java.util.regex.Matcher;
import java.util.regex.Pattern; import java.util.regex.Pattern;
...@@ -30,13 +33,17 @@ import javax.transaction.Transactional; ...@@ -30,13 +33,17 @@ import javax.transaction.Transactional;
import org.modelmapper.ModelMapper; import org.modelmapper.ModelMapper;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service; import org.springframework.stereotype.Service;
import org.springframework.web.multipart.MultipartFile;
import org.springframework.web.reactive.function.client.WebClientResponseException; import org.springframework.web.reactive.function.client.WebClientResponseException;
import com.opencsv.exceptions.CsvValidationException;
import nl.tudelft.labracore.api.StudentGroupControllerApi; import nl.tudelft.labracore.api.StudentGroupControllerApi;
import nl.tudelft.labracore.api.dto.*; import nl.tudelft.labracore.api.dto.*;
import nl.tudelft.librador.dto.view.View; import nl.tudelft.librador.dto.view.View;
import nl.tudelft.submit.cache.PersonCacheManager; import nl.tudelft.submit.cache.PersonCacheManager;
import nl.tudelft.submit.cache.StudentGroupCacheManager; import nl.tudelft.submit.cache.StudentGroupCacheManager;
import nl.tudelft.submit.csv.CSVService;
import nl.tudelft.submit.dto.id.GroupId; import nl.tudelft.submit.dto.id.GroupId;
import nl.tudelft.submit.dto.view.labracore.SubmitGroupDetailsDTO; import nl.tudelft.submit.dto.view.labracore.SubmitGroupDetailsDTO;
import nl.tudelft.submit.dto.view.labracore.SubmitGroupSummaryDTO; import nl.tudelft.submit.dto.view.labracore.SubmitGroupSummaryDTO;
...@@ -71,6 +78,11 @@ public class StudentGroupService { ...@@ -71,6 +78,11 @@ public class StudentGroupService {
private ModuleDivisionService moduleDivisionService; private ModuleDivisionService moduleDivisionService;
@Autowired @Autowired
private SubmissionService submissionService; private SubmissionService submissionService;
@Autowired
private ModuleService moduleService;
@Autowired
private CSVService csvService;
private ModelMapper mapper = new ModelMapper(); private ModelMapper mapper = new ModelMapper();
...@@ -215,6 +227,31 @@ public class StudentGroupService { ...@@ -215,6 +227,31 @@ public class StudentGroupService {
groupApi.allocateGroupsForModule(capacity, moduleId).block(); groupApi.allocateGroupsForModule(capacity, moduleId).block();
} }
/**
* Imports student groups from a csv file. Group members must already be a part of the edition.
*
* @param moduleId the id of the module to import for
* @param file the csv file
*/
@Transactional
public void importStudentGroups(Long moduleId, MultipartFile file)
throws IOException, CsvValidationException {
Map<String, List<String>> groupAssignment = new HashMap<>();
csvService.readCSV(new InputStreamReader(file.getInputStream(), StandardCharsets.UTF_8), false, 1)
.forEach(line -> groupAssignment
.computeIfAbsent(line[1], key -> new ArrayList<>())
.add(line[0]));
for (Map.Entry<String, List<String>> group : groupAssignment.entrySet()) {
createGroup(new StudentGroupCreateDTO().members(group.getValue().stream()
.map(x -> new RoleIdDTO()
.id(new Id().editionId(moduleService.getModuleById(moduleId).getEdition().getId())
.personId(personCache.get(x).getId())))
.toList()).name(group.getKey())
.module(new ModuleIdDTO().id(moduleId)).capacity(group.getValue().size()));
}
}
/** /**
* Attempts to add a person to a group and, if successful, creates a switching event if the group is * Attempts to add a person to a group and, if successful, creates a switching event if the group is
* locked. Throws GroupSwitchingException if is already in a group in the module. * locked. Throws GroupSwitchingException if is already in a group in the module.
...@@ -246,6 +283,7 @@ public class StudentGroupService { ...@@ -246,6 +283,7 @@ public class StudentGroupService {
* @param id the id of the group * @param id the id of the group
* @param usernames the list of usernames of the people to be added * @param usernames the list of usernames of the people to be added
*/ */
@Transactional
public void addPeopleToGroup(Long id, List<String> usernames) throws GroupSwitchingException { public void addPeopleToGroup(Long id, List<String> usernames) throws GroupSwitchingException {
Long moduleId = getGroupDetails(id).getModule().getId(); Long moduleId = getGroupDetails(id).getModule().getId();
......
...@@ -140,6 +140,7 @@ module.no_group = No group ...@@ -140,6 +140,7 @@ module.no_group = No group
module.progress = Progress module.progress = Progress
module.student_search = Username / Group / Student number / Student name module.student_search = Username / Group / Student number / Student name
module.create_groups = Create groups module.create_groups = Create groups
module.create_groups.import = Import groups
module.create_groups.allocate = Allocate module.create_groups.allocate = Allocate
module.create_groups.generate = Generate module.create_groups.generate = Generate
module.create_groups.amount = Amount of groups module.create_groups.amount = Amount of groups
......
...@@ -34,6 +34,7 @@ ...@@ -34,6 +34,7 @@
<div class="flex gap-3"> <div class="flex gap-3">
<button class="button" data-dialog="allocate">Allocate groups</button> <button class="button" data-dialog="allocate">Allocate groups</button>
<button class="button" data-dialog="generate">Generate groups</button> <button class="button" data-dialog="generate">Generate groups</button>
<button class="button" data-dialog="import">Import groups</button>
</div> </div>
<dialog id="allocate" class="dialog"> <dialog id="allocate" class="dialog">
...@@ -103,6 +104,55 @@ ...@@ -103,6 +104,55 @@
</div> </div>
</form> </form>
</dialog> </dialog>
<dialog id="import" class="dialog">
<form
class="flex vertical p-7"
th:action="@{|/group/${module.id}/import|}"
method="post"
enctype="multipart/form-data">
<h2 class="underlined font-500" th:text="#{module.create_groups.import}"></h2>
<p>
Note: students must already be part of the edition. You can do so by using
the import feature.
</p>
<div class="grid col-2 align-center gap-3" style="--col-1: minmax(0, 8rem)">
<label for="file" th:text="#{general.file}"></label>
<input
type="file"
id="file"
name="file"
accept="text/csv"
onchange="readColumnOptions()"
required />
</div>
<h3>File format</h3>
<div class="surface">
<div class="flex gap-2">
<div class="grid align-center gap-0">
<span class="font-100">1.</span>
<span class="font-100">2.</span>
<span class="font-100">3.</span>
</div>
<div class="grid gap-0">
<code>Header 1,Header 2</code>
<code>NetID,group name</code>
<code>NetID,group name</code>
</div>
</div>
<em style="font-style: italic">The header row will be ignored</em>
</div>
<div class="flex space-between">
<button class="button p-less" data-style="outlined" data-cancel>
Cancel
</button>
<button
class="button p-less"
type="submit"
th:text="#{module.create_groups.import}"></button>
</div>
</form>
</dialog>
</main> </main>
</body> </body>
</html> </html>
...@@ -36,6 +36,7 @@ import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMock ...@@ -36,6 +36,7 @@ import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMock
import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.http.MediaType; import org.springframework.http.MediaType;
import org.springframework.mock.web.MockMultipartFile;
import org.springframework.security.test.context.support.WithUserDetails; import org.springframework.security.test.context.support.WithUserDetails;
import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
...@@ -217,6 +218,24 @@ class StudentGroupControllerTest { ...@@ -217,6 +218,24 @@ class StudentGroupControllerTest {
verify(groupService).changeCapacities(MODULE_ID, 2); verify(groupService).changeCapacities(MODULE_ID, 2);
} }
@Test
@WithUserDetails("username")
void importStudentGroupsTest() throws Exception {
when(authService.canImportStudentGroups(anyLong())).thenReturn(true);
doNothing().when(groupService)
.importStudentGroups(anyLong(), any());
MockMultipartFile file = new MockMultipartFile("file", "importfile".getBytes());
mockMvc.perform(
multipart("/group/{moduleId}/import", MODULE_ID)
.file(file).with(csrf())
.contentType(MediaType.MULTIPART_FORM_DATA))
.andExpect(status().is3xxRedirection())
.andExpect(redirectedUrl("/module/" + MODULE_ID + "/groups"));;
verify(authService).canImportStudentGroups(MODULE_ID);
verify(groupService).importStudentGroups(MODULE_ID, file);
}
@Test @Test
@WithUserDetails("username") @WithUserDetails("username")
void generateGroupsAllowsIfCanCreateGroup() throws Exception { void generateGroupsAllowsIfCanCreateGroup() throws Exception {
......
...@@ -874,6 +874,14 @@ class AuthorizationServiceTest { ...@@ -874,6 +874,14 @@ class AuthorizationServiceTest {
assertThat(service.canCreateGroup(MODULE_ID)).isEqualTo(expected); assertThat(service.canCreateGroup(MODULE_ID)).isEqualTo(expected);
} }
@ParameterizedTest
@CsvSource({ "TEACHER,true", "HEAD_TA,true", "TA,false", "STUDENT,false", ",false" })
@WithUserDetails("username")
void canImportStudentGroups(RoleDetailsDTO.TypeEnum role, boolean expected) {
mockRole(role);
assertThat(service.canImportStudentGroups(MODULE_ID)).isEqualTo(expected);
}
@ParameterizedTest @ParameterizedTest
@CsvSource({ "TEACHER,true", "HEAD_TA,true", "TA,true", "STUDENT,false", ",false" }) @CsvSource({ "TEACHER,true", "HEAD_TA,true", "TA,true", "STUDENT,false", ",false" })
@WithUserDetails("username") @WithUserDetails("username")
......
...@@ -23,6 +23,8 @@ import static org.junit.jupiter.api.Assertions.assertThrows; ...@@ -23,6 +23,8 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.*; import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*; import static org.mockito.Mockito.*;
import java.io.IOException;
import java.io.InputStreamReader;
import java.time.LocalDateTime; import java.time.LocalDateTime;
import java.util.*; import java.util.*;
...@@ -33,6 +35,9 @@ import org.mockito.Mockito; ...@@ -33,6 +35,9 @@ import org.mockito.Mockito;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.mock.web.MockMultipartFile;
import com.opencsv.exceptions.CsvValidationException;
import application.test.TestSubmitApplication; import application.test.TestSubmitApplication;
import nl.tudelft.labracore.api.StudentGroupControllerApi; import nl.tudelft.labracore.api.StudentGroupControllerApi;
...@@ -40,6 +45,7 @@ import nl.tudelft.labracore.api.dto.*; ...@@ -40,6 +45,7 @@ import nl.tudelft.labracore.api.dto.*;
import nl.tudelft.librador.dto.view.View; import nl.tudelft.librador.dto.view.View;
import nl.tudelft.submit.cache.PersonCacheManager; import nl.tudelft.submit.cache.PersonCacheManager;
import nl.tudelft.submit.cache.StudentGroupCacheManager; import nl.tudelft.submit.cache.StudentGroupCacheManager;
import nl.tudelft.submit.csv.CSVService;
import nl.tudelft.submit.dto.id.GroupId; import nl.tudelft.submit.dto.id.GroupId;
import nl.tudelft.submit.dto.view.labracore.SubmitGroupDetailsDTO; import nl.tudelft.submit.dto.view.labracore.SubmitGroupDetailsDTO;
import nl.tudelft.submit.dto.view.labracore.SubmitGroupSummaryDTO; import nl.tudelft.submit.dto.view.labracore.SubmitGroupSummaryDTO;
...@@ -47,6 +53,7 @@ import nl.tudelft.submit.enums.SwitchAction; ...@@ -47,6 +53,7 @@ import nl.tudelft.submit.enums.SwitchAction;
import nl.tudelft.submit.exception.GroupSwitchingException; import nl.tudelft.submit.exception.GroupSwitchingException;
import nl.tudelft.submit.model.*; import nl.tudelft.submit.model.*;
import nl.tudelft.submit.model.note.GroupNote; import nl.tudelft.submit.model.note.GroupNote;
import nl.tudelft.submit.repository.SubmitGroupRepository;
import nl.tudelft.submit.repository.SwitchEventRepository; import nl.tudelft.submit.repository.SwitchEventRepository;
import nl.tudelft.submit.repository.note.GroupNoteRepository; import nl.tudelft.submit.repository.note.GroupNoteRepository;
import reactor.core.publisher.Flux; import reactor.core.publisher.Flux;
...@@ -58,6 +65,7 @@ class StudentGroupServiceTest { ...@@ -58,6 +65,7 @@ class StudentGroupServiceTest {
private final Long GROUP_ID = 5L; private final Long GROUP_ID = 5L;
private final Long MODULE_ID = 42L; private final Long MODULE_ID = 42L;
private final Long EDITION_ID = 33L;
private final Long COURSE_ID = 139L; private final Long COURSE_ID = 139L;
private final Long MEMBER_ID = 56123L; private final Long MEMBER_ID = 56123L;
private final Long PERSON_ID = 98451L; private final Long PERSON_ID = 98451L;
...@@ -85,6 +93,9 @@ class StudentGroupServiceTest { ...@@ -85,6 +93,9 @@ class StudentGroupServiceTest {
@Autowired @Autowired
private GroupNoteRepository groupNoteRepository; private GroupNoteRepository groupNoteRepository;
@MockBean
private SubmitGroupRepository groupRepository;
@MockBean @MockBean
private ModuleDivisionService moduleDivisionService; private ModuleDivisionService moduleDivisionService;
...@@ -94,6 +105,12 @@ class StudentGroupServiceTest { ...@@ -94,6 +105,12 @@ class StudentGroupServiceTest {
@MockBean @MockBean
private SubmissionService submissionService; private SubmissionService submissionService;
@MockBean
private CSVService csvService;
@MockBean
private ModuleService moduleService;
@Test @Test
void getGroupDetails() { void getGroupDetails() {
StudentGroupDetailsDTO dto = new StudentGroupDetailsDTO().id(GROUP_ID).name("Test group") StudentGroupDetailsDTO dto = new StudentGroupDetailsDTO().id(GROUP_ID).name("Test group")
...@@ -219,6 +236,40 @@ class StudentGroupServiceTest { ...@@ -219,6 +236,40 @@ class StudentGroupServiceTest {
Mockito.verify(groupApi).addGroupMembers(GROUP_ID, usernames); Mockito.verify(groupApi).addGroupMembers(GROUP_ID, usernames);
} }
@Test
public void importStudentGroupsTest() throws IOException, CsvValidationException {
MockMultipartFile file = new MockMultipartFile("file", "test.csv", "text/csv", "content".getBytes());
List<String[]> lines = List.of(
new String[] { "dduck", "Group1" },
new String[] { "mmouse", "Group1" },
new String[] { "batman", "Group2" });
PersonSummaryDTO person1 = new PersonSummaryDTO().id(1L).username("dduck").displayName("Donald Duck");
PersonSummaryDTO person2 = new PersonSummaryDTO().id(2L).username("mmouse")
.displayName("Mickey Mouse");
PersonSummaryDTO person3 = new PersonSummaryDTO().id(3L).username("batman").displayName("Batman");
when(csvService.readCSV(any(InputStreamReader.class), eq(false), eq(1))).thenReturn(lines);
when(moduleService.getModuleById(eq(MODULE_ID))).thenReturn(
new ModuleDetailsDTO().id(MODULE_ID).edition(new EditionSummaryDTO().id(EDITION_ID)));
when(personCache.get(eq("dduck"))).thenReturn(person1);
when(personCache.get(eq("mmouse"))).thenReturn(person2);
when(personCache.get(eq("batman"))).thenReturn(person3);
when(personCache.get(eq(1L))).thenReturn(Optional.ofNullable(person1));
when(personCache.get(eq(2L))).thenReturn(Optional.ofNullable(person2));
when(personCache.get(eq(3L))).thenReturn(Optional.ofNullable(person3));
when(groupApi.getGroupForPersonAndModule(anyLong(), anyLong())).thenReturn(Mono.empty());
when(groupApi.addGroup(any(StudentGroupCreateDTO.class))).thenReturn(Mono.just(5L));
when(groupRepository.save(any(SubmitGroup.class))).thenReturn(SubmitGroup.builder().id(5L).build());
groupService.importStudentGroups(MODULE_ID, file);
verify(csvService).readCSV(any(InputStreamReader.class), eq(false), eq(1));
verify(moduleService, times(3)).getModuleById(eq(MODULE_ID));
verify(personCache, times(6)).get(anyString());
verify(personCache, times(3)).get(anyLong());
verify(groupRepository, times(2)).save(any(SubmitGroup.class));
}
@Test @Test
void addPeopleToLockedGroup() { void addPeopleToLockedGroup() {
List<String> usernames = List.of("hpotter", "rweasley", "hgranger"); List<String> usernames = List.of("hpotter", "rweasley", "hgranger");
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please to comment