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

Merge branch '617-divide-requests-over-tas' into 'development'

Resolve "Divide requests over TAs"

Closes #617

See merge request !667
parents a28b3fad 09833afc
Branches
Tags
2 merge requests!711Version 2.2.1,!667Resolve "Divide requests over TAs"
Showing with 376 additions and 6 deletions
......@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
### Added
- Requests in slotted labs can now be distributed to assistants in advanced. [@hpage](https://gitlab.ewi.tudelft.nl/hpage)
### Changed
- Screens should no longer sleep when presenting (not supported on Firefox). [@rwbackx](https://gitlab.ewi.tudelft.nl/rwbackx)
......
......@@ -54,6 +54,7 @@ import nl.tudelft.queue.dto.patch.labs.CapacitySessionPatchDTO;
import nl.tudelft.queue.dto.patch.labs.ExamLabPatchDTO;
import nl.tudelft.queue.dto.patch.labs.RegularLabPatchDTO;
import nl.tudelft.queue.dto.patch.labs.SlottedLabPatchDTO;
import nl.tudelft.queue.dto.util.DistributeRequestsDTO;
import nl.tudelft.queue.dto.util.RequestTableFilterDTO;
import nl.tudelft.queue.dto.view.RequestViewDTO;
import nl.tudelft.queue.model.QueueSession;
......@@ -144,6 +145,9 @@ public class LabController {
@Autowired
private RoleDTOService roleService;
@Autowired
private RequestService requestService;
@Autowired
private StudentGroupControllerApi sgApi;
......@@ -251,6 +255,26 @@ public class LabController {
return "redirect:/lab/" + lab.getId();
}
/**
*
* @param lab The lab that we will distribute requests for
* @param person The distributor of the requests
* @param dto The DTO containing the assignments and assistants to distribute
* @return A redirect to the lab overview page.
*/
@Transactional
@PostMapping("/lab/{lab}/distribute")
@PreAuthorize("@permissionService.canManageSession(#lab)")
public String distributeRequests(@PathEntity Lab lab, @AuthenticatedPerson Person person,
DistributeRequestsDTO dto) {
dto.getEditionSelections()
.forEach(edition -> requestService.distributeRequests(edition.getSelectedAssignments(),
edition.getSelectedAssistants(), person, lab));
return "redirect:/lab/" + lab.getId();
}
/**
* Gets the student enqueue view. This page displays the form that needs to be filled out by the student
* to successfully enrol into the given session.
......@@ -765,6 +789,9 @@ public class LabController {
return isStaffInAll || roleService.rolesForPersonInEdition(edition, person).stream()
.noneMatch(roleService::isStaff);
}).toList();
Map<Long, CourseSummaryDTO> editionsToCourses = session.getEditions().stream()
.collect(Collectors.toMap(EditionSummaryDTO::getId,
edition -> eCache.getRequired(edition.getId()).getCourse()));
Map<Long, String> assignments = allowedAssignments.stream()
.collect(Collectors.toMap(AssignmentDetailsDTO::getId, a -> {
var edition = mCache.getRequired(a.getModule().getId()).getEdition();
......@@ -773,6 +800,7 @@ public class LabController {
: eCache.getRequired(edition.getId()).getCourse().getName() + " - "
+ a.getName();
}));
Map<Long, Long> notEnqueueAble = allowedAssignments.stream().filter(a -> {
var edition = mCache.getRequired(a.getModule().getId()).getEdition();
return roleService.rolesForPersonInEdition(edition, person).isEmpty();
......@@ -781,6 +809,11 @@ public class LabController {
return edition.getId();
}));
if (qSession instanceof AbstractSlottedLab<?>) {
model.addAttribute("distributeRequestsDto",
new DistributeRequestsDTO(editionsToCourses.size()));
}
Set<Long> alreadyInGroup = sgCache.getByPerson(person.getId()).stream()
.map(g -> g.getModule().getId()).collect(Collectors.toSet());
Set<Long> hasEmptyGroups = qSession.getModules().stream()
......@@ -793,6 +826,7 @@ public class LabController {
allowedAssignments.stream().filter(a -> hasEmptyGroups.contains(a.getModule().getId()))
.map(AssignmentDetailsDTO::getId).collect(Collectors.toSet()));
model.addAttribute("assignments", assignments);
model.addAttribute("editionsToCourses", editionsToCourses);
model.addAttribute("notEnqueueAble", notEnqueueAble);
model.addAttribute("types", lab.getAllowedRequests().stream()
.collect(Collectors.groupingBy(AllowedRequest::getAssignment,
......
/*
* Queue - A Queueing system that can be used to handle labs in higher education
* Copyright (C) 2016-2021 Delft University of Technology
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
package nl.tudelft.queue.dto.util;
import java.util.ArrayList;
import java.util.List;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.EqualsAndHashCode;
import lombok.NoArgsConstructor;
@Data
@Builder
@NoArgsConstructor
@AllArgsConstructor
@EqualsAndHashCode(callSuper = false)
public class DistributeRequestsDTO {
@Builder.Default
private List<EditionRequestDistributionDTO> editionSelections = new ArrayList<>();
public DistributeRequestsDTO(int numEditions) {
this.editionSelections = new ArrayList<>();
for (int i = 0; i < numEditions; i++) {
this.editionSelections.add(new EditionRequestDistributionDTO());
}
}
@Data
@NoArgsConstructor
@AllArgsConstructor
@EqualsAndHashCode(callSuper = false)
public static class EditionRequestDistributionDTO {
private List<Long> selectedAssignments = new ArrayList<>();
private List<Long> selectedAssistants = new ArrayList<>();
}
}
......@@ -18,6 +18,7 @@
package nl.tudelft.queue.service;
import static java.time.LocalDateTime.now;
import static nl.tudelft.labracore.api.dto.RolePersonDetailsDTO.TypeEnum.*;
import static nl.tudelft.queue.misc.QueueSessionStatus.*;
import java.io.IOException;
......
/*
* Queue - A Queueing system that can be used to handle labs in higher education
* Copyright (C) 2016-2021 Delft University of Technology
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
package nl.tudelft.queue.service;
import java.util.List;
import nl.tudelft.labracore.api.dto.AssignmentSummaryDTO;
import nl.tudelft.labracore.api.dto.ModuleDetailsDTO;
import org.springframework.stereotype.Service;
@Service
public class ModuleDTOService {
/**
* Gets assignments with respect to a certain edition within several modules.
*
* @param dto The list of modules to consider.
* @param editionId The edition to consider.
* @return The assignments that belong to that edition within the modules.
*/
public List<AssignmentSummaryDTO> getAssignmentsInEdition(List<ModuleDetailsDTO> dto, Long editionId) {
return dto.stream().filter(module -> module.getEdition().getId().equals(editionId))
.flatMap(module -> module.getAssignments().stream())
.collect(java.util.stream.Collectors.toList());
}
}
......@@ -39,6 +39,7 @@ import nl.tudelft.queue.model.LabRequest;
import nl.tudelft.queue.model.Request;
import nl.tudelft.queue.model.SelectionRequest;
import nl.tudelft.queue.model.enums.OnlineMode;
import nl.tudelft.queue.model.enums.RequestStatus;
import nl.tudelft.queue.model.enums.SelectionProcedure;
import nl.tudelft.queue.model.events.*;
import nl.tudelft.queue.model.labs.AbstractSlottedLab;
......@@ -589,4 +590,34 @@ public class RequestService {
.flatMap(id -> sgApi.getStudentGroupsById(List.of(id)).collectList().map(l -> l.get(0)))
.block();
}
/**
* Distributes requests by abusing the forwarding mechanism. Assigns requests in a round-robin fashion.
*
* @param selectedAssignments Assignments that should be distributed
* @param selectedAssistants Assistants that should receieve the distributed requests
* @param distributor Person who is responsible for distributing the requests
* @param lab The lab session that the requests belong to.
*/
@Transactional
public void distributeRequests(List<Long> selectedAssignments, List<Long> selectedAssistants,
Person distributor, Lab lab) {
var assistants = pCache
.getAndIgnoreMissing(selectedAssistants);
if (assistants.isEmpty())
return;
var requests = lab.getRequests().stream()
.filter(rq -> selectedAssignments.contains(rq.getAssignment())
&& rq.getEventInfo().getStatus() == RequestStatus.PENDING)
.sorted(Comparator.comparing(Request::getCreatedAt))
.toList();
// round robin request assignment
for (int i = 0; i < requests.size(); i++) {
forwardRequestToPerson(requests.get(i), distributor, assistants.get(i % assistants.size()),
"Distributed by " + distributor.getDisplayName());
}
}
}
......@@ -27,7 +27,9 @@ import java.util.stream.Collectors;
import nl.tudelft.labracore.api.RoleControllerApi;
import nl.tudelft.labracore.api.dto.*;
import nl.tudelft.labracore.lib.security.user.Person;
import nl.tudelft.queue.cache.EditionCacheManager;
import nl.tudelft.queue.cache.EditionRolesCacheManager;
import nl.tudelft.queue.cache.SessionCacheManager;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
......@@ -40,6 +42,12 @@ public class RoleDTOService {
@Autowired
private RoleControllerApi rApi;
@Autowired
private EditionCacheManager eCache;
@Autowired
private SessionCacheManager sessionCacheManager;
public List<String> names(List<PersonSummaryDTO> people) {
return people.stream().map(PersonSummaryDTO::getDisplayName).collect(Collectors.toList());
}
......@@ -77,6 +85,10 @@ public class RoleDTOService {
return roles(erCache.getRequired(eDto.getId()).getRoles(), types);
}
public List<PersonSummaryDTO> roles(EditionSummaryDTO eDto, Set<RolePersonDetailsDTO.TypeEnum> types) {
return roles(eCache.getRequired(eDto.getId()), types);
}
/**
* Gets the display name of the role type.
*
......@@ -131,6 +143,10 @@ public class RoleDTOService {
return roles(eDto, Set.of(TA));
}
public List<PersonSummaryDTO> staff(EditionSummaryDTO eDto) {
return roles(eDto, Set.of(TA, HEAD_TA, TEACHER, TEACHER_RO));
}
public List<PersonSummaryDTO> headTAs(EditionDetailsDTO eDto) {
return roles(eDto, Set.of(HEAD_TA));
}
......
......@@ -22,6 +22,8 @@
<!--@thymesVar id="qSession" type="nl.tudelft.queue.dto.view.labs.AbstractSlottedLabViewDTO"-->
<!--@thymesVar id="distributeRequestsDto" type="nl.tudelft.queue.dto.util.DistributeRequestsDTO"-->
<body>
<div th:fragment="slots-info">
<div th:if="${@permissionService.canManageSession(qSession.data)}" class="tabs mb-5" role="tablist">
......@@ -33,6 +35,10 @@
<i class="fa fa-chair"></i>
Edit time slot Capacity
</button>
<button type="button" role="tab" aria-controls="ts-distribution-tab" aria-selected="false">
<i class="fa fa-users-cog"></i>
Distribute Requests
</button>
</div>
<div id="basic-tab">
......@@ -168,6 +174,81 @@
</form>
</div>
</th:block>
<th:block th:if="${@permissionService.canManageSession(qSession.data)}">
<div id="ts-distribution-tab" hidden>
<div class="flex vertical gap-5">
<h2 class="font-600 fw-400">Distribute Requests</h2>
<div class="banner" data-type="info">
<span class="fa-solid fa-info-circle"></span>
<p>
Distribute pending requests relating to specific assignments in a round-robin fashion among selected assistants. The
distribution will be done in the order of the assistants selected.
</p>
</div>
<form
th:object="${distributeRequestsDto}"
th:action="@{/lab/{id}/distribute(id=${qSession.id})}"
th:method="POST"
class="flex vertical gap-5">
<div id="distribute-edition-cards" class="flex vertical space-between">
<th:block th:each="edition, editionItemStat : ${qSession.session.editions}">
<div
class="surface p-0"
th:id="'distribute-card-' + ${edition.id}"
th:if="${@permissionService.canManageEdition(edition.id)}">
<h3 class="surface__header" th:text="${editionsToCourses.get(edition.id).name}"></h3>
<div class="surface__content">
<div class="grid col-2 align-center" style="--col-1: minmax(0, 10rem)">
<label th:for="'assignment-distribute-select-' + ${edition.id}">Select Assignments</label>
<div>
<select
data-select
multiple
class="textfield"
data-style="variant"
th:id="'assignment-distribute-select-' + ${edition.id}"
data-title="Select assignment(s) to distribute"
th:field="*{editionSelections[__${editionItemStat.index}__].selectedAssignments}">
<th:block
th:each="assignment : ${@moduleDTOService.getAssignmentsInEdition(modules, edition.id)}">
<option th:value="${assignment.id}" th:text="|${assignment.name}|"></option>
</th:block>
</select>
</div>
<label th:for="'assistant-distribute-select-' + ${edition.id}">Select Assistants</label>
<div>
<select
data-select
multiple
class="textfield"
data-style="variant"
th:id="'assistant-distribute-select-' + ${edition.id}"
data-title="Select assistant(s) to distribute among"
th:field="*{editionSelections[__${editionItemStat.index}__].selectedAssistants}">
<th:block th:each="person : ${@roleDTOService.staff(edition)}">
<option th:value="${person.id}" th:text="${person.displayName}"></option>
</th:block>
</select>
</div>
</div>
</div>
</div>
</th:block>
</div>
<div>
<button type="submit" data-type="primary" data-style="filled" class="button">Distribute Requests</button>
</div>
</form>
</div>
</div>
</th:block>
<script type="text/javascript" src="/js/form_submission_enhancer.js"></script>
</div>
</body>
</html>
......@@ -42,6 +42,7 @@ import nl.tudelft.queue.dto.create.requests.SelectionRequestCreateDTO;
import nl.tudelft.queue.dto.patch.labs.ExamLabPatchDTO;
import nl.tudelft.queue.dto.patch.labs.RegularLabPatchDTO;
import nl.tudelft.queue.dto.patch.labs.SlottedLabPatchDTO;
import nl.tudelft.queue.dto.util.DistributeRequestsDTO;
import nl.tudelft.queue.model.LabRequest;
import nl.tudelft.queue.model.QueueSession;
import nl.tudelft.queue.model.SelectionRequest;
......@@ -69,6 +70,7 @@ import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.SpyBean;
import org.springframework.http.MediaType;
import org.springframework.security.test.context.support.WithUserDetails;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
......@@ -78,6 +80,8 @@ import reactor.core.publisher.Mono;
import test.TestDatabaseLoader;
import test.test.TestQueueApplication;
import com.fasterxml.jackson.databind.ObjectMapper;
@Transactional
@AutoConfigureMockMvc
@SpringBootTest(classes = TestQueueApplication.class)
......@@ -712,6 +716,39 @@ class LabControllerTest {
}
@Test
@WithUserDetails("admin")
void distributingRequestsWithoutFaultWorks() throws Exception {
DistributeRequestsDTO dto = new DistributeRequestsDTO(2);
dto.getEditionSelections().get(0).getSelectedAssignments().addAll(List.of(1L, 2L));
dto.getEditionSelections().get(0).getSelectedAssistants().addAll(List.of(1L, 2L, 10L));
mvc.perform(post("/lab/" + slottedLab1.getId() + "/distribute")
.with(csrf())
.content(new ObjectMapper().writeValueAsString(dto))
.contentType(MediaType.APPLICATION_JSON))
.andExpect(status().is3xxRedirection())
.andExpect(redirectedUrl("/lab/" + slottedLab1.getId()));
}
@Test
@WithUserDetails("admin")
void distributingRequestsWithoutDtoDoesNotCrash() throws Exception {
mvc.perform(post("/lab/" + slottedLab1.getId() + "/distribute")
.with(csrf()))
.andExpect(status().is3xxRedirection());
verify(rs, never()).distributeRequests(any(), any(), any(), any());
}
@Test
@WithUserDetails("student50")
void sendingDistributionRequestWorksWhenAuthorised() throws Exception {
mvc.perform(post("/lab/" + regLab1.getId() + "/distribute")
.with(csrf()))
.andExpect(view().name("error/403"));
}
@Test
@WithUserDetails("student150")
void redirectToEnrollPageWhenNotEnrolled() throws Exception {
......@@ -769,6 +806,7 @@ class LabControllerTest {
get("/lab/1/enqueue"),
post("/lab/1/enqueue"),
post("/lab/1/revoke"),
post("/lab/1/distribute"),
get("/edition/1/lab/create"),
get("/shared-edition/1/lab/create"),
......
......@@ -20,6 +20,7 @@ package nl.tudelft.queue.service;
import static nl.tudelft.queue.model.enums.RequestStatus.*;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.AdditionalMatchers.or;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;
......@@ -33,7 +34,8 @@ import nl.tudelft.labracore.api.AssignmentControllerApi;
import nl.tudelft.labracore.api.RoleControllerApi;
import nl.tudelft.labracore.api.StudentGroupControllerApi;
import nl.tudelft.labracore.api.dto.*;
import nl.tudelft.queue.cache.SessionCacheManager;
import nl.tudelft.labracore.lib.security.user.Person;
import nl.tudelft.queue.cache.PersonCacheManager;
import nl.tudelft.queue.dto.create.requests.SelectionRequestCreateDTO;
import nl.tudelft.queue.model.LabRequest;
import nl.tudelft.queue.model.QSelectionRequest;
......@@ -44,6 +46,7 @@ import nl.tudelft.queue.model.enums.RequestStatus;
import nl.tudelft.queue.model.enums.SelectionProcedure;
import nl.tudelft.queue.model.labs.CapacitySession;
import nl.tudelft.queue.model.labs.RegularLab;
import nl.tudelft.queue.model.labs.SlottedLab;
import nl.tudelft.queue.repository.CapacitySessionRepository;
import nl.tudelft.queue.repository.SelectionRequestRepository;
......@@ -64,7 +67,6 @@ import test.test.TestQueueApplication;
@Transactional
@SpringBootTest(classes = TestQueueApplication.class)
@DirtiesContext(classMode = DirtiesContext.ClassMode.BEFORE_CLASS)
public class RequestServiceTest {
private final RoomDetailsDTO room1 = new RoomDetailsDTO()
.id(8932L)
......@@ -98,6 +100,8 @@ public class RequestServiceTest {
private RegularLab oopNowRegularLab1;
private SlottedLab oopNowSlottedLab1;
private RoleDetailsDTO[] oopNowTAs;
private LabRequest[] rlOopNowSharedLabRequests;
......@@ -113,12 +117,9 @@ public class RequestServiceTest {
@Autowired
private SelectionRequestRepository srr;
@Autowired
@SpyBean
private RequestService rs;
@Autowired
private SessionCacheManager sCache;
@Autowired
private SessionApiMocker sApiMocker;
......@@ -146,6 +147,9 @@ public class RequestServiceTest {
@Autowired
private StudentGroupControllerApi sgApi;
@SpyBean
private PersonCacheManager pCache;
@Autowired
private RoleControllerApi rlApi;
......@@ -163,6 +167,8 @@ public class RequestServiceTest {
oopNowRegularLab1 = db.getOopNowRegularLab1();
oopNowSlottedLab1 = db.getOopNowSlottedLab1();
oopNowTAs = db.getOopNowTAs();
rlOopNowSharedLabRequests = db.getRlOopNowSharedLabRequests();
......@@ -348,6 +354,67 @@ public class RequestServiceTest {
}
@Test
void distributingEmptyRequestsNothingHappens() {
rs.distributeRequests(List.of(), List.of(),
Person.builder().id(1L).displayName("Test Person").build(), oopNowSlottedLab1);
rs.distributeRequests(List.of(), List.of(1L, 2L),
Person.builder().id(1L).displayName("Test Person").build(), oopNowSlottedLab1);
verify(rs, never()).forwardRequestToPerson(any(), any(), any(), any());
}
@Test
@DirtiesContext(methodMode = DirtiesContext.MethodMode.BEFORE_METHOD)
void onlyPendingRequestsWithCorrectAssignmentsGetDistributed() {
pApiMocker.save(new PersonSummaryDTO().id(1L));
pApiMocker.save(new PersonSummaryDTO().id(2L));
doNothing().when(rs).forwardRequestToPerson(any(), any(), any(), any());
oopNowSlottedLab1.getRequests().addAll(List.of(LabRequest.builder().assignment(1L).build(),
LabRequest.builder().assignment(1L).build(), LabRequest.builder().assignment(2L).build(),
LabRequest.builder().assignment(3L).build(), LabRequest.builder().assignment(3L).build()));
oopNowSlottedLab1.getRequests().get(1).getEventInfo().setStatus(APPROVED);
oopNowSlottedLab1.getRequests().get(2).getEventInfo().setStatus(PROCESSING);
rs.distributeRequests(List.of(1L), List.of(1L, 2L), Person.builder().build(), oopNowSlottedLab1);
verify(rs, times(1)).forwardRequestToPerson(eq(oopNowSlottedLab1.getRequests().get(0)), any(), any(),
any());
}
@Test
@DirtiesContext(methodMode = DirtiesContext.MethodMode.BEFORE_METHOD)
void distributingRequestsWithMissingPeopleBehavesAsExpected() {
pApiMocker.save(new PersonSummaryDTO().id(1L));
doNothing().when(rs).forwardRequestToPerson(any(), any(), any(), any());
oopNowSlottedLab1.getRequests().addAll(List.of(LabRequest.builder().assignment(9801L).build(),
LabRequest.builder().assignment(9801L).build(),
LabRequest.builder().assignment(9802L).build(),
LabRequest.builder().assignment(9803L).build(),
LabRequest.builder().assignment(9803L).build()));
oopNowSlottedLab1.getRequests().get(1).getEventInfo().setStatus(APPROVED);
oopNowSlottedLab1.getRequests().get(2).getEventInfo().setStatus(PENDING);
rs.distributeRequests(List.of(9801L), List.of(2L), Person.builder().build(), oopNowSlottedLab1);
verify(rs, never()).forwardRequestToPerson(
or(eq(oopNowSlottedLab1.getRequests().get(0)), eq(oopNowSlottedLab1.getRequests().get(1))),
any(), any(), any());
rs.distributeRequests(List.of(9801L, 9802L), List.of(1L, 2L), Person.builder().build(),
oopNowSlottedLab1);
verify(rs, times(2)).forwardRequestToPerson(any(), any(), any(), any());
}
@Test
@WithUserDetails("student200")
void oopTaGetsOopRequestsOnlyInSharedSession() {
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please to comment