From d008bec815d4bbb611d5db700dcc2a97ca8aca0f Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Wed, 5 Mar 2025 10:05:37 +0100 Subject: [PATCH 1/2] Allow supervisors to request improvements from final seminar opponents (#78) Fixes #36 ## How to test 1. (Optional) Log in as `sid@example.com` and submit an opposition report 1. Go to the tab "Opposition & Active participation" 2. Open the opposition "Putting the it in supervising" on the right 3. Submit the report 2. Log in as `eric@example.com` 3. Go to the final seminar in the "Putting the it in supervising" project (or follow the notification if you did step 1) 4. Request improvements 5. Log in as `sid@example.com` 6. Follow the notification to submit the new opposition report Click the "Re-run all jobs" button (top right) on https://gitea.dsv.su.se/DMC/scipro/actions/runs/457 to reset the database. It takes a few minutes. Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/78 Reviewed-by: Nico Athanassiadis <nico@dsv.su.se> Co-authored-by: Andreas Svanberg <andreass@dsv.su.se> Co-committed-by: Andreas Svanberg <andreass@dsv.su.se> --- .../java/se/su/dsv/scipro/CoreConfig.java | 36 ++- .../finalseminar/AbstractOppositionEvent.java | 14 ++ ...fulfilledOppositionImprovementsWorker.java | 32 +++ .../finalseminar/FinalSeminarOpposition.java | 25 +++ .../FinalSeminarOppositionGrading.java | 7 + .../FinalSeminarOppositionRepo.java | 2 + .../FinalSeminarOppositionRepoImpl.java | 9 + .../FinalSeminarOppositionService.java | 16 +- .../FinalSeminarOppositionServiceImpl.java | 163 +++++++++++++- .../finalseminar/FinalSeminarServiceImpl.java | 28 ++- .../finalseminar/FinalSeminarSettings.java | 14 ++ .../dsv/scipro/finalseminar/Opposition.java | 10 + .../finalseminar/OppositionCriteria.java | 7 + ...itionReportImprovementsRequestedEvent.java | 9 + .../finalseminar/PointNotValidException.java | 27 +++ .../se/su/dsv/scipro/misc/DaysService.java | 10 + .../scipro/notifications/Notifications.java | 37 ++++ .../dataobject/SeminarEvent.java | 3 + .../notifications/notifications.properties | 8 + .../scipro/report/GradingReportService.java | 3 - .../report/GradingReportServiceImpl.java | 48 +++- .../report/OppositionReportService.java | 8 + .../report/OppositionReportServiceImpl.java | 40 +++- .../OppositionReportSubmittedEvent.java | 9 + .../su/dsv/scipro/report/ReportService.java | 13 -- .../dsv/scipro/report/ReportServiceImpl.java | 52 ----- ...eminar_opposition_request_improvements.sql | 3 + ...nar_work_days_to_fix_opposition_report.sql | 2 + ...rOppositionServiceImplIntegrationTest.java | 80 ++++++- ...inalSeminarServiceImplIntegrationTest.java | 45 +++- ...adingReportServiceImplIntegrationTest.java | 25 ++- .../se/su/dsv/scipro/test/SpringTest.java | 23 ++ .../dsv/scipro/testdata/DataInitializer.java | 74 ++++++- .../AdminFinalSeminarSettingsPage.html | 7 + .../AdminFinalSeminarSettingsPage.java | 11 + .../finalseminar/DownloadPdfReportPanel.java | 7 +- .../finalseminar/OppositionReportPage.html | 35 +-- .../finalseminar/OppositionReportPage.java | 53 +++-- .../finalseminar/SeminarOppositionPanel.html | 45 +++- .../finalseminar/SeminarOppositionPanel.java | 207 +++++++++++++----- .../SeminarOppositionPanel.properties | 13 +- .../SeminarOppositionReportPanel.html | 6 +- .../SeminarOppositionReportPanel.java | 13 ++ .../scipro/grading/FillOutReportPanel.html | 40 ++-- .../scipro/grading/FillOutReportPanel.java | 16 +- .../pages/NotificationLandingPage.java | 15 ++ .../dsv/scipro/wicket-package.utf8.properties | 3 + .../java/se/su/dsv/scipro/SciProTest.java | 3 - .../OppositionReportPageTest.java | 22 +- .../SeminarOppositionPanelTest.java | 9 + .../scipro/finalseminar/SeminarPanelTest.java | 8 + .../grading/FillOutReportPanelTest.java | 4 +- .../se/su/dsv/scipro/war/WorkerConfig.java | 17 ++ 53 files changed, 1153 insertions(+), 263 deletions(-) create mode 100644 core/src/main/java/se/su/dsv/scipro/finalseminar/ExpireUnfulfilledOppositionImprovementsWorker.java create mode 100644 core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionGrading.java create mode 100644 core/src/main/java/se/su/dsv/scipro/finalseminar/Opposition.java create mode 100644 core/src/main/java/se/su/dsv/scipro/finalseminar/OppositionCriteria.java create mode 100644 core/src/main/java/se/su/dsv/scipro/finalseminar/OppositionReportImprovementsRequestedEvent.java create mode 100644 core/src/main/java/se/su/dsv/scipro/finalseminar/PointNotValidException.java create mode 100644 core/src/main/java/se/su/dsv/scipro/report/OppositionReportSubmittedEvent.java delete mode 100644 core/src/main/java/se/su/dsv/scipro/report/ReportService.java delete mode 100644 core/src/main/java/se/su/dsv/scipro/report/ReportServiceImpl.java create mode 100644 core/src/main/resources/db/migration/V5__final_seminar_opposition_request_improvements.sql create mode 100644 core/src/main/resources/db/migration/V6__final_seminar_work_days_to_fix_opposition_report.sql 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 19b8f04a13..352c43456a 100644 --- a/core/src/main/java/se/su/dsv/scipro/CoreConfig.java +++ b/core/src/main/java/se/su/dsv/scipro/CoreConfig.java @@ -31,6 +31,7 @@ import se.su.dsv.scipro.finalseminar.AuthorRepository; import se.su.dsv.scipro.finalseminar.FinalSeminarActiveParticipationRepository; import se.su.dsv.scipro.finalseminar.FinalSeminarActiveParticipationServiceImpl; import se.su.dsv.scipro.finalseminar.FinalSeminarCreationSubscribers; +import se.su.dsv.scipro.finalseminar.FinalSeminarOppositionGrading; import se.su.dsv.scipro.finalseminar.FinalSeminarOppositionRepo; import se.su.dsv.scipro.finalseminar.FinalSeminarOppositionServiceImpl; import se.su.dsv.scipro.finalseminar.FinalSeminarRepository; @@ -153,8 +154,8 @@ import se.su.dsv.scipro.report.GradingReportServiceImpl; import se.su.dsv.scipro.report.GradingReportTemplateRepo; import se.su.dsv.scipro.report.GradingReportTemplateRepoImpl; import se.su.dsv.scipro.report.OppositionReportRepo; +import se.su.dsv.scipro.report.OppositionReportService; import se.su.dsv.scipro.report.OppositionReportServiceImpl; -import se.su.dsv.scipro.report.ReportServiceImpl; import se.su.dsv.scipro.report.SupervisorGradingReportRepository; import se.su.dsv.scipro.reviewing.DecisionRepository; import se.su.dsv.scipro.reviewing.FinalSeminarApprovalService; @@ -430,8 +431,26 @@ public class CoreConfig { } @Bean - public FinalSeminarOppositionServiceImpl finalSeminarOppositionService(Provider<EntityManager> em) { - return new FinalSeminarOppositionServiceImpl(em); + public FinalSeminarOppositionServiceImpl finalSeminarOppositionService( + Provider<EntityManager> em, + FinalSeminarOppositionGrading finalSeminarOppositionGrading, + EventBus eventBus, + FinalSeminarOppositionRepo finalSeminarOppositionRepository, + Clock clock, + FinalSeminarSettingsService finalSeminarSettingsService, + DaysService daysService, + OppositionReportService oppositionReportService + ) { + return new FinalSeminarOppositionServiceImpl( + em, + finalSeminarOppositionGrading, + eventBus, + finalSeminarOppositionRepository, + clock, + finalSeminarSettingsService, + daysService, + oppositionReportService + ); } @Bean @@ -669,13 +688,15 @@ public class CoreConfig { OppositionReportRepo oppositionReportRepository, GradingReportTemplateRepo gradingReportTemplateRepository, FileService fileService, - FinalSeminarOppositionRepo finalSeminarOppositionRepository + FinalSeminarOppositionRepo finalSeminarOppositionRepository, + EventBus eventBus ) { return new OppositionReportServiceImpl( oppositionReportRepository, gradingReportTemplateRepository, fileService, - finalSeminarOppositionRepository + finalSeminarOppositionRepository, + eventBus ); } @@ -855,11 +876,6 @@ public class CoreConfig { ); } - @Bean - public ReportServiceImpl reportService(Provider<EntityManager> em, FileService fileService) { - return new ReportServiceImpl(em, fileService); - } - @Bean public ResearchAreaServiceImpl researchAreaService(Provider<EntityManager> em) { return new ResearchAreaServiceImpl(em); diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/AbstractOppositionEvent.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/AbstractOppositionEvent.java index e60a7b2bc7..2143479147 100644 --- a/core/src/main/java/se/su/dsv/scipro/finalseminar/AbstractOppositionEvent.java +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/AbstractOppositionEvent.java @@ -1,5 +1,6 @@ package se.su.dsv.scipro.finalseminar; +import java.util.Objects; import se.su.dsv.scipro.project.Project; import se.su.dsv.scipro.system.User; @@ -26,4 +27,17 @@ class AbstractOppositionEvent { public FinalSeminarOpposition getOpposition() { return opposition; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + AbstractOppositionEvent that = (AbstractOppositionEvent) o; + return Objects.equals(opposition, that.opposition); + } + + @Override + public int hashCode() { + return Objects.hashCode(opposition); + } } diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/ExpireUnfulfilledOppositionImprovementsWorker.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/ExpireUnfulfilledOppositionImprovementsWorker.java new file mode 100644 index 0000000000..9a92f0c18b --- /dev/null +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/ExpireUnfulfilledOppositionImprovementsWorker.java @@ -0,0 +1,32 @@ +package se.su.dsv.scipro.finalseminar; + +import jakarta.inject.Inject; +import jakarta.inject.Provider; +import java.util.concurrent.TimeUnit; +import se.su.dsv.scipro.workerthreads.AbstractWorker; +import se.su.dsv.scipro.workerthreads.Scheduler; + +public class ExpireUnfulfilledOppositionImprovementsWorker extends AbstractWorker { + + private final FinalSeminarOppositionServiceImpl oppositionService; + + public ExpireUnfulfilledOppositionImprovementsWorker(FinalSeminarOppositionServiceImpl oppositionService) { + this.oppositionService = oppositionService; + } + + @Override + protected void doWork() { + oppositionService.expireUnfulfilledOppositionImprovements(); + } + + public static class Schedule { + + @Inject + public Schedule(Scheduler scheduler, Provider<ExpireUnfulfilledOppositionImprovementsWorker> worker) { + scheduler + .schedule("Fail opponents that have not submitted improvements") + .runBy(worker) + .every(1, TimeUnit.HOURS); + } + } +} diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOpposition.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOpposition.java index 9825e981a8..8a1533fedd 100755 --- a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOpposition.java +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOpposition.java @@ -8,6 +8,7 @@ import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; import jakarta.persistence.OneToOne; import jakarta.persistence.Table; +import java.time.Instant; import java.util.Objects; import se.su.dsv.scipro.file.FileReference; import se.su.dsv.scipro.project.Project; @@ -31,6 +32,14 @@ public class FinalSeminarOpposition extends FinalSeminarParticipation { @Column(name = "feedback", length = FEEDBACK_LENGTH) private String feedback; + @Basic + @Column(name = "improvements_requested_at") + private Instant improvementsRequestedAt; + + @Basic + @Column(name = "supervisor_improvements_comment") + private String supervisorCommentForImprovements; + // ---------------------------------------------------------------------------------- // JPA-mappings of foreign keys in this table (final_seminar_opposition) referencing // other tables. @@ -92,6 +101,22 @@ public class FinalSeminarOpposition extends FinalSeminarParticipation { this.oppositionReport = oppositionReport; } + public Instant getImprovementsRequestedAt() { + return improvementsRequestedAt; + } + + public void setImprovementsRequestedAt(Instant improvementsRequestedAt) { + this.improvementsRequestedAt = improvementsRequestedAt; + } + + public String getSupervisorCommentForImprovements() { + return supervisorCommentForImprovements; + } + + public void setSupervisorCommentForImprovements(String supervisorCommentsForImprovements) { + this.supervisorCommentForImprovements = supervisorCommentsForImprovements; + } + // ---------------------------------------------------------------------------------- // Methods Common To All Objects // ---------------------------------------------------------------------------------- diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionGrading.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionGrading.java new file mode 100644 index 0000000000..2962d1b25e --- /dev/null +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionGrading.java @@ -0,0 +1,7 @@ +package se.su.dsv.scipro.finalseminar; + +import java.util.List; + +public interface FinalSeminarOppositionGrading { + OppositionCriteria oppositionCriteria(FinalSeminarOpposition opposition); +} diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionRepo.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionRepo.java index e636597075..b6e7a348c5 100755 --- a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionRepo.java +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionRepo.java @@ -11,4 +11,6 @@ import se.su.dsv.scipro.system.User; public interface FinalSeminarOppositionRepo extends JpaRepository<FinalSeminarOpposition, Long>, QueryDslPredicateExecutor<FinalSeminarOpposition> { List<FinalSeminarOpposition> findByOpposingUserAndType(User user, ProjectType projectType); + + Collection<FinalSeminarOpposition> findUnfulfilledOppositionImprovements(); } diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionRepoImpl.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionRepoImpl.java index 00ad589634..a48da4e198 100644 --- a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionRepoImpl.java +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionRepoImpl.java @@ -24,4 +24,13 @@ public class FinalSeminarOppositionRepoImpl .where(QFinalSeminarOpposition.finalSeminarOpposition.project.projectType.eq(projectType)) .fetch(); } + + @Override + public Collection<FinalSeminarOpposition> findUnfulfilledOppositionImprovements() { + return createQuery() + .innerJoin(QFinalSeminarOpposition.finalSeminarOpposition.oppositionReport) + .where(QFinalSeminarOpposition.finalSeminarOpposition.improvementsRequestedAt.isNotNull()) + .where(QFinalSeminarOpposition.finalSeminarOpposition.oppositionReport.submitted.isFalse()) + .fetch(); + } } diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionService.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionService.java index 60eeacc1a9..e7d7916583 100755 --- a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionService.java +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionService.java @@ -1,8 +1,18 @@ package se.su.dsv.scipro.finalseminar; +import java.time.Instant; import se.su.dsv.scipro.system.GenericService; -public interface FinalSeminarOppositionService extends GenericService<FinalSeminarOpposition, Long> { - @Override - void delete(Long aLong); +public interface FinalSeminarOppositionService { + OppositionCriteria getCriteriaForOpposition(FinalSeminarOpposition opposition); + + FinalSeminarOpposition gradeOpponent(FinalSeminarOpposition opposition, int points, String feedback) + throws PointNotValidException; + + /** + * @return the deadline by which the improvements must have been submitted + */ + Instant requestImprovements(FinalSeminarOpposition opposition, String supervisorComment); + + Opposition getOpposition(long id); } diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionServiceImpl.java index 27550bb3ac..1de2066727 100755 --- a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionServiceImpl.java +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionServiceImpl.java @@ -1,16 +1,177 @@ package se.su.dsv.scipro.finalseminar; +import com.google.common.eventbus.EventBus; import jakarta.inject.Inject; import jakarta.inject.Provider; import jakarta.persistence.EntityManager; +import jakarta.transaction.Transactional; +import java.time.Clock; +import java.time.Instant; +import java.util.Collection; +import java.util.List; +import java.util.Optional; +import se.su.dsv.scipro.misc.DaysService; +import se.su.dsv.scipro.report.OppositionReport; +import se.su.dsv.scipro.report.OppositionReportService; import se.su.dsv.scipro.system.AbstractServiceImpl; public class FinalSeminarOppositionServiceImpl extends AbstractServiceImpl<FinalSeminarOpposition, Long> implements FinalSeminarOppositionService { + private final FinalSeminarOppositionGrading finalSeminarOppositionGrading; + private final EventBus eventBus; + private final FinalSeminarOppositionRepo finalSeminarOppositionRepository; + private final Clock clock; + private final FinalSeminarSettingsService finalSeminarSettingsService; + private final DaysService daysService; + private final OppositionReportService oppositionReportService; + @Inject - public FinalSeminarOppositionServiceImpl(Provider<EntityManager> em) { + public FinalSeminarOppositionServiceImpl( + Provider<EntityManager> em, + FinalSeminarOppositionGrading finalSeminarOppositionGrading, + EventBus eventBus, + FinalSeminarOppositionRepo finalSeminarOppositionRepository, + Clock clock, + FinalSeminarSettingsService finalSeminarSettingsService, + DaysService daysService, + OppositionReportService oppositionReportService + ) { super(em, FinalSeminarOpposition.class, QFinalSeminarOpposition.finalSeminarOpposition); + this.finalSeminarOppositionGrading = finalSeminarOppositionGrading; + this.eventBus = eventBus; + this.finalSeminarOppositionRepository = finalSeminarOppositionRepository; + this.clock = clock; + this.finalSeminarSettingsService = finalSeminarSettingsService; + this.daysService = daysService; + this.oppositionReportService = oppositionReportService; + } + + @Override + public OppositionCriteria getCriteriaForOpposition(FinalSeminarOpposition opposition) { + return finalSeminarOppositionGrading.oppositionCriteria(opposition); + } + + @Override + @Transactional + public FinalSeminarOpposition gradeOpponent(FinalSeminarOpposition opposition, int points, String feedback) + throws PointNotValidException { + OppositionCriteria criteriaForOpposition = getCriteriaForOpposition(opposition); + boolean validPoints = criteriaForOpposition + .pointsAvailable() + .stream() + .anyMatch(criterion -> criterion.value() == points); + if (!validPoints) { + throw new PointNotValidException(points, List.of(0, 1)); + } + + FinalSeminarGrade notApproved = criteriaForOpposition.pointsToPass() > points + ? FinalSeminarGrade.NOT_APPROVED + : FinalSeminarGrade.APPROVED; + return internalGradeOpponent(opposition, points, feedback, notApproved); + } + + private FinalSeminarOpposition internalGradeOpponent( + FinalSeminarOpposition opposition, + int points, + String feedback, + FinalSeminarGrade grade + ) { + opposition.setGrade(grade); + opposition.setPoints(points); + opposition.setFeedback(feedback); + FinalSeminarOpposition assessedOpposition = finalSeminarOppositionRepository.save(opposition); + + if (grade == FinalSeminarGrade.NOT_APPROVED) { + eventBus.post(new OppositionFailedEvent(assessedOpposition)); + } else { + eventBus.post(new OppositionApprovedEvent(assessedOpposition)); + } + + return assessedOpposition; + } + + @Override + @Transactional + public Instant requestImprovements(FinalSeminarOpposition opposition, String supervisorComment) { + OppositionReport oppositionReport = opposition.getOppositionReport(); + if (oppositionReport == null) { + throw new IllegalStateException("There is no opposition report submitted"); + } + + FinalSeminarSettings finalSeminarSettings = finalSeminarSettingsService.getInstance(); + + Instant now = clock.instant(); + Instant deadline = daysService.workDaysAfter( + now, + finalSeminarSettings.getWorkDaysToFixRequestedImprovementsToOppositionReport() + ); + + oppositionReport.setSubmitted(false); + opposition.setImprovementsRequestedAt(now); + opposition.setSupervisorCommentForImprovements(supervisorComment); + + eventBus.post(new OppositionReportImprovementsRequestedEvent(opposition, supervisorComment, deadline)); + + return deadline; + } + + @Override + public Opposition getOpposition(long id) { + FinalSeminarOpposition finalSeminarOpposition = finalSeminarOppositionRepository.findOne(id); + if (finalSeminarOpposition == null) { + return null; + } + OppositionReport report = oppositionReportService.findOrCreateReport(finalSeminarOpposition); + Optional<Opposition.ImprovementsNeeded> improvements = getImprovementsNeeded(finalSeminarOpposition); + return new Opposition(finalSeminarOpposition.getUser(), report, improvements); + } + + private Optional<Opposition.ImprovementsNeeded> getImprovementsNeeded( + FinalSeminarOpposition finalSeminarOpposition + ) { + if (finalSeminarOpposition.getSupervisorCommentForImprovements() != null) { + return Optional.of( + new Opposition.ImprovementsNeeded( + finalSeminarOpposition.getSupervisorCommentForImprovements(), + finalSeminarOpposition.getImprovementsRequestedAt() + ) + ); + } else { + return Optional.empty(); + } + } + + void expireUnfulfilledOppositionImprovements() { + Collection<FinalSeminarOpposition> unfulfilledOppositions = + finalSeminarOppositionRepository.findUnfulfilledOppositionImprovements(); + + Instant now = clock.instant(); + int workDaysToFixRequestedImprovementsToOppositionReport = finalSeminarSettingsService + .getInstance() + .getWorkDaysToFixRequestedImprovementsToOppositionReport(); + for (FinalSeminarOpposition unfulfilledOpposition : unfulfilledOppositions) { + Instant deadline = daysService.workDaysAfter( + unfulfilledOpposition.getImprovementsRequestedAt(), + workDaysToFixRequestedImprovementsToOppositionReport + ); + if (now.isAfter(deadline)) { + internalGradeOpponent( + unfulfilledOpposition, + 0, + unfulfilledOpposition.getSupervisorCommentForImprovements(), + FinalSeminarGrade.NOT_APPROVED + ); + + OppositionReport oppositionReport = unfulfilledOpposition.getOppositionReport(); + if (oppositionReport != null) { + // Lock the report so it's not possible to submit it again + oppositionReport.setSubmitted(true); + } + + finalSeminarOppositionRepository.save(unfulfilledOpposition); + } + } } } diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarServiceImpl.java index 0af91dca55..02892489a0 100755 --- a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarServiceImpl.java +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarServiceImpl.java @@ -2,7 +2,10 @@ package se.su.dsv.scipro.finalseminar; import com.google.common.eventbus.EventBus; import com.querydsl.core.BooleanBuilder; +import com.querydsl.core.types.SubQueryExpressionImpl; import com.querydsl.core.types.dsl.BooleanExpression; +import com.querydsl.core.types.dsl.Expressions; +import com.querydsl.jpa.JPAExpressions; import jakarta.inject.Inject; import jakarta.inject.Provider; import jakarta.persistence.EntityManager; @@ -542,19 +545,22 @@ public class FinalSeminarServiceImpl extends AbstractServiceImpl<FinalSeminar, L private BooleanExpression unfinishedSeminars(Date after, Date before) { QFinalSeminar seminar = QFinalSeminar.finalSeminar; - if (after == null && before == null) { - return seminar.oppositions + QFinalSeminarOpposition opposition = QFinalSeminarOpposition.finalSeminarOpposition; + BooleanExpression ungradedParticipant = Expressions.anyOf( + seminar.oppositions .any() - .grade.isNull() - .or(seminar.activeParticipations.any().grade.isNull().or(seminar.respondents.any().grade.isNull())); + .id.in( + JPAExpressions.select(opposition.id) + .from(opposition) + .where(opposition.grade.isNull().and(opposition.improvementsRequestedAt.isNull())) + ), + seminar.activeParticipations.any().grade.isNull(), + seminar.respondents.any().grade.isNull() + ); + if (after == null && before == null) { + return ungradedParticipant; } else { - return seminar.startDate - .between(after, before) - .andAnyOf( - seminar.oppositions.any().grade.isNull(), - seminar.activeParticipations.any().grade.isNull(), - seminar.respondents.any().grade.isNull() - ); + return seminar.startDate.between(after, before).and(ungradedParticipant); } } diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarSettings.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarSettings.java index 6468b01d85..012c9f4c38 100644 --- a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarSettings.java +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarSettings.java @@ -34,6 +34,9 @@ public class FinalSeminarSettings extends DomainObject { @Column(name = "days_ahead_to_upload_thesis", nullable = false) private int daysAheadToUploadThesis = DEFAULT_DAYS_AHEAD_TO_UPLOAD_THESIS; + @Column(name = "work_days_to_fix_requested_improvements_to_opposition_report", nullable = false) + private int workDaysToFixRequestedImprovementsToOppositionReport = 10; + @Column(name = "thesis_must_be_pdf", nullable = false) private boolean thesisMustBePDF = false; @@ -113,6 +116,17 @@ public class FinalSeminarSettings extends DomainObject { this.oppositionPriorityDays = oppositionPriorityDays; } + public int getWorkDaysToFixRequestedImprovementsToOppositionReport() { + return workDaysToFixRequestedImprovementsToOppositionReport; + } + + public void setWorkDaysToFixRequestedImprovementsToOppositionReport( + int workDaysToFixRequestedImprovementsToOppositionReport + ) { + this.workDaysToFixRequestedImprovementsToOppositionReport = + workDaysToFixRequestedImprovementsToOppositionReport; + } + @Override public String toString() { return ( diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/Opposition.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/Opposition.java new file mode 100644 index 0000000000..5ff6a71e5b --- /dev/null +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/Opposition.java @@ -0,0 +1,10 @@ +package se.su.dsv.scipro.finalseminar; + +import java.time.Instant; +import java.util.Optional; +import se.su.dsv.scipro.report.OppositionReport; +import se.su.dsv.scipro.system.User; + +public record Opposition(User user, OppositionReport report, Optional<ImprovementsNeeded> improvementsNeeded) { + record ImprovementsNeeded(String comment, Instant deadline) {} +} diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/OppositionCriteria.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/OppositionCriteria.java new file mode 100644 index 0000000000..96be8aa6a1 --- /dev/null +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/OppositionCriteria.java @@ -0,0 +1,7 @@ +package se.su.dsv.scipro.finalseminar; + +import java.util.List; + +public record OppositionCriteria(int pointsToPass, List<Point> pointsAvailable) { + public record Point(int value, String requirement) {} +} diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/OppositionReportImprovementsRequestedEvent.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/OppositionReportImprovementsRequestedEvent.java new file mode 100644 index 0000000000..0c063e49cf --- /dev/null +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/OppositionReportImprovementsRequestedEvent.java @@ -0,0 +1,9 @@ +package se.su.dsv.scipro.finalseminar; + +import java.time.Instant; + +public record OppositionReportImprovementsRequestedEvent( + FinalSeminarOpposition opposition, + String supervisorComment, + Instant deadline +) {} diff --git a/core/src/main/java/se/su/dsv/scipro/finalseminar/PointNotValidException.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/PointNotValidException.java new file mode 100644 index 0000000000..82e92fc68f --- /dev/null +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/PointNotValidException.java @@ -0,0 +1,27 @@ +package se.su.dsv.scipro.finalseminar; + +import java.util.List; + +public class PointNotValidException extends Exception { + + private final int givenValue; + private final List<Integer> acceptableValues; + + public PointNotValidException(int givenValue, List<Integer> acceptableValues) { + this.givenValue = givenValue; + this.acceptableValues = acceptableValues; + } + + public int givenValue() { + return givenValue; + } + + public List<Integer> acceptableValues() { + return acceptableValues; + } + + @Override + public String toString() { + return "PointNotValidException{" + "givenValue=" + givenValue + ", acceptableValues=" + acceptableValues + '}'; + } +} diff --git a/core/src/main/java/se/su/dsv/scipro/misc/DaysService.java b/core/src/main/java/se/su/dsv/scipro/misc/DaysService.java index d52c0e467e..865d495bf7 100644 --- a/core/src/main/java/se/su/dsv/scipro/misc/DaysService.java +++ b/core/src/main/java/se/su/dsv/scipro/misc/DaysService.java @@ -1,6 +1,9 @@ package se.su.dsv.scipro.misc; +import java.time.Instant; import java.time.LocalDate; +import java.time.ZoneId; +import java.time.ZonedDateTime; import java.util.Date; public interface DaysService { @@ -9,4 +12,11 @@ public interface DaysService { int workDaysBetween(Date startDate, Date endDate); LocalDate workDaysAhead(LocalDate date, int days); LocalDate workDaysAfter(LocalDate date, int days); + + default Instant workDaysAfter(Instant instant, int days) { + ZonedDateTime zonedDateTime = instant.atZone(ZoneId.systemDefault()); + LocalDate localDate = zonedDateTime.toLocalDate(); + LocalDate newDate = workDaysAfter(localDate, days); + return newDate.atTime(zonedDateTime.toLocalTime()).atZone(ZoneId.systemDefault()).toInstant(); + } } diff --git a/core/src/main/java/se/su/dsv/scipro/notifications/Notifications.java b/core/src/main/java/se/su/dsv/scipro/notifications/Notifications.java index 00cbe6b0da..55dd6be255 100644 --- a/core/src/main/java/se/su/dsv/scipro/notifications/Notifications.java +++ b/core/src/main/java/se/su/dsv/scipro/notifications/Notifications.java @@ -11,7 +11,9 @@ import se.su.dsv.scipro.finalseminar.FinalSeminarCreatedEvent; import se.su.dsv.scipro.finalseminar.FinalSeminarDeletedEvent; import se.su.dsv.scipro.finalseminar.FinalSeminarThesisDeletedEvent; import se.su.dsv.scipro.finalseminar.FinalSeminarThesisUploadedEvent; +import se.su.dsv.scipro.finalseminar.OppositionApprovedEvent; import se.su.dsv.scipro.finalseminar.OppositionFailedEvent; +import se.su.dsv.scipro.finalseminar.OppositionReportImprovementsRequestedEvent; import se.su.dsv.scipro.finalseminar.ParticipationFailedEvent; import se.su.dsv.scipro.notifications.dataobject.NotificationSource; import se.su.dsv.scipro.notifications.dataobject.PeerEvent; @@ -23,6 +25,7 @@ import se.su.dsv.scipro.project.ProjectActivatedEvent; import se.su.dsv.scipro.project.ProjectCompletedEvent; import se.su.dsv.scipro.project.ProjectDeactivatedEvent; import se.su.dsv.scipro.project.ReviewerAssignedEvent; +import se.su.dsv.scipro.report.OppositionReportSubmittedEvent; @Singleton public class Notifications { @@ -168,6 +171,40 @@ public class Notifications { ); } + @Subscribe + public void oppositionApproved(OppositionApprovedEvent event) { + notificationController.notifySeminar( + event.getFinalSeminar(), + SeminarEvent.Event.OPPOSITION_APPROVED, + new NotificationSource() + ); + } + + @Subscribe + public void oppositionReportImprovementsRequested(OppositionReportImprovementsRequestedEvent event) { + Member recipient = new Member(event.opposition().getUser(), Member.Type.OPPONENT); + Set<Member> recipients = Set.of(recipient); + NotificationSource source = new NotificationSource(); + source.setMessage(event.supervisorComment()); + notificationController.notifyCustomSeminar( + event.opposition().getFinalSeminar(), + SeminarEvent.Event.OPPOSITION_REPORT_IMPROVEMENTS_REQUESTED, + source, + recipients + ); + } + + @Subscribe + public void oppositionReportSubmitted(OppositionReportSubmittedEvent event) { + NotificationSource source = new NotificationSource(); + source.setMessage(event.report().getAuthorName()); + notificationController.notifySeminar( + event.finalSeminar(), + SeminarEvent.Event.OPPOSITION_REPORT_SUBMITTED, + source + ); + } + @Subscribe public void reviewersChanged(ReviewerAssignedEvent event) { notificationController.notifyProject( diff --git a/core/src/main/java/se/su/dsv/scipro/notifications/dataobject/SeminarEvent.java b/core/src/main/java/se/su/dsv/scipro/notifications/dataobject/SeminarEvent.java index e2a6484ed6..4a655018f1 100755 --- a/core/src/main/java/se/su/dsv/scipro/notifications/dataobject/SeminarEvent.java +++ b/core/src/main/java/se/su/dsv/scipro/notifications/dataobject/SeminarEvent.java @@ -26,6 +26,9 @@ public class SeminarEvent extends NotificationEvent { THESIS_DELETED, THESIS_UPLOAD_REMIND, CANCELLED, + OPPOSITION_REPORT_SUBMITTED, + OPPOSITION_REPORT_IMPROVEMENTS_REQUESTED, + OPPOSITION_APPROVED, } @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 c5cb11b76a..c754728273 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 @@ -140,6 +140,14 @@ FINAL_SEMINAR.THESIS_UPLOAD_REMIND.body = No final seminar thesis has been uploa If no final thesis has been uploaded by {0}, the final seminar will be automatically cancelled. FINAL_SEMINAR.CANCELLED.title = Final seminar for project {1} was cancelled FINAL_SEMINAR.CANCELLED.body = The final seminar for project {0} was cancelled, supervisor must select a new date for the final seminar. +FINAL_SEMINAR.OPPOSITION_REPORT_SUBMITTED.title=Opposition report submitted by {1} for the seminar on project {0} +FINAL_SEMINAR.OPPOSITION_REPORT_SUBMITTED.body=The opposition report from {0} has been submitted. +FINAL_SEMINAR.OPPOSITION_REPORT_IMPROVEMENTS_REQUESTED.title = Opposition report improvements requested +FINAL_SEMINAR.OPPOSITION_REPORT_IMPROVEMENTS_REQUESTED.body = The supervisor has deemed that the opposition report submitted \ + does not meet the minimum requirements and has requested improvements. Please log into SciPro and submit a new \ + opposition report. Their comments can be seen below:\n\n{0} +FINAL_SEMINAR.OPPOSITION_APPROVED.title = Opposition approved +FINAL_SEMINAR.OPPOSITION_APPROVED.body = Your opposition report has been approved by the final seminar supervisor. FINAL_SEMINAR.compilationSuffix = , project: {0} PEER.REVIEW_COMPLETED.title = Peer review completed diff --git a/core/src/main/java/se/su/dsv/scipro/report/GradingReportService.java b/core/src/main/java/se/su/dsv/scipro/report/GradingReportService.java index 1c38af9b44..409bf6385d 100644 --- a/core/src/main/java/se/su/dsv/scipro/report/GradingReportService.java +++ b/core/src/main/java/se/su/dsv/scipro/report/GradingReportService.java @@ -2,7 +2,6 @@ package se.su.dsv.scipro.report; import java.time.Instant; import java.util.List; -import se.su.dsv.scipro.finalseminar.FinalSeminarOpposition; import se.su.dsv.scipro.grading.GradingBasis; import se.su.dsv.scipro.project.Project; import se.su.dsv.scipro.system.User; @@ -19,8 +18,6 @@ public interface GradingReportService { SupervisorGradingReport supervisorGradingReport ); - boolean updateOppositionCriteria(SupervisorGradingReport report, FinalSeminarOpposition opposition); - GradingBasis getGradingBasis(Project project); GradingBasis updateGradingBasis(Project project, GradingBasis gradingBasis); diff --git a/core/src/main/java/se/su/dsv/scipro/report/GradingReportServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/report/GradingReportServiceImpl.java index ea9363d06c..3956cb3115 100644 --- a/core/src/main/java/se/su/dsv/scipro/report/GradingReportServiceImpl.java +++ b/core/src/main/java/se/su/dsv/scipro/report/GradingReportServiceImpl.java @@ -1,6 +1,7 @@ package se.su.dsv.scipro.report; import com.google.common.eventbus.EventBus; +import com.google.common.eventbus.Subscribe; import jakarta.inject.Inject; import jakarta.transaction.Transactional; import java.time.Clock; @@ -8,6 +9,9 @@ import java.time.Instant; import java.time.LocalDate; import java.util.*; import se.su.dsv.scipro.finalseminar.FinalSeminarOpposition; +import se.su.dsv.scipro.finalseminar.FinalSeminarOppositionGrading; +import se.su.dsv.scipro.finalseminar.OppositionApprovedEvent; +import se.su.dsv.scipro.finalseminar.OppositionCriteria; import se.su.dsv.scipro.grading.GradingBasis; import se.su.dsv.scipro.grading.GradingReportTemplateService; import se.su.dsv.scipro.grading.GradingReportTemplateUpdate; @@ -20,7 +24,8 @@ import se.su.dsv.scipro.system.ProjectTypeService; import se.su.dsv.scipro.system.User; import se.su.dsv.scipro.util.Either; -public class GradingReportServiceImpl implements GradingReportTemplateService, GradingReportService { +public class GradingReportServiceImpl + implements GradingReportTemplateService, GradingReportService, FinalSeminarOppositionGrading { private final EventBus eventBus; private final ThesisSubmissionHistoryService thesisSubmissionHistoryService; @@ -44,11 +49,11 @@ public class GradingReportServiceImpl implements GradingReportTemplateService, G this.supervisorGradingReportRepository = supervisorGradingReportRepository; this.gradingReportTemplateRepo = gradingReportTemplateRepo; this.projectTypeService = projectTypeService; + + eventBus.register(this); } - @Override - @Transactional - public boolean updateOppositionCriteria(SupervisorGradingReport report, FinalSeminarOpposition opposition) { + private boolean updateOppositionCriteria(SupervisorGradingReport report, FinalSeminarOpposition opposition) { for (GradingCriterion gradingCriterion : report.getIndividualCriteria()) { boolean isOppositionCriterion = gradingCriterion.getFlag() == GradingCriterion.Flag.OPPOSITION; boolean betterGrade = @@ -294,4 +299,39 @@ public class GradingReportServiceImpl implements GradingReportTemplateService, G return gradingReportTemplateRepo.createTemplate(projectType, update); } + + @Subscribe + public void opponentApproved(OppositionApprovedEvent event) { + SupervisorGradingReport report = getSupervisorGradingReport(event.getProject(), event.getStudent()); + updateOppositionCriteria(report, event.getOpposition()); + } + + @Override + @Transactional + public OppositionCriteria oppositionCriteria(FinalSeminarOpposition opposition) { + SupervisorGradingReport supervisorGradingReport = getSupervisorGradingReport( + opposition.getProject(), + opposition.getUser() + ); + Optional<GradingCriterion> oppositionGradingCriteria = supervisorGradingReport + .getIndividualCriteria() + .stream() + .filter(individualCriterion -> individualCriterion.getFlag() == AbstractGradingCriterion.Flag.OPPOSITION) + .findAny(); + if (oppositionGradingCriteria.isEmpty()) { + return new OppositionCriteria(0, List.of()); + } + List<OppositionCriteria.Point> points = oppositionGradingCriteria + .stream() + .map(GradingCriterion::getGradingCriterionPoints) + .flatMap(Collection::stream) + .map(gcp -> + new OppositionCriteria.Point( + gcp.getPoint(), + Objects.requireNonNullElse(gcp.getDescription(Language.ENGLISH), "") + ) + ) + .toList(); + return new OppositionCriteria(oppositionGradingCriteria.get().getPointsRequiredToPass(), points); + } } diff --git a/core/src/main/java/se/su/dsv/scipro/report/OppositionReportService.java b/core/src/main/java/se/su/dsv/scipro/report/OppositionReportService.java index 60f1db90b0..3ec394e7a5 100644 --- a/core/src/main/java/se/su/dsv/scipro/report/OppositionReportService.java +++ b/core/src/main/java/se/su/dsv/scipro/report/OppositionReportService.java @@ -1,5 +1,7 @@ package se.su.dsv.scipro.report; +import java.util.Optional; +import se.su.dsv.scipro.file.FileUpload; import se.su.dsv.scipro.finalseminar.FinalSeminarOpposition; public interface OppositionReportService { @@ -7,4 +9,10 @@ public interface OppositionReportService { void save(OppositionReport oppositionReport); void deleteOppositionReport(FinalSeminarOpposition finalSeminarOpposition); void deleteOpponentReport(FinalSeminarOpposition modelObject); + + AttachmentReport submit(OppositionReport report); + + void save(OppositionReport report, Optional<FileUpload> fileUpload); + + void deleteAttachment(OppositionReport report); } diff --git a/core/src/main/java/se/su/dsv/scipro/report/OppositionReportServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/report/OppositionReportServiceImpl.java index f993503d09..65a35aae78 100644 --- a/core/src/main/java/se/su/dsv/scipro/report/OppositionReportServiceImpl.java +++ b/core/src/main/java/se/su/dsv/scipro/report/OppositionReportServiceImpl.java @@ -1,10 +1,13 @@ package se.su.dsv.scipro.report; +import com.google.common.eventbus.EventBus; import jakarta.inject.Inject; import jakarta.inject.Named; import jakarta.transaction.Transactional; +import java.util.Optional; import se.su.dsv.scipro.file.FileReference; import se.su.dsv.scipro.file.FileService; +import se.su.dsv.scipro.file.FileUpload; import se.su.dsv.scipro.finalseminar.FinalSeminarOpposition; import se.su.dsv.scipro.finalseminar.FinalSeminarOppositionRepo; @@ -15,18 +18,21 @@ public class OppositionReportServiceImpl implements OppositionReportService { private GradingReportTemplateRepo gradingReportTemplateRepo; private FileService fileService; private FinalSeminarOppositionRepo finalSeminarOppositionRepo; + private final EventBus eventBus; @Inject public OppositionReportServiceImpl( OppositionReportRepo oppositionReportRepo, GradingReportTemplateRepo gradingReportTemplateRepo, FileService fileService, - FinalSeminarOppositionRepo finalSeminarOppositionRepo + FinalSeminarOppositionRepo finalSeminarOppositionRepo, + EventBus eventBus ) { this.oppositionReportRepo = oppositionReportRepo; this.gradingReportTemplateRepo = gradingReportTemplateRepo; this.fileService = fileService; this.finalSeminarOppositionRepo = finalSeminarOppositionRepo; + this.eventBus = eventBus; } @Override @@ -74,4 +80,36 @@ public class OppositionReportServiceImpl implements OppositionReportService { finalSeminarOppositionRepo.save(finalSeminarOpposition); } } + + @Override + @Transactional + public OppositionReport submit(OppositionReport report) { + report.submit(); + OppositionReport submitted = oppositionReportRepo.save(report); + eventBus.post(new OppositionReportSubmittedEvent(submitted)); + return submitted; + } + + @Override + @Transactional + public void save(OppositionReport report, Optional<FileUpload> fileUpload) { + storeReportFile(report, fileUpload); + save(report); + } + + @Override + @Transactional + public void deleteAttachment(OppositionReport report) { + FileReference attachment = report.getAttachment(); + report.setAttachment(null); + fileService.delete(attachment); + oppositionReportRepo.save(report); + } + + private void storeReportFile(OppositionReport report, Optional<FileUpload> fileUpload) { + if (fileUpload.isPresent()) { + final FileReference reference = fileService.storeFile(fileUpload.get()); + report.setAttachment(reference); + } + } } diff --git a/core/src/main/java/se/su/dsv/scipro/report/OppositionReportSubmittedEvent.java b/core/src/main/java/se/su/dsv/scipro/report/OppositionReportSubmittedEvent.java new file mode 100644 index 0000000000..e479091fd7 --- /dev/null +++ b/core/src/main/java/se/su/dsv/scipro/report/OppositionReportSubmittedEvent.java @@ -0,0 +1,9 @@ +package se.su.dsv.scipro.report; + +import se.su.dsv.scipro.finalseminar.FinalSeminar; + +public record OppositionReportSubmittedEvent(OppositionReport report) { + public FinalSeminar finalSeminar() { + return report().getFinalSeminarOpposition().getFinalSeminar(); + } +} diff --git a/core/src/main/java/se/su/dsv/scipro/report/ReportService.java b/core/src/main/java/se/su/dsv/scipro/report/ReportService.java deleted file mode 100644 index a077143c0e..0000000000 --- a/core/src/main/java/se/su/dsv/scipro/report/ReportService.java +++ /dev/null @@ -1,13 +0,0 @@ -package se.su.dsv.scipro.report; - -import java.util.Optional; -import se.su.dsv.scipro.file.FileUpload; -import se.su.dsv.scipro.system.GenericService; - -public interface ReportService extends GenericService<Report, Long> { - AttachmentReport submit(AttachmentReport report); - - void save(AttachmentReport report, Optional<FileUpload> fileUpload); - - void deleteAttachment(AttachmentReport report); -} diff --git a/core/src/main/java/se/su/dsv/scipro/report/ReportServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/report/ReportServiceImpl.java deleted file mode 100644 index 41b6e7a2a6..0000000000 --- a/core/src/main/java/se/su/dsv/scipro/report/ReportServiceImpl.java +++ /dev/null @@ -1,52 +0,0 @@ -package se.su.dsv.scipro.report; - -import jakarta.inject.Inject; -import jakarta.inject.Provider; -import jakarta.persistence.EntityManager; -import jakarta.transaction.Transactional; -import java.util.Optional; -import se.su.dsv.scipro.file.FileReference; -import se.su.dsv.scipro.file.FileService; -import se.su.dsv.scipro.file.FileUpload; -import se.su.dsv.scipro.system.AbstractServiceImpl; - -public class ReportServiceImpl extends AbstractServiceImpl<Report, Long> implements ReportService { - - private final FileService fileDescriptionService; - - @Inject - public ReportServiceImpl(Provider<EntityManager> em, final FileService fileDescriptionService) { - super(em, Report.class, QReport.report); - this.fileDescriptionService = fileDescriptionService; - } - - @Override - @Transactional - public AttachmentReport submit(AttachmentReport report) { - report.submit(); - return save(report); - } - - @Override - @Transactional - public void save(AttachmentReport report, Optional<FileUpload> fileUpload) { - storeReportFile(report, fileUpload); - save(report); - } - - @Override - @Transactional - public void deleteAttachment(AttachmentReport report) { - FileReference attachment = report.getAttachment(); - report.setAttachment(null); - fileDescriptionService.delete(attachment); - save(report); - } - - private void storeReportFile(AttachmentReport report, Optional<FileUpload> fileUpload) { - if (fileUpload.isPresent()) { - final FileReference reference = fileDescriptionService.storeFile(fileUpload.get()); - report.setAttachment(reference); - } - } -} diff --git a/core/src/main/resources/db/migration/V5__final_seminar_opposition_request_improvements.sql b/core/src/main/resources/db/migration/V5__final_seminar_opposition_request_improvements.sql new file mode 100644 index 0000000000..e922687232 --- /dev/null +++ b/core/src/main/resources/db/migration/V5__final_seminar_opposition_request_improvements.sql @@ -0,0 +1,3 @@ +ALTER TABLE `final_seminar_opposition` + ADD COLUMN `improvements_requested_at` DATETIME NULL, + ADD COLUMN `supervisor_improvements_comment` TEXT NULL; diff --git a/core/src/main/resources/db/migration/V6__final_seminar_work_days_to_fix_opposition_report.sql b/core/src/main/resources/db/migration/V6__final_seminar_work_days_to_fix_opposition_report.sql new file mode 100644 index 0000000000..f102c8fe14 --- /dev/null +++ b/core/src/main/resources/db/migration/V6__final_seminar_work_days_to_fix_opposition_report.sql @@ -0,0 +1,2 @@ +ALTER TABLE `final_seminar_settings` + ADD COLUMN `work_days_to_fix_requested_improvements_to_opposition_report` INT(11) NOT NULL DEFAULT 10; diff --git a/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionServiceImplIntegrationTest.java b/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionServiceImplIntegrationTest.java index 9b27323fd4..fb39050f2d 100644 --- a/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionServiceImplIntegrationTest.java +++ b/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionServiceImplIntegrationTest.java @@ -1,14 +1,22 @@ package se.su.dsv.scipro.finalseminar; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import jakarta.inject.Inject; import java.time.LocalDate; import java.time.Month; import java.util.Date; +import java.util.List; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import se.su.dsv.scipro.project.Project; +import se.su.dsv.scipro.report.AbstractGradingCriterion; +import se.su.dsv.scipro.report.GradingCriterionPointTemplate; import se.su.dsv.scipro.report.GradingReportTemplate; import se.su.dsv.scipro.report.OppositionReport; import se.su.dsv.scipro.system.DegreeType; @@ -46,6 +54,76 @@ public class FinalSeminarOppositionServiceImplIntegrationTest extends Integratio assertEquals(0, finalSeminarOppositionService.count()); } + @Test + public void opposition_criteria_are_taken_from_the_grading_report_template() { + FinalSeminarOpposition opposition = createOpposition(finalSeminar, createUser()); + createSimpleGradingReportTemplateWithPassFail(); + + assertEquals(2, finalSeminarOppositionService.getCriteriaForOpposition(opposition).pointsAvailable().size()); + } + + @Test + public void can_not_grade_outside_criterion() { + FinalSeminarOpposition opposition = createOpposition(finalSeminar, createUser()); + createSimpleGradingReportTemplateWithPassFail(); + + PointNotValidException exception = assertThrows(PointNotValidException.class, () -> + finalSeminarOppositionService.gradeOpponent(opposition, 2, "Feedback") + ); + assertEquals(2, exception.givenValue()); + assertThat(exception.acceptableValues(), hasSize(2)); + assertThat(exception.acceptableValues(), contains(0, 1)); + } + + @Test + public void publishes_failed_event_when_grading_with_failing_criterion() throws Exception { + FinalSeminarOpposition opposition = createOpposition(finalSeminar, createUser()); + createSimpleGradingReportTemplateWithPassFail(); + + finalSeminarOppositionService.gradeOpponent(opposition, 0, "Feedback"); + + assertThat(getPublishedEvents(), hasItem(new OppositionFailedEvent(opposition))); + } + + @Test + public void publishes_approved_event_when_grading_with_passing_criterion() throws Exception { + FinalSeminarOpposition opposition = createOpposition(finalSeminar, createUser()); + createSimpleGradingReportTemplateWithPassFail(); + + finalSeminarOppositionService.gradeOpponent(opposition, 1, "Feedback"); + + assertThat(getPublishedEvents(), hasItem(new OppositionApprovedEvent(opposition))); + } + + @Test + public void stores_assessment() throws Exception { + FinalSeminarOpposition opposition = createOpposition(finalSeminar, createUser()); + createSimpleGradingReportTemplateWithPassFail(); + + FinalSeminarOpposition graded = finalSeminarOppositionService.gradeOpponent(opposition, 1, "Feedback"); + + assertEquals(1, graded.getPoints()); + assertEquals("Feedback", graded.getFeedback()); + } + + private void createSimpleGradingReportTemplateWithPassFail() { + GradingReportTemplate gradingReportTemplate = createGradingReportTemplate(); + + GradingCriterionPointTemplate failingCriterion = new GradingCriterionPointTemplate(); + failingCriterion.setPoint(0); + GradingCriterionPointTemplate passingCriterion = new GradingCriterionPointTemplate(); + passingCriterion.setPoint(1); + + gradingReportTemplate.addIndividualCriterion( + "Criterion 1", + "Criterion 1", + 1, + List.of(failingCriterion, passingCriterion), + AbstractGradingCriterion.Flag.OPPOSITION + ); + save(gradingReportTemplate); + } + private void createOppositionReport(FinalSeminarOpposition opposition) { OppositionReport report = new OppositionReport(createGradingReportTemplate(), opposition); opposition.setOppositionReport(report); @@ -93,7 +171,7 @@ public class FinalSeminarOppositionServiceImplIntegrationTest extends Integratio FinalSeminarOpposition opposition = new FinalSeminarOpposition(); opposition.setFinalSeminar(finalSeminar); opposition.setUser(student); - opposition.setProject(createProject(createProjectType())); + opposition.setProject(createProject(projectType)); return save(opposition); } } diff --git a/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarServiceImplIntegrationTest.java b/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarServiceImplIntegrationTest.java index 3cced9804a..9d86984f17 100644 --- a/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarServiceImplIntegrationTest.java +++ b/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarServiceImplIntegrationTest.java @@ -8,6 +8,7 @@ import static se.su.dsv.scipro.test.Matchers.isRight; import com.google.common.collect.Lists; import jakarta.inject.Inject; +import java.time.Duration; import java.time.LocalDate; import java.time.Month; import java.time.ZonedDateTime; @@ -30,6 +31,9 @@ public class FinalSeminarServiceImplIntegrationTest extends IntegrationTest { @Inject private FinalSeminarService finalSeminarService; + @Inject + private FinalSeminarOppositionService finalSeminarOppositionService; + private ProjectType projectType; private FinalSeminar futureFinalSeminar; private User user; @@ -309,6 +313,43 @@ public class FinalSeminarServiceImplIntegrationTest extends IntegrationTest { assertThat(finalSeminarService.canOppose(user, finalSeminar, otherProject), isRight(anything())); } + @Test + public void seminar_is_not_unfinished_if_opponent_has_improvements_requested() { + FinalSeminar seminar = createFinalSeminar(createProject(), -6); + FinalSeminarOpposition finalSeminarOpposition = addOpposition(seminar, null); + addOppositionReport(finalSeminarOpposition); + + Date after = Date.from(seminar.getStartDate().toInstant().minus(Duration.ofDays(1))); + Date before = Date.from(seminar.getStartDate().toInstant().plus(Duration.ofDays(1))); + + List<FinalSeminar> unfinishedSeminars = finalSeminarService.findUnfinishedSeminars( + after, + before, + new PageRequest(0, 5) + ); + + assertThat(unfinishedSeminars, hasItem(seminar)); + + finalSeminarOppositionService.requestImprovements(finalSeminarOpposition, "improvements"); + + List<FinalSeminar> afterImprovements = finalSeminarService.findUnfinishedSeminars( + after, + before, + new PageRequest(0, 5) + ); + + assertThat(afterImprovements, not(hasItem(seminar))); + } + + private static void addOppositionReport(FinalSeminarOpposition finalSeminarOpposition) { + GradingReportTemplate gradingReportTemplate = new GradingReportTemplate( + finalSeminarOpposition.getProjectType(), + LocalDate.now() + ); + OppositionReport oppositionReport = new OppositionReport(gradingReportTemplate, finalSeminarOpposition); + finalSeminarOpposition.setOppositionReport(oppositionReport); + } + private FinalSeminar createFutureFinalSeminarSomeDaysAgo(final int daysAgo) { FinalSeminar finalSeminar = initFinalSeminar(createProject(), 5); final Date dateCreated = Date.from(ZonedDateTime.now().minusDays(daysAgo).toInstant()); @@ -340,10 +381,10 @@ public class FinalSeminarServiceImplIntegrationTest extends IntegrationTest { save(seminar); } - private void addOpposition(FinalSeminar seminar, FinalSeminarGrade grade) { + private FinalSeminarOpposition addOpposition(FinalSeminar seminar, FinalSeminarGrade grade) { FinalSeminarOpposition opposition = createOpposition(seminar); opposition.setGrade(grade); - save(opposition); + return save(opposition); } private OppositionReport createOppositionReport(FinalSeminarOpposition opposition) { diff --git a/core/src/test/java/se/su/dsv/scipro/report/GradingReportServiceImplIntegrationTest.java b/core/src/test/java/se/su/dsv/scipro/report/GradingReportServiceImplIntegrationTest.java index 4eec189c37..597396bbea 100644 --- a/core/src/test/java/se/su/dsv/scipro/report/GradingReportServiceImplIntegrationTest.java +++ b/core/src/test/java/se/su/dsv/scipro/report/GradingReportServiceImplIntegrationTest.java @@ -5,6 +5,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import com.google.common.eventbus.EventBus; import jakarta.inject.Inject; import java.time.LocalDate; import java.time.Month; @@ -13,6 +14,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import se.su.dsv.scipro.finalseminar.FinalSeminar; import se.su.dsv.scipro.finalseminar.FinalSeminarOpposition; +import se.su.dsv.scipro.finalseminar.OppositionApprovedEvent; import se.su.dsv.scipro.project.Project; import se.su.dsv.scipro.security.auth.roles.Roles; import se.su.dsv.scipro.system.DegreeType; @@ -31,6 +33,9 @@ public class GradingReportServiceImplIntegrationTest extends IntegrationTest { @Inject private GradingReportServiceImpl gradingReportService; + @Inject + private EventBus eventBus; + private ProjectType projectType; private GradingReportTemplate gradingReportTemplate; private Project project; @@ -45,7 +50,6 @@ public class GradingReportServiceImplIntegrationTest extends IntegrationTest { project = createProject(projectType, 30); gradingReportTemplate = createProjectGradingCriterion(gradingReportTemplate, 2); gradingReportTemplate = createIndividualGradingCriterion(gradingReportTemplate, 2); - gradingReport = createGradingReport(project, student); } @Test @@ -68,6 +72,7 @@ public class GradingReportServiceImplIntegrationTest extends IntegrationTest { @Test public void submit_supervisor_grading_report_flags_report_as_submitted() { + gradingReport = createGradingReport(project, student); assessAllCriteria(gradingReport); Either<List<SubmissionError>, SupervisorGradingReport> result = gradingReportService.submitReport( gradingReport @@ -77,6 +82,7 @@ public class GradingReportServiceImplIntegrationTest extends IntegrationTest { @Test public void submitting_supervisor_report_throws_exception_if_report_is_not_finished() { + gradingReport = createGradingReport(project, student); Either<List<SubmissionError>, SupervisorGradingReport> result = gradingReportService.submitReport( gradingReport ); @@ -86,38 +92,35 @@ public class GradingReportServiceImplIntegrationTest extends IntegrationTest { @Test public void update_opposition_criterion() { addOppositionCriterion(); - boolean updated = updateOppositionCriterion(); + updateOppositionCriterion(); GradingCriterion oppositionCriterion = findOppositionCriterion(); assert oppositionCriterion != null; assertEquals(FEEDBACK_ON_OPPOSITION, oppositionCriterion.getFeedback()); assertEquals((Integer) OPPOSITION_CRITERION_POINTS, oppositionCriterion.getPoints()); - assertTrue(updated); } @Test public void update_opposition_if_title_matches_english_title() { addOppositionCriterion(); - boolean updated = updateOppositionCriterion(); + updateOppositionCriterion(); GradingCriterion oppositionCriterion = findEnglishOppositionCriterion("Ö1 Opposition report"); assert oppositionCriterion != null; assertEquals(FEEDBACK_ON_OPPOSITION, oppositionCriterion.getFeedback()); assertEquals((Integer) OPPOSITION_CRITERION_POINTS, oppositionCriterion.getPoints()); - assertTrue(updated); } @Test public void updating_opposition_criterion_does_nothing_if_criterion_already_has_values() { addOppositionCriterion(); assessAllCriteria(gradingReport); - boolean updated = updateOppositionCriterion(); + updateOppositionCriterion(); GradingCriterion oppositionCriterion = findOppositionCriterion(); assert oppositionCriterion != null; assertEquals(FEEDBACK, oppositionCriterion.getFeedback()); assertEquals((Integer) oppositionCriterion.getMaxPoints(), oppositionCriterion.getPoints()); - assertFalse(updated); } @Test @@ -151,9 +154,9 @@ public class GradingReportServiceImplIntegrationTest extends IntegrationTest { gradingReport = createGradingReport(project, student); } - private boolean updateOppositionCriterion() { + private void updateOppositionCriterion() { FinalSeminarOpposition opposition = createFinalSeminarOpposition(); - return gradingReportService.updateOppositionCriteria(gradingReport, opposition); + eventBus.post(new OppositionApprovedEvent(opposition)); } private GradingCriterion findOppositionCriterion() { @@ -176,8 +179,8 @@ public class GradingReportServiceImplIntegrationTest extends IntegrationTest { private FinalSeminarOpposition createFinalSeminarOpposition() { FinalSeminarOpposition finalSeminarOpposition = new FinalSeminarOpposition(); - finalSeminarOpposition.setProject(createProject(projectType, 30)); - finalSeminarOpposition.setUser(createStudent()); + finalSeminarOpposition.setProject(project); + finalSeminarOpposition.setUser(student); finalSeminarOpposition.setFinalSeminar(createFinalSeminar()); finalSeminarOpposition.setFeedback(FEEDBACK_ON_OPPOSITION); diff --git a/core/src/test/java/se/su/dsv/scipro/test/SpringTest.java b/core/src/test/java/se/su/dsv/scipro/test/SpringTest.java index 04d0f70da7..2d52bd0d36 100644 --- a/core/src/test/java/se/su/dsv/scipro/test/SpringTest.java +++ b/core/src/test/java/se/su/dsv/scipro/test/SpringTest.java @@ -1,11 +1,14 @@ package se.su.dsv.scipro.test; +import com.google.common.eventbus.EventBus; import jakarta.persistence.EntityManager; import jakarta.persistence.EntityManagerFactory; import jakarta.persistence.EntityTransaction; import jakarta.persistence.Persistence; import java.sql.SQLException; import java.time.Clock; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.Optional; import org.flywaydb.core.Flyway; @@ -35,6 +38,8 @@ public abstract class SpringTest { @Container static MariaDBContainer<?> mariaDBContainer = new MariaDBContainer<>("mariadb:10.11"); + private CapturingEventBus capturingEventBus; + @BeforeEach public final void prepareSpring() throws SQLException { MariaDbDataSource dataSource = new MariaDbDataSource(mariaDBContainer.getJdbcUrl()); @@ -50,8 +55,11 @@ public abstract class SpringTest { transaction.begin(); transaction.setRollbackOnly(); + capturingEventBus = new CapturingEventBus(); + AnnotationConfigApplicationContext annotationConfigApplicationContext = new AnnotationConfigApplicationContext(); + annotationConfigApplicationContext.registerBean("eventBus", EventBus.class, () -> this.capturingEventBus); annotationConfigApplicationContext.register(TestContext.class); annotationConfigApplicationContext.getBeanFactory().registerSingleton("entityManager", this.entityManager); annotationConfigApplicationContext.refresh(); @@ -75,6 +83,10 @@ public abstract class SpringTest { } } + protected List<Object> getPublishedEvents() { + return capturingEventBus.publishedEvents; + } + @Configuration(proxyBeanMethods = false) @Import({ CoreConfig.class, RepositoryConfiguration.class }) public static class TestContext { @@ -106,4 +118,15 @@ public abstract class SpringTest { return currentProfile; } } + + private static class CapturingEventBus extends EventBus { + + private List<Object> publishedEvents = new ArrayList<>(); + + @Override + public void post(Object event) { + publishedEvents.add(event); + super.post(event); + } + } } diff --git a/test-data/src/main/java/se/su/dsv/scipro/testdata/DataInitializer.java b/test-data/src/main/java/se/su/dsv/scipro/testdata/DataInitializer.java index b2fec13a8e..cf01ab2315 100644 --- a/test-data/src/main/java/se/su/dsv/scipro/testdata/DataInitializer.java +++ b/test-data/src/main/java/se/su/dsv/scipro/testdata/DataInitializer.java @@ -9,18 +9,35 @@ import java.io.InputStream; import java.time.LocalDate; import java.time.LocalTime; import java.time.Month; +import java.time.ZonedDateTime; import java.util.*; import java.util.function.Function; import se.su.dsv.scipro.checklist.ChecklistCategory; +import se.su.dsv.scipro.data.dataobjects.Member; +import se.su.dsv.scipro.file.FileReference; +import se.su.dsv.scipro.file.FileService; import se.su.dsv.scipro.file.FileUpload; -import se.su.dsv.scipro.match.*; +import se.su.dsv.scipro.finalseminar.FinalSeminar; +import se.su.dsv.scipro.finalseminar.FinalSeminarOpposition; import se.su.dsv.scipro.match.ApplicationPeriod; import se.su.dsv.scipro.match.Idea; import se.su.dsv.scipro.match.IdeaService; import se.su.dsv.scipro.match.Keyword; +import se.su.dsv.scipro.match.Target; +import se.su.dsv.scipro.match.TholanderBox; import se.su.dsv.scipro.milestones.dataobjects.MilestoneActivityTemplate; import se.su.dsv.scipro.milestones.dataobjects.MilestonePhaseTemplate; import se.su.dsv.scipro.milestones.service.MilestoneActivityTemplateService; +import se.su.dsv.scipro.notifications.dataobject.CustomEvent; +import se.su.dsv.scipro.notifications.dataobject.GroupEvent; +import se.su.dsv.scipro.notifications.dataobject.IdeaEvent; +import se.su.dsv.scipro.notifications.dataobject.MileStoneEvent; +import se.su.dsv.scipro.notifications.dataobject.Notification; +import se.su.dsv.scipro.notifications.dataobject.PeerEvent; +import se.su.dsv.scipro.notifications.dataobject.ProjectEvent; +import se.su.dsv.scipro.notifications.dataobject.ProjectForumEvent; +import se.su.dsv.scipro.notifications.dataobject.SeminarEvent; +import se.su.dsv.scipro.notifications.settings.service.ReceiverConfigurationService; import se.su.dsv.scipro.profiles.CurrentProfile; import se.su.dsv.scipro.profiles.Profiles; import se.su.dsv.scipro.project.Project; @@ -56,12 +73,18 @@ public class DataInitializer implements Lifecycle, BaseData, Factory { @Inject private MilestoneActivityTemplateService milestoneActivityTemplateService; + @Inject + private FileService fileService; + @Inject private CurrentProfile profile; @Inject private Provider<EntityManager> em; + @Inject + private ReceiverConfigurationService receiverConfigurationService; + @Inject private RoughDraftApprovalService roughDraftApprovalService; @@ -103,6 +126,7 @@ public class DataInitializer implements Lifecycle, BaseData, Factory { private ProjectType masterClass; private ProjectType magisterClass; private ApplicationPeriod applicationPeriod; + private Project project1; private Project project2; @Transactional @@ -123,6 +147,8 @@ public class DataInitializer implements Lifecycle, BaseData, Factory { createTarget(); createStudentIdea(); createRoughDraftApproval(); + createPastFinalSeminar(); + setUpNotifications(); Collection<TestDataPopulator> availablePopulators = testDataPopulators.orElseGet(Collections::emptySet); for (TestDataPopulator testDataPopulator : availablePopulators) { testDataPopulator.populate(this, this); @@ -143,6 +169,47 @@ public class DataInitializer implements Lifecycle, BaseData, Factory { reviewerAssignmentService.assignReviewer(project2, eric_employee); } + private void setUpNotifications() { + enableAllNotifications(Notification.Type.PEER, PeerEvent.Event.values()); + enableAllNotifications(Notification.Type.FORUM, ProjectForumEvent.Event.values()); + enableAllNotifications(Notification.Type.GROUP, GroupEvent.Event.values()); + enableAllNotifications(Notification.Type.MILESTONE, MileStoneEvent.Event.values()); + enableAllNotifications(Notification.Type.PROJECT, ProjectEvent.Event.values()); + enableAllNotifications(Notification.Type.IDEA, IdeaEvent.Event.values()); + enableAllNotifications(Notification.Type.CUSTOM, CustomEvent.Event.values()); + enableAllNotifications(Notification.Type.FINAL_SEMINAR, SeminarEvent.Event.values()); + } + + private void enableAllNotifications(Notification.Type type, Enum<?>[] events) { + for (Enum<?> event : events) { + for (Member.Type member : Member.Type.values()) { + receiverConfigurationService.setReceiving(type, event, member, true); + } + } + } + + private void createPastFinalSeminar() { + FileReference document = fileService.storeFile( + new SimpleTextFile(sture_student, "document.txt", "Hello World") + ); + + FinalSeminar finalSeminar = new FinalSeminar(); + finalSeminar.setStartDate(Date.from(ZonedDateTime.now().minusDays(1).toInstant())); + finalSeminar.setProject(project1); + finalSeminar.setRoom("zoom"); + finalSeminar.setPresentationLanguage(Language.ENGLISH); + finalSeminar.setDocument(document); + finalSeminar.setDocumentUploadDate(document.getFileDescription().getDateCreated()); + + FinalSeminarOpposition opponent = new FinalSeminarOpposition(); + opponent.setProject(project2); + opponent.setFinalSeminar(finalSeminar); + opponent.setUser(sid_student); + finalSeminar.addOpposition(opponent); + + save(finalSeminar); + } + @Override public void stop() {} @@ -202,7 +269,7 @@ public class DataInitializer implements Lifecycle, BaseData, Factory { } private void createProjects() { - createProject(PROJECT_1, eric_employee, sture_student, stina_student, eve_employee, 135); + project1 = createProject(PROJECT_1, eric_employee, sture_student, stina_student, eve_employee, 135); project2 = createProject(PROJECT_2, eve_employee, sid_student, simon_student, eric_employee, 246); } @@ -1916,9 +1983,6 @@ public class DataInitializer implements Lifecycle, BaseData, Factory { magisterClass = new ProjectType(ProjectType.MAGISTER, "Magister", "One-year-Master degree thesis project"); save(magisterClass); - - final ProjectType phdClass = new ProjectType(DegreeType.NONE, "PhD", "PhD project"); - save(phdClass); } private void createDefaultChecklistCategoriesIfNotDone() { diff --git a/view/src/main/java/se/su/dsv/scipro/finalseminar/AdminFinalSeminarSettingsPage.html b/view/src/main/java/se/su/dsv/scipro/finalseminar/AdminFinalSeminarSettingsPage.html index a17eec8b77..7b95106394 100755 --- a/view/src/main/java/se/su/dsv/scipro/finalseminar/AdminFinalSeminarSettingsPage.html +++ b/view/src/main/java/se/su/dsv/scipro/finalseminar/AdminFinalSeminarSettingsPage.html @@ -42,6 +42,13 @@ </div> </div> + <div class="mb-3"> + <label class="col-lg-4">How many work days opponents have to resubmit their report</label> + <div class="col-lg-1"> + <input class="form-control" type="text" wicket:id="work_days_to_fix_requested_improvements_to_opposition_report" /> + </div> + </div> + <div class="mb-3"> <div class="col-lg-offset-4 col-lg-4"> <div class="form-check"> diff --git a/view/src/main/java/se/su/dsv/scipro/finalseminar/AdminFinalSeminarSettingsPage.java b/view/src/main/java/se/su/dsv/scipro/finalseminar/AdminFinalSeminarSettingsPage.java index 59dad683ab..f04ffe4136 100755 --- a/view/src/main/java/se/su/dsv/scipro/finalseminar/AdminFinalSeminarSettingsPage.java +++ b/view/src/main/java/se/su/dsv/scipro/finalseminar/AdminFinalSeminarSettingsPage.java @@ -131,6 +131,17 @@ public class AdminFinalSeminarSettingsPage extends AbstractAdminSystemPage { Integer.class ) ); + add( + new RequiredTextField<>( + "work_days_to_fix_requested_improvements_to_opposition_report", + LambdaModel.of( + model, + FinalSeminarSettings::getWorkDaysToFixRequestedImprovementsToOppositionReport, + FinalSeminarSettings::setWorkDaysToFixRequestedImprovementsToOppositionReport + ), + Integer.class + ) + ); add( new CheckBox( SEMINAR_PDF, diff --git a/view/src/main/java/se/su/dsv/scipro/finalseminar/DownloadPdfReportPanel.java b/view/src/main/java/se/su/dsv/scipro/finalseminar/DownloadPdfReportPanel.java index fe82b15be8..dacb0325fa 100644 --- a/view/src/main/java/se/su/dsv/scipro/finalseminar/DownloadPdfReportPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/finalseminar/DownloadPdfReportPanel.java @@ -6,6 +6,7 @@ import org.apache.wicket.markup.html.panel.Panel; import org.apache.wicket.model.IModel; import org.apache.wicket.model.LambdaModel; import se.su.dsv.scipro.components.DateLabel; +import se.su.dsv.scipro.data.enums.DateStyle; import se.su.dsv.scipro.grading.ReportPdfResource; import se.su.dsv.scipro.report.Report; @@ -32,7 +33,11 @@ public class DownloadPdfReportPanel extends Panel { add(resourceLink); resourceLink.add(new Label(PDF_LABEL, reportPdfResource.getFileName())); resourceLink.add( - new DateLabel(PDF_UPLOAD_DATE, LambdaModel.of(report, Report::getLastModified, Report::setLastModified)) + new DateLabel( + PDF_UPLOAD_DATE, + LambdaModel.of(report, Report::getLastModified, Report::setLastModified), + DateStyle.DATETIME + ) ); } } diff --git a/view/src/main/java/se/su/dsv/scipro/finalseminar/OppositionReportPage.html b/view/src/main/java/se/su/dsv/scipro/finalseminar/OppositionReportPage.html index f4df0cf012..a3dbfe1e51 100644 --- a/view/src/main/java/se/su/dsv/scipro/finalseminar/OppositionReportPage.html +++ b/view/src/main/java/se/su/dsv/scipro/finalseminar/OppositionReportPage.html @@ -6,29 +6,36 @@ <div class="col-lg-8"> <h4>Opposition report</h4> - <div class="row mb-4"> - <div class="col-lg-8"> - <div class="help-box"> - Använd bedömningskriterierna i denna rapport och skriv dina synpunkter som opponent i fritextfälten - under varje kriterium. Du gör dock ingen poängbedömning men - är fri att skriva så mycket som du önskar på varje bedömningskriterium. - </div> - </div> + <div class="help-box mb-3"> + Use the assessment criteria in this report and write your views as an opponent in the text + fields under each criterion. However, you do not make a point assessment but are free to + write as much as you wish on each criterion. </div> <div class="mb-3"> <strong>Final seminar file:</strong> <span wicket:id="thesisFile"></span> </div> - <div wicket:id="fillOutReport"> - <strong>Thesis summary</strong> - + <wicket:enclosure child="improvements_requested_comment"> + <div class="alert alert-info"> <p> - Ge en kort sammanfattning av det utvärderade arbetet. + The supervisor has requested improvements to your opposition report. + You have until <span wicket:id="improvements_requested_deadline"></span> + to make the requested changes. See below for the comments from the supervisor. </p> - <label> - <textarea class="form-control mb-4" rows="8" wicket:id="thesisSummary"></textarea> + <p class="mb-0" wicket:id="improvements_requested_comment"></p> + </div> + </wicket:enclosure> + + <div wicket:id="fillOutReport"> + <label wicket:for="thesisSummary"> + Thesis summary </label> + <p> + Give a short summary of the evaluated work. + </p> + <textarea class="form-control mb-4" rows="8" wicket:id="thesisSummary"></textarea> + </div> </div> </div> diff --git a/view/src/main/java/se/su/dsv/scipro/finalseminar/OppositionReportPage.java b/view/src/main/java/se/su/dsv/scipro/finalseminar/OppositionReportPage.java index fc35785ba1..802625bdc2 100644 --- a/view/src/main/java/se/su/dsv/scipro/finalseminar/OppositionReportPage.java +++ b/view/src/main/java/se/su/dsv/scipro/finalseminar/OppositionReportPage.java @@ -1,8 +1,11 @@ package se.su.dsv.scipro.finalseminar; import jakarta.inject.Inject; +import java.time.ZoneId; +import java.util.Optional; import org.apache.wicket.RestartResponseException; import org.apache.wicket.ajax.AjaxRequestTarget; +import org.apache.wicket.markup.html.basic.Label; import org.apache.wicket.markup.html.form.TextArea; import org.apache.wicket.model.IModel; import org.apache.wicket.model.LambdaModel; @@ -24,7 +27,7 @@ public class OppositionReportPage extends AbstractProjectDetailsPage implements public static final String FILL_OUT_REPORT = "fillOutReport"; @Inject - private FinalSeminarOppositionRepo finalSeminarOppositionRepo; + private FinalSeminarOppositionService finalSeminarOppositionService; @Inject private OppositionReportService oppositionReportService; @@ -35,13 +38,15 @@ public class OppositionReportPage extends AbstractProjectDetailsPage implements throw new RestartResponseException(ProjectDetailsPage.class, pp); } - final FinalSeminarOpposition opposition = finalSeminarOppositionRepo.findOne(pp.get("oid").toLong()); + final IModel<Opposition> opposition = LoadableDetachableModel.of(() -> + finalSeminarOppositionService.getOpposition(pp.get("oid").toLong()) + ); - if (opposition == null) { + if (opposition.getObject() == null) { throw new RestartResponseException(ProjectDetailsPage.class, pp); } - final IModel<OppositionReport> report = getOppositionReport(opposition); + final IModel<OppositionReport> report = opposition.map(Opposition::report); add( new ViewAttachmentPanel( @@ -50,8 +55,35 @@ public class OppositionReportPage extends AbstractProjectDetailsPage implements ) ); + IModel<Opposition.ImprovementsNeeded> improvements = opposition + .map(Opposition::improvementsNeeded) + .map(OppositionReportPage::orNull); add( - new FillOutReportPanel<>(FILL_OUT_REPORT, report) { + new Label("improvements_requested_comment", improvements.map(Opposition.ImprovementsNeeded::comment)) { + @Override + protected void onConfigure() { + super.onConfigure(); + setVisible(!getDefaultModelObjectAsString().isBlank()); + } + } + ); + add( + new Label( + "improvements_requested_deadline", + improvements + .map(Opposition.ImprovementsNeeded::deadline) + .map(deadline -> deadline.atZone(ZoneId.systemDefault())) + ) { + @Override + protected void onConfigure() { + super.onConfigure(); + setVisible(getDefaultModelObject() != null); + } + } + ); + + add( + new FillOutReportPanel(FILL_OUT_REPORT, report) { { TextArea<String> textArea = new TextArea<>( THESIS_SUMMARY, @@ -71,18 +103,13 @@ public class OppositionReportPage extends AbstractProjectDetailsPage implements @Override protected void onConfigure() { super.onConfigure(); - setEnabled(opposition.getUser().equals(SciProSession.get().getUser())); + setEnabled(opposition.getObject().user().equals(SciProSession.get().getUser())); } } ); } - private IModel<OppositionReport> getOppositionReport(final FinalSeminarOpposition opposition) { - return new LoadableDetachableModel<>() { - @Override - protected OppositionReport load() { - return oppositionReportService.findOrCreateReport(opposition); - } - }; + private static <A> A orNull(Optional<A> optional) { + return optional.orElse(null); } } diff --git a/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanel.html b/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanel.html index 4467d0a779..7d665d4e31 100644 --- a/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanel.html +++ b/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanel.html @@ -11,7 +11,7 @@ <div wicket:id="container"> <div wicket:id="opponents"> <div class="row"> - <div class="col-lg-7 mb-3"> + <div class="col-lg-7"> <span wicket:id="user"></span><a href="#" wicket:id="remove"><span class="fa fa-times"></span></a><br> <div wicket:id="report"></div> </div> @@ -19,13 +19,31 @@ <div class="col-lg-5"> <form wicket:id="form"> - <div class="card mb-3 bg-info text-white"> - <wicket:message key="criteria"/> + <div class="card mb-3 text-bg-info"> + <div class="card-body"> + <p class="card-text"> + <wicket:message key="criteria"/> + </p> + <p class="card-text" wicket:id="requirements"> + <wicket:container wicket:id="requirement"/> + </p> + </div> </div> + <wicket:enclosure> + <div class="alert alert-info"> + <p> + You've requested improvements to the opposition report with the below comment. + If they do not make the requested improvements in time, they will get an automatic failing grade. + The system will notify you when they've submitted a new report. + </p> + <span wicket:id="improvements_requested"></span> + </div> + </wicket:enclosure> + <div class="mb-3"> <label>Points:</label> - <input type="text" class="form-control gradingPoints" wicket:id="points"/> + <select class="form-select" wicket:id="points"></select> </div> <label>Motivation:</label> @@ -34,6 +52,25 @@ <button wicket:id="submit" type="submit" class="btn btn-sm btn-success"> <wicket:message key="submit"/> </button> + <a class="btn btn-outline-secondary btn-sm" wicket:id="request_improvements"> + Request improvements + </a> + </form> + + <form wicket:id="request_improvements"> + <p> + Once you request improvements the student have a limited time to make the requested changes. + If they do not make the requested improvements in time, they will get an automatic failing grade. + You will be notified when they've submitted a new report. + </p> + <div class="mb-3"> + <label class="form-label" wicket:for="feedback_to_opponent"> + Provide feedback to the opponent on how to improve the opposition + </label> + <textarea class="form-control" wicket:id="feedback_to_opponent" rows="8"></textarea> + </div> + <button class="btn btn-sm btn-success">Request improvements</button> + <a class="btn btn-outline-secondary btn-sm" wicket:id="cancel">Cancel</a> </form> <div wicket:id="gradeContainer"> diff --git a/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanel.java b/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanel.java index 5315c6ff3d..9a788bc885 100644 --- a/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanel.java @@ -1,19 +1,23 @@ package se.su.dsv.scipro.finalseminar; -import com.google.common.eventbus.EventBus; import jakarta.inject.Inject; +import java.time.Instant; +import java.time.ZoneId; +import java.time.ZonedDateTime; import java.util.Date; import java.util.List; import java.util.Objects; 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.feedback.FencedFeedbackPanel; import org.apache.wicket.markup.html.WebMarkupContainer; import org.apache.wicket.markup.html.basic.Label; +import org.apache.wicket.markup.html.form.DropDownChoice; import org.apache.wicket.markup.html.form.Form; import org.apache.wicket.markup.html.form.FormComponent; +import org.apache.wicket.markup.html.form.LambdaChoiceRenderer; import org.apache.wicket.markup.html.form.TextArea; -import org.apache.wicket.markup.html.form.TextField; import org.apache.wicket.markup.html.link.Link; import org.apache.wicket.markup.html.list.ListItem; import org.apache.wicket.markup.html.list.ListView; @@ -22,14 +26,14 @@ import org.apache.wicket.markup.html.panel.FeedbackPanel; import org.apache.wicket.markup.html.panel.Panel; import org.apache.wicket.model.IModel; import org.apache.wicket.model.LambdaModel; +import org.apache.wicket.model.LoadableDetachableModel; +import org.apache.wicket.model.Model; import org.apache.wicket.model.ResourceModel; -import org.apache.wicket.validation.validator.RangeValidator; import org.apache.wicket.validation.validator.StringValidator; import se.su.dsv.scipro.components.ListAdapterModel; +import se.su.dsv.scipro.components.StatelessModel; import se.su.dsv.scipro.profile.UserLinkPanel; -import se.su.dsv.scipro.report.GradingReportService; import se.su.dsv.scipro.report.OppositionReportService; -import se.su.dsv.scipro.report.SupervisorGradingReport; import se.su.dsv.scipro.security.auth.roles.Roles; import se.su.dsv.scipro.session.SciProSession; import se.su.dsv.scipro.system.ProjectModule; @@ -43,8 +47,6 @@ public class SeminarOppositionPanel extends Panel { public static final String REMOVE = "remove"; public static final String FORM = "form"; public static final String POINTS = "points"; - public static final int MIN_POINTS = 0; - public static final int MAX_POINTS = 2; public static final String GRADING_FEEDBACK = "gradingFeedback"; public static final int FEEDBACK_MAX_LENGTH = 2000; public static final String SUBMIT = "submit"; @@ -55,18 +57,12 @@ public class SeminarOppositionPanel extends Panel { @Inject private FinalSeminarOppositionService finalSeminarOppositionService; - @Inject - private EventBus eventBus; - @Inject private ProjectTypeService projectTypeService; @Inject private FinalSeminarService finalSeminarService; - @Inject - private GradingReportService gradingReportService; - @Inject private OppositionReportService oppositionReportService; @@ -75,6 +71,9 @@ public class SeminarOppositionPanel extends Panel { private final WebMarkupContainer oppositionContainer; private final ListView<FinalSeminarOpposition> opponents; + private FinalSeminarOppositionForm gradeForm; + private RequestImprovementsForm requestImprovementsForm; + public SeminarOppositionPanel(String id, final IModel<FinalSeminar> seminar) { super(id, seminar); this.seminar = seminar; @@ -107,6 +106,12 @@ public class SeminarOppositionPanel extends Panel { private ListView<FinalSeminarOpposition> getOpponentsList(final IModel<List<FinalSeminarOpposition>> oppositions) { return new ListView<>(OPPONENTS, oppositions) { + { + // Need to reuse child list items since they contain form components + // and if they're recreated all the state and error messages are lost + setReuseItems(true); + } + @Override protected void populateItem(final ListItem<FinalSeminarOpposition> item) { final FinalSeminarOpposition opposition = item.getModelObject(); @@ -121,7 +126,14 @@ public class SeminarOppositionPanel extends Panel { item.add(getRemoveLink(item.getModel())); - item.add(getFinalSeminarOppositionForm(item)); + gradeForm = getFinalSeminarOppositionForm(item); + gradeForm.setOutputMarkupPlaceholderTag(true); + item.add(gradeForm); + + requestImprovementsForm = new RequestImprovementsForm("request_improvements", item.getModel()); + requestImprovementsForm.setVisible(false); + requestImprovementsForm.setOutputMarkupPlaceholderTag(true); + item.add(requestImprovementsForm); if (gradingModuleIsOnForProjectType()) { item.add(new SeminarOppositionReportPanel("report", item.getModel())); @@ -211,29 +223,47 @@ public class SeminarOppositionPanel extends Panel { private class FinalSeminarOppositionForm extends Form<FinalSeminarOpposition> { + private IModel<OppositionCriteria.Point> pointsModel = new StatelessModel<>(); + private IModel<String> feedbackModel = new Model<>(); + public FinalSeminarOppositionForm(String id, final IModel<FinalSeminarOpposition> finalSeminarOpposition) { super(id, finalSeminarOpposition); - FormComponent<Integer> pointsField = new TextField<>( + IModel<OppositionCriteria> criteriaModel = LoadableDetachableModel.of(() -> + finalSeminarOppositionService.getCriteriaForOpposition(finalSeminarOpposition.getObject()) + ); + + add( + new ListView<>("requirements", criteriaModel.map(this::getPointsWithRequirements)) { + @Override + protected void populateItem(ListItem<OppositionCriteria.Point> item) { + item.add(new Label("requirement", item.getModel().map(OppositionCriteria.Point::requirement))); + } + } + ); + + IModel<String> improvementComment = finalSeminarOpposition.map( + FinalSeminarOpposition::getSupervisorCommentForImprovements + ); + add( + new Label("improvements_requested", improvementComment) { + @Override + protected void onConfigure() { + super.onConfigure(); + setVisible(!getDefaultModelObjectAsString().isBlank()); + } + } + ); + + FormComponent<OppositionCriteria.Point> pointsField = new DropDownChoice<>( POINTS, - LambdaModel.of( - finalSeminarOpposition, - FinalSeminarOpposition::getPoints, - FinalSeminarOpposition::setPoints - ) - ) - .add(RangeValidator.range(MIN_POINTS, MAX_POINTS)) - .setType(Integer.class) - .setRequired(true); + pointsModel, + criteriaModel.map(OppositionCriteria::pointsAvailable), + new LambdaChoiceRenderer<>(OppositionCriteria.Point::value) + ); + pointsField.setRequired(true); add(pointsField); - TextArea<String> feedback = new TextArea<>( - GRADING_FEEDBACK, - LambdaModel.of( - finalSeminarOpposition, - FinalSeminarOpposition::getFeedback, - FinalSeminarOpposition::setFeedback - ) - ); + TextArea<String> feedback = new TextArea<>(GRADING_FEEDBACK, feedbackModel); feedback.add(StringValidator.maximumLength(FEEDBACK_MAX_LENGTH)); feedback.setRequired(true); add(feedback); @@ -242,33 +272,19 @@ public class SeminarOppositionPanel extends Panel { new AjaxSubmitLink(SUBMIT) { @Override protected void onSubmit(AjaxRequestTarget target) { - if (getModelObject().getPoints().equals(0)) { - finalSeminarOpposition.getObject().setGrade(FinalSeminarGrade.NOT_APPROVED); - eventBus.post(new OppositionFailedEvent(finalSeminarOpposition.getObject())); - } else { - finalSeminarOpposition.getObject().setGrade(FinalSeminarGrade.APPROVED); - eventBus.post(new OppositionApprovedEvent(finalSeminarOpposition.getObject())); - } - finalSeminarOppositionService.save(finalSeminarOpposition.getObject()); - boolean updated = true; - if (gradingModuleIsOnForProjectType()) { - SupervisorGradingReport report = gradingReportService.getSupervisorGradingReport( - finalSeminarOpposition.getObject().getProject(), - finalSeminarOpposition.getObject().getUser() - ); - updated = gradingReportService.updateOppositionCriteria( - report, - finalSeminarOpposition.getObject() + try { + finalSeminarOppositionService.gradeOpponent( + finalSeminarOpposition.getObject(), + pointsModel.getObject().value(), + feedbackModel.getObject() ); + success(getString("feedback.opponent.updated", finalSeminarOpposition)); + target.add(feedbackPanel); + target.add(oppositionContainer); + } catch (PointNotValidException e) { + error(getString("point.not.valid")); + target.add(feedbackPanel); } - success( - getString( - updated ? "feedback.opponent.updated" : "feedback.opponent.not.updated", - finalSeminarOpposition - ) - ); - target.add(feedbackPanel); - target.add(oppositionContainer); } @Override @@ -277,17 +293,47 @@ public class SeminarOppositionPanel extends Panel { } } ); + + add( + new AjaxLink<Void>("request_improvements") { + @Override + public void onClick(AjaxRequestTarget target) { + requestImprovementsForm.setVisible(true); + target.add(requestImprovementsForm); + gradeForm.setVisible(false); + target.add(gradeForm); + target.appendJavaScript( + "document.getElementById('" + + requestImprovementsForm.get("feedback_to_opponent").getMarkupId() + + "').focus();" + ); + } + } + ); + } + + private List<OppositionCriteria.Point> getPointsWithRequirements(OppositionCriteria oppositionCriteria) { + return oppositionCriteria + .pointsAvailable() + .stream() + .filter(point -> !point.requirement().isBlank()) + .toList(); } @Override protected void onConfigure() { super.onConfigure(); + FinalSeminarOpposition opposition = getModelObject(); setVisibilityAllowed( startDateHasPassed() && - getModelObject().getPoints() == null && - getModelObject().getFeedback() == null && + opposition.getPoints() == null && + opposition.getFeedback() == null && isHeadSupervisor() ); + boolean hasRequestedImprovements = opposition.getImprovementsRequestedAt() != null; + boolean reportIsSubmitted = + opposition.getOppositionReport() != null && opposition.getOppositionReport().isSubmitted(); + setEnabled(!hasRequestedImprovements || reportIsSubmitted); } } @@ -298,4 +344,47 @@ public class SeminarOppositionPanel extends Panel { private boolean hasSubmittedOppositionReport(FinalSeminarOpposition opposition) { return oppositionReportService.findOrCreateReport(opposition).isSubmitted(); } + + private class RequestImprovementsForm extends Form<FinalSeminarOpposition> { + + private final Model<String> feedbackToOpponentModel = new Model<>(); + + public RequestImprovementsForm(String id, IModel<FinalSeminarOpposition> model) { + super(id, model); + TextArea<String> feedbackToOpponentField = new TextArea<>("feedback_to_opponent", feedbackToOpponentModel); + feedbackToOpponentField.setRequired(true); + add(feedbackToOpponentField); + + add( + new AjaxLink<Void>("cancel") { + @Override + public void onClick(AjaxRequestTarget target) { + requestImprovementsForm.setVisible(false); + target.add(requestImprovementsForm); + gradeForm.setVisible(true); + target.add(gradeForm); + } + } + ); + } + + @Override + protected void onSubmit() { + Instant deadline = finalSeminarOppositionService.requestImprovements( + getModelObject(), + feedbackToOpponentModel.getObject() + ); + + record ImprovementFeedback(String fullName, ZonedDateTime deadline) {} + ZonedDateTime localDeadline = deadline.atZone(ZoneId.systemDefault()); + success( + getString("feedback.opponent.requested.improvements", () -> + new ImprovementFeedback(getModelObject().getUser().getFullName(), localDeadline) + ) + ); + + requestImprovementsForm.setVisible(false); + gradeForm.setVisible(true); + } + } } diff --git a/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanel.properties b/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanel.properties index 4976fecc4d..0a041037c6 100644 --- a/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanel.properties +++ b/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanel.properties @@ -8,16 +8,15 @@ gradingFeedback.Required = You need to write a motivation points.Required = Points are required opponents.form.points.RangeValidator.range= Points assigned must be between ${minimum} and ${maximum} feedback.opponent.updated= Opponent ${user.fullName} feedback updated. +point.not.valid=You need to assign points from the available selection. feedback.opponent.not.updated= Opponent ${user.fullName} feedback updated. Point and motivation could not be transferred to the students final grading report since the opponents supervisor already filled it in. -criteria= As the supervisor on this final seminar you are also required to grade the opponents opposition report.\ - <\br><\br>Requirement for 1 point: <\br>That the opposition report provides a short summary of the evaluated thesis, that it deliberates about the scientific basis, originality, \ - significance, and formulation of the problem and research question, as well as that it contains clear suggestions for improvements. \ - <\br><\br>For 2 points the following is also required: \ - <\br>That the opposition report thoroughly and in a well-balanced way describes from numerous aspects the strengths and weaknesses of the evaluated thesis and that it \ - offers clear and well- motivated suggestions for improvements. +criteria= As the supervisor on this final seminar you are also required to grade the opponents opposition report. opposition.report= Opposition report: removed= Opponent ${user.fullName} successfully removed opposition.report.removed= Opposition report successfully removed are.you.sure= Are you sure you want to remove this opponent report? no.opponents= There are no opponents registered yet. -noOppositionReportYet= No opposition report has been submitted yet. \ No newline at end of file +noOppositionReportYet= No opposition report has been submitted yet. +feedback.opponent.requested.improvements = You've requested improvements from ${fullName}. \ + They have until ${deadline} to make the changes. If they fail to resubmit by that point they \ + will automatically get a failing grade. diff --git a/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionReportPanel.html b/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionReportPanel.html index b490fdc9db..5327a3cac0 100644 --- a/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionReportPanel.html +++ b/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionReportPanel.html @@ -7,9 +7,13 @@ <div wicket:id="wmc"> <wicket:enclosure child="newReport"> <wicket:container wicket:id="newReport"/> + <div class="alert alert-info mt-1 mb-1" wicket:id="improvements_requested"> + The supervisor has requested improvements to your opposition report. + Click the link below to see detailed comments from the supervisor and to make the requested changes. + </div> <span wicket:id="oppositionReportLabel"></span> <span wicket:id="noOppositionReportYet"></span> <a href="#" wicket:id="oppositionReportLink">Fill out opposition report</a> - <span wicket:id="downloadPdfPanel"></span><br /> + <div wicket:id="downloadPdfPanel"></div> <wicket:enclosure child="downloadAttachment"> Report attachment: <span wicket:id="downloadAttachment"></span> </wicket:enclosure> diff --git a/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionReportPanel.java b/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionReportPanel.java index fd975f51ad..38fc6aeb41 100644 --- a/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionReportPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionReportPanel.java @@ -70,6 +70,19 @@ public class SeminarOppositionReportPanel extends GenericPanel<FinalSeminarOppos wmc.add(getDeleteOpponentReportLink(model)); wmc.add(getDeleteOppositionReportLink(model)); + + wmc.add( + new WebMarkupContainer("improvements_requested") { + @Override + protected void onConfigure() { + super.onConfigure(); + FinalSeminarOpposition opp = model.getObject(); + boolean notGraded = opp.getGrade() == null; + boolean improvementsRequested = opp.getImprovementsRequestedAt() != null; + setVisible(isOpponentAndNotSubmitted(opp) && notGraded && improvementsRequested); + } + } + ); } private Component getNewReportContainer(ViewAttachmentPanel oldReport) { diff --git a/view/src/main/java/se/su/dsv/scipro/grading/FillOutReportPanel.html b/view/src/main/java/se/su/dsv/scipro/grading/FillOutReportPanel.html index c18fd20db6..6877da1204 100644 --- a/view/src/main/java/se/su/dsv/scipro/grading/FillOutReportPanel.html +++ b/view/src/main/java/se/su/dsv/scipro/grading/FillOutReportPanel.html @@ -5,30 +5,26 @@ </head> <body> <wicket:border> - <div class="row"> - <div class="col-lg-8"> - <div wicket:id="save"></div> - <form wicket:id="form"> - <div wicket:id="feedbackPanel"></div> - <wicket:body/> - <div wicket:id="criteria"> - <strong><span wicket:id="title"></span></strong> + <div wicket:id="save"></div> + <form wicket:id="form"> + <div wicket:id="feedbackPanel"></div> + <wicket:body/> + <div wicket:id="criteria"> + <strong><span wicket:id="title"></span></strong> - <p><span wicket:id="description" class="gradingCriteria"></span></p> - <textarea class="form-control mb-4" rows="8" cols="5" wicket:id="feedback"></textarea> - </div> - <div> - <strong><wicket:message key="attachment" /></strong> - <span wicket:id="viewAttachment"></span> - <a wicket:id="deleteAttachment"><span class="fa fa-times"></span></a> - <input type="file" wicket:id="uploadAttachment" class="mb-3"/> - </div> - <button type="button" class="btn btn-success btn-sm mb-3" wicket:id="submit"> - Submit - </button> - </form> + <p><span wicket:id="description" class="gradingCriteria"></span></p> + <textarea class="form-control mb-4" rows="8" cols="5" wicket:id="feedback"></textarea> </div> - </div> + <div> + <strong><wicket:message key="attachment" /></strong> + <span wicket:id="viewAttachment"></span> + <a wicket:id="deleteAttachment"><span class="fa fa-times"></span></a> + <input type="file" wicket:id="uploadAttachment" class="mb-3"/> + </div> + <button type="button" class="btn btn-success btn-sm mb-3" wicket:id="submit"> + Submit + </button> + </form> </wicket:border> </body> </html> \ No newline at end of file diff --git a/view/src/main/java/se/su/dsv/scipro/grading/FillOutReportPanel.java b/view/src/main/java/se/su/dsv/scipro/grading/FillOutReportPanel.java index 95bfd72a34..e35ab7c896 100644 --- a/view/src/main/java/se/su/dsv/scipro/grading/FillOutReportPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/grading/FillOutReportPanel.java @@ -25,12 +25,12 @@ import se.su.dsv.scipro.files.WicketFileUpload; import se.su.dsv.scipro.report.AttachmentReport; import se.su.dsv.scipro.report.Criterion; import se.su.dsv.scipro.report.OppositionReport; -import se.su.dsv.scipro.report.ReportService; +import se.su.dsv.scipro.report.OppositionReportService; import se.su.dsv.scipro.repository.panels.ViewAttachmentPanel; import se.su.dsv.scipro.system.Language; import se.su.dsv.scipro.util.JavascriptEventConfirmation; -public class FillOutReportPanel<T extends OppositionReport> extends Border { +public class FillOutReportPanel extends Border { public static final String FORM = "form"; public static final String GRADING_CRITERIA = "criteria"; @@ -42,20 +42,20 @@ public class FillOutReportPanel<T extends OppositionReport> extends Border { public static final String FEEDBACK_PANEL = "feedbackPanel"; @Inject - private ReportService reportService; + private OppositionReportService reportService; - public FillOutReportPanel(String id, final IModel<T> model) { + public FillOutReportPanel(String id, final IModel<OppositionReport> model) { super(id, model); ReportForm form = new ReportForm(FORM, model); addToBorder(new ScrollingSaveButtonPanel(SAVE, form)); addToBorder(form); } - private class ReportForm extends StatelessForm<T> { + private class ReportForm extends StatelessForm<OppositionReport> { private final FileUploadField attachment; - public ReportForm(String id, final IModel<T> model) { + public ReportForm(String id, final IModel<OppositionReport> model) { super(id, model); add(new ComponentFeedbackPanel(FEEDBACK_PANEL, this)); IModel<Language> language = model.map(OppositionReport::getLanguage); @@ -139,9 +139,9 @@ public class FillOutReportPanel<T extends OppositionReport> extends Border { } } - private class DeleteAttachmentLink extends Link<T> { + private class DeleteAttachmentLink extends Link<OppositionReport> { - public DeleteAttachmentLink(String id, IModel<T> model) { + public DeleteAttachmentLink(String id, IModel<OppositionReport> model) { super(id, model); add(new JavascriptEventConfirmation("click", new ResourceModel("delete.attachment"))); } 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 4a999d66f4..743fb8b158 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 @@ -10,6 +10,8 @@ import org.apache.wicket.util.string.StringValueConversionException; import se.su.dsv.scipro.activityplan.ProjectActivityPlanPage; import se.su.dsv.scipro.activityplan.SupervisorActivityPlanPage; import se.su.dsv.scipro.finalseminar.FinalSeminar; +import se.su.dsv.scipro.finalseminar.FinalSeminarOpposition; +import se.su.dsv.scipro.finalseminar.OppositionReportPage; import se.su.dsv.scipro.finalseminar.ProjectFinalSeminarDetailsPage; import se.su.dsv.scipro.finalseminar.ProjectFinalSeminarPage; import se.su.dsv.scipro.finalseminar.ProjectOppositionPage; @@ -217,6 +219,19 @@ public class NotificationLandingPage extends WebPage { } else if ( seminar.getActiveParticipants().contains(currentUser) || seminar.getOpponents().contains(currentUser) ) { + if (seminarEvent.getEvent() == SeminarEvent.Event.OPPOSITION_REPORT_IMPROVEMENTS_REQUESTED) { + Optional<FinalSeminarOpposition> opposition = seminar + .getOppositions() + .stream() + .filter(op -> op.getUser().equals(currentUser)) + .findFirst(); + if (opposition.isPresent()) { + final PageParameters oppPP = new PageParameters(); + oppPP.set("oid", opposition.get().getId()); + setResponsePage(OppositionReportPage.class, oppPP); + return; + } + } setResponsePage(ProjectFinalSeminarDetailsPage.class, pp); } } 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 97ea5d57c3..8a9ded9954 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 @@ -83,6 +83,9 @@ SeminarEvent.OPPOSITION_REPORT_UPLOADED = Opposition report created. SeminarEvent.THESIS_DELETED = Final seminar thesis deleted. SeminarEvent.THESIS_UPLOAD_REMIND = Authors reminded to upload final seminar thesis. SeminarEvent.CANCELLED = Final seminar cancelled. +SeminarEvent.OPPOSITION_REPORT_SUBMITTED = Opposition report submitted. +SeminarEvent.OPPOSITION_REPORT_IMPROVEMENTS_REQUESTED = Opposition report improvements requested. +SeminarEvent.OPPOSITION_APPROVED = Opposition approved. IdeaEvent.STATUS_CHANGE = Idea status changed. IdeaEvent.PARTNER_ACCEPT = Partner (author) accepted partnering idea. 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 10b02264f4..d83e739cb0 100755 --- a/view/src/test/java/se/su/dsv/scipro/SciProTest.java +++ b/view/src/test/java/se/su/dsv/scipro/SciProTest.java @@ -327,9 +327,6 @@ public abstract class SciProTest { @Mock protected FinalSeminarUploadController finalSeminarUploadController; - @Mock - protected FinalSeminarOppositionRepo finalSeminarOppositionRepo; - @Mock protected PlagiarismControl plagiarismControl; diff --git a/view/src/test/java/se/su/dsv/scipro/finalseminar/OppositionReportPageTest.java b/view/src/test/java/se/su/dsv/scipro/finalseminar/OppositionReportPageTest.java index 6e9d239fa4..aeb5e9c9ef 100644 --- a/view/src/test/java/se/su/dsv/scipro/finalseminar/OppositionReportPageTest.java +++ b/view/src/test/java/se/su/dsv/scipro/finalseminar/OppositionReportPageTest.java @@ -17,7 +17,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatchers; -import org.mockito.Mock; import org.mockito.Mockito; import se.su.dsv.scipro.SciProTest; import se.su.dsv.scipro.file.FileDescription; @@ -28,7 +27,6 @@ import se.su.dsv.scipro.project.pages.ProjectDetailsPage; import se.su.dsv.scipro.report.GradingCriterionPointTemplate; import se.su.dsv.scipro.report.GradingReportTemplate; import se.su.dsv.scipro.report.OppositionReport; -import se.su.dsv.scipro.report.ReportService; import se.su.dsv.scipro.system.DegreeType; import se.su.dsv.scipro.system.ProjectType; import se.su.dsv.scipro.system.User; @@ -41,9 +39,6 @@ public class OppositionReportPageTest extends SciProTest { public static final String CRITERION_DESCRIPTION = "For 1 point: Be nice to your supervisor"; public static final String CRITERTION_TITLE = "U1 Sammanfattning"; - @Mock - private ReportService reportService; - private FinalSeminarOpposition finalSeminarOpposition; private User user; private ProjectType bachelor; @@ -76,10 +71,9 @@ public class OppositionReportPageTest extends SciProTest { Mockito.when(finalSeminarService.findByProject(opponentsProject)).thenReturn(finalSeminar); } - private void mockReport(ProjectType bachelor) { + private OppositionReport mockReport(ProjectType bachelor) { GradingReportTemplate reportTemplate = createTemplate(bachelor); - OppositionReport oppositionReport = reportTemplate.createOppositionReport(finalSeminarOpposition); - Mockito.when(oppositionReportService.findOrCreateReport(finalSeminarOpposition)).thenReturn(oppositionReport); + return reportTemplate.createOppositionReport(finalSeminarOpposition); } @Test @@ -104,14 +98,16 @@ public class OppositionReportPageTest extends SciProTest { public void disable_form_if_opposition_does_not_belong_to_logged_in_user() { mockReport(bachelor); long oppositionId = 4L; - Mockito.when(finalSeminarOppositionRepo.findOne(oppositionId)).thenReturn(finalSeminarOpposition); + Mockito.when(finalSeminarOppositionService.getOpposition(oppositionId)).thenReturn( + new Opposition(user, mockReport(bachelor), Optional.empty()) + ); startPage(oppositionId); tester.assertDisabled(FILL_OUT_REPORT); } @Test public void redirect_if_no_opposition_is_found_from_id() { - Mockito.when(finalSeminarOppositionRepo.findOne(ArgumentMatchers.anyLong())).thenReturn(null); + Mockito.when(finalSeminarOppositionService.getOpposition(ArgumentMatchers.anyLong())).thenReturn(null); startPage(1L); tester.assertRenderedPage(ProjectDetailsPage.class); } @@ -138,7 +134,7 @@ public class OppositionReportPageTest extends SciProTest { formTester.submit(); ArgumentCaptor<OppositionReport> captor = ArgumentCaptor.forClass(OppositionReport.class); - Mockito.verify(reportService).save(captor.capture(), eq(Optional.empty())); + Mockito.verify(oppositionReportService).save(captor.capture(), eq(Optional.empty())); Assertions.assertEquals(summary, captor.getValue().getThesisSummary()); } @@ -156,7 +152,9 @@ public class OppositionReportPageTest extends SciProTest { private void startOppositionPage() { long oppositionId = 4L; setLoggedInAs(user); - Mockito.when(finalSeminarOppositionRepo.findOne(oppositionId)).thenReturn(finalSeminarOpposition); + Mockito.when(finalSeminarOppositionService.getOpposition(oppositionId)).thenReturn( + new Opposition(user, mockReport(bachelor), Optional.empty()) + ); startPage(oppositionId); } diff --git a/view/src/test/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanelTest.java b/view/src/test/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanelTest.java index 66e5c2224d..587e4559d2 100644 --- a/view/src/test/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanelTest.java +++ b/view/src/test/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanelTest.java @@ -61,6 +61,15 @@ public class SeminarOppositionPanelTest extends SciProTest { finalSeminar.setProject(project); setLoggedInAs(supervisorUser); + + Mockito.lenient() + .when(finalSeminarOppositionService.getCriteriaForOpposition(opposition)) + .thenReturn( + new OppositionCriteria( + 1, + List.of(new OppositionCriteria.Point(0, ""), new OppositionCriteria.Point(1, "Filled in report")) + ) + ); } @Test diff --git a/view/src/test/java/se/su/dsv/scipro/finalseminar/SeminarPanelTest.java b/view/src/test/java/se/su/dsv/scipro/finalseminar/SeminarPanelTest.java index 2edb1a7727..da4f5a63ca 100644 --- a/view/src/test/java/se/su/dsv/scipro/finalseminar/SeminarPanelTest.java +++ b/view/src/test/java/se/su/dsv/scipro/finalseminar/SeminarPanelTest.java @@ -66,6 +66,14 @@ public class SeminarPanelTest extends SciProTest { Mockito.when(plagiarismControl.getStatus(any(FileDescription.class))).thenReturn( new PlagiarismControl.Status.NotSubmitted() ); + Mockito.lenient() + .when(finalSeminarOppositionService.getCriteriaForOpposition(opposition)) + .thenReturn( + new OppositionCriteria( + 1, + List.of(new OppositionCriteria.Point(0, ""), new OppositionCriteria.Point(1, "Filled in report")) + ) + ); } private void addCoSupervisorToProject() { diff --git a/view/src/test/java/se/su/dsv/scipro/grading/FillOutReportPanelTest.java b/view/src/test/java/se/su/dsv/scipro/grading/FillOutReportPanelTest.java index e8e9367d12..5444ce2902 100644 --- a/view/src/test/java/se/su/dsv/scipro/grading/FillOutReportPanelTest.java +++ b/view/src/test/java/se/su/dsv/scipro/grading/FillOutReportPanelTest.java @@ -35,7 +35,7 @@ public class FillOutReportPanelTest extends SciProTest { private FillOutReportPanel panel; @Mock - private ReportService reportService; + private OppositionReportService reportService; @BeforeEach public void setUp() throws Exception { @@ -197,6 +197,6 @@ public class FillOutReportPanelTest extends SciProTest { } private void startPanel() { - panel = tester.startComponentInPage(new FillOutReportPanel<>("id", Model.of(oppositionReport))); + panel = tester.startComponentInPage(new FillOutReportPanel("id", Model.of(oppositionReport))); } } diff --git a/war/src/main/java/se/su/dsv/scipro/war/WorkerConfig.java b/war/src/main/java/se/su/dsv/scipro/war/WorkerConfig.java index 4acdf186a7..69e4d9050a 100644 --- a/war/src/main/java/se/su/dsv/scipro/war/WorkerConfig.java +++ b/war/src/main/java/se/su/dsv/scipro/war/WorkerConfig.java @@ -13,6 +13,8 @@ import org.springframework.context.annotation.Import; import org.springframework.context.annotation.Scope; import org.springframework.transaction.PlatformTransactionManager; import se.su.dsv.scipro.file.FileService; +import se.su.dsv.scipro.finalseminar.ExpireUnfulfilledOppositionImprovementsWorker; +import se.su.dsv.scipro.finalseminar.FinalSeminarOppositionServiceImpl; import se.su.dsv.scipro.finalseminar.FinalSeminarService; import se.su.dsv.scipro.firstmeeting.FirstMeetingReminderWorker; import se.su.dsv.scipro.firstmeeting.FirstMeetingService; @@ -150,6 +152,14 @@ public class WorkerConfig { return new SpringManagedWorkerTransactions(platformTransactionManager); } + @Bean + public ExpireUnfulfilledOppositionImprovementsWorker.Schedule expireUnfulfilledOppositionImprovementsWorkerSchedule( + Scheduler scheduler, + Provider<ExpireUnfulfilledOppositionImprovementsWorker> worker + ) { + return new ExpireUnfulfilledOppositionImprovementsWorker.Schedule(scheduler, worker); + } + @Configuration public static class Workers { @@ -279,5 +289,12 @@ public class WorkerConfig { public ExpiredRequestWorker expiredRequestWorker() { return new ExpiredRequestWorker(); } + + @Bean + public ExpireUnfulfilledOppositionImprovementsWorker expireUnfulfilledOppositionImprovementsWorker( + FinalSeminarOppositionServiceImpl finalSeminarOppositionService + ) { + return new ExpireUnfulfilledOppositionImprovementsWorker(finalSeminarOppositionService); + } } } From 1aa0a4e3eff23d0326fd455c53a9a838ff7b137e Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Wed, 5 Mar 2025 11:01:37 +0100 Subject: [PATCH 2/2] Improve the UX when creating groups as a supervisor (#123) The main problem was that the supervisor did not get enough information about each project, mainly who the authors were, when selecting them in the dropdown. To remedy this, the dropdown has been completely replaced with a checkbox based approach showing the title as well as project type, authors, and start date for each project. The projects are sorted first by start date (descending) and then title, based on the assumptions that newly created projects are the most relevant when setting up groups. In addition extra "quick buttons" have been added in an effort to reduce the number of clicks required to accomplish varying tasks. Fixes #89 ## How to test 1. Log in as `evan@example.com` 2. Go to "My groups" 3. Click "Create new group" Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/123 Reviewed-by: Nico Athanassiadis <nico@dsv.su.se> Co-authored-by: Andreas Svanberg <andreass@dsv.su.se> Co-committed-by: Andreas Svanberg <andreass@dsv.su.se> --- .../GroupCreationUXImprovement.java | 65 ++++++ .../su/dsv/scipro/group/EditGroupPanel.html | 107 ++++------ .../su/dsv/scipro/group/EditGroupPanel.java | 188 +++++++----------- view/src/main/webapp/css/scipro_m.css | 24 +++ .../dsv/scipro/group/EditGroupPanelTest.java | 1 - 5 files changed, 201 insertions(+), 184 deletions(-) create mode 100644 test-data/src/main/java/se/su/dsv/scipro/testdata/populators/GroupCreationUXImprovement.java diff --git a/test-data/src/main/java/se/su/dsv/scipro/testdata/populators/GroupCreationUXImprovement.java b/test-data/src/main/java/se/su/dsv/scipro/testdata/populators/GroupCreationUXImprovement.java new file mode 100644 index 0000000000..7d3c30523a --- /dev/null +++ b/test-data/src/main/java/se/su/dsv/scipro/testdata/populators/GroupCreationUXImprovement.java @@ -0,0 +1,65 @@ +package se.su.dsv.scipro.testdata.populators; + +import jakarta.inject.Inject; +import java.time.LocalDate; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import org.springframework.stereotype.Service; +import se.su.dsv.scipro.project.Project; +import se.su.dsv.scipro.project.ProjectService; +import se.su.dsv.scipro.system.ProjectType; +import se.su.dsv.scipro.system.User; +import se.su.dsv.scipro.testdata.BaseData; +import se.su.dsv.scipro.testdata.Factory; +import se.su.dsv.scipro.testdata.TestDataPopulator; + +@Service +public class GroupCreationUXImprovement implements TestDataPopulator { + + private static final String[] STUDENT_NAMES = { "Alice", "Bob", "Charlie", "David", "Emma" }; + + private final ProjectService projectService; + + @Inject + public GroupCreationUXImprovement(ProjectService projectService) { + this.projectService = projectService; + } + + @Override + public void populate(BaseData baseData, Factory factory) { + User supervisor = factory.createSupervisor("Evan"); + List<User> students = createStudents(factory); + for (int i = 1; i <= 20; i++) { + projectService.save(createProject(baseData, i, supervisor, students)); + } + } + + private List<User> createStudents(Factory factory) { + return Arrays.stream(STUDENT_NAMES).map(factory::createAuthor).toList(); + } + + private Project createProject(BaseData baseData, int i, User supervisor, List<User> students) { + User author1 = students.get(i % students.size()); + User author2 = students.get((i + 1) % students.size()); + + String title = "Test project " + i; + if (i % 6 == 0) { + title = title + " with a very long title that makes the project special"; + } + + ProjectType projectType = + switch (i % 3) { + case 1 -> baseData.magister(); + case 2 -> baseData.master(); + default -> baseData.bachelor(); + }; + return Project.builder() + .title(title) + .projectType(projectType) + .startDate(LocalDate.now()) + .headSupervisor(supervisor) + .projectParticipants(Set.of(author1, author2)) + .build(); + } +} diff --git a/view/src/main/java/se/su/dsv/scipro/group/EditGroupPanel.html b/view/src/main/java/se/su/dsv/scipro/group/EditGroupPanel.html index f6f9edc0a7..e0db52aaa7 100644 --- a/view/src/main/java/se/su/dsv/scipro/group/EditGroupPanel.html +++ b/view/src/main/java/se/su/dsv/scipro/group/EditGroupPanel.html @@ -2,85 +2,50 @@ <html xmlns="http://www.w3.org/1999/xhtml" xmlns:wicket="http://wicket.apache.org"> <body> <wicket:panel> - <div class="row"> - <div class="col-lg-12"> - <form wicket:id="form"> + <div class="line-length-limit"> + <form wicket:id="form"> - <div class="row"> - <div class="col-lg-12"> - <div wicket:id="feedback"></div> - </div> - </div> + <div wicket:id="feedback"></div> - <div class="row"> - <div class="col-lg-5 col-md-10"> - <label wicket:for="title">Title: </label> - <input type="text" wicket:id="title" class="form-control"> - </div> - </div> + <div class="mb-3"> + <label wicket:for="title" class="form-label">Title</label> + <input type="text" wicket:id="title" class="form-control"> + </div> - <div class="row"> - <div class="col-lg-5 col-md-10"> - <label wicket:for="description">Description: </label> - <textarea wicket:id="description" class="form-control"></textarea> - </div> - </div> + <div class="mb-3"> + <label wicket:for="description" class="form-label">Description</label> + <textarea wicket:id="description" class="form-control"></textarea> + </div> - <div class="row"> - <div class="col-lg-5 col-md-10"> - <div class="form-check"> - <input class="form-check-input" wicket:id="active" type="checkbox"/> - <label class="form-check-label" wicket:for="active">Active</label> + <div class="form-check mb-3"> + <input class="form-check-input" wicket:id="active" type="checkbox"/> + <label class="form-check-label" wicket:for="active">Active</label> + </div> + + <fieldset class="mb-3"> + <legend>Projects</legend> + <div class="group-project-grid"> + <label wicket:id="available_projects"> + <div> + <input class="form-check-input mt-0" type="checkbox" wicket:id="selected"> </div> - </div> - </div> - - <div class="row"> - <div class="col-lg-12"> - <div wicket:id="wmc"> - - <div class="row"> - <div class="col-lg-5 col-md-10"> - <strong>Add projects to group: </strong> - <div wicket:id="projectTypes"></div> - <select class="form-select" wicket:id="addProjects"></select> - </div> + <div> + <h4 wicket:id="title"></h4> + <span wicket:id="type"></span> + <br> + Started at <span wicket:id="start_date"></span> + <div wicket:id="authors"> + <span wicket:id="author"></span> </div> - - <div class="row"> - <div class="col-lg-12"> - <strong>Projects in group: </strong> - <table class="table table-striped table-hover"> - <thead> - <tr> - <th>Type</th> - <th>Title</th> - <th>Authors</th> - <th>Remove</th> - </tr> - </thead> - <tbody> - <tr wicket:id="projects"> - <td><span wicket:id="type"></span></td> - <td><span wicket:id="title"></span></td> - <td><div wicket:id="authors"> - <div wicket:id="author"></div> - </div></td> - <td><a wicket:id="remove"><span class="fa fa-times"></span></a></td> - </tr> - </tbody> - </table> - <div wicket:id="noProjects"></div> - </div> - </div> - </div> - </div> + </label> </div> - <br> - <button type="submit" class="btn btn-success" >Save</button> - </form> - </div> + </fieldset> + <button type="submit" class="btn btn-success">Save</button> + <button type="submit" wicket:id="save_and_close" class="btn btn-success">Save and close</button> + <button type="submit" wicket:id="save_and_create" class="btn btn-success">Save and create another</button> + <a wicket:id="cancel" class="btn btn-outline-secondary">Cancel</a> + </form> </div> </wicket:panel> </body> diff --git a/view/src/main/java/se/su/dsv/scipro/group/EditGroupPanel.java b/view/src/main/java/se/su/dsv/scipro/group/EditGroupPanel.java index 853cedc848..831986ffad 100644 --- a/view/src/main/java/se/su/dsv/scipro/group/EditGroupPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/group/EditGroupPanel.java @@ -2,11 +2,10 @@ package se.su.dsv.scipro.group; import jakarta.inject.Inject; import java.util.*; -import org.apache.wicket.ajax.AjaxRequestTarget; -import org.apache.wicket.ajax.markup.html.AjaxLink; -import org.apache.wicket.markup.html.WebMarkupContainer; +import org.apache.wicket.extensions.model.AbstractCheckBoxModel; import org.apache.wicket.markup.html.basic.Label; import org.apache.wicket.markup.html.form.*; +import org.apache.wicket.markup.html.link.BookmarkablePageLink; import org.apache.wicket.markup.html.list.ListItem; import org.apache.wicket.markup.html.list.ListView; import org.apache.wicket.markup.html.panel.FeedbackPanel; @@ -14,10 +13,6 @@ import org.apache.wicket.markup.html.panel.Panel; import org.apache.wicket.model.IModel; import org.apache.wicket.model.LambdaModel; import org.apache.wicket.model.LoadableDetachableModel; -import org.apache.wicket.model.Model; -import org.apache.wicket.model.util.ListModel; -import se.su.dsv.scipro.components.AjaxCheckBoxMultipleChoice; -import se.su.dsv.scipro.components.AjaxDropDownChoice; import se.su.dsv.scipro.components.ListAdapterModel; import se.su.dsv.scipro.profile.UserLinkPanel; import se.su.dsv.scipro.project.Project; @@ -25,8 +20,8 @@ import se.su.dsv.scipro.project.ProjectService; import se.su.dsv.scipro.project.ProjectStatus; import se.su.dsv.scipro.project.ProjectTeamMemberRoles; import se.su.dsv.scipro.session.SciProSession; -import se.su.dsv.scipro.system.ProjectType; -import se.su.dsv.scipro.system.ProjectTypeService; +import se.su.dsv.scipro.supervisor.pages.SupervisorEditGroupPage; +import se.su.dsv.scipro.supervisor.pages.SupervisorMyGroupsPage; import se.su.dsv.scipro.system.User; public class EditGroupPanel extends Panel { @@ -37,9 +32,6 @@ public class EditGroupPanel extends Panel { @Inject private GroupService groupService; - @Inject - private ProjectTypeService projectTypeService; - public EditGroupPanel(String id, final IModel<Group> model) { super(id, model); add(new GroupForm("form", model)); @@ -47,18 +39,46 @@ public class EditGroupPanel extends Panel { private class GroupForm extends Form<Group> { - private final AjaxDropDownChoice<Project> addProjects; - private final ListView<Project> projects; - private final List<Project> currentProjects; - private final AjaxCheckBoxMultipleChoice<ProjectType> projectTypes; - public GroupForm(String form, final IModel<Group> model) { super(form, model); final FeedbackPanel feedbackPanel = new FeedbackPanel("feedback"); feedbackPanel.setOutputMarkupId(true); add(feedbackPanel); - currentProjects = new ArrayList<>(getModelObject().getProjects()); + IModel<List<Project>> availableProjects = LoadableDetachableModel.of(() -> { + Set<Project> projects = new HashSet<>(); + projects.addAll(getAllRelevantProjects()); + // Have to add the projects that are already in the group to the list of available projects + // since they may not be included in the relevant projects if they're inactive or completed. + // To allow them to be removed from the group, it will not be possible to add them again. + projects.addAll(model.getObject().getProjects()); + return projects + .stream() + .sorted(Comparator.comparing(Project::getStartDate).reversed().thenComparing(Project::getTitle)) + .toList(); + }); + add( + new ListView<>("available_projects", availableProjects) { + @Override + protected void populateItem(ListItem<Project> item) { + CheckBox checkbox = new CheckBox("selected", new SelectProjectModel(model, item.getModel())); + checkbox.setOutputMarkupId(true); + item.add(checkbox); + item.add(new Label("title", item.getModel().map(Project::getTitle))); + item.add(new Label("type", item.getModel().map(Project::getProjectTypeName))); + item.add(new Label("start_date", item.getModel().map(Project::getStartDate))); + IModel<SortedSet<User>> authors = item.getModel().map(Project::getProjectParticipants); + item.add( + new ListView<>("authors", new ListAdapterModel<>(authors)) { + @Override + protected void populateItem(ListItem<User> item) { + item.add(new UserLinkPanel("author", item.getModel())); + } + } + ); + } + } + ); add(new RequiredTextField<>("title", LambdaModel.of(model, Group::getTitle, Group::setTitle))); add(new TextArea<>("description", LambdaModel.of(model, Group::getDescription, Group::setDescription))); @@ -66,120 +86,64 @@ public class EditGroupPanel extends Panel { new CheckBox("active", LambdaModel.of(model, Group::isActive, Group::setActive)).setOutputMarkupId(true) ); - final WebMarkupContainer wmc = new WebMarkupContainer("wmc"); - wmc.setOutputMarkupId(true); - - projectTypes = projectTypeSelection(wmc); - wmc.add(projectTypes); - - addProjects = new AjaxDropDownChoice<>( - "addProjects", - new Model<>(), - getSelectableProjects(currentProjects), - new LambdaChoiceRenderer<>(Project::getTitle, Project::getId) - ) { - @Override - public void onNewSelection(AjaxRequestTarget target, Project objectSelected) { - if (objectSelected != null && !currentProjects.contains(objectSelected)) { - currentProjects.add(objectSelected); - projects.setList(currentProjects); - addProjects.setChoices(getSelectableProjects(currentProjects)); - target.add(wmc); - } - } - }; - addProjects.setRequired(false); - addProjects.setNullValid(true); - wmc.add(addProjects); - - projects = new ListView<>("projects", new ArrayList<>(currentProjects)) { - @Override - protected void populateItem(final ListItem<Project> item) { - item.add(new Label("type", item.getModel().map(Project::getProjectTypeName))); - item.add(new Label("title", item.getModel().map(Project::getTitle))); - item.add( - new ListView<>( - "authors", - new ListAdapterModel<>( - getLoaded(item.getModelObject()).map(Project::getProjectParticipants) - ) - ) { - @Override - public void populateItem(ListItem<User> item) { - item.add(new UserLinkPanel("author", item.getModel())); - } - } - ); - item.add( - new AjaxLink<>("remove", item.getModel()) { - @Override - public void onClick(AjaxRequestTarget target) { - currentProjects.remove(item.getModelObject()); - projects.setList(currentProjects); - addProjects.setChoices(getSelectableProjects(currentProjects)); - target.add(wmc); - } - } - ); - } - }; - wmc.add(projects); - - wmc.add( - new Label("noProjects", "None") { + add( + new SubmitLink("save_and_close") { @Override - protected void onConfigure() { - super.onConfigure(); - setVisibilityAllowed(currentProjects.isEmpty()); + public void onAfterSubmit() { + setResponsePage(SupervisorMyGroupsPage.class); } } ); - - add(wmc); - } - - private AjaxCheckBoxMultipleChoice<ProjectType> projectTypeSelection(final WebMarkupContainer wmc) { - return new AjaxCheckBoxMultipleChoice<>( - "projectTypes", - projectTypeService.findAllActive(), - projectTypeService.findAllActive(), - new LambdaChoiceRenderer<>(ProjectType::getName, ProjectType::getId) - ) { - @Override - public void onUpdate(AjaxRequestTarget target) { - addProjects.setChoices(getSelectableProjects(currentProjects)); - target.add(wmc); + add( + new SubmitLink("save_and_create") { + @Override + public void onAfterSubmit() { + setResponsePage(SupervisorEditGroupPage.class); + } } - }; + ); + add(new BookmarkablePageLink<>("cancel", SupervisorMyGroupsPage.class)); } @Override protected void onSubmit() { Group group = getModelObject(); - group.setProjects(new HashSet<>(currentProjects)); groupService.save(group); info(getString("saved")); } - private ListModel<Project> getSelectableProjects(List<Project> currentProjects) { + private List<Project> getAllRelevantProjects() { final ProjectService.Filter filter = new ProjectService.Filter(); filter.setSupervisor(SciProSession.get().getUser()); filter.setRoles(Collections.singleton(ProjectTeamMemberRoles.CO_SUPERVISOR)); filter.setStatuses(Collections.singletonList(ProjectStatus.ACTIVE)); - filter.setProjectTypes(projectTypes.getModelObject()); - List<Project> all = projectService.findAll(filter); - all.removeAll(currentProjects); - all.remove(null); - return new ListModel<>(all); + return projectService.findAll(filter); } - private LoadableDetachableModel<Project> getLoaded(final Project project) { - return new LoadableDetachableModel<>() { - @Override - protected Project load() { - return projectService.findOne(project.getId()); - } - }; + private static final class SelectProjectModel extends AbstractCheckBoxModel { + + private final IModel<Group> groupModel; + private final IModel<Project> projectModel; + + public SelectProjectModel(IModel<Group> groupModel, IModel<Project> projectModel) { + this.groupModel = groupModel; + this.projectModel = projectModel; + } + + @Override + public boolean isSelected() { + return groupModel.getObject().getProjects().contains(projectModel.getObject()); + } + + @Override + public void select() { + groupModel.getObject().getProjects().add(projectModel.getObject()); + } + + @Override + public void unselect() { + groupModel.getObject().getProjects().remove(projectModel.getObject()); + } } } } diff --git a/view/src/main/webapp/css/scipro_m.css b/view/src/main/webapp/css/scipro_m.css index 7ce4954a00..49c8b96510 100755 --- a/view/src/main/webapp/css/scipro_m.css +++ b/view/src/main/webapp/css/scipro_m.css @@ -607,3 +607,27 @@ th.wicket_orderUp, th.sorting_asc { .line-length-limit { max-width: 80em; } +.group-project-grid { + display: flex; + flex-wrap: wrap; + gap: 1em; + grid-template-columns: repeat(auto-fill, minmax(30em, 1fr)); +} +.group-project-grid > * { + background: linear-gradient(to left, white 40%, var(--bs-success-bg-subtle) 60%) right; + background-size: 250% 100%; + transition: background 0.4s ease; + cursor: pointer; + border: 1px solid black; + border-radius: 0.25em; + display: flex; + padding: 0.5em; + align-items: center; + flex-grow: 1; +} +.group-project-grid > *:has(:checked) { + background-position: left; +} +.group-project-grid label { + font-weight: normal; +} diff --git a/view/src/test/java/se/su/dsv/scipro/group/EditGroupPanelTest.java b/view/src/test/java/se/su/dsv/scipro/group/EditGroupPanelTest.java index d73dc26f54..2436037335 100644 --- a/view/src/test/java/se/su/dsv/scipro/group/EditGroupPanelTest.java +++ b/view/src/test/java/se/su/dsv/scipro/group/EditGroupPanelTest.java @@ -27,7 +27,6 @@ public class EditGroupPanelTest extends SciProTest { group.setId(1L); Project project = createProject(); group.setProjects(new HashSet<>(Collections.singletonList(project))); - when(projectService.findOne(anyLong())).thenReturn(project); startPanel(); }