From 12338425116bdfaccd080ad4116a24f578dd13e7 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Fri, 17 Jan 2025 12:36:08 +0100 Subject: [PATCH] Implement gradeOpponent --- .../java/se/su/dsv/scipro/CoreConfig.java | 11 +++- .../finalseminar/AbstractOppositionEvent.java | 14 +++++ .../FinalSeminarOppositionGrading.java | 2 +- .../FinalSeminarOppositionService.java | 6 +- .../FinalSeminarOppositionServiceImpl.java | 39 +++++++++++-- .../finalseminar/OppositionCriteria.java | 7 +++ .../finalseminar/OppositionCriterion.java | 3 - .../finalseminar/PointNotValidException.java | 27 +++++++++ .../report/GradingReportServiceImpl.java | 19 +++++-- ...rOppositionServiceImplIntegrationTest.java | 57 ++++++++++++++++++- .../se/su/dsv/scipro/test/SpringTest.java | 23 ++++++++ .../finalseminar/SeminarOppositionPanel.java | 31 +++++----- .../SeminarOppositionPanel.properties | 1 + .../SeminarOppositionPanelTest.java | 9 +++ 14 files changed, 215 insertions(+), 34 deletions(-) create mode 100644 core/src/main/java/se/su/dsv/scipro/finalseminar/OppositionCriteria.java delete mode 100644 core/src/main/java/se/su/dsv/scipro/finalseminar/OppositionCriterion.java create mode 100644 core/src/main/java/se/su/dsv/scipro/finalseminar/PointNotValidException.java 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 ee89a28365..537fdb9976 100644 --- a/core/src/main/java/se/su/dsv/scipro/CoreConfig.java +++ b/core/src/main/java/se/su/dsv/scipro/CoreConfig.java @@ -433,9 +433,16 @@ public class CoreConfig { @Bean public FinalSeminarOppositionServiceImpl finalSeminarOppositionService( Provider<EntityManager> em, - FinalSeminarOppositionGrading finalSeminarOppositionGrading + FinalSeminarOppositionGrading finalSeminarOppositionGrading, + EventBus eventBus, + FinalSeminarOppositionRepo finalSeminarOppositionRepository ) { - return new FinalSeminarOppositionServiceImpl(em, finalSeminarOppositionGrading); + return new FinalSeminarOppositionServiceImpl( + em, + finalSeminarOppositionGrading, + eventBus, + finalSeminarOppositionRepository + ); } @Bean 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/FinalSeminarOppositionGrading.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionGrading.java index 0ac5bba547..2962d1b25e 100644 --- a/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionGrading.java +++ b/core/src/main/java/se/su/dsv/scipro/finalseminar/FinalSeminarOppositionGrading.java @@ -3,5 +3,5 @@ package se.su.dsv.scipro.finalseminar; import java.util.List; public interface FinalSeminarOppositionGrading { - List<OppositionCriterion> oppositionCriteria(FinalSeminarOpposition opposition); + OppositionCriteria oppositionCriteria(FinalSeminarOpposition opposition); } 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 a751149db1..e320c1da2e 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,13 +1,13 @@ package se.su.dsv.scipro.finalseminar; -import java.util.List; import se.su.dsv.scipro.system.GenericService; public interface FinalSeminarOppositionService extends GenericService<FinalSeminarOpposition, Long> { @Override void delete(Long aLong); - List<OppositionCriterion> getCriteriaForOpposition(FinalSeminarOpposition opposition); + OppositionCriteria getCriteriaForOpposition(FinalSeminarOpposition opposition); - void gradeOpponent(FinalSeminarOpposition opposition, int points, String feedback); + FinalSeminarOpposition gradeOpponent(FinalSeminarOpposition opposition, int points, String feedback) + throws PointNotValidException; } 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 4d902f540f..abf7e61246 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,8 +1,10 @@ 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.util.List; import se.su.dsv.scipro.system.AbstractServiceImpl; @@ -11,25 +13,50 @@ public class FinalSeminarOppositionServiceImpl implements FinalSeminarOppositionService { private final FinalSeminarOppositionGrading finalSeminarOppositionGrading; + private final EventBus eventBus; + private final FinalSeminarOppositionRepo finalSeminarOppositionRepository; @Inject public FinalSeminarOppositionServiceImpl( Provider<EntityManager> em, - FinalSeminarOppositionGrading finalSeminarOppositionGrading + FinalSeminarOppositionGrading finalSeminarOppositionGrading, + EventBus eventBus, + FinalSeminarOppositionRepo finalSeminarOppositionRepository ) { super(em, FinalSeminarOpposition.class, QFinalSeminarOpposition.finalSeminarOpposition); this.finalSeminarOppositionGrading = finalSeminarOppositionGrading; + this.eventBus = eventBus; + this.finalSeminarOppositionRepository = finalSeminarOppositionRepository; } @Override - public List<OppositionCriterion> getCriteriaForOpposition(FinalSeminarOpposition opposition) { + public OppositionCriteria getCriteriaForOpposition(FinalSeminarOpposition opposition) { return finalSeminarOppositionGrading.oppositionCriteria(opposition); } @Override - public void gradeOpponent(FinalSeminarOpposition opposition, int points, String feedback) { - // TODO: - // * save assessment - // * post events + @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)); + } + + opposition.setPoints(points); + opposition.setFeedback(feedback); + FinalSeminarOpposition assessedOpposition = finalSeminarOppositionRepository.save(opposition); + + if (criteriaForOpposition.pointsToPass() > points) { + eventBus.post(new OppositionFailedEvent(assessedOpposition)); + } else { + eventBus.post(new OppositionApprovedEvent(assessedOpposition)); + } + + return assessedOpposition; } } 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/OppositionCriterion.java b/core/src/main/java/se/su/dsv/scipro/finalseminar/OppositionCriterion.java deleted file mode 100644 index 9915cf55f1..0000000000 --- a/core/src/main/java/se/su/dsv/scipro/finalseminar/OppositionCriterion.java +++ /dev/null @@ -1,3 +0,0 @@ -package se.su.dsv.scipro.finalseminar; - -public record OppositionCriterion(int points, String requirement) {} 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/report/GradingReportServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/report/GradingReportServiceImpl.java index 9afe32c8ab..66d47f0887 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 @@ -11,7 +11,7 @@ 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.OppositionCriterion; +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; @@ -303,14 +303,25 @@ public class GradingReportServiceImpl @Override @Transactional - public List<OppositionCriterion> oppositionCriteria(FinalSeminarOpposition opposition) { - return getSupervisorGradingReport(opposition.getProject(), opposition.getUser()) + 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 OppositionCriterion(gcp.getPoint(), gcp.getDescription(Language.ENGLISH))) + .map(gcp -> new OppositionCriteria.Point(gcp.getPoint(), gcp.getDescription(Language.ENGLISH))) .toList(); + return new OppositionCriteria(oppositionGradingCriteria.get().getPointsRequiredToPass(), points); } } 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 053ccee8d8..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,6 +1,11 @@ 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; @@ -52,6 +57,56 @@ public class FinalSeminarOppositionServiceImplIntegrationTest extends Integratio @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(); @@ -67,8 +122,6 @@ public class FinalSeminarOppositionServiceImplIntegrationTest extends Integratio AbstractGradingCriterion.Flag.OPPOSITION ); save(gradingReportTemplate); - - assertEquals(2, finalSeminarOppositionService.getCriteriaForOpposition(opposition).size()); } private void createOppositionReport(FinalSeminarOpposition 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/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanel.java b/view/src/main/java/se/su/dsv/scipro/finalseminar/SeminarOppositionPanel.java index 5e2799b0b4..f8ed61ccce 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 @@ -228,20 +228,20 @@ public class SeminarOppositionPanel extends Panel { private class FinalSeminarOppositionForm extends Form<FinalSeminarOpposition> { - private IModel<OppositionCriterion> pointsModel = new StatelessModel<>(); + private IModel<OppositionCriteria.Point> pointsModel = new StatelessModel<>(); private IModel<String> feedbackModel = new Model<>(); public FinalSeminarOppositionForm(String id, final IModel<FinalSeminarOpposition> finalSeminarOpposition) { super(id, finalSeminarOpposition); - IModel<List<OppositionCriterion>> criteriaModel = LoadableDetachableModel.of(() -> + IModel<OppositionCriteria> criteriaModel = LoadableDetachableModel.of(() -> finalSeminarOppositionService.getCriteriaForOpposition(finalSeminarOpposition.getObject()) ); - FormComponent<OppositionCriterion> pointsField = new DropDownChoice<>( + FormComponent<OppositionCriteria.Point> pointsField = new DropDownChoice<>( POINTS, pointsModel, - criteriaModel, - new LambdaChoiceRenderer<>(OppositionCriterion::points) + criteriaModel.map(OppositionCriteria::pointsAvailable), + new LambdaChoiceRenderer<>(OppositionCriteria.Point::value) ); pointsField.setRequired(true); add(pointsField); @@ -255,14 +255,19 @@ public class SeminarOppositionPanel extends Panel { new AjaxSubmitLink(SUBMIT) { @Override protected void onSubmit(AjaxRequestTarget target) { - finalSeminarOppositionService.gradeOpponent( - finalSeminarOpposition.getObject(), - pointsModel.getObject().points(), - feedbackModel.getObject() - ); - success(getString("feedback.opponent.updated", finalSeminarOpposition)); - target.add(feedbackPanel); - target.add(oppositionContainer); + 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); + } } @Override 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..24ac972a57 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,6 +8,7 @@ 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, \ 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