Merge branch '670-remove-ability-to-leave-ta-review-if-not-found-status' into 'development'

Resolve "Remove ability to leave TA review if "Not found" status"

Closes #670

See merge request !736

CHANGELOG.md

@@ -17,13 +17,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - A few general statistics was added in the lab statistics page. [@hpage](https://gitlab.ewi.tudelft.nl/hpage)
 - A bar-graph detailing the assignment breakdown between questions and submissions was added in the lab statistics page. [@hpage](https://gitlab.ewi.tudelft.nl/hpage)
 
-### Changed
-
-### Fixed
-
 ### Changed
 - Written feedback sorting now follows a reverse chronological order. [@hpage](https://gitlab.ewi.tudelft.nl/hpage)
 - Text used to indicate the number of times a lab is rescheduled is now more intuitive. [@hpage](https://gitlab.ewi.tudelft.nl/hpage)
+- Assistants who couldn't locate students are no longer eligible to receive feedback from those students. [@hpage](https://gitlab.ewi.tudelft.nl/hpage)
 
 ### Fixed
 - The request history page can now be seen for participants. [@mmadara](https://gitlab.ewi.tudelft.nl/mmadara)
@@ -32,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - The assignment count graph on the edition statistic page was fixed. [@hpage](https://gitlab.ewi.tudelft.nl/hpage)
 - Limited capacity sessions were not shown on the home page [@rwbackx](https://gitlab.ewi.tudelft.nl/rwbackx)
 - Limited capacity sessions can now have extra information. [@hpage](https://gitlab.ewi.tudelft.nl/hpage)
+- Limited capacity sessions were not shown on the home page
 
 ## [2.2.2]
 

src/main/java/nl/tudelft/queue/model/embeddables/RequestEventInfo.java

@@ -21,6 +21,7 @@ import java.time.LocalDateTime;
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 import javax.persistence.*;
@@ -31,6 +32,7 @@ import nl.tudelft.queue.cqsr.Aggregate;
 import nl.tudelft.queue.model.RequestEvent;
 import nl.tudelft.queue.model.enums.RequestStatus;
 import nl.tudelft.queue.model.events.EventWithAssistant;
+import nl.tudelft.queue.model.events.StudentNotFoundEvent;
 
 @Data
 @Builder
@@ -114,15 +116,22 @@ public class RequestEventInfo extends Aggregate<RequestEventInfo, RequestEvent<?
 
 	/**
 	 * Collects a list of ids of the assistants that are involved with this request. The assistants are given
-	 * in order of most-recently involved to least-recently involved.
+	 * in order of most-recently involved to least-recently involved. It also excludes assistants which could
+	 * not find the student. This is done to ensure that students do not give feedback to a TA who never
+	 * interacted with them.
 	 *
 	 * @return The list of assistant ids.
 	 */
 	public List<Long> involvedAssistants() {
+		Set<Long> disqualifiedAssistants = events.stream()
+				.filter(event -> event instanceof StudentNotFoundEvent)
+				.map(event -> ((StudentNotFoundEvent) event).getAssistant()).collect(Collectors.toSet());
+
 		return events.stream()
 				.sorted(Comparator.comparing((RequestEvent<?> re) -> re.getTimestamp()).reversed())
 				.filter(event -> event instanceof EventWithAssistant)
 				.map(event -> ((EventWithAssistant) event).getAssistant())
+				.filter(assistant -> !disqualifiedAssistants.contains(assistant))
 				.distinct().collect(Collectors.toList());
 	}
 }

src/main/java/nl/tudelft/queue/service/FeedbackService.java

@@ -64,9 +64,12 @@ public class FeedbackService {
 	private SessionCacheManager sessionCacheManager;
 
 	/**
-	 * Finds all assistant that are involved in the given request. This list of people is generated for
-	 * students to be able to give feedback on the one TA they want to give feedback on. This list is ordered
-	 * by event occurrence: the last occurred event will see its assistant at the top of the list.
+	 * Finds all assistants that are involved in the given request. This list of people is generated for
+	 * students to be able to give feedback to TAs that helped them. This list is ordered by event occurrence:
+	 * the last occurred event will see its assistant at the top of the list. It also excludes assistants
+	 * which could not find the student. This is done to ensure that students do not give feedback to a TA who
+	 * never interacted with them.
+	 *
 	 *
 	 * @param  requestId The id of the request to find all assistants for.
 	 * @return           A list of person summaries representing the various assistants helping out with the

src/main/resources/migrations.yml

@@ -1233,3 +1233,30 @@ databaseChangeLog:
             referencedTableName: lab
             validate: true
 
+  # Delete feedbacks where student could not be found
+  - changeSet:
+      id: delete-feedbacks-where-student-not-found-mariadb
+      author: Henry Page
+      changes:
+        - sql:
+            comment: (MariaDB) Sets DELETED_AT on feedback if feedback is present on TA that could not find student
+            dbms: mariadb
+            sql:
+              UPDATE feedback AS f 
+              INNER JOIN request_event AS re ON f.assistant_id = re.assistant AND f.request_id = re.request_id 
+              SET f.deleted_at = re.timestamp WHERE re.dtype = 'StudentNotFoundEvent';
+  - changeSet:
+      id: delete-feedbacks-where-student-not-found-pgsql
+      author: Henry Page
+      changes:
+        - sql:
+            comment: (PGSQL) Sets DELETED_AT on feedback if feedback is present on TA that could not find student
+            dbms: postgresql
+            sql:
+              UPDATE feedback AS f 
+              SET deleted_at = re.timestamp 
+              FROM request_event AS re 
+              WHERE f.assistant_id = re.assistant AND f.request_id = re.request_id AND re.dtype = 'StudentNotFoundEvent';
+
+
+

src/test/java/nl/tudelft/queue/service/FeedbackServiceTest.java

@@ -132,9 +132,6 @@ public class FeedbackServiceTest {
 				new RequestForwardedToAnyEvent(request1, people[0].getId(), "Hey"),
 				new RequestTakenEvent(request1, people[1].getId()),
 				new RequestApprovedEvent(request1, people[1].getId(), "")));
-		for (RequestEvent<?> event : request1.getEventInfo().getEvents()) {
-			rer.applyAndSave(event);
-		}
 
 		request2 = rr.save(LabRequest.builder()
 				.room(32L).assignment(32L).studentGroup(32L).requester(32L).comment("Hello")
@@ -148,7 +145,26 @@ public class FeedbackServiceTest {
 				new RequestTakenEvent(request2, people[3].getId()),
 				new RequestForwardedToPersonEvent(request2, people[3].getId(), people[4].getId(), ""),
 				new RequestApprovedEvent(request2, people[4].getId(), "")));
-		for (RequestEvent<?> event : request2.getEventInfo().getEvents()) {
+
+		request3 = rr.save(LabRequest.builder()
+				.room(32L).assignment(32L).studentGroup(32L).requester(32L).comment("Hello")
+				.requestType(RequestType.QUESTION)
+				.session(lab)
+				.build());
+
+		request3.getEventInfo().getEvents().addAll(List.of(
+				new RequestCreatedEvent(request3),
+				new RequestTakenEvent(request3, people[0].getId()),
+				new StudentNotFoundEvent(request3, people[0].getId()),
+				new RequestTakenEvent(request3, people[1].getId()),
+				new RequestForwardedToAnyEvent(request3, people[1].getId(), "Hey"),
+				new RequestTakenEvent(request3, people[2].getId()),
+				new RequestApprovedEvent(request3, people[2].getId(), "Approved"),
+				new RequestTakenEvent(request3, people[3].getId()),
+				new StudentNotFoundEvent(request3, people[3].getId())));
+
+		for (RequestEvent<?> event : Stream.of(request1, request2, request3)
+				.flatMap(rq -> rq.getEventInfo().getEvents().stream()).toList()) {
 			rer.applyAndSave(event);
 		}
 
@@ -189,6 +205,13 @@ public class FeedbackServiceTest {
 				.containsExactlyInAnyOrder(people[4], people[3], people[2], people[0]);
 	}
 
+	@Test
+	void requestsDoNotHaveAssistantsInvolvedThatCouldNotFindStudent() {
+		assertThat(fs.assistantsInvolvedInRequest(request3.getId()))
+				.containsExactlyElementsOf(List.of(people[2], people[1]));
+
+	}
+
 	@Test
 	void countRatingsWorks() {
 		var p0FeedBacks = fr.findAllByAssistant(people[0].getId());