From ff4c5b58b40db5fcb7754c259c3854194668c1e1 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Thu, 21 Nov 2024 19:20:47 +0100 Subject: [PATCH] Allow changes to the reflection to be made after it's been submitted (#13) Replaces #12 Fixes card 3213 and 3412 There are minimum requirements for the reflection document submitted by authors at the end of the thesis process. Before now there was no way to handle the case when the reflection did not meet these minimum requirements. This change makes it possible in two ways; 1. The supervisor can request improvements to be made requiring the author to re-submit a new reflection inside SciPro 2. The supervisor can directly edit the reflection themselves if it has been submitted out-of-band or for any other reason Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/13 Reviewed-by: Nico Athanassiadis <nico@dsv.su.se> --- .../java/se/su/dsv/scipro/CoreConfig.java | 5 +- .../dataobject/ProjectEvent.java | 3 +- .../notifications/notifications.properties | 7 + .../java/se/su/dsv/scipro/project/Author.java | 27 +++ .../dsv/scipro/project/ReflectionStatus.java | 7 + .../su/dsv/scipro/reflection/Reflection.java | 22 ++ .../ReflectionImprovementsRequestedEvent.java | 7 + .../ReflectionImprovementsSubmittedEvent.java | 10 + .../scipro/reflection/ReflectionService.java | 11 + .../reflection/ReflectionServiceImpl.java | 59 +++++- .../V392_1__reflection_resubmission.sql | 4 + ...92_2__reflection_comment_by_supervisor.sql | 1 + .../reflection/ReflectionServiceTest.java | 20 ++ .../crosscutting/NotifyFailedReflection.java | 46 +++++ .../su/dsv/scipro/grading/CriteriaPanel.html | 10 +- .../su/dsv/scipro/grading/CriteriaPanel.java | 27 ++- .../grading/ReflectionModalBodyPanel.html | 82 ++++++++ .../grading/ReflectionModalBodyPanel.java | 191 ++++++++++++++++++ .../ReflectionModalBodyPanel.utf8.properties | 10 + .../pages/NotificationLandingPage.java | 2 +- .../project/panels/FinalStepsPanel.html | 24 +++ .../project/panels/FinalStepsPanel.java | 58 +++++- .../panels/FinalStepsPanel.utf8.properties | 5 + .../dsv/scipro/wicket-package.utf8.properties | 2 + .../java/se/su/dsv/scipro/SciProTest.java | 3 + .../dsv/scipro/war/WicketConfiguration.java | 10 + 26 files changed, 628 insertions(+), 25 deletions(-) create mode 100644 core/src/main/java/se/su/dsv/scipro/project/ReflectionStatus.java create mode 100644 core/src/main/java/se/su/dsv/scipro/reflection/Reflection.java create mode 100644 core/src/main/java/se/su/dsv/scipro/reflection/ReflectionImprovementsRequestedEvent.java create mode 100644 core/src/main/java/se/su/dsv/scipro/reflection/ReflectionImprovementsSubmittedEvent.java create mode 100644 core/src/main/resources/db/migration/V392_1__reflection_resubmission.sql create mode 100644 core/src/main/resources/db/migration/V392_2__reflection_comment_by_supervisor.sql create mode 100644 view/src/main/java/se/su/dsv/scipro/crosscutting/NotifyFailedReflection.java create mode 100644 view/src/main/java/se/su/dsv/scipro/grading/ReflectionModalBodyPanel.html create mode 100644 view/src/main/java/se/su/dsv/scipro/grading/ReflectionModalBodyPanel.java create mode 100644 view/src/main/java/se/su/dsv/scipro/grading/ReflectionModalBodyPanel.utf8.properties diff --git a/core/src/main/java/se/su/dsv/scipro/CoreConfig.java b/core/src/main/java/se/su/dsv/scipro/CoreConfig.java index a6ec1b8ffe..918df6478f 100644 --- a/core/src/main/java/se/su/dsv/scipro/CoreConfig.java +++ b/core/src/main/java/se/su/dsv/scipro/CoreConfig.java @@ -744,9 +744,10 @@ public class CoreConfig { @Bean public ReflectionServiceImpl reflectionService( AuthorRepository authorRepository, - FinalSeminarServiceImpl finalSeminarService) + FinalSeminarServiceImpl finalSeminarService, + EventBus eventBus) { - return new ReflectionServiceImpl(authorRepository, finalSeminarService); + return new ReflectionServiceImpl(authorRepository, finalSeminarService, eventBus); } @Bean diff --git a/core/src/main/java/se/su/dsv/scipro/notifications/dataobject/ProjectEvent.java b/core/src/main/java/se/su/dsv/scipro/notifications/dataobject/ProjectEvent.java index a44a3760be..ae72d48f16 100755 --- a/core/src/main/java/se/su/dsv/scipro/notifications/dataobject/ProjectEvent.java +++ b/core/src/main/java/se/su/dsv/scipro/notifications/dataobject/ProjectEvent.java @@ -23,7 +23,8 @@ public class ProjectEvent extends NotificationEvent { ROUGH_DRAFT_APPROVAL_APPROVED, ROUGH_DRAFT_APPROVAL_REJECTED, REVIEWER_GRADING_REPORT_SUBMITTED, ONE_YEAR_PASSED_FROM_LATEST_ANNUAL_REVIEW, SUPERVISOR_GRADING_INITIAL_ASSESSMENT_DONE, EXPORTED_SUCCESS, REVIEWER_GRADING_INITIAL_ASSESSMENT_DONE, FIRST_MEETING, OPPOSITION_FAILED, - PARTICIPATION_APPROVED, COMPLETED, PARTICIPATION_FAILED + PARTICIPATION_APPROVED, COMPLETED, PARTICIPATION_FAILED, REFLECTION_IMPROVEMENTS_REQUESTED, + REFLECTION_IMPROVEMENTS_SUBMITTED } @Basic diff --git a/core/src/main/java/se/su/dsv/scipro/notifications/notifications.properties b/core/src/main/java/se/su/dsv/scipro/notifications/notifications.properties index a972eac30e..c5cb11b76a 100755 --- a/core/src/main/java/se/su/dsv/scipro/notifications/notifications.properties +++ b/core/src/main/java/se/su/dsv/scipro/notifications/notifications.properties @@ -85,6 +85,13 @@ PROJECT.PARTICIPATION_APPROVED.body = Your active participation on {0} has been PROJECT.PARTICIPATION_FAILED.title = Your active participation on {1} did not meet the minimum requirements. PROJECT.PARTICIPATION_FAILED.body = Your active participation did not meet the minimum requirements set, and you will \ have to be an active participant on a different final seminar to pass this step. +PROJECT.REFLECTION_IMPROVEMENTS_REQUESTED.title = Reflection improvements requested +PROJECT.REFLECTION_IMPROVEMENTS_REQUESTED.body = The supervisor has deemed that the reflection submitted does not meet \ + the minimum requirements and has requested improvements. Please log into SciPro and submit a new reflection. \ + Their comments can be seen below:\n\n{0} +PROJECT.REFLECTION_IMPROVEMENTS_SUBMITTED.title = Reflection improvements submitted +PROJECT.REFLECTION_IMPROVEMENTS_SUBMITTED.body = The reflection improvements have been submitted. \ + \n\n{0} FORUM.NEW_FORUM_POST.title = Forum post: {2} FORUM.NEW_FORUM_POST.body = New forum post submitted:<br /><br />{0} diff --git a/core/src/main/java/se/su/dsv/scipro/project/Author.java b/core/src/main/java/se/su/dsv/scipro/project/Author.java index 8adaa5fed3..96ec7ce284 100644 --- a/core/src/main/java/se/su/dsv/scipro/project/Author.java +++ b/core/src/main/java/se/su/dsv/scipro/project/Author.java @@ -5,6 +5,8 @@ import jakarta.persistence.Column; import jakarta.persistence.Embeddable; import jakarta.persistence.EmbeddedId; import jakarta.persistence.Entity; +import jakarta.persistence.EnumType; +import jakarta.persistence.Enumerated; import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; import jakarta.persistence.MapsId; @@ -30,6 +32,15 @@ public class Author { @Column(name = "reflection") private String reflection; + @Basic + @Enumerated(EnumType.STRING) + @Column(name = "reflection_status") + private ReflectionStatus reflectionStatus = ReflectionStatus.NOT_SUBMITTED; + + @Basic + @Column(name = "reflection_comment_by_supervisor") + private String reflectionSupervisorComment; + /** * If this author wants to be notified when a final seminar created * as long as they have not yet completed both an opposition and @@ -85,6 +96,22 @@ public class Author { return user; } + public ReflectionStatus getReflectionStatus() { + return reflectionStatus; + } + + public void setReflectionStatus(ReflectionStatus reflectionStatus) { + this.reflectionStatus = reflectionStatus; + } + + public void setReflectionSupervisorComment(String reflectionSupervisorComment) { + this.reflectionSupervisorComment = reflectionSupervisorComment; + } + + public String getReflectionSupervisorComment() { + return reflectionSupervisorComment; + } + // ---------------------------------------------------------------------------------- // Nested class // ---------------------------------------------------------------------------------- diff --git a/core/src/main/java/se/su/dsv/scipro/project/ReflectionStatus.java b/core/src/main/java/se/su/dsv/scipro/project/ReflectionStatus.java new file mode 100644 index 0000000000..92b94cd7b6 --- /dev/null +++ b/core/src/main/java/se/su/dsv/scipro/project/ReflectionStatus.java @@ -0,0 +1,7 @@ +package se.su.dsv.scipro.project; + +public enum ReflectionStatus { + NOT_SUBMITTED, + SUBMITTED, + IMPROVEMENTS_NEEDED +} diff --git a/core/src/main/java/se/su/dsv/scipro/reflection/Reflection.java b/core/src/main/java/se/su/dsv/scipro/reflection/Reflection.java new file mode 100644 index 0000000000..898d961a2c --- /dev/null +++ b/core/src/main/java/se/su/dsv/scipro/reflection/Reflection.java @@ -0,0 +1,22 @@ +package se.su.dsv.scipro.reflection; + +public sealed interface Reflection { + boolean isSubmittable(); + + record NotSubmitted() implements Reflection { + @Override + public boolean isSubmittable() { return true; } + } + + record Submitted(String reflection) implements Reflection { + @Override + public boolean isSubmittable() { + return false; + } + } + + record ImprovementsNeeded(String oldReflection, String commentBySupervisor) implements Reflection { + @Override + public boolean isSubmittable() { return true; } + } +} diff --git a/core/src/main/java/se/su/dsv/scipro/reflection/ReflectionImprovementsRequestedEvent.java b/core/src/main/java/se/su/dsv/scipro/reflection/ReflectionImprovementsRequestedEvent.java new file mode 100644 index 0000000000..07d8811f99 --- /dev/null +++ b/core/src/main/java/se/su/dsv/scipro/reflection/ReflectionImprovementsRequestedEvent.java @@ -0,0 +1,7 @@ +package se.su.dsv.scipro.reflection; + +import se.su.dsv.scipro.project.Project; +import se.su.dsv.scipro.system.User; + +public record ReflectionImprovementsRequestedEvent(Project project, User author, String supervisorComment) { +} diff --git a/core/src/main/java/se/su/dsv/scipro/reflection/ReflectionImprovementsSubmittedEvent.java b/core/src/main/java/se/su/dsv/scipro/reflection/ReflectionImprovementsSubmittedEvent.java new file mode 100644 index 0000000000..ce43d5eed4 --- /dev/null +++ b/core/src/main/java/se/su/dsv/scipro/reflection/ReflectionImprovementsSubmittedEvent.java @@ -0,0 +1,10 @@ +package se.su.dsv.scipro.reflection; + +import se.su.dsv.scipro.project.Project; +import se.su.dsv.scipro.system.User; + +/** + * This event may be triggered by the supervisor if they edit the reflection after requesting improvements. + */ +public record ReflectionImprovementsSubmittedEvent(Project project, User author, String reflection) { +} diff --git a/core/src/main/java/se/su/dsv/scipro/reflection/ReflectionService.java b/core/src/main/java/se/su/dsv/scipro/reflection/ReflectionService.java index b4a206bf76..139dc9a6d9 100644 --- a/core/src/main/java/se/su/dsv/scipro/reflection/ReflectionService.java +++ b/core/src/main/java/se/su/dsv/scipro/reflection/ReflectionService.java @@ -14,4 +14,15 @@ public interface ReflectionService { * @return the reflection, or {@code null} if none has been submitted */ String getSubmittedReflection(Project project, User author); + + /** + * Used by the supervisor when the currently submitted reflection does not meet the minimum requirements. + * This is done individually by author. + * + * @param author the author whose reflection does not meet the minimum requirements. + * @param supervisorComment feedback provided by the supervisor so the author knows what to improve. + */ + void requestNewReflection(Project project, User author, String supervisorComment); + + Reflection getReflection(Project project, User author); } diff --git a/core/src/main/java/se/su/dsv/scipro/reflection/ReflectionServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/reflection/ReflectionServiceImpl.java index bdc36bc904..73d934ca32 100644 --- a/core/src/main/java/se/su/dsv/scipro/reflection/ReflectionServiceImpl.java +++ b/core/src/main/java/se/su/dsv/scipro/reflection/ReflectionServiceImpl.java @@ -1,22 +1,32 @@ package se.su.dsv.scipro.reflection; +import com.google.common.eventbus.EventBus; import jakarta.transaction.Transactional; import se.su.dsv.scipro.finalseminar.AuthorRepository; import se.su.dsv.scipro.finalseminar.FinalSeminarService; import se.su.dsv.scipro.project.Author; import se.su.dsv.scipro.project.Project; +import se.su.dsv.scipro.project.ReflectionStatus; import se.su.dsv.scipro.system.User; import jakarta.inject.Inject; +import java.util.Optional; + public class ReflectionServiceImpl implements ReflectionService { private final AuthorRepository authorRepository; private final FinalSeminarService finalSeminarService; + private final EventBus eventBus; @Inject - public ReflectionServiceImpl(AuthorRepository authorRepository, FinalSeminarService finalSeminarService) { + public ReflectionServiceImpl( + AuthorRepository authorRepository, + FinalSeminarService finalSeminarService, + EventBus eventBus) + { this.authorRepository = authorRepository; this.finalSeminarService = finalSeminarService; + this.eventBus = eventBus; } @Override @@ -25,10 +35,13 @@ public class ReflectionServiceImpl implements ReflectionService { } @Override - public boolean hasToFillInReflection(Project project, User author) { - boolean noReflectionSubmitted = authorRepository.findByProjectAndUser(project, author) - .map(Author::getReflection) - .isEmpty(); + public boolean hasToFillInReflection(Project project, User user) { + Optional<Author> optionalAuthor = authorRepository.findByProjectAndUser(project, user); + if (optionalAuthor.isEmpty()) { + return false; + } + Author author = optionalAuthor.get(); + boolean noReflectionSubmitted = author.getReflectionStatus() != ReflectionStatus.SUBMITTED; return hasReachedReflectionProcess(project) && noReflectionSubmitted; } @@ -36,7 +49,13 @@ public class ReflectionServiceImpl implements ReflectionService { @Transactional public void submitReflection(Project project, User user, String reflection) { authorRepository.findByProjectAndUser(project, user) - .ifPresent(author -> author.setReflection(reflection)); + .ifPresent(author -> { + if (author.getReflectionStatus() == ReflectionStatus.IMPROVEMENTS_NEEDED) { + eventBus.post(new ReflectionImprovementsSubmittedEvent(project, user, reflection)); + } + author.setReflection(reflection); + author.setReflectionStatus(ReflectionStatus.SUBMITTED); + }); } @Override @@ -45,4 +64,32 @@ public class ReflectionServiceImpl implements ReflectionService { .map(Author::getReflection) .orElse(null); } + + @Override + @Transactional + public void requestNewReflection(Project project, User user, String supervisorComment) { + authorRepository.findByProjectAndUser(project, user) + .ifPresent(author -> { + author.setReflectionStatus(ReflectionStatus.IMPROVEMENTS_NEEDED); + author.setReflectionSupervisorComment(supervisorComment); + }); + eventBus.post(new ReflectionImprovementsRequestedEvent(project, user, supervisorComment)); + } + + @Override + public Reflection getReflection(Project project, User author) { + return authorRepository.findByProjectAndUser(project, author) + .map(this::toReflection) + .orElseGet(Reflection.NotSubmitted::new); + } + + private Reflection toReflection(Author author) { + return switch (author.getReflectionStatus()) { + case SUBMITTED -> new Reflection.Submitted(author.getReflection()); + case IMPROVEMENTS_NEEDED -> new Reflection.ImprovementsNeeded( + author.getReflection(), + author.getReflectionSupervisorComment()); + default -> new Reflection.NotSubmitted(); + }; + } } diff --git a/core/src/main/resources/db/migration/V392_1__reflection_resubmission.sql b/core/src/main/resources/db/migration/V392_1__reflection_resubmission.sql new file mode 100644 index 0000000000..d80e266c9d --- /dev/null +++ b/core/src/main/resources/db/migration/V392_1__reflection_resubmission.sql @@ -0,0 +1,4 @@ +ALTER TABLE `project_user` + ADD COLUMN `reflection_status` VARCHAR(32) NOT NULL DEFAULT 'NOT_SUBMITTED'; + +UPDATE `project_user` SET `reflection_status` = 'SUBMITTED' WHERE `reflection` IS NOT NULL; diff --git a/core/src/main/resources/db/migration/V392_2__reflection_comment_by_supervisor.sql b/core/src/main/resources/db/migration/V392_2__reflection_comment_by_supervisor.sql new file mode 100644 index 0000000000..f3cc5f2ce1 --- /dev/null +++ b/core/src/main/resources/db/migration/V392_2__reflection_comment_by_supervisor.sql @@ -0,0 +1 @@ +ALTER TABLE `project_user` ADD COLUMN `reflection_comment_by_supervisor` TEXT NULL; diff --git a/core/src/test/java/se/su/dsv/scipro/reflection/ReflectionServiceTest.java b/core/src/test/java/se/su/dsv/scipro/reflection/ReflectionServiceTest.java index 8d8d462e31..745def4bcd 100644 --- a/core/src/test/java/se/su/dsv/scipro/reflection/ReflectionServiceTest.java +++ b/core/src/test/java/se/su/dsv/scipro/reflection/ReflectionServiceTest.java @@ -101,6 +101,26 @@ public class ReflectionServiceTest extends IntegrationTest { assertTrue(reflectionService.hasReachedReflectionProcess(project)); } + @Test + public void request_resubmission() { + LocalDate seminarDate = scheduleSeminar(); + clock.setDate(seminarDate.plusDays(1)); + assertTrue(reflectionService.hasToFillInReflection(project, author), + "After the final seminar the author should be required to submit a reflection"); + + String myReflection = "my reflection"; + reflectionService.submitReflection(project, author, myReflection); + assertEquals(myReflection, reflectionService.getSubmittedReflection(project, author)); + assertFalse(reflectionService.hasToFillInReflection(project, author), + "After submitting the initial reflection it should no longer be required"); + + reflectionService.requestNewReflection(project, author, "Very bad reflection"); + assertTrue(reflectionService.hasToFillInReflection(project, author), + "After supervisor requests resubmission the author should now be required to submit a new reflection"); + assertEquals(myReflection, reflectionService.getSubmittedReflection(project, author), + "The old reflection should be saved to make it easier for the student to update it"); + } + private LocalDate scheduleSeminar() { project.setFinalSeminarRuleExempted(true); // to bypass rough draft approval FinalSeminarDetails details = new FinalSeminarDetails("Zoom", false, 1, 1, Language.SWEDISH, Language.ENGLISH, "zoom id 123"); diff --git a/view/src/main/java/se/su/dsv/scipro/crosscutting/NotifyFailedReflection.java b/view/src/main/java/se/su/dsv/scipro/crosscutting/NotifyFailedReflection.java new file mode 100644 index 0000000000..867703dde2 --- /dev/null +++ b/view/src/main/java/se/su/dsv/scipro/crosscutting/NotifyFailedReflection.java @@ -0,0 +1,46 @@ +package se.su.dsv.scipro.crosscutting; + +import com.google.common.eventbus.EventBus; +import com.google.common.eventbus.Subscribe; +import jakarta.inject.Inject; +import se.su.dsv.scipro.data.dataobjects.Member; +import se.su.dsv.scipro.notifications.NotificationController; +import se.su.dsv.scipro.notifications.dataobject.NotificationSource; +import se.su.dsv.scipro.notifications.dataobject.ProjectEvent; +import se.su.dsv.scipro.reflection.ReflectionImprovementsRequestedEvent; +import se.su.dsv.scipro.reflection.ReflectionImprovementsSubmittedEvent; + +import java.util.Set; + +public class NotifyFailedReflection { + private final NotificationController notificationController; + + @Inject + public NotifyFailedReflection(NotificationController notificationController, EventBus eventBus) { + this.notificationController = notificationController; + eventBus.register(this); + } + + @Subscribe + public void reflectionImprovementsRequested(ReflectionImprovementsRequestedEvent event) { + NotificationSource source = new NotificationSource(); + source.setMessage(event.supervisorComment()); + notificationController.notifyCustomProject( + event.project(), + ProjectEvent.Event.REFLECTION_IMPROVEMENTS_REQUESTED, + source, + Set.of(new Member(event.author(), Member.Type.AUTHOR))); + } + + @Subscribe + public void reflectionImprovementsSubmittted(ReflectionImprovementsSubmittedEvent event) { + NotificationSource source = new NotificationSource(); + source.setMessage(event.reflection()); + source.setAdditionalMessage(event.author().getFullName()); + notificationController.notifyCustomProject( + event.project(), + ProjectEvent.Event.REFLECTION_IMPROVEMENTS_SUBMITTED, + source, + Set.of(new Member(event.project().getHeadSupervisor(), Member.Type.SUPERVISOR))); + } +} diff --git a/view/src/main/java/se/su/dsv/scipro/grading/CriteriaPanel.html b/view/src/main/java/se/su/dsv/scipro/grading/CriteriaPanel.html index 079fb7b067..8794749252 100644 --- a/view/src/main/java/se/su/dsv/scipro/grading/CriteriaPanel.html +++ b/view/src/main/java/se/su/dsv/scipro/grading/CriteriaPanel.html @@ -49,10 +49,16 @@ </div> </wicket:fragment> </div> - <wicket:container wicket:id="reflection"> + <div wicket:id="reflection"> + <wicket:enclosure> + <div class="alert alert-danger"> + <h4 class="alert-heading">Improvements requested</h4> + <wicket:container wicket:id="improvementsNeeded"/> + </div> + </wicket:enclosure> <a wicket:id="showReflection" href="#">View reflection</a> <div wicket:id="modal"></div> - </wicket:container> + </div> </fieldset> </wicket:panel> </body> diff --git a/view/src/main/java/se/su/dsv/scipro/grading/CriteriaPanel.java b/view/src/main/java/se/su/dsv/scipro/grading/CriteriaPanel.java index e5475c42ed..011cb74f9f 100644 --- a/view/src/main/java/se/su/dsv/scipro/grading/CriteriaPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/grading/CriteriaPanel.java @@ -30,10 +30,12 @@ import se.su.dsv.scipro.finalseminar.FinalSeminar; import se.su.dsv.scipro.finalseminar.FinalSeminarOpposition; import se.su.dsv.scipro.finalseminar.FinalSeminarService; import se.su.dsv.scipro.project.Project; +import se.su.dsv.scipro.reflection.Reflection; import se.su.dsv.scipro.reflection.ReflectionService; import se.su.dsv.scipro.report.AbstractGradingCriterion; import se.su.dsv.scipro.report.GradingCriterion; import se.su.dsv.scipro.report.GradingCriterionPoint; +import se.su.dsv.scipro.report.GradingReport; import se.su.dsv.scipro.report.SupervisorGradingReport; import se.su.dsv.scipro.system.Language; import se.su.dsv.scipro.system.User; @@ -264,14 +266,33 @@ public class CriteriaPanel extends GenericPanel<SupervisorGradingReport> { super(id, author); this.gradingCriterion = gradingCriterion; - IModel<String> reflection = LoadableDetachableModel.of(() -> { + IModel<Reflection> reflection = LoadableDetachableModel.of(() -> { Project project = CriteriaPanel.this.getModelObject().getProject(); - return reflectionService.getSubmittedReflection(project, author.getObject()); + return reflectionService.getReflection(project, author.getObject()); + }); + IModel<String> improvementsNeeded = reflection + .as(Reflection.ImprovementsNeeded.class) + .map(Reflection.ImprovementsNeeded::commentBySupervisor); + + add(new MultiLineLabel("improvementsNeeded", improvementsNeeded) { + @Override + protected void onConfigure() { + super.onConfigure(); + setVisible(!getDefaultModelObjectAsString().isBlank()); + } }); modal = new LargeModalWindow("modal"); modal.setTitle("Reflection"); - modal.setContent(id_ -> new MultiLineLabel(id_, new NullReplacementModel(reflection, "No reflection filled in."))); + modal.setContent(modalBodyId -> { + IModel<Project> projectModel = CriteriaPanel.this.getModel().map(GradingReport::getProject); + return new ReflectionModalBodyPanel(modalBodyId, projectModel, author); + }); + this.setOutputMarkupId(true); + this.setOutputMarkupPlaceholderTag(true); + modal.onClose(target -> { + target.add(ReflectionFeedbackPanel.this); + }); add(modal); WebMarkupContainer showReflection = new WebMarkupContainer("showReflection") { diff --git a/view/src/main/java/se/su/dsv/scipro/grading/ReflectionModalBodyPanel.html b/view/src/main/java/se/su/dsv/scipro/grading/ReflectionModalBodyPanel.html new file mode 100644 index 0000000000..7840492505 --- /dev/null +++ b/view/src/main/java/se/su/dsv/scipro/grading/ReflectionModalBodyPanel.html @@ -0,0 +1,82 @@ +<?xml version="1.0" encoding="UTF-8"?> +<html lang="en" xmlns="http://www.w3.org/1999/xhtml" xmlns:wicket="http://wicket.apache.org"> +<body> +<wicket:panel> + <wicket:enclosure> + <div class="alert alert-info"> + <h4 class="alert-heading"> + <wicket:message key="improvements_requested"> + You've requested improvements to the submitted version. + </wicket:message> + </h4> + <wicket:container wicket:id="improvements_needed_supervisor_feedback"> + [Supervisor feedback on needed improvements] + </wicket:container> + </div> + </wicket:enclosure> + + <wicket:container wicket:id="reflection_text"> + [Authors submitted reflection] + </wicket:container> + + <button class="btn btn-outline-secondary" wicket:id="show_edit_reflection_form"> + <wicket:message key="edit_reflection"> + Edit reflection + </wicket:message> + </button> + <button class="btn btn-outline-secondary" wicket:id="show_request_improvements_form"> + <wicket:message key="request_improvements"> + Request improvements + </wicket:message> + </button> + + <form wicket:id="request_improvements_form"> + <hr> + <p> + <wicket:message key="request_improvements_text"> + Please provide feedback on what improvements are needed in the submitted version. + </wicket:message> + </p> + <div class="mb-3"> + <label class="form-label" wicket:for="comment"> + <wicket:message key="comment"> + Comment + </wicket:message> + </label> + <textarea class="form-control" wicket:id="comment" rows="5"></textarea> + </div> + <button class="btn btn-primary" wicket:id="submit"> + <wicket:message key="request_improvements"> + Request improvements + </wicket:message> + </button> + <button class="btn btn-link" wicket:id="cancel"> + <wicket:message key="cancel"> + Cancel + </wicket:message> + </button> + </form> + + <form wicket:id="edit_reflection_form"> + <div class="mb-3"> + <label class="form-label"> + <wicket:message key="reflection"> + Reflection + </wicket:message> + </label> + <textarea class="form-control" wicket:id="reflection" rows="10"></textarea> + </div> + <button class="btn btn-primary" wicket:id="submit"> + <wicket:message key="save"> + Save + </wicket:message> + </button> + <button class="btn btn-link" wicket:id="cancel"> + <wicket:message key="cancel"> + Cancel + </wicket:message> + </button> + </form> +</wicket:panel> +</body> +</html> \ No newline at end of file diff --git a/view/src/main/java/se/su/dsv/scipro/grading/ReflectionModalBodyPanel.java b/view/src/main/java/se/su/dsv/scipro/grading/ReflectionModalBodyPanel.java new file mode 100644 index 0000000000..895e65018f --- /dev/null +++ b/view/src/main/java/se/su/dsv/scipro/grading/ReflectionModalBodyPanel.java @@ -0,0 +1,191 @@ +package se.su.dsv.scipro.grading; + +import jakarta.inject.Inject; +import org.apache.wicket.ajax.AjaxRequestTarget; +import org.apache.wicket.ajax.markup.html.AjaxLink; +import org.apache.wicket.ajax.markup.html.form.AjaxSubmitLink; +import org.apache.wicket.markup.html.basic.MultiLineLabel; +import org.apache.wicket.markup.html.form.Form; +import org.apache.wicket.markup.html.form.TextArea; +import org.apache.wicket.markup.html.panel.Panel; +import org.apache.wicket.model.IModel; +import org.apache.wicket.model.Model; +import se.su.dsv.scipro.project.Project; +import se.su.dsv.scipro.reflection.Reflection; +import se.su.dsv.scipro.reflection.ReflectionService; +import se.su.dsv.scipro.system.User; + +/** + * This is not meant to be a re-usable panel and is made specifically to be used + * as the body of the modal dialog that opens when a supervisor views the + * author's reflection as they're doing their final individual assessment. + */ +class ReflectionModalBodyPanel extends Panel { + @Inject + private ReflectionService reflectionService; + + private final IModel<Project> projectModel; + private final IModel<User> authorModel; + + private enum State { VIEWING, REQUESTING_IMPROVEMENTS, EDITING } + + private State state = State.VIEWING; + + ReflectionModalBodyPanel(String id, IModel<Project> projectModel, IModel<User> authorModel) { + super(id); + this.projectModel = projectModel; + this.authorModel = authorModel; + + setOutputMarkupPlaceholderTag(true); // enable ajax refreshing of the entire body + + IModel<Reflection> reflectionModel = projectModel.combineWith(authorModel, reflectionService::getReflection); + + add(new MultiLineLabel("reflection_text", reflectionModel.map(this::getReflectionText)) { + @Override + protected void onConfigure() { + super.onConfigure(); + setVisible(state != State.EDITING); + } + }); + + add(new MultiLineLabel("improvements_needed_supervisor_feedback", reflectionModel + .as(Reflection.ImprovementsNeeded.class) + .map(Reflection.ImprovementsNeeded::commentBySupervisor)) { + @Override + protected void onConfigure() { + super.onConfigure(); + setVisible(!getDefaultModelObjectAsString().isBlank()); + } + }); + + add(new RequestImprovementsForm("request_improvements_form", reflectionModel)); + add(new SupervisorEditReflectionForm("edit_reflection_form", reflectionModel)); + + add(new AjaxLink<>("show_request_improvements_form", reflectionModel) { + @Override + public void onClick(AjaxRequestTarget target) { + ReflectionModalBodyPanel.this.state = State.REQUESTING_IMPROVEMENTS; + target.add(ReflectionModalBodyPanel.this); + } + + @Override + protected void onConfigure() { + super.onConfigure(); + Reflection reflection = getModelObject(); + boolean canRequestImprovements = reflection instanceof Reflection.Submitted; + setVisible(state == State.VIEWING && canRequestImprovements && isEnabledInHierarchy()); + } + }); + + add(new AjaxLink<>("show_edit_reflection_form", reflectionModel) { + @Override + public void onClick(AjaxRequestTarget target) { + ReflectionModalBodyPanel.this.state = State.EDITING; + target.add(ReflectionModalBodyPanel.this); + } + + @Override + protected void onConfigure() { + super.onConfigure(); + Reflection reflection = getModelObject(); + boolean canEditReflection = reflection instanceof Reflection.Submitted || reflection instanceof Reflection.ImprovementsNeeded; + setVisible(state == State.VIEWING && canEditReflection && isEnabledInHierarchy()); + } + }); + } + + private String getReflectionText(Reflection reflection) { + if (reflection instanceof Reflection.Submitted submitted) { + return submitted.reflection(); + } else if (reflection instanceof Reflection.ImprovementsNeeded improvementsNeeded) { + return improvementsNeeded.oldReflection(); + } else { + return getString("reflection_not_submitted"); + } + } + + @Override + protected void onDetach() { + this.projectModel.detach(); + this.authorModel.detach(); + super.onDetach(); + } + + private class RequestImprovementsForm extends Form<Reflection> { + public RequestImprovementsForm(String id, IModel<Reflection> reflectionModel) { + super(id, reflectionModel); + + IModel<String> commentModel = new Model<>(); + + TextArea<String> comment = new TextArea<>("comment", commentModel); + comment.setRequired(true); + add(comment); + + add(new AjaxSubmitLink("submit") { + @Override + protected void onSubmit(AjaxRequestTarget target) { + super.onSubmit(target); + + reflectionService.requestNewReflection( + projectModel.getObject(), + authorModel.getObject(), + commentModel.getObject()); + + ReflectionModalBodyPanel.this.state = State.VIEWING; + target.add(ReflectionModalBodyPanel.this); + } + }); + add(new AjaxLink<>("cancel") { + @Override + public void onClick(AjaxRequestTarget target) { + ReflectionModalBodyPanel.this.state = State.VIEWING; + target.add(ReflectionModalBodyPanel.this); + } + }); + } + + @Override + protected void onConfigure() { + super.onConfigure(); + setVisible(state == State.REQUESTING_IMPROVEMENTS && getModelObject() instanceof Reflection.Submitted); + } + } + + private class SupervisorEditReflectionForm extends Form<Reflection> { + public SupervisorEditReflectionForm(String id, IModel<Reflection> reflectionModel) { + super(id, reflectionModel); + + IModel<String> reflectionTextModel = new Model<>(getReflectionText(reflectionModel.getObject())); + + TextArea<String> reflectionTextArea = new TextArea<>("reflection", reflectionTextModel); + reflectionTextArea.setRequired(true); + add(reflectionTextArea); + + add(new AjaxSubmitLink("submit") { + @Override + protected void onSubmit(AjaxRequestTarget target) { + reflectionService.submitReflection( + projectModel.getObject(), + authorModel.getObject(), + reflectionTextModel.getObject()); + + ReflectionModalBodyPanel.this.state = State.VIEWING; + target.add(ReflectionModalBodyPanel.this); + } + }); + add(new AjaxLink<>("cancel") { + @Override + public void onClick(AjaxRequestTarget target) { + ReflectionModalBodyPanel.this.state = State.VIEWING; + target.add(ReflectionModalBodyPanel.this); + } + }); + } + + @Override + protected void onConfigure() { + super.onConfigure(); + setVisible(state == State.EDITING); + } + } +} diff --git a/view/src/main/java/se/su/dsv/scipro/grading/ReflectionModalBodyPanel.utf8.properties b/view/src/main/java/se/su/dsv/scipro/grading/ReflectionModalBodyPanel.utf8.properties new file mode 100644 index 0000000000..cf818f1e92 --- /dev/null +++ b/view/src/main/java/se/su/dsv/scipro/grading/ReflectionModalBodyPanel.utf8.properties @@ -0,0 +1,10 @@ +improvements_requested=You've requested improvements to the submitted version. +request_improvements=Request improvements +comment=Comment +reflection_not_submitted=Reflection not submitted yet +request_improvements_text=If the submitted reflection does not meet the minimum requirements \ + you can request improvements from the student. The student will be notified and can submit a new reflection. \ + Use the comment field to provide feedback to the student. +edit_reflection=Edit reflection +reflection=Reflection +cancel=Cancel diff --git a/view/src/main/java/se/su/dsv/scipro/notifications/pages/NotificationLandingPage.java b/view/src/main/java/se/su/dsv/scipro/notifications/pages/NotificationLandingPage.java index b3b73a4bec..0fc12af49a 100644 --- a/view/src/main/java/se/su/dsv/scipro/notifications/pages/NotificationLandingPage.java +++ b/view/src/main/java/se/su/dsv/scipro/notifications/pages/NotificationLandingPage.java @@ -158,7 +158,7 @@ public class NotificationLandingPage extends WebPage { setResponsePage(RoughDraftApprovalDecisionPage.class, reviewerParameters); } break; - case REVIEWER_GRADING_INITIAL_ASSESSMENT_DONE, REVIEWER_GRADING_REPORT_SUBMITTED: + case REVIEWER_GRADING_INITIAL_ASSESSMENT_DONE, REVIEWER_GRADING_REPORT_SUBMITTED, REFLECTION_IMPROVEMENTS_SUBMITTED: if (project.isSupervisor(currentUser)) { setResponsePage(SupervisorGradingReportPage.class, pp); } diff --git a/view/src/main/java/se/su/dsv/scipro/project/panels/FinalStepsPanel.html b/view/src/main/java/se/su/dsv/scipro/project/panels/FinalStepsPanel.html index a50d6d7828..6cc7d5661d 100644 --- a/view/src/main/java/se/su/dsv/scipro/project/panels/FinalStepsPanel.html +++ b/view/src/main/java/se/su/dsv/scipro/project/panels/FinalStepsPanel.html @@ -38,6 +38,30 @@ <div wicket:id="current_thesis_file"></div> </div> </wicket:enclosure> + <wicket:enclosure child="old_reflection"> + <div class="alert alert-info"> + <h4 class="alert-heading"> + <wicket:message key="reflection_improvements_needed_heading"> + Reflection improvements needed + </wicket:message> + </h4> + <p> + <wicket:message key="reflection_improvements_needed"> + Your supervisor has requested that you improve and resubmit your reflection. + See their comments below about what changes are necessary. + </wicket:message> + </p> + <p wicket:id="supervisor_comment"> + [You need to reflect more on the methods you used.] + </p> + </div> + <h4> + <wicket:message key="old_reflection"> + Your previous reflection + </wicket:message> + </h4> + <p wicket:id="old_reflection"></p> + </wicket:enclosure> <div class="mb-3"> <label class="form-label" wicket:for="reflection"> <wicket:message key="reflection">[Reflection]</wicket:message> diff --git a/view/src/main/java/se/su/dsv/scipro/project/panels/FinalStepsPanel.java b/view/src/main/java/se/su/dsv/scipro/project/panels/FinalStepsPanel.java index 0283f15c13..95d9fb319a 100644 --- a/view/src/main/java/se/su/dsv/scipro/project/panels/FinalStepsPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/project/panels/FinalStepsPanel.java @@ -24,6 +24,7 @@ import se.su.dsv.scipro.grading.PublicationMetadata; import se.su.dsv.scipro.grading.PublicationMetadataFormComponentPanel; import se.su.dsv.scipro.grading.PublicationMetadataService; import se.su.dsv.scipro.project.Project; +import se.su.dsv.scipro.reflection.Reflection; import se.su.dsv.scipro.reflection.ReflectionService; import se.su.dsv.scipro.session.SciProSession; @@ -45,17 +46,20 @@ public class FinalStepsPanel extends GenericPanel<Project> { add(new FencedFeedbackPanel("feedback", this)); - IModel<String> reflection = LoadableDetachableModel.of(() -> - reflectionService.getSubmittedReflection(projectModel.getObject(), SciProSession.get().getUser())); - add(new MultiLineLabel("reflection", reflection) { + IModel<Reflection> currentReflection = LoadableDetachableModel.of(() -> + reflectionService.getReflection(projectModel.getObject(), SciProSession.get().getUser())); + IModel<String> reflectionText = currentReflection + .as(Reflection.Submitted.class) + .map(Reflection.Submitted::reflection); + add(new MultiLineLabel("reflection", reflectionText) { @Override protected void onConfigure() { super.onConfigure(); - setVisible(getDefaultModelObject() != null); + setVisible(!getDefaultModelObjectAsString().isBlank()); } }); - add(new FinalStepsForm("submit_reflection", projectModel)); + add(new FinalStepsForm("submit_reflection", projectModel, currentReflection)); } @Override @@ -67,22 +71,49 @@ public class FinalStepsPanel extends GenericPanel<Project> { private class FinalStepsForm extends Form<Project> { private final FinalThesisUploadComponent thesisFileUpload; private final IModel<PublicationMetadata> publicationMetadataModel; + private final IModel<Reflection> currentReflection; private IModel<String> reflectionModel; private IModel<PublishingConsentService.Level> levelModel; - public FinalStepsForm(String id, IModel<Project> projectModel) { + public FinalStepsForm(String id, IModel<Project> projectModel, IModel<Reflection> currentReflection) { super(id, projectModel); + this.currentReflection = currentReflection; + + IModel<Reflection.ImprovementsNeeded> improvementsNeeded = this.currentReflection.as(Reflection.ImprovementsNeeded.class); + + IModel<String> oldReflection = improvementsNeeded.map(Reflection.ImprovementsNeeded::oldReflection); + add(new MultiLineLabel("old_reflection", oldReflection) { + @Override + protected void onConfigure() { + super.onConfigure(); + setVisibilityAllowed(!getDefaultModelObjectAsString().isBlank()); + } + }); + + add(new MultiLineLabel("supervisor_comment", improvementsNeeded.map(Reflection.ImprovementsNeeded::commentBySupervisor)) { + @Override + protected void onConfigure() { + super.onConfigure(); + setVisibilityAllowed(!getDefaultModelObjectAsString().isBlank()); + } + }); reflectionModel = new Model<>(); - TextArea<String> reflectionTextArea = new TextArea<>("reflection", reflectionModel); + TextArea<String> reflectionTextArea = new TextArea<>("reflection", reflectionModel) { + @Override + protected void onConfigure() { + super.onConfigure(); + setVisible(FinalStepsForm.this.currentReflection.getObject().isSubmittable()); + } + }; reflectionTextArea.setRequired(true); add(reflectionTextArea); IModel<PublishingConsentService.PublishingConsent> publishingConsent = LoadableDetachableModel.of(() -> publishingConsentService.getPublishingConsent(getModelObject(), SciProSession.get().getUser())); - levelModel = new Model<>(); + levelModel = new Model<>(publishingConsent.getObject().selected()); FormComponent<PublishingConsentService.Level> publishingConsentLevel = new BootstrapRadioChoice<>( "publishingConsentLevel", levelModel, @@ -111,7 +142,13 @@ public class FinalStepsPanel extends GenericPanel<Project> { currentThesisFile.add(new OppositeVisibility(thesisFileUpload)); add(currentThesisFile); publicationMetadataModel = LoadableDetachableModel.of(() -> publicationMetadataService.getByProject(getModelObject())); - WebMarkupContainer publicationMetadata = new WebMarkupContainer("publication_metadata"); + WebMarkupContainer publicationMetadata = new WebMarkupContainer("publication_metadata") { + @Override + protected void onConfigure() { + super.onConfigure(); + setEnabled(finalThesisService.isUploadAllowed(FinalStepsPanel.FinalStepsForm.this.getModelObject())); + } + }; add(publicationMetadata); publicationMetadata.add(new PublicationMetadataFormComponentPanel("publication_metadata_components", publicationMetadataModel)); } @@ -119,7 +156,7 @@ public class FinalStepsPanel extends GenericPanel<Project> { @Override protected void onConfigure() { super.onConfigure(); - setVisibilityAllowed(reflectionService.hasToFillInReflection(getModelObject(), SciProSession.get().getUser())); + setVisibilityAllowed(currentReflection.getObject().isSubmittable()); } @Override @@ -131,6 +168,7 @@ public class FinalStepsPanel extends GenericPanel<Project> { new WicketProjectFileUpload(finalThesisUpload.fileUpload(), FinalStepsPanel.this.getModelObject()), finalThesisUpload.englishTitle(), finalThesisUpload.swedishTitle()); + success(getString("final_thesis_uploaded")); } reflectionService.submitReflection(getModelObject(), SciProSession.get().getUser(), reflectionModel.getObject()); if (levelModel.getObject() != null) { diff --git a/view/src/main/java/se/su/dsv/scipro/project/panels/FinalStepsPanel.utf8.properties b/view/src/main/java/se/su/dsv/scipro/project/panels/FinalStepsPanel.utf8.properties index a0f79eb71a..f21ad3bd37 100644 --- a/view/src/main/java/se/su/dsv/scipro/project/panels/FinalStepsPanel.utf8.properties +++ b/view/src/main/java/se/su/dsv/scipro/project/panels/FinalStepsPanel.utf8.properties @@ -9,3 +9,8 @@ current_final_thesis=Final thesis publication_metadata_why=Please provide the following metadata. englishTitle=English title swedishTitle=Swedish title +reflection_improvements_needed_heading=Reflection improvements needed +reflection_improvements_needed=Your supervisor has requested that you improve and resubmit your reflection. \ +See their comments below about what changes are necessary. +old_reflection=Your previous reflection +final_thesis_uploaded=Final thesis uploaded diff --git a/view/src/main/java/se/su/dsv/scipro/wicket-package.utf8.properties b/view/src/main/java/se/su/dsv/scipro/wicket-package.utf8.properties index 74aa56c867..97ea5d57c3 100644 --- a/view/src/main/java/se/su/dsv/scipro/wicket-package.utf8.properties +++ b/view/src/main/java/se/su/dsv/scipro/wicket-package.utf8.properties @@ -65,6 +65,8 @@ ProjectEvent.FIRST_MEETING = First meeting created. (with date, time, place/room ProjectEvent.OPPOSITION_FAILED = An author fails their opposition. ProjectEvent.PARTICIPATION_APPROVED = An author's active participation is approved. ProjectEvent.PARTICIPATION_FAILED = An author fails their active participation. +ProjectEvent.REFLECTION_IMPROVEMENTS_REQUESTED = Reflection improvements requested. +ProjectEvent.REFLECTION_IMPROVEMENTS_SUBMITTED = Reflection improvements submitted. ProjectForumEvent.NEW_FORUM_POST = Forum thread created. ProjectForumEvent.NEW_FORUM_POST_COMMENT = Comment posted in forum thread. diff --git a/view/src/test/java/se/su/dsv/scipro/SciProTest.java b/view/src/test/java/se/su/dsv/scipro/SciProTest.java index e712c36790..2b469c39ec 100755 --- a/view/src/test/java/se/su/dsv/scipro/SciProTest.java +++ b/view/src/test/java/se/su/dsv/scipro/SciProTest.java @@ -139,6 +139,7 @@ import java.util.*; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) public abstract class SciProTest { @@ -390,6 +391,8 @@ public abstract class SciProTest { publicationMetadata.setProject(answer.getArgument(0)); return publicationMetadata; }); + lenient().when(publishingConsentService.getPublishingConsent(any(), any())) + .thenReturn(new PublishingConsentService.PublishingConsent(null, List.of())); } @BeforeEach diff --git a/war/src/main/java/se/su/dsv/scipro/war/WicketConfiguration.java b/war/src/main/java/se/su/dsv/scipro/war/WicketConfiguration.java index f31d390c16..bb255d4915 100644 --- a/war/src/main/java/se/su/dsv/scipro/war/WicketConfiguration.java +++ b/war/src/main/java/se/su/dsv/scipro/war/WicketConfiguration.java @@ -10,6 +10,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import se.su.dsv.scipro.SciProApplication; import se.su.dsv.scipro.crosscutting.ForwardPhase2Feedback; +import se.su.dsv.scipro.crosscutting.NotifyFailedReflection; import se.su.dsv.scipro.crosscutting.ReviewerAssignedNotifications; import se.su.dsv.scipro.crosscutting.ReviewerSupportMailer; import se.su.dsv.scipro.crosscutting.ReviewingNotifications; @@ -87,4 +88,13 @@ public class WicketConfiguration { return new ReviewerAssignedNotifications(roughDraftApprovalService, finalSeminarApprovalService, notificationController, eventBus); } + + // Not sure why this dependency lives in the view module + @Bean + public NotifyFailedReflection notifyFailedReflection( + EventBus eventBus, + NotificationController notificationController) + { + return new NotifyFailedReflection(notificationController, eventBus); + } }