WIP: Allow supervisors to request improvements from final seminar opponents #78

Draft
ansv7779 wants to merge 23 commits from opponent-completion into develop
14 changed files with 215 additions and 34 deletions
Showing only changes of commit 1233842511 - Show all commits

View File

@ -433,9 +433,16 @@ public class CoreConfig {
@Bean @Bean
public FinalSeminarOppositionServiceImpl finalSeminarOppositionService( public FinalSeminarOppositionServiceImpl finalSeminarOppositionService(
Provider<EntityManager> em, Provider<EntityManager> em,
FinalSeminarOppositionGrading finalSeminarOppositionGrading FinalSeminarOppositionGrading finalSeminarOppositionGrading,
EventBus eventBus,
FinalSeminarOppositionRepo finalSeminarOppositionRepository
) { ) {
return new FinalSeminarOppositionServiceImpl(em, finalSeminarOppositionGrading); return new FinalSeminarOppositionServiceImpl(
em,
finalSeminarOppositionGrading,
eventBus,
finalSeminarOppositionRepository
);
} }
@Bean @Bean

View File

@ -1,5 +1,6 @@
package se.su.dsv.scipro.finalseminar; package se.su.dsv.scipro.finalseminar;
import java.util.Objects;
import se.su.dsv.scipro.project.Project; import se.su.dsv.scipro.project.Project;
import se.su.dsv.scipro.system.User; import se.su.dsv.scipro.system.User;
@ -26,4 +27,17 @@ class AbstractOppositionEvent {
public FinalSeminarOpposition getOpposition() { public FinalSeminarOpposition getOpposition() {
return opposition; 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);
}
} }

View File

@ -3,5 +3,5 @@ package se.su.dsv.scipro.finalseminar;
import java.util.List; import java.util.List;
public interface FinalSeminarOppositionGrading { public interface FinalSeminarOppositionGrading {
List<OppositionCriterion> oppositionCriteria(FinalSeminarOpposition opposition); OppositionCriteria oppositionCriteria(FinalSeminarOpposition opposition);
} }

View File

@ -1,13 +1,13 @@
package se.su.dsv.scipro.finalseminar; package se.su.dsv.scipro.finalseminar;
import java.util.List;
import se.su.dsv.scipro.system.GenericService; import se.su.dsv.scipro.system.GenericService;
public interface FinalSeminarOppositionService extends GenericService<FinalSeminarOpposition, Long> { public interface FinalSeminarOppositionService extends GenericService<FinalSeminarOpposition, Long> {
@Override @Override
void delete(Long aLong); 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;
} }

View File

@ -1,8 +1,10 @@
package se.su.dsv.scipro.finalseminar; package se.su.dsv.scipro.finalseminar;
import com.google.common.eventbus.EventBus;
import jakarta.inject.Inject; import jakarta.inject.Inject;
import jakarta.inject.Provider; import jakarta.inject.Provider;
import jakarta.persistence.EntityManager; import jakarta.persistence.EntityManager;
import jakarta.transaction.Transactional;
import java.util.List; import java.util.List;
import se.su.dsv.scipro.system.AbstractServiceImpl; import se.su.dsv.scipro.system.AbstractServiceImpl;
@ -11,25 +13,50 @@ public class FinalSeminarOppositionServiceImpl
implements FinalSeminarOppositionService { implements FinalSeminarOppositionService {
private final FinalSeminarOppositionGrading finalSeminarOppositionGrading; private final FinalSeminarOppositionGrading finalSeminarOppositionGrading;
private final EventBus eventBus;
private final FinalSeminarOppositionRepo finalSeminarOppositionRepository;
@Inject @Inject
public FinalSeminarOppositionServiceImpl( public FinalSeminarOppositionServiceImpl(
Provider<EntityManager> em, Provider<EntityManager> em,
FinalSeminarOppositionGrading finalSeminarOppositionGrading FinalSeminarOppositionGrading finalSeminarOppositionGrading,
EventBus eventBus,
FinalSeminarOppositionRepo finalSeminarOppositionRepository
) { ) {
super(em, FinalSeminarOpposition.class, QFinalSeminarOpposition.finalSeminarOpposition); super(em, FinalSeminarOpposition.class, QFinalSeminarOpposition.finalSeminarOpposition);
this.finalSeminarOppositionGrading = finalSeminarOppositionGrading; this.finalSeminarOppositionGrading = finalSeminarOppositionGrading;
this.eventBus = eventBus;
this.finalSeminarOppositionRepository = finalSeminarOppositionRepository;
} }
@Override @Override
public List<OppositionCriterion> getCriteriaForOpposition(FinalSeminarOpposition opposition) { public OppositionCriteria getCriteriaForOpposition(FinalSeminarOpposition opposition) {
return finalSeminarOppositionGrading.oppositionCriteria(opposition); return finalSeminarOppositionGrading.oppositionCriteria(opposition);
} }
@Override @Override
public void gradeOpponent(FinalSeminarOpposition opposition, int points, String feedback) { @Transactional
// TODO: public FinalSeminarOpposition gradeOpponent(FinalSeminarOpposition opposition, int points, String feedback)
// * save assessment throws PointNotValidException {
// * post events 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;
} }
} }

View File

@ -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) {}
}

View File

@ -1,3 +0,0 @@
package se.su.dsv.scipro.finalseminar;
public record OppositionCriterion(int points, String requirement) {}

View File

@ -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 + '}';
}
}

View File

@ -11,7 +11,7 @@ import java.util.*;
import se.su.dsv.scipro.finalseminar.FinalSeminarOpposition; import se.su.dsv.scipro.finalseminar.FinalSeminarOpposition;
import se.su.dsv.scipro.finalseminar.FinalSeminarOppositionGrading; import se.su.dsv.scipro.finalseminar.FinalSeminarOppositionGrading;
import se.su.dsv.scipro.finalseminar.OppositionApprovedEvent; 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.GradingBasis;
import se.su.dsv.scipro.grading.GradingReportTemplateService; import se.su.dsv.scipro.grading.GradingReportTemplateService;
import se.su.dsv.scipro.grading.GradingReportTemplateUpdate; import se.su.dsv.scipro.grading.GradingReportTemplateUpdate;
@ -303,14 +303,25 @@ public class GradingReportServiceImpl
@Override @Override
@Transactional @Transactional
public List<OppositionCriterion> oppositionCriteria(FinalSeminarOpposition opposition) { public OppositionCriteria oppositionCriteria(FinalSeminarOpposition opposition) {
return getSupervisorGradingReport(opposition.getProject(), opposition.getUser()) SupervisorGradingReport supervisorGradingReport = getSupervisorGradingReport(
opposition.getProject(),
opposition.getUser()
);
Optional<GradingCriterion> oppositionGradingCriteria = supervisorGradingReport
.getIndividualCriteria() .getIndividualCriteria()
.stream() .stream()
.filter(individualCriterion -> individualCriterion.getFlag() == AbstractGradingCriterion.Flag.OPPOSITION) .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) .map(GradingCriterion::getGradingCriterionPoints)
.flatMap(Collection::stream) .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(); .toList();
return new OppositionCriteria(oppositionGradingCriteria.get().getPointsRequiredToPass(), points);
} }
} }

View File

@ -1,6 +1,11 @@
package se.su.dsv.scipro.finalseminar; 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.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import jakarta.inject.Inject; import jakarta.inject.Inject;
import java.time.LocalDate; import java.time.LocalDate;
@ -52,6 +57,56 @@ public class FinalSeminarOppositionServiceImplIntegrationTest extends Integratio
@Test @Test
public void opposition_criteria_are_taken_from_the_grading_report_template() { public void opposition_criteria_are_taken_from_the_grading_report_template() {
FinalSeminarOpposition opposition = createOpposition(finalSeminar, createUser()); 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(); GradingReportTemplate gradingReportTemplate = createGradingReportTemplate();
GradingCriterionPointTemplate failingCriterion = new GradingCriterionPointTemplate(); GradingCriterionPointTemplate failingCriterion = new GradingCriterionPointTemplate();
@ -67,8 +122,6 @@ public class FinalSeminarOppositionServiceImplIntegrationTest extends Integratio
AbstractGradingCriterion.Flag.OPPOSITION AbstractGradingCriterion.Flag.OPPOSITION
); );
save(gradingReportTemplate); save(gradingReportTemplate);
assertEquals(2, finalSeminarOppositionService.getCriteriaForOpposition(opposition).size());
} }
private void createOppositionReport(FinalSeminarOpposition opposition) { private void createOppositionReport(FinalSeminarOpposition opposition) {

View File

@ -1,11 +1,14 @@
package se.su.dsv.scipro.test; package se.su.dsv.scipro.test;
import com.google.common.eventbus.EventBus;
import jakarta.persistence.EntityManager; import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityManagerFactory; import jakarta.persistence.EntityManagerFactory;
import jakarta.persistence.EntityTransaction; import jakarta.persistence.EntityTransaction;
import jakarta.persistence.Persistence; import jakarta.persistence.Persistence;
import java.sql.SQLException; import java.sql.SQLException;
import java.time.Clock; import java.time.Clock;
import java.util.ArrayList;
import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Optional; import java.util.Optional;
import org.flywaydb.core.Flyway; import org.flywaydb.core.Flyway;
@ -35,6 +38,8 @@ public abstract class SpringTest {
@Container @Container
static MariaDBContainer<?> mariaDBContainer = new MariaDBContainer<>("mariadb:10.11"); static MariaDBContainer<?> mariaDBContainer = new MariaDBContainer<>("mariadb:10.11");
private CapturingEventBus capturingEventBus;
@BeforeEach @BeforeEach
public final void prepareSpring() throws SQLException { public final void prepareSpring() throws SQLException {
MariaDbDataSource dataSource = new MariaDbDataSource(mariaDBContainer.getJdbcUrl()); MariaDbDataSource dataSource = new MariaDbDataSource(mariaDBContainer.getJdbcUrl());
@ -50,8 +55,11 @@ public abstract class SpringTest {
transaction.begin(); transaction.begin();
transaction.setRollbackOnly(); transaction.setRollbackOnly();
capturingEventBus = new CapturingEventBus();
AnnotationConfigApplicationContext annotationConfigApplicationContext = AnnotationConfigApplicationContext annotationConfigApplicationContext =
new AnnotationConfigApplicationContext(); new AnnotationConfigApplicationContext();
annotationConfigApplicationContext.registerBean("eventBus", EventBus.class, () -> this.capturingEventBus);
annotationConfigApplicationContext.register(TestContext.class); annotationConfigApplicationContext.register(TestContext.class);
annotationConfigApplicationContext.getBeanFactory().registerSingleton("entityManager", this.entityManager); annotationConfigApplicationContext.getBeanFactory().registerSingleton("entityManager", this.entityManager);
annotationConfigApplicationContext.refresh(); annotationConfigApplicationContext.refresh();
@ -75,6 +83,10 @@ public abstract class SpringTest {
} }
} }
protected List<Object> getPublishedEvents() {
return capturingEventBus.publishedEvents;
}
@Configuration(proxyBeanMethods = false) @Configuration(proxyBeanMethods = false)
@Import({ CoreConfig.class, RepositoryConfiguration.class }) @Import({ CoreConfig.class, RepositoryConfiguration.class })
public static class TestContext { public static class TestContext {
@ -106,4 +118,15 @@ public abstract class SpringTest {
return currentProfile; 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);
}
}
} }

View File

@ -228,20 +228,20 @@ public class SeminarOppositionPanel extends Panel {
private class FinalSeminarOppositionForm extends Form<FinalSeminarOpposition> { 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<>(); private IModel<String> feedbackModel = new Model<>();
public FinalSeminarOppositionForm(String id, final IModel<FinalSeminarOpposition> finalSeminarOpposition) { public FinalSeminarOppositionForm(String id, final IModel<FinalSeminarOpposition> finalSeminarOpposition) {
super(id, finalSeminarOpposition); super(id, finalSeminarOpposition);
IModel<List<OppositionCriterion>> criteriaModel = LoadableDetachableModel.of(() -> IModel<OppositionCriteria> criteriaModel = LoadableDetachableModel.of(() ->
finalSeminarOppositionService.getCriteriaForOpposition(finalSeminarOpposition.getObject()) finalSeminarOppositionService.getCriteriaForOpposition(finalSeminarOpposition.getObject())
); );
FormComponent<OppositionCriterion> pointsField = new DropDownChoice<>( FormComponent<OppositionCriteria.Point> pointsField = new DropDownChoice<>(
POINTS, POINTS,
pointsModel, pointsModel,
criteriaModel, criteriaModel.map(OppositionCriteria::pointsAvailable),
new LambdaChoiceRenderer<>(OppositionCriterion::points) new LambdaChoiceRenderer<>(OppositionCriteria.Point::value)
); );
pointsField.setRequired(true); pointsField.setRequired(true);
add(pointsField); add(pointsField);
@ -255,14 +255,19 @@ public class SeminarOppositionPanel extends Panel {
new AjaxSubmitLink(SUBMIT) { new AjaxSubmitLink(SUBMIT) {
@Override @Override
protected void onSubmit(AjaxRequestTarget target) { protected void onSubmit(AjaxRequestTarget target) {
finalSeminarOppositionService.gradeOpponent( try {
finalSeminarOpposition.getObject(), finalSeminarOppositionService.gradeOpponent(
pointsModel.getObject().points(), finalSeminarOpposition.getObject(),
feedbackModel.getObject() pointsModel.getObject().value(),
); feedbackModel.getObject()
success(getString("feedback.opponent.updated", finalSeminarOpposition)); );
target.add(feedbackPanel); success(getString("feedback.opponent.updated", finalSeminarOpposition));
target.add(oppositionContainer); target.add(feedbackPanel);
target.add(oppositionContainer);
} catch (PointNotValidException e) {
error(getString("point.not.valid"));
target.add(feedbackPanel);
}
} }
@Override @Override

View File

@ -8,6 +8,7 @@ gradingFeedback.Required = You need to write a motivation
points.Required = Points are required points.Required = Points are required
opponents.form.points.RangeValidator.range= Points assigned must be between ${minimum} and ${maximum} opponents.form.points.RangeValidator.range= Points assigned must be between ${minimum} and ${maximum}
feedback.opponent.updated= Opponent ${user.fullName} feedback updated. 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. 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.\ 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, \ <\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, \

View File

@ -61,6 +61,15 @@ public class SeminarOppositionPanelTest extends SciProTest {
finalSeminar.setProject(project); finalSeminar.setProject(project);
setLoggedInAs(supervisorUser); 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 @Test