Merge branch 'add-feedback-to-submissions-page' into 'development'

Add feedback to submissions page

See merge request !56

src/main/java/nl/tudelft/submit/controller/AssignmentController.java

@@ -32,7 +32,7 @@ import nl.tudelft.labracore.lib.security.user.AuthenticatedPerson;
 import nl.tudelft.librador.dto.view.View;
 import nl.tudelft.submit.csv.CSVService;
 import nl.tudelft.submit.dto.create.SubmitAssignmentCreateDTO;
-import nl.tudelft.submit.dto.create.grading.SubmitGradeCreateDTO;
+import nl.tudelft.submit.dto.create.grading.GradedFeedbackCreateDTO;
 import nl.tudelft.submit.dto.create.note.AssignmentNoteCreateDTO;
 import nl.tudelft.submit.dto.patch.SubmitAssignmentPatchDTO;
 import nl.tudelft.submit.dto.patch.grading.AssignmentGradeImportDTO;
@@ -153,8 +153,7 @@ public class AssignmentController {
 		SubmitAssignmentDetailsDTO assignment = assignmentService.getSubmitAssignmentDetails(id);
 		model.addAttribute("assignment", assignment);
 		model.addAttribute("submissions", assignmentService.getAllSubmissions(id));
-		model.addAttribute("gradeCreate",
-				SubmitGradeCreateDTO.builder().submission(new SubmissionIdDTO()).build());
+		model.addAttribute("gradeCreate", new GradedFeedbackCreateDTO());
 		return "assignment/submissions";
 	}
 

src/main/java/nl/tudelft/submit/controller/GradeController.java

@@ -31,11 +31,9 @@ import nl.tudelft.labracore.api.dto.GradeCreateDTO;
 import nl.tudelft.labracore.api.dto.Person;
 import nl.tudelft.labracore.lib.security.user.AuthenticatedPerson;
 import nl.tudelft.submit.dto.create.grading.GradedFeedbackCreateDTO;
-import nl.tudelft.submit.dto.create.grading.SubmitGradeCreateDTO;
 import nl.tudelft.submit.dto.patch.grading.GradingFormulaPatchDTO;
 import nl.tudelft.submit.dto.view.FormulaTestDTO;
 import nl.tudelft.submit.dto.view.TestGradeViewDTO;
-import nl.tudelft.submit.model.SubmitAssignment;
 import nl.tudelft.submit.model.grading.CalculatedScore;
 import nl.tudelft.submit.model.grading.GradingFormula;
 import nl.tudelft.submit.service.AssignmentService;
@@ -68,16 +66,20 @@ public class GradeController {
 		create.setIsScriptFeedback(false);
 		create.setTimestamp(LocalDateTime.now());
 
+		Long gradeId = null;
 		if (create.getGrade() != null) {
-			Long id = gradeService
+			gradeId = gradeService
 					.addGrade(create.getGradeCreateDTO(GradeCreateDTO.SchemeEnum.valueOf(assignmentService
 							.getOrCreateSubmitAssignment(assignmentId).getScoreType().name())));
 			gradeService.updateGradesAfterGradedSubmission(create.getSubmissionId());
-			feedbackService.addFeedback(create.getFeedbackCreateDTO(id));
-		} else {
-			feedbackService.addFeedback(create.getFeedbackCreateDTO(null));
 		}
-		return "redirect:/group/{groupId}?assignment={assignmentId}&submission=" + create.getSubmissionId();
+
+		if (create.getTextualFeedback() != null && !create.getTextualFeedback().isBlank()) {
+			feedbackService.addFeedback(create.getFeedbackCreateDTO(gradeId));
+			return "redirect:/group/{groupId}?assignment={assignmentId}&submission="
+					+ create.getSubmissionId();
+		}
+		return "redirect:/group/{groupId}";
 	}
 
 	@PostMapping("/group/remove-highest/{groupId}/{submissionId}")
@@ -89,12 +91,23 @@ public class GradeController {
 
 	@PostMapping("/assignment/{assignmentId}")
 	@PreAuthorize("@authorizationService.hasStaffRoleInAssignment(#assignmentId)")
-	public String addGradeForAssignment(@PathVariable Long assignmentId,
-			@ModelAttribute("gradeCreate") SubmitGradeCreateDTO create) {
-		SubmitAssignment assignment = assignmentService.getOrCreateSubmitAssignment(assignmentId);
-		gradeService.addGrade(
-				create.getCreateDTO(GradeCreateDTO.SchemeEnum.valueOf(assignment.getScoreType().name())));
-		gradeService.updateGradesAfterGradedSubmission(create.getSubmission().getId());
+	public String addGradeForAssignment(@AuthenticatedPerson Person person, @PathVariable Long assignmentId,
+			@ModelAttribute("gradeCreate") GradedFeedbackCreateDTO create) {
+		create.setAuthorId(person.getId());
+		create.setIsScriptFeedback(false);
+		create.setTimestamp(LocalDateTime.now());
+
+		Long gradeId = null;
+		if (create.getGrade() != null) {
+			gradeId = gradeService
+					.addGrade(create.getGradeCreateDTO(GradeCreateDTO.SchemeEnum.valueOf(assignmentService
+							.getOrCreateSubmitAssignment(assignmentId).getScoreType().name())));
+			gradeService.updateGradesAfterGradedSubmission(create.getSubmissionId());
+		}
+		if (create.getTextualFeedback() != null && !create.getTextualFeedback().isBlank()) {
+			feedbackService.addFeedback(create.getFeedbackCreateDTO(gradeId));
+		}
+
 		return "redirect:/assignment/{assignmentId}/submissions";
 	}
 

src/main/java/nl/tudelft/submit/dto/create/grading/GradedFeedbackCreateDTO.java

@@ -19,7 +19,6 @@ package nl.tudelft.submit.dto.create.grading;
 
 import java.time.LocalDateTime;
 
-import javax.validation.constraints.NotEmpty;
 import javax.validation.constraints.NotNull;
 
 import lombok.*;
@@ -40,7 +39,6 @@ public class GradedFeedbackCreateDTO {
 
 	private Role.TypeEnum visibleFor;
 
-	@NotEmpty
 	private String textualFeedback;
 
 	private String fileFeedback;

src/main/resources/templates/assignment/submissions.html

@@ -93,15 +93,24 @@
         </div>
     </div>
 
-    <div th:each="submission : ${submissions}" th:id="|submission-${submission.id}-new-grade|" class="overlay">
+
+    <div layout:fragment="submission-view" th:if="${staff}" th:id="|submission-${submission.id}-overlay|"
+         th:class="${param.submission?.toString() == '__${submission.id}__'} ? 'visible overlay' : 'overlay'">
         <div class="boxed-group">
-            <h1 class="section-title" th:text="#{submission.new_grade}"></h1>
-            <form class="form" th:object="${gradeCreate}" th:action="@{|/grade/assignment/${assignment.id}|}"
-                  method="post">
+            <div class="boxed-header">
+                <h1 class="collapsible-title" th:text="#{submission.text}"></h1>
+                <div class="float-right">
+                    <button type="button" class="overlay-close fas fa-times" aria-label="Close overlay"
+                            th:onclick="|toggleOverlay('submission-${submission.id}-overlay')|"></button>
+                </div>
+            </div>
+            <div class="boxed-content">
+                <form th:if="${submission.isLatest}" class="form" th:object="${feedbackCreate}" th:action="@{|/grade/group/${group.id}/${assignment.id}|}" method="post">
+                    <input type="hidden" th:name="submissionId" th:value="${submission.id}"/>
                     <div class="form-grid">
-                    <input type="hidden" th:name="submission.id" th:value="${submission.id}"/>
-                    <label th:for="|new-grade-${submission.id}|" class="center-label"
-                           th:text="#{submission.new_grade}"></label>
+                        <label th:for="|new-feedback-${submission.id}|" class="center-label" th:text="#{assignment.feedback}"></label>
+                        <textarea th:id="|new-feedback-${submission.id}|" th:field="*{textualFeedback}" class="textarea" th:placeholder="#{feedback.text}" rows=5></textarea>
+                        <label th:for="|new-grade-${submission.id}|" class="center-label" th:text="#{submission.grade}"></label>
                         <input th:if="${assignment.scoreType == T(nl.tudelft.labracore.api.dto.Grade.SchemeEnum).DUTCH_GRADE or assignment.scoreType == T(nl.tudelft.labracore.api.dto.Grade.SchemeEnum).DUTCH_GRADE_1000}"
                                th:id="|new-grade-${submission.id}|" th:field="*{grade}" class="textfield" type="number"
                                step=".1" min="1" max="10"/>
@@ -109,17 +118,63 @@
                                th:id="|new-grade-${submission.id}|" th:field="*{grade}" class="textfield" type="number"
                                step="1" th:min="${@gradeService.getMinScore(assignment.scoreType)}" th:max="${@gradeService.getMaxScore(assignment.scoreType)}"/>
                         <select th:if="${assignment.scoreType == T(nl.tudelft.labracore.api.dto.Grade.SchemeEnum).PASS_FAIL}"
-                            th:id="|new-grade-${submission.id}|" th:field="*{grade}" class="selectbox">
+                                th:id="|new-grade-${submission.id}|" th:name="grade" class="selectbox">
+                            <option th:value="${null}" th:text="#{submission.no_grade}"></option>
                             <option value="0" th:text="#{stats.failed}"></option>
                             <option value="1" th:text="#{stats.passed}"></option>
                         </select>
-                </div>
-                <div class="form-buttons">
                         <button type="submit" class="button" th:text="#{generic.submit}"></button>
-                    <button type="button" class="button" th:text="#{generic.cancel}"
+                    </div>
+                </form>
+                <div th:unless="${submission.isLatest}" th:text="#{submission.not_latest}"></div>
+                <div th:unless="${submission.feedback.isEmpty()}" class="feedbacks">
+                    <div th:each="feedback : ${submission.feedback}" class="feedback">
+                        <div class="feedback-text">
+                            <p th:utext="${#strings.replace(#strings.escapeXml(feedback.textualFeedback), '&#10;', '&lt;br&gt;')}">Test</p>
+                            <span th:if="${grades.containsKey(feedback.gradeId)}" class="feedback-grade"
+                                  th:text="${@gradeService.getScoreDisplayString(grades[feedback.gradeId])}"></span>
+                        </div>
+                        <span class="feedback-time" th:text="${#temporals.format(feedback.timestamp, 'dd-MM-yyyy HH:mm')}"></span>
+                    </div>
+                </div>
+            </div>
+        </div>
+    </div>
+
+
+    <div th:each="submission : ${submissions}" th:id="|submission-${submission.id}-new-grade|" class="overlay">
+        <div class="boxed-group">
+            <div class="boxed-header">
+                <h1 class="collapsible-title" th:text="#{submission.text}"></h1>
+                <div class="float-right">
+                    <button type="button" class="overlay-close fas fa-times" aria-label="Close overlay"
                             th:onclick="|toggleOverlay('submission-${submission.id}-new-grade')|"></button>
                 </div>
+            </div>
+            <div class="boxed-content">
+                <form th:if="${submission.isLatest}" class="form" th:object="${gradeCreate}" th:action="@{|/grade/assignment/${assignment.id}|}" method="post">
+                    <input type="hidden" th:name="submissionId" th:value="${submission.id}"/>
+                    <div class="form-grid">
+                        <label th:for="|new-feedback-${submission.id}|" class="center-label" th:text="#{assignment.feedback}"></label>
+                        <textarea th:id="|new-feedback-${submission.id}|" th:field="*{textualFeedback}" class="textarea" th:placeholder="#{feedback.text}" rows=5></textarea>
+                        <label th:for="|new-grade-${submission.id}|" class="center-label" th:text="#{submission.grade}"></label>
+                        <input th:if="${assignment.scoreType == T(nl.tudelft.labracore.api.dto.Grade.SchemeEnum).DUTCH_GRADE or assignment.scoreType == T(nl.tudelft.labracore.api.dto.Grade.SchemeEnum).DUTCH_GRADE_1000}"
+                               th:id="|new-grade-${submission.id}|" th:field="*{grade}" class="textfield" type="number"
+                               step=".1" min="1" max="10"/>
+                        <input th:unless="${assignment.scoreType == T(nl.tudelft.labracore.api.dto.Grade.SchemeEnum).PASS_FAIL or assignment.scoreType == T(nl.tudelft.labracore.api.dto.Grade.SchemeEnum).DUTCH_GRADE or assignment.scoreType == T(nl.tudelft.labracore.api.dto.Grade.SchemeEnum).DUTCH_GRADE_1000}"
+                               th:id="|new-grade-${submission.id}|" th:field="*{grade}" class="textfield" type="number"
+                               step="1" th:min="${@gradeService.getMinScore(assignment.scoreType)}" th:max="${@gradeService.getMaxScore(assignment.scoreType)}"/>
+                        <select th:if="${assignment.scoreType == T(nl.tudelft.labracore.api.dto.Grade.SchemeEnum).PASS_FAIL}"
+                                th:id="|new-grade-${submission.id}|" th:name="grade" class="selectbox">
+                            <option th:value="${null}" th:text="#{submission.no_grade}"></option>
+                            <option value="0" th:text="#{stats.failed}"></option>
+                            <option value="1" th:text="#{stats.passed}"></option>
+                        </select>
+                        <button type="submit" class="button" th:text="#{generic.submit}"></button>
+                    </div>
                 </form>
+                <div th:unless="${submission.isLatest}" th:text="#{submission.not_latest}"></div>
+            </div>
         </div>
     </div>
 

src/test/java/nl/tudelft/submit/controller/GradeControllerTest.java

@@ -28,11 +28,9 @@ import java.util.*;
 
 import nl.tudelft.labracore.api.dto.Grade;
 import nl.tudelft.labracore.api.dto.GradeCreateDTO;
-import nl.tudelft.labracore.api.dto.SubmissionIdDTO;
 import nl.tudelft.submit.TestSubmitApplication;
 import nl.tudelft.submit.dto.create.FeedbackCreateDTO;
 import nl.tudelft.submit.dto.create.grading.GradedFeedbackCreateDTO;
-import nl.tudelft.submit.dto.create.grading.SubmitGradeCreateDTO;
 import nl.tudelft.submit.dto.view.FormulaTestDTO;
 import nl.tudelft.submit.model.SubmitAssignment;
 import nl.tudelft.submit.model.grading.CalculatedScore;
@@ -137,7 +135,7 @@ public class GradeControllerTest {
 		when(authService.hasStaffRoleInAssignment(anyLong())).thenReturn(false);
 
 		mockMvc.perform(post("/grade/assignment/{assignmentId}", ASSIGNMENT_ID).with(csrf())
-				.flashAttr("gradeCreate", SubmitGradeCreateDTO.builder().grade(6.0).build()))
+				.flashAttr("gradeCreate", GradedFeedbackCreateDTO.builder().grade(6.0).build()))
 				.andExpect(status().isForbidden());
 
 		verify(authService).hasStaffRoleInAssignment(ASSIGNMENT_ID);
@@ -153,7 +151,7 @@ public class GradeControllerTest {
 
 		mockMvc.perform(post("/grade/assignment/{assignmentId}", ASSIGNMENT_ID).with(csrf())
 				.flashAttr("gradeCreate",
-						SubmitGradeCreateDTO.builder().submission(new SubmissionIdDTO()).grade(6.0).build()))
+						GradedFeedbackCreateDTO.builder().grade(6.0).build()))
 				.andExpect(status().is3xxRedirection());
 
 		verify(authService).hasStaffRoleInAssignment(ASSIGNMENT_ID);
@@ -173,7 +171,7 @@ public class GradeControllerTest {
 
 		mockMvc.perform(post("/grade/assignment/{assignmentId}", ASSIGNMENT_ID).with(csrf())
 				.flashAttr("gradeCreate",
-						SubmitGradeCreateDTO.builder().submission(new SubmissionIdDTO()).grade(6.0).build()))
+						GradedFeedbackCreateDTO.builder().grade(6.0).build()))
 				.andExpect(status().is3xxRedirection());
 
 		verify(authService).hasStaffRoleInAssignment(ASSIGNMENT_ID);
@@ -193,7 +191,7 @@ public class GradeControllerTest {
 
 		mockMvc.perform(post("/grade/assignment/{assignmentId}", ASSIGNMENT_ID).with(csrf())
 				.flashAttr("gradeCreate",
-						SubmitGradeCreateDTO.builder().submission(new SubmissionIdDTO()).grade(5.0).build()))
+						GradedFeedbackCreateDTO.builder().grade(5.0).build()))
 				.andExpect(status().is3xxRedirection());
 
 		verify(authService).hasStaffRoleInAssignment(ASSIGNMENT_ID);