From ec70ea55963ceb518593c62e0099264ff9dd00a4 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Mon, 3 Mar 2025 07:32:25 +0100 Subject: [PATCH 01/16] Make session serializable (#121) When re-deploying the application, or restarting Tomcat, it will attempt to serialize the active sessions to prevent users from getting logged out and losing in-progess work. This requires that all attributes that are stored in the session implement `java.io.Serializable`. Spring stores the entire security context in the session which includes a reference to the principal, and that principal may be of type "WicketControlledPrincipal" and it must therefore be serializable. ## How to test 1. Be on the `develop` branch 2. Make sure session preservation is turned on (in IntelliJ check "Preserve sessions across restarts and redeploys", or read https://tomcat.apache.org/tomcat-10.0-doc/config/manager.html#Persistence_Across_Restarts) 3. Log in as the default admin `dev@localhost` 4. Switch to "Sture Student" under "Admin / Users / Switch user" 5. Restart Tomcat 6. Refresh page and you'll be prompted to log in again 7. Switch to this branch and repeat step 1-6 Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/121 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> --- .../se/su/dsv/scipro/war/CurrentUserFromSpringSecurity.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/war/src/main/java/se/su/dsv/scipro/war/CurrentUserFromSpringSecurity.java b/war/src/main/java/se/su/dsv/scipro/war/CurrentUserFromSpringSecurity.java index 6f209f38aa..3d71fd12a3 100644 --- a/war/src/main/java/se/su/dsv/scipro/war/CurrentUserFromSpringSecurity.java +++ b/war/src/main/java/se/su/dsv/scipro/war/CurrentUserFromSpringSecurity.java @@ -4,6 +4,7 @@ import jakarta.inject.Inject; import jakarta.inject.Provider; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import java.io.Serializable; import java.security.Principal; import java.util.Collections; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; @@ -81,7 +82,7 @@ public class CurrentUserFromSpringSecurity implements AuthenticationContext { return authentication.getName(); } - private static final class WicketControlledPrincipal implements Principal { + private static final class WicketControlledPrincipal implements Principal, Serializable { private final String username; From a71eeb5e2ce35e38a39dfa9630f39fd41b093859 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Mon, 3 Mar 2025 07:48:46 +0100 Subject: [PATCH 02/16] Fix crash when editing an application period (#117) Fixes #68 ## How to test 1. Log in as admin 2. Go to "Match / Application periods" 3. Click the edit icon (6th column) 4. Click "Save" Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/117 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> --- .../se/su/dsv/scipro/match/ApplicationPeriod.java | 12 +++++------- .../AdminApplicationPeriodsPanel.java | 8 +------- .../scipro/datatables/target/AddTargetLinkPanel.java | 7 +------ .../scipro/projectpartner/ProjectPartnerPage.java | 8 +------- 4 files changed, 8 insertions(+), 27 deletions(-) diff --git a/core/src/main/java/se/su/dsv/scipro/match/ApplicationPeriod.java b/core/src/main/java/se/su/dsv/scipro/match/ApplicationPeriod.java index 9dc48661d9..b21a586607 100755 --- a/core/src/main/java/se/su/dsv/scipro/match/ApplicationPeriod.java +++ b/core/src/main/java/se/su/dsv/scipro/match/ApplicationPeriod.java @@ -176,18 +176,16 @@ public class ApplicationPeriod extends DomainObject { return Collections.unmodifiableSet(answerSet); } - public void setProjectTypes(Iterable<ProjectType> projectTypes) { - this.projectTypes.clear(); + public void setProjectTypes(Set<ProjectType> projectTypes) { + this.projectTypes.removeIf(appt -> !projectTypes.contains(appt.getProjectType())); for (ProjectType pt : projectTypes) { - this.projectTypes.add(new ApplicationPeriodProjectType(this, pt)); + if (this.projectTypes.stream().noneMatch(appt -> appt.getProjectType().equals(pt))) { + addProjectType(pt); + } } } public void addProjectType(ProjectType projectType) { this.projectTypes.add(new ApplicationPeriodProjectType(this, projectType)); } - - public void removeProjectType(ProjectType projectType) { - this.projectTypes.removeIf(next -> next.getProjectType().equals(projectType)); - } } diff --git a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminApplicationPeriodsPanel.java b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminApplicationPeriodsPanel.java index 307d7858f3..b8bb7d9e76 100755 --- a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminApplicationPeriodsPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminApplicationPeriodsPanel.java @@ -123,13 +123,7 @@ public class AdminApplicationPeriodsPanel extends Panel { item.add( new DisplayMultiplesPanel<>( s, - new ListAdapterModel<>( - LambdaModel.of( - iModel, - ApplicationPeriod::getProjectTypes, - ApplicationPeriod::setProjectTypes - ) - ) + new ListAdapterModel<>(iModel.map(ApplicationPeriod::getProjectTypes)) ) { @Override public Component getComponent(String componentId, IModel<ProjectType> t) { diff --git a/view/src/main/java/se/su/dsv/scipro/datatables/target/AddTargetLinkPanel.java b/view/src/main/java/se/su/dsv/scipro/datatables/target/AddTargetLinkPanel.java index 2f4d027af5..8c4c98a6d0 100644 --- a/view/src/main/java/se/su/dsv/scipro/datatables/target/AddTargetLinkPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/datatables/target/AddTargetLinkPanel.java @@ -22,12 +22,7 @@ public class AddTargetLinkPanel extends Panel { public AddTargetLinkPanel(String id, final IModel<ApplicationPeriod> model) { super(id, model); add( - new ListView<>( - "list", - new ListAdapterModel<>( - LambdaModel.of(model, ApplicationPeriod::getProjectTypes, ApplicationPeriod::setProjectTypes) - ) - ) { + new ListView<>("list", new ListAdapterModel<>(model.map(ApplicationPeriod::getProjectTypes))) { @Override protected void populateItem(ListItem<ProjectType> item) { item.add(new Label("pc", item.getModelObject().getName())); diff --git a/view/src/main/java/se/su/dsv/scipro/projectpartner/ProjectPartnerPage.java b/view/src/main/java/se/su/dsv/scipro/projectpartner/ProjectPartnerPage.java index a70588801a..ca3736e027 100755 --- a/view/src/main/java/se/su/dsv/scipro/projectpartner/ProjectPartnerPage.java +++ b/view/src/main/java/se/su/dsv/scipro/projectpartner/ProjectPartnerPage.java @@ -76,13 +76,7 @@ public class ProjectPartnerPage extends AbstractIdeaProjectPage implements MenuH } ); final IModel<? extends List<ProjectType>> matchableTypes = getMatchableTypes( - new ListAdapterModel<>( - LambdaModel.of( - applicationPeriod, - ApplicationPeriod::getProjectTypes, - ApplicationPeriod::setProjectTypes - ) - ) + new ListAdapterModel<>(applicationPeriod.map(ApplicationPeriod::getProjectTypes)) ); panelContainer.add( new ListView<>("ads", matchableTypes) { From 7f9e72484aaf00e1550374af2f644ec33eb0c94c Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Mon, 3 Mar 2025 07:59:14 +0100 Subject: [PATCH 03/16] Remove unused javax.inject and jersey-hk2 dependencies (#118) Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/118 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> --- core/pom.xml | 22 ---------------------- pom.xml | 7 ------- 2 files changed, 29 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index e34cb44596..532b1d4d04 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -23,36 +23,14 @@ <dependency> <groupId>org.glassfish.jersey.core</groupId> <artifactId>jersey-client</artifactId> - <exclusions> - <exclusion> - <groupId>org.glassfish.hk2.external</groupId> - <artifactId>javax.inject</artifactId> - </exclusion> - </exclusions> </dependency> <dependency> <groupId>org.glassfish.jersey.media</groupId> <artifactId>jersey-media-jaxb</artifactId> - <exclusions> - <exclusion> - <groupId>org.glassfish.hk2.external</groupId> - <artifactId>javax.inject</artifactId> - </exclusion> - </exclusions> </dependency> <dependency> <groupId>org.glassfish.jersey.media</groupId> <artifactId>jersey-media-json-jackson</artifactId> - <exclusions> - <exclusion> - <groupId>org.glassfish.hk2.external</groupId> - <artifactId>javax.inject</artifactId> - </exclusion> - </exclusions> - </dependency> - <dependency> - <groupId>org.glassfish.jersey.inject</groupId> - <artifactId>jersey-hk2</artifactId> </dependency> <dependency> <groupId>org.springframework</groupId> diff --git a/pom.xml b/pom.xml index a43a6491cd..1940f2ed25 100755 --- a/pom.xml +++ b/pom.xml @@ -295,13 +295,6 @@ <artifactId>slf4j-api</artifactId> </dependency> - <!-- Additional dependencies --> - <dependency> - <groupId>javax.inject</groupId> - <artifactId>javax.inject</artifactId> - <version>1</version> - </dependency> - <!-- Test stuff --> <dependency> <groupId>org.junit.jupiter</groupId> From a76b317b1cb48f178efb1071f6b3f81ad6c15648 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Mon, 3 Mar 2025 12:38:35 +0100 Subject: [PATCH 04/16] Improve test data creation (#112) Currently there is only one class used to add test data; [`DataInitializer`](https://gitea.dsv.su.se/DMC/scipro/src/commit/b9f7dd5a49aeebd64a5f44b66ac0775901550476/core/src/main/java/se/su/dsv/scipro/DataInitializer.java). That class is already very large and causes a lot of merge conflicts when multiple changes are in the pipeline as noted by #109. This change makes it possible to have multiple classes adding test data so that each change adds its own class and thus there are no conflicts. It also has the benefit of making each class smaller and more self-contained for testing a specific feature. Some additional infrastructure was added in the form of the `BaseData` and `Factory` (naming improvements notwithstanding) interfaces to help each class add its own test data and re-use common data. Finally all test data related classes have been moved to their own module so they can be properly excluded when building for production but are included by default while developing. Fixes #109 ## Future work * Add a mechanism to work with date and time. Many processes (and therefore service method implementations) rely on the time between certain events. For example a final seminar must be scheduled a set amount of days in advance. In the ideal world, the test data is populated using these service methods to more accurately represent an achievable real world state. Therefore there must be a way to manipulate time when adding test data. * Add more methods to the `Factory` interface as we discover more common steps that many populators must take. * Add more base data available through the `BaseData` interface as we discover more common data that many populators need Care must be taken that this data is final and useful in its base state since populators will rely on that state. Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/112 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> --- .gitignore | 1 + Dockerfile | 2 + .../java/se/su/dsv/scipro/CoreConfig.java | 5 -- pom.xml | 1 + test-data/pom.xml | 20 ++++++++ .../se/su/dsv/scipro/testdata/BaseData.java | 19 +++++++ .../dsv/scipro/testdata}/DataInitializer.java | 49 +++++++++++++++++-- .../se/su/dsv/scipro/testdata/Factory.java | 46 +++++++++++++++++ .../testdata/TestDataConfiguration.java | 16 ++++++ .../scipro/testdata/TestDataPopulator.java | 11 +++++ .../su/dsv/scipro/testdata/package-info.java | 8 +++ .../testdata/populators/package-info.java | 13 +++++ .../se.su.dsv.scipro.war.PluginConfiguration | 1 + war/pom.xml | 13 +++++ 14 files changed, 196 insertions(+), 9 deletions(-) create mode 100644 test-data/pom.xml create mode 100644 test-data/src/main/java/se/su/dsv/scipro/testdata/BaseData.java rename {core/src/main/java/se/su/dsv/scipro => test-data/src/main/java/se/su/dsv/scipro/testdata}/DataInitializer.java (99%) create mode 100644 test-data/src/main/java/se/su/dsv/scipro/testdata/Factory.java create mode 100644 test-data/src/main/java/se/su/dsv/scipro/testdata/TestDataConfiguration.java create mode 100644 test-data/src/main/java/se/su/dsv/scipro/testdata/TestDataPopulator.java create mode 100644 test-data/src/main/java/se/su/dsv/scipro/testdata/package-info.java create mode 100644 test-data/src/main/java/se/su/dsv/scipro/testdata/populators/package-info.java create mode 100644 test-data/src/main/resources/META-INF/services/se.su.dsv.scipro.war.PluginConfiguration diff --git a/.gitignore b/.gitignore index a4397c6105..8393ae6082 100755 --- a/.gitignore +++ b/.gitignore @@ -25,4 +25,5 @@ fitnesse/target/ daisy-integration/target/ war/target/ api/target/ +test-data/target/ node_modules/ diff --git a/Dockerfile b/Dockerfile index ac53fd07f3..acf3c889a3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -12,12 +12,14 @@ COPY core/pom.xml core/pom.xml COPY view/pom.xml view/pom.xml COPY war/pom.xml war/pom.xml COPY daisy-integration/pom.xml daisy-integration/pom.xml +COPY test-data/pom.xml test-data/pom.xml COPY api/src/ api/src/ COPY core/src/ core/src/ COPY view/src/ view/src/ COPY war/src/ war/src/ COPY daisy-integration/src/ daisy-integration/src/ +COPY test-data/src/ test-data/src/ RUN ./mvnw package \ --define skipTests \ 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 b7c51b41f2..19b8f04a13 100644 --- a/core/src/main/java/se/su/dsv/scipro/CoreConfig.java +++ b/core/src/main/java/se/su/dsv/scipro/CoreConfig.java @@ -1017,11 +1017,6 @@ public class CoreConfig { ); } - @Bean - public DataInitializer dataInitializer() { - return new DataInitializer(); - } - @Bean public FinalSeminarActivityHandler finalSeminarActivityHandler( ActivityPlanFacade activityPlanFacade, diff --git a/pom.xml b/pom.xml index 1940f2ed25..896b1269e5 100755 --- a/pom.xml +++ b/pom.xml @@ -15,6 +15,7 @@ <module>daisy-integration</module> <module>war</module> <module>api</module> + <module>test-data</module> </modules> <properties> diff --git a/test-data/pom.xml b/test-data/pom.xml new file mode 100644 index 0000000000..127cef4d71 --- /dev/null +++ b/test-data/pom.xml @@ -0,0 +1,20 @@ +<?xml version="1.0" encoding="UTF-8"?> +<project xmlns="http://maven.apache.org/POM/4.0.0" + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>se.su.dsv.scipro</groupId> + <artifactId>SciPro</artifactId> + <version>0.1-SNAPSHOT</version> + </parent> + + <artifactId>test-data</artifactId> + + <dependencies> + <dependency> + <groupId>se.su.dsv.scipro</groupId> + <artifactId>core</artifactId> + </dependency> + </dependencies> +</project> diff --git a/test-data/src/main/java/se/su/dsv/scipro/testdata/BaseData.java b/test-data/src/main/java/se/su/dsv/scipro/testdata/BaseData.java new file mode 100644 index 0000000000..1dd6b18132 --- /dev/null +++ b/test-data/src/main/java/se/su/dsv/scipro/testdata/BaseData.java @@ -0,0 +1,19 @@ +package se.su.dsv.scipro.testdata; + +import se.su.dsv.scipro.system.ProjectType; + +/// All the base test data that can be re-used in different test cases. +/// +/// **Do not modify any of this data.** There are many +/// [TestDataPopulator]s that rely on this data to be in a specific state. +/// +/// In addition to the data that is available here there is also much additional +/// data that has been created; +/// +/// - A grading report template for each [ProjectType] +/// +public interface BaseData { + ProjectType bachelor(); + ProjectType magister(); + ProjectType master(); +} diff --git a/core/src/main/java/se/su/dsv/scipro/DataInitializer.java b/test-data/src/main/java/se/su/dsv/scipro/testdata/DataInitializer.java similarity index 99% rename from core/src/main/java/se/su/dsv/scipro/DataInitializer.java rename to test-data/src/main/java/se/su/dsv/scipro/testdata/DataInitializer.java index 15a093006b..8b157598c4 100644 --- a/core/src/main/java/se/su/dsv/scipro/DataInitializer.java +++ b/test-data/src/main/java/se/su/dsv/scipro/testdata/DataInitializer.java @@ -1,4 +1,4 @@ -package se.su.dsv.scipro; +package se.su.dsv.scipro.testdata; import jakarta.inject.Inject; import jakarta.inject.Provider; @@ -34,13 +34,16 @@ import se.su.dsv.scipro.security.auth.roles.Roles; import se.su.dsv.scipro.system.*; import se.su.dsv.scipro.util.Pair; -public class DataInitializer implements Lifecycle { +public class DataInitializer implements Lifecycle, BaseData, Factory { public static final int APPLICATION_PERIOD_START_MINUS_DAYS = 1; public static final int APPLICATION_PERIOD_END_PLUS_DAYS = 3; public static final int APPLICATION_PERIOD_COURSE_START_PLUS_DAYS = 5; public static final long RESEARCH_AREA_ID = 12L; + @Inject + private Collection<TestDataPopulator> testDataPopulators = new ArrayList<>(); + @Inject private UserService userService; @@ -120,6 +123,9 @@ public class DataInitializer implements Lifecycle { createTarget(); createStudentIdea(); createRoughDraftApproval(); + for (TestDataPopulator testDataPopulator : testDataPopulators) { + testDataPopulator.populate(this, this); + } } if (profile.getCurrentProfile() == Profiles.DEV && noAdminUser()) { createAdmin(); @@ -243,13 +249,18 @@ public class DataInitializer implements Lifecycle { sofia_student = createStudent("Sofia", 17); } - private User createStudent(String firstName, int identifier) { + private User createStudent(String firstName) { User user = createUser(firstName, STUDENT_LAST); - user.setIdentifier(identifier); createBeta(user); return user; } + private User createStudent(String firstName, int identifier) { + User user = createStudent(firstName); + user.setIdentifier(identifier); + return user; + } + private User createEmployee(String firstName) { User user = createUser(firstName, EMPLOYEE_LAST); Unit u = createUnit(); @@ -2087,6 +2098,36 @@ public class DataInitializer implements Lifecycle { return entity; } + @Override + public ProjectType bachelor() { + return bachelorClass; + } + + @Override + public ProjectType magister() { + return magisterClass; + } + + @Override + public ProjectType master() { + return masterClass; + } + + @Override + public User createAuthor(String firstName) { + return createStudent(firstName); + } + + @Override + public User createSupervisor(String firstName) { + return createEmployee(firstName); + } + + @Override + public User createReviewer(String firstName) { + return createEmployee(firstName); + } + private static final class SimpleTextFile implements FileUpload { private final User uploader; diff --git a/test-data/src/main/java/se/su/dsv/scipro/testdata/Factory.java b/test-data/src/main/java/se/su/dsv/scipro/testdata/Factory.java new file mode 100644 index 0000000000..fc4c0910f1 --- /dev/null +++ b/test-data/src/main/java/se/su/dsv/scipro/testdata/Factory.java @@ -0,0 +1,46 @@ +package se.su.dsv.scipro.testdata; + +import se.su.dsv.scipro.security.auth.roles.Roles; +import se.su.dsv.scipro.system.User; + +/** + * A factory to help with repetitive tasks when populating test data. + */ +public interface Factory { + /** + * Creates a user with the given first name and last name "Student". + * The user is given the role {@link Roles#AUTHOR}. + * <p> + * A username is created of the form {@code <first_name>@example.com} that + * can be used to log in. + */ + User createAuthor(String firstName); + + /** + * Creates a user with the given first name and last name "Employee". + * <p> + * The user is given the role {@link Roles#SUPERVISOR}, {@link Roles#REVIEWER}, + * and {@link Roles#EXAMINER}. + * <p> + * The user gets a default research area, unit, and language. It is also + * marked as {@link User#setActiveAsSupervisor(boolean) an active supervisor}. + * <p> + * A username is created of the form {@code <first_name>@example.com} that + * can be used to log in. + */ + User createSupervisor(String firstName); + + /** + * Creates a user with the given first name and last name "Employee". + * <p> + * The user is given the role {@link Roles#SUPERVISOR}, {@link Roles#REVIEWER}, + * and {@link Roles#EXAMINER}. + * <p> + * The user gets a default research area, unit, and language. It is also + * marked as {@link User#setActiveAsSupervisor(boolean) an active supervisor}. + * <p> + * A username is created of the form {@code <first_name>@example.com} that + * can be used to log in. + */ + User createReviewer(String firstName); +} diff --git a/test-data/src/main/java/se/su/dsv/scipro/testdata/TestDataConfiguration.java b/test-data/src/main/java/se/su/dsv/scipro/testdata/TestDataConfiguration.java new file mode 100644 index 0000000000..ffd372628b --- /dev/null +++ b/test-data/src/main/java/se/su/dsv/scipro/testdata/TestDataConfiguration.java @@ -0,0 +1,16 @@ +package se.su.dsv.scipro.testdata; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.Configuration; +import se.su.dsv.scipro.war.PluginConfiguration; + +@Configuration(proxyBeanMethods = false) +@ComponentScan(basePackages = "se.su.dsv.scipro.testdata.populators") +public class TestDataConfiguration implements PluginConfiguration { + + @Bean + public DataInitializer dataInitializer() { + return new DataInitializer(); + } +} diff --git a/test-data/src/main/java/se/su/dsv/scipro/testdata/TestDataPopulator.java b/test-data/src/main/java/se/su/dsv/scipro/testdata/TestDataPopulator.java new file mode 100644 index 0000000000..41d250ac71 --- /dev/null +++ b/test-data/src/main/java/se/su/dsv/scipro/testdata/TestDataPopulator.java @@ -0,0 +1,11 @@ +package se.su.dsv.scipro.testdata; + +public interface TestDataPopulator { + /** + * Add test data to the system to help with testing a specific feature. + * + * @param baseData the base data already populated + * @param factory helper object to make repetitive tasks easier (such as creating users) + */ + void populate(BaseData baseData, Factory factory); +} diff --git a/test-data/src/main/java/se/su/dsv/scipro/testdata/package-info.java b/test-data/src/main/java/se/su/dsv/scipro/testdata/package-info.java new file mode 100644 index 0000000000..6b93353f29 --- /dev/null +++ b/test-data/src/main/java/se/su/dsv/scipro/testdata/package-info.java @@ -0,0 +1,8 @@ +/** + * This package contains the infrastructure that is used when generating test data for the application. To add new test + * data to the system, add a new class to the {@link se.su.dsv.scipro.testdata.populators} package that implements the + * {@link se.su.dsv.scipro.testdata.TestDataPopulator} interface and annotate it with + * {@link org.springframework.stereotype.Service @Service}. Inject dependencies as needed using + * {@link jakarta.inject.Inject @Inject}. + */ +package se.su.dsv.scipro.testdata; diff --git a/test-data/src/main/java/se/su/dsv/scipro/testdata/populators/package-info.java b/test-data/src/main/java/se/su/dsv/scipro/testdata/populators/package-info.java new file mode 100644 index 0000000000..c9d116ca5a --- /dev/null +++ b/test-data/src/main/java/se/su/dsv/scipro/testdata/populators/package-info.java @@ -0,0 +1,13 @@ +/** + * Contains classes that populate the database with test data. + * <p> + * Prefer to use methods on the various services to create data, rather than directly interacting with the database + * using an {@link jakarta.persistence.EntityManager}. This is to make sure all business rules are enforced and that + * any additional logic is executed such as sending notifications or calculating statistics. + * + * @see se.su.dsv.scipro.testdata how to add new populators + * @see se.su.dsv.scipro.testdata.TestDataPopulator + * @see se.su.dsv.scipro.testdata.BaseData + * @see se.su.dsv.scipro.testdata.Factory + */ +package se.su.dsv.scipro.testdata.populators; diff --git a/test-data/src/main/resources/META-INF/services/se.su.dsv.scipro.war.PluginConfiguration b/test-data/src/main/resources/META-INF/services/se.su.dsv.scipro.war.PluginConfiguration new file mode 100644 index 0000000000..99b97f03e4 --- /dev/null +++ b/test-data/src/main/resources/META-INF/services/se.su.dsv.scipro.war.PluginConfiguration @@ -0,0 +1 @@ +se.su.dsv.scipro.testdata.TestDataConfiguration diff --git a/war/pom.xml b/war/pom.xml index c1dd889587..6a347c9b09 100644 --- a/war/pom.xml +++ b/war/pom.xml @@ -140,5 +140,18 @@ <spring.profile.active>branch</spring.profile.active> </properties> </profile> + <profile> + <id>DEV</id> + <activation> + <activeByDefault>true</activeByDefault> + </activation> + <dependencies> + <dependency> + <groupId>se.su.dsv.scipro</groupId> + <artifactId>test-data</artifactId> + <version>${project.version}</version> + </dependency> + </dependencies> + </profile> </profiles> </project> From 17192f9eb9b103c0f442450ae643da5c7ea95fb0 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Tue, 4 Mar 2025 06:12:15 +0100 Subject: [PATCH 05/16] Handle the case with no test data populators (#122) Since there is no populator yet Spring fails when trying to inject since it does not support empty collections. Mark the dependency as optional until we have at least one populator at which point we can simply the code again. Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/122 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> --- .../main/java/se/su/dsv/scipro/testdata/DataInitializer.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 8b157598c4..b2fec13a8e 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 @@ -42,7 +42,7 @@ public class DataInitializer implements Lifecycle, BaseData, Factory { public static final long RESEARCH_AREA_ID = 12L; @Inject - private Collection<TestDataPopulator> testDataPopulators = new ArrayList<>(); + private Optional<Collection<TestDataPopulator>> testDataPopulators = Optional.empty(); @Inject private UserService userService; @@ -123,7 +123,8 @@ public class DataInitializer implements Lifecycle, BaseData, Factory { createTarget(); createStudentIdea(); createRoughDraftApproval(); - for (TestDataPopulator testDataPopulator : testDataPopulators) { + Collection<TestDataPopulator> availablePopulators = testDataPopulators.orElseGet(Collections::emptySet); + for (TestDataPopulator testDataPopulator : availablePopulators) { testDataPopulator.populate(this, this); } } 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 06/16] 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 07/16] 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(); } From 5c5f03bd78da3552258d630381eb1eb9f86d8a14 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Wed, 5 Mar 2025 11:53:10 +0100 Subject: [PATCH 08/16] Let Spring Boot manage dependency versions (#124) Since SciPro is now a Spring Boot-based application it is counter-productive to manage our own dependency versions. * They could conflict with what Spring Boot assumes * All libraries that are managed by Spring Boot are tested to work together QueryDSL and JPA version properties are left in because they're needed for annotation processors (they can't be dependency managed). Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/124 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> --- core/pom.xml | 2 - pom.xml | 142 --------------------------------------------------- 2 files changed, 144 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 532b1d4d04..d2a9506ee7 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -113,12 +113,10 @@ <dependency> <groupId>jakarta.xml.bind</groupId> <artifactId>jakarta.xml.bind-api</artifactId> - <version>4.0.2</version> </dependency> <dependency> <groupId>org.glassfish.jaxb</groupId> <artifactId>jaxb-runtime</artifactId> - <version>4.0.5</version> <scope>runtime</scope> </dependency> </dependencies> diff --git a/pom.xml b/pom.xml index 896b1269e5..f79b662c36 100755 --- a/pom.xml +++ b/pom.xml @@ -23,23 +23,10 @@ <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding> <!-- Dependency versions --> - <slf4j.version>2.0.7</slf4j.version> - <log4j2.version>2.20.0</log4j2.version> <wicket.version>10.4.0</wicket.version> - - <!-- See https://hibernate.org/orm/releases/ for which version Hibernate implements --> <jakarta.persistence-api.version>3.1.0</jakarta.persistence-api.version> - <hibernate.version>6.5.0.Final</hibernate.version> - <mariadb-java-client.version>3.2.0</mariadb-java-client.version> - <querydsl.version>5.0.0</querydsl.version> - <jakarta.servlet.version>5.0.0</jakarta.servlet.version> - <junit.version>5.9.3</junit.version> - <mockito.version>5.3.1</mockito.version> - <flyway.version>9.19.1</flyway.version> - <jersey.version>3.1.6</jersey.version> <poi.version>5.2.5</poi.version> - <jackson.version>2.17.0</jackson.version> <!-- When updating spring-boot check if the transitive dependency on json-smart has been @@ -114,47 +101,6 @@ <type>pom</type> </dependency> - <!-- Servlet API, needed for compilation. --> - <dependency> - <groupId>jakarta.servlet</groupId> - <artifactId>jakarta.servlet-api</artifactId> - <version>${jakarta.servlet.version}</version> - <scope>provided</scope> - </dependency> - - <!-- LOGGING DEPENDENCIES --> - <dependency> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-api</artifactId> - <version>${slf4j.version}</version> - </dependency> - <dependency> - <groupId>org.slf4j</groupId> - <artifactId>jcl-over-slf4j</artifactId> - <version>${slf4j.version}</version> - </dependency> - <dependency> - <groupId>org.apache.logging.log4j</groupId> - <artifactId>log4j-bom</artifactId> - <version>${log4j2.version}</version> - <type>pom</type> - <scope>import</scope> - </dependency> - - <dependency> - <groupId>org.mariadb.jdbc</groupId> - <artifactId>mariadb-java-client</artifactId> - <version>${mariadb-java-client.version}</version> - </dependency> - - <!--QueryDSL--> - <dependency> - <groupId>com.querydsl</groupId> - <artifactId>querydsl-bom</artifactId> - <version>${querydsl.version}</version> - <type>pom</type> - <scope>import</scope> - </dependency> <dependency> <groupId>com.querydsl</groupId> <artifactId>querydsl-jpa</artifactId> @@ -162,41 +108,6 @@ <classifier>jakarta</classifier> </dependency> - <!-- JPA --> - <dependency> - <groupId>jakarta.persistence</groupId> - <artifactId>jakarta.persistence-api</artifactId> - <version>${jakarta.persistence-api.version}</version> - </dependency> - <!-- Hibernate impl --> - <dependency> - <groupId>org.hibernate.orm</groupId> - <artifactId>hibernate-core</artifactId> - <version>${hibernate.version}</version> - <scope>runtime</scope> - </dependency> - - <!-- Jersey/Jax-Rs --> - <dependency> - <groupId>jakarta.ws.rs</groupId> - <artifactId>jakarta.ws.rs-api</artifactId> - <version>3.1.0</version> - </dependency> - <dependency> - <groupId>org.glassfish.jersey</groupId> - <artifactId>jersey-bom</artifactId> - <version>${jersey.version}</version> - <type>pom</type> - <scope>import</scope> - </dependency> - <dependency> - <groupId>com.fasterxml.jackson</groupId> - <artifactId>jackson-bom</artifactId> - <version>${jackson.version}</version> - <type>pom</type> - <scope>import</scope> - </dependency> - <!-- Additional dependencies --> <dependency> <groupId>com.google.guava</groupId> @@ -204,22 +115,6 @@ <version>32.0.1-jre</version> </dependency> - <dependency> - <groupId>jakarta.mail</groupId> - <artifactId>jakarta.mail-api</artifactId> - <version>2.1.3</version> - </dependency> - <dependency> - <groupId>jakarta.activation</groupId> - <artifactId>jakarta.activation-api</artifactId> - <version>2.1.3</version> - </dependency> - <dependency> - <groupId>org.eclipse.angus</groupId> - <artifactId>jakarta.mail</artifactId> - <version>2.0.2</version> - <scope>runtime</scope> - </dependency> <dependency> <!-- 2.5.1 is brought in transitively by @@ -235,32 +130,6 @@ <version>2.5.2</version> </dependency> - <!-- Test stuff --> - <dependency> - <groupId>org.junit</groupId> - <artifactId>junit-bom</artifactId> - <version>${junit.version}</version> - <type>pom</type> - <scope>import</scope> - </dependency> - <dependency> - <groupId>org.mockito</groupId> - <artifactId>mockito-core</artifactId> - <version>${mockito.version}</version> - <scope>test</scope> - </dependency> - <dependency> - <groupId>org.mockito</groupId> - <artifactId>mockito-junit-jupiter</artifactId> - <version>${mockito.version}</version> - <scope>test</scope> - </dependency> - <dependency> - <groupId>org.hamcrest</groupId> - <artifactId>hamcrest</artifactId> - <version>2.2</version> - <scope>test</scope> - </dependency> <dependency> <groupId>org.apache.poi</groupId> <artifactId>poi</artifactId> @@ -271,16 +140,6 @@ <artifactId>poi-ooxml</artifactId> <version>${poi.version}</version> </dependency> - <dependency> - <groupId>org.flywaydb</groupId> - <artifactId>flyway-core</artifactId> - <version>${flyway.version}</version> - </dependency> - <dependency> - <groupId>org.flywaydb</groupId> - <artifactId>flyway-mysql</artifactId> - <version>${flyway.version}</version> - </dependency> <dependency> <groupId>org.springdoc</groupId> <artifactId>springdoc-openapi-starter-webmvc-ui</artifactId> @@ -320,7 +179,6 @@ <dependency> <groupId>org.slf4j</groupId> <artifactId>slf4j-simple</artifactId> - <version>${slf4j.version}</version> <scope>test</scope> </dependency> </dependencies> From ed365bd7f56d67e2df1b9692296ef102f4fa5f20 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Wed, 5 Mar 2025 13:01:20 +0100 Subject: [PATCH 09/16] Update library used to generate Excel files (#125) Fixes #94 Fixes #93 ## How to test 1. Log in as the default admin 2. Go to "Project management / Projects" 3. Click "Download as Excel" under the table 4. See that it's still a valid Excel-file Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/125 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> --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f79b662c36..3473242939 100755 --- a/pom.xml +++ b/pom.xml @@ -26,7 +26,7 @@ <wicket.version>10.4.0</wicket.version> <jakarta.persistence-api.version>3.1.0</jakarta.persistence-api.version> <querydsl.version>5.0.0</querydsl.version> - <poi.version>5.2.5</poi.version> + <poi.version>5.4.0</poi.version> <!-- When updating spring-boot check if the transitive dependency on json-smart has been From 23e0a7f5ead02dba3724ce7fdbdcb6f24d8b7d5b Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Wed, 5 Mar 2025 14:07:47 +0100 Subject: [PATCH 10/16] Improvements to the Excel export of projects (#126) The research area column show the string "null" instead of being an empty cell for projects without a research area. This has been fixed everywhere and not just on the project export. The reviewer column showed weird technical details (`User#toString()`) instead of the reviewers name. ## How to test 1. On `develop` branch 2. Log in as the default admin 3. Go to "Project management / Projects" 4. Click "Excel export" under the table 5. Open the file and see 6. Repeat 1-5 on this branch Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/126 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> --- .../dsv/scipro/datatables/project/ProjectDataPanel.java | 8 +++++--- view/src/main/java/se/su/dsv/scipro/io/ExcelExporter.java | 4 +++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/view/src/main/java/se/su/dsv/scipro/datatables/project/ProjectDataPanel.java b/view/src/main/java/se/su/dsv/scipro/datatables/project/ProjectDataPanel.java index 04b5d7b3f8..e58b5cce94 100644 --- a/view/src/main/java/se/su/dsv/scipro/datatables/project/ProjectDataPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/datatables/project/ProjectDataPanel.java @@ -1,6 +1,5 @@ package se.su.dsv.scipro.datatables.project; -import com.google.common.eventbus.EventBus; import jakarta.inject.Inject; import java.util.*; import org.apache.wicket.ajax.AjaxRequestTarget; @@ -33,7 +32,6 @@ import se.su.dsv.scipro.components.datatables.MultipleUsersColumn; import se.su.dsv.scipro.components.datatables.UserColumn; import se.su.dsv.scipro.dataproviders.FilteredDataProvider; import se.su.dsv.scipro.datatables.AjaxCheckboxWrapper; -import se.su.dsv.scipro.notifications.NotificationController; import se.su.dsv.scipro.profile.UserLinkPanel; import se.su.dsv.scipro.project.Project; import se.su.dsv.scipro.project.ProjectService; @@ -45,7 +43,6 @@ import se.su.dsv.scipro.system.ProjectType; import se.su.dsv.scipro.system.ProjectTypeService; import se.su.dsv.scipro.system.ResearchArea; import se.su.dsv.scipro.system.User; -import se.su.dsv.scipro.system.UserService; import se.su.dsv.scipro.util.PageParameterKeys; public class ProjectDataPanel extends Panel { @@ -170,6 +167,11 @@ public class ProjectDataPanel extends Panel { ) { cellItem.add(new ReviewerColumnCell(componentId, rowModel)); } + + @Override + public IModel<?> getDataModel(IModel<Project> rowModel) { + return rowModel.map(Project::getReviewer).map(User::getFullName); + } }; } diff --git a/view/src/main/java/se/su/dsv/scipro/io/ExcelExporter.java b/view/src/main/java/se/su/dsv/scipro/io/ExcelExporter.java index 14946fc9a4..7e4edfd722 100644 --- a/view/src/main/java/se/su/dsv/scipro/io/ExcelExporter.java +++ b/view/src/main/java/se/su/dsv/scipro/io/ExcelExporter.java @@ -72,7 +72,9 @@ public class ExcelExporter extends AbstractDataExporter { for (int i = 0; i < columns.size(); i++) { Object cellValue = columns.get(i).getDataModel(data).getObject(); Cell cell = row.createCell(i); - cell.setCellValue(String.valueOf(cellValue)); + if (cellValue != null) { + cell.setCellValue(cellValue.toString()); + } } } } From de18f200a7b95ad9e1e7bc9026335f80bd2a4412 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Mon, 10 Mar 2025 07:17:28 +0100 Subject: [PATCH 11/16] Make exemptions for project type apply to partner as well (#128) Read #119 for context first When authors select supervisor ideas during an open application period they are only allowed to select ideas corresponding to their degree type classification. This limitation can be lifted by giving the author an exemption for "Project type limitation" on the corresponding application period. However, this limitation is still enforced for any potential partner *even if* the have been given the same exemption. This change makes it so the exemption applies to any selected partner as well and not just the author selecting the supervisor idea. ## How to test 1. Log in as `oskar@example.com` 2. Go to "Ideas / My ideas" page 3. Click "Select from available ideas" on the application period "Supervisor ideas" 4. Open the one available idea 5. Try to select it with "Johan Student" as a partner 6. Log in as admin 7. Go to "Match / Application periods" 8. Click "Edit exemptions" on the "Supervisor ideas" period 9. Give "Johan Student" an exemption to "Project type limitation" 10. Repeat steps 1-5 Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/128 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> --- .../su/dsv/scipro/match/IdeaServiceImpl.java | 6 +- .../dsv/scipro/match/IdeaServiceImplTest.java | 34 ++++++++++ .../se/su/dsv/scipro/testdata/BaseData.java | 10 +++ .../dsv/scipro/testdata/DataInitializer.java | 5 ++ .../populators/PartnerTypeExemption.java | 68 +++++++++++++++++++ 5 files changed, 119 insertions(+), 4 deletions(-) create mode 100644 test-data/src/main/java/se/su/dsv/scipro/testdata/populators/PartnerTypeExemption.java diff --git a/core/src/main/java/se/su/dsv/scipro/match/IdeaServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/match/IdeaServiceImpl.java index 08845a8a3f..2e97a2b07a 100755 --- a/core/src/main/java/se/su/dsv/scipro/match/IdeaServiceImpl.java +++ b/core/src/main/java/se/su/dsv/scipro/match/IdeaServiceImpl.java @@ -235,10 +235,8 @@ public class IdeaServiceImpl extends AbstractServiceImpl<Idea, Long> implements if (authorParticipatingOnActiveIdea(coAuthor, ap)) { return new Pair<>(Boolean.FALSE, PARTNER_ALREADY_PARTICIPATING_ERROR); } - if ( - coAuthor.getDegreeType() != ProjectType.UNKNOWN && - coAuthor.getDegreeType() != idea.getProjectType().getDegreeType() - ) { + List<ProjectType> typesForCoAuthor = applicationPeriodService.getTypesForStudent(ap, coAuthor); + if (!typesForCoAuthor.contains(idea.getProjectType())) { return new Pair<>(Boolean.FALSE, WRONG_LEVEL_FOR_YOUR_PARTNER); } if (!projectService.getActiveProjectsByUserAndProjectType(coAuthor, idea.getProjectType()).isEmpty()) { diff --git a/core/src/test/java/se/su/dsv/scipro/match/IdeaServiceImplTest.java b/core/src/test/java/se/su/dsv/scipro/match/IdeaServiceImplTest.java index 6365ba546a..871ea15351 100755 --- a/core/src/test/java/se/su/dsv/scipro/match/IdeaServiceImplTest.java +++ b/core/src/test/java/se/su/dsv/scipro/match/IdeaServiceImplTest.java @@ -241,6 +241,7 @@ public class IdeaServiceImplTest { when(generalSystemSettingsService.getGeneralSystemSettingsInstance()).thenReturn(new GeneralSystemSettings()); Idea idea = createBachelorIdea(Idea.Status.UNMATCHED); when(applicationPeriodService.getTypesForStudent(applicationPeriod, student)).thenReturn(List.of(bachelor)); + when(applicationPeriodService.getTypesForStudent(applicationPeriod, coAuthor)).thenReturn(List.of(bachelor)); Pair<Boolean, String> acceptance = ideaService.validateStudentAcceptance( idea, @@ -401,6 +402,39 @@ public class IdeaServiceImplTest { assertEquals(expected, ideaService.countAuthorsByApplicationPeriod(applicationPeriod, params)); } + @Test + public void wrong_type_for_author() { + when(applicationPeriodService.getTypesForStudent(applicationPeriod, student)).thenReturn(List.of(master)); + when(applicationPeriodService.getTypesForStudent(applicationPeriod, coAuthor)).thenReturn(List.of(bachelor)); + + assertPair( + false, + "The idea is the wrong level for you, please pick another one.", + ideaService.validateStudentAcceptance( + createBachelorIdea(Idea.Status.UNMATCHED), + student, + coAuthor, + applicationPeriod + ) + ); + } + + @Test + public void wrong_type_for_partner() { + when(applicationPeriodService.getTypesForStudent(applicationPeriod, coAuthor)).thenReturn(List.of(master)); + + assertPair( + false, + "The idea is the wrong level for your partner, please pick another one.", + ideaService.validateStudentAcceptance( + createBachelorIdea(Idea.Status.UNMATCHED), + student, + coAuthor, + applicationPeriod + ) + ); + } + private Idea mockInactiveIdea() { Idea idea = new Idea(); Match match = new Match(); diff --git a/test-data/src/main/java/se/su/dsv/scipro/testdata/BaseData.java b/test-data/src/main/java/se/su/dsv/scipro/testdata/BaseData.java index 1dd6b18132..56bb1313a4 100644 --- a/test-data/src/main/java/se/su/dsv/scipro/testdata/BaseData.java +++ b/test-data/src/main/java/se/su/dsv/scipro/testdata/BaseData.java @@ -1,6 +1,9 @@ package se.su.dsv.scipro.testdata; +import java.util.List; +import se.su.dsv.scipro.match.Keyword; import se.su.dsv.scipro.system.ProjectType; +import se.su.dsv.scipro.system.ResearchArea; /// All the base test data that can be re-used in different test cases. /// @@ -16,4 +19,11 @@ public interface BaseData { ProjectType bachelor(); ProjectType magister(); ProjectType master(); + + /** + * @return generic research area with some keywords attached to it + */ + ResearchAreaAndKeywords researchArea(); + + record ResearchAreaAndKeywords(ResearchArea researchArea, List<Keyword> keywords) {} } 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 cf01ab2315..c12cb602b2 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 @@ -2193,6 +2193,11 @@ public class DataInitializer implements Lifecycle, BaseData, Factory { return createEmployee(firstName); } + @Override + public ResearchAreaAndKeywords researchArea() { + return new ResearchAreaAndKeywords(researchArea1, List.of(keyword1, keyword2)); + } + private static final class SimpleTextFile implements FileUpload { private final User uploader; diff --git a/test-data/src/main/java/se/su/dsv/scipro/testdata/populators/PartnerTypeExemption.java b/test-data/src/main/java/se/su/dsv/scipro/testdata/populators/PartnerTypeExemption.java new file mode 100644 index 0000000000..c490d0b069 --- /dev/null +++ b/test-data/src/main/java/se/su/dsv/scipro/testdata/populators/PartnerTypeExemption.java @@ -0,0 +1,68 @@ +package se.su.dsv.scipro.testdata.populators; + +import jakarta.inject.Inject; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.util.Set; +import org.springframework.stereotype.Service; +import se.su.dsv.scipro.match.ApplicationPeriod; +import se.su.dsv.scipro.match.ApplicationPeriodService; +import se.su.dsv.scipro.match.Idea; +import se.su.dsv.scipro.match.IdeaService; +import se.su.dsv.scipro.match.Target; +import se.su.dsv.scipro.match.TargetService; +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 PartnerTypeExemption implements TestDataPopulator { + + private final ApplicationPeriodService applicationPeriodService; + private final IdeaService ideaService; + private final TargetService targetService; + + @Inject + public PartnerTypeExemption( + ApplicationPeriodService applicationPeriodService, + IdeaService ideaService, + TargetService targetService + ) { + this.applicationPeriodService = applicationPeriodService; + this.ideaService = ideaService; + this.targetService = targetService; + } + + @Override + public void populate(BaseData baseData, Factory factory) { + factory.createAuthor("Oskar"); + + User johan = factory.createAuthor("Johan"); + johan.setDegreeType(baseData.master().getDegreeType()); + + User supervisor = factory.createSupervisor("Elsa"); + + ApplicationPeriod applicationPeriod = new ApplicationPeriod("Supervisor ideas"); + applicationPeriod.setStartDate(LocalDate.now()); + applicationPeriod.setEndDate(LocalDate.now().plusDays(14)); + applicationPeriod.setCourseStartDateTime(LocalDateTime.now().plusDays(15)); + applicationPeriod.setProjectTypes(Set.of(baseData.bachelor())); + applicationPeriodService.save(applicationPeriod); + + Target target = targetService.findOne(applicationPeriod, supervisor, baseData.bachelor()); + target.setTarget(10); + targetService.save(target); + + Idea idea = new Idea(); + idea.setPublished(true); + idea.setTitle("The next gen AI 2.0 turbo edition"); + idea.setPrerequisites("Hacker experience"); + idea.setDescription("Better than all the rest"); + idea.setProjectType(baseData.bachelor()); + idea.setApplicationPeriod(applicationPeriod); + idea.setResearchArea(baseData.researchArea().researchArea()); + + ideaService.saveSupervisorIdea(idea, supervisor, baseData.researchArea().keywords(), true); + } +} From 9ede262e7bd7767933e0ce4a349bb73436a8786b Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Tue, 11 Mar 2025 08:49:16 +0100 Subject: [PATCH 12/16] Fix crash due to JSON parsing on "Finishing up" tab (#132) The seemingly unused library `jersey-hk2` that got removed in #118 is used, if present, internally by the Jersey client to find and register Jackson modules (such as those that provide `java.time` support). ## How to test 1. Turn on the `DAISY-INTEGRATION` Maven profile (alongside `DEV`) 2. Configure some projects and their authors to have a Daisy connection 3. Log in as the supervisor 4. Go to the "Finishing up" tab in the project 5. See that it works compared to `develop` branch Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/132 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> --- core/pom.xml | 4 ++ ...adingReportServiceImplIntegrationTest.java | 45 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/core/pom.xml b/core/pom.xml index d2a9506ee7..cda6ec6cdd 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -32,6 +32,10 @@ <groupId>org.glassfish.jersey.media</groupId> <artifactId>jersey-media-json-jackson</artifactId> </dependency> + <dependency> + <groupId>org.glassfish.jersey.inject</groupId> + <artifactId>jersey-hk2</artifactId> + </dependency> <dependency> <groupId>org.springframework</groupId> <artifactId>spring-context</artifactId> 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 597396bbea..63808671e3 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 @@ -6,7 +6,11 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.common.eventbus.EventBus; +import com.sun.net.httpserver.HttpServer; import jakarta.inject.Inject; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; import java.time.LocalDate; import java.time.Month; import java.util.*; @@ -15,6 +19,9 @@ 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.grading.GetGradeError; +import se.su.dsv.scipro.grading.GradingServiceImpl; +import se.su.dsv.scipro.grading.Result; import se.su.dsv.scipro.project.Project; import se.su.dsv.scipro.security.auth.roles.Roles; import se.su.dsv.scipro.system.DegreeType; @@ -149,6 +156,44 @@ public class GradingReportServiceImplIntegrationTest extends IntegrationTest { assertNull(oppositionCriterion.getFeedback()); } + @Test + public void test_json_deserialization() throws IOException { + String json = + """ + { + "grade": "A", + "reported": "2021-01-01" + } + """; + HttpServer httpServer = startHttpServerWithJsonResponse(json); + + int port = httpServer.getAddress().getPort(); + + GradingServiceImpl gradingService = new GradingServiceImpl("http://localhost:" + port); + Either<GetGradeError, Optional<Result>> result = gradingService.getResult("token", 1, 2, 3); + + Optional<Result> right = result.right(); + assertTrue(right.isPresent()); + assertEquals(LocalDate.of(2021, 1, 1), right.get().reported()); + + httpServer.stop(0); + } + + private static HttpServer startHttpServerWithJsonResponse(String json) throws IOException { + HttpServer httpServer = HttpServer.create(); + httpServer.createContext("/", exchange -> { + try (exchange) { + byte[] response = json.getBytes(StandardCharsets.UTF_8); + exchange.getResponseHeaders().add("Content-Type", "application/json"); + exchange.sendResponseHeaders(200, response.length); + exchange.getResponseBody().write(response); + } + }); + httpServer.bind(new InetSocketAddress("localhost", 0), 0); + httpServer.start(); + return httpServer; + } + private void addOppositionCriterion() { gradingReportTemplate = createOppositionCriteria(gradingReportTemplate, 2); gradingReport = createGradingReport(project, student); From 59e3ec3fd940bf257546d67301233979c27de239 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Tue, 11 Mar 2025 09:15:01 +0100 Subject: [PATCH 13/16] Maintain project selection on validation failure during group creation (#133) Fixes #129 ## How to test 1. Log in as `evan@example.com` 2. Go to "My groups" 3. Click "Create new group" 4. Select some projects but do *not* fill in the "Title" 5. Click save 6. Error message should be presented 7. Project selection should be maintained Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/133 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> --- .../src/main/java/se/su/dsv/scipro/group/EditGroupPanel.java | 5 +++++ 1 file changed, 5 insertions(+) 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 831986ffad..05b8396ffa 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 @@ -59,6 +59,11 @@ public class EditGroupPanel extends Panel { }); add( new ListView<>("available_projects", availableProjects) { + { + // must re-use list items to maintain form component (checkboxes) state + setReuseItems(true); + } + @Override protected void populateItem(ListItem<Project> item) { CheckBox checkbox = new CheckBox("selected", new SelectProjectModel(model, item.getModel())); From 6b77142a067597557522007a51bd928fdc394153 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Tue, 18 Mar 2025 07:33:38 +0100 Subject: [PATCH 14/16] New Daisy API XSD (#134) Allows a way to solve #119 Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/134 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> --- core/src/main/xsd/daisy_api.xsd | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/main/xsd/daisy_api.xsd b/core/src/main/xsd/daisy_api.xsd index fa68eef856..7ff97ec5a7 100755 --- a/core/src/main/xsd/daisy_api.xsd +++ b/core/src/main/xsd/daisy_api.xsd @@ -559,7 +559,7 @@ <xs:complexType name="course"> <xs:sequence> - <xs:element name="courseCode" type="xs:string" minOccurs="0"> + <xs:element name="courseCode" type="xs:string" minOccurs="1"> </xs:element> <xs:element name="credits" type="xs:float" minOccurs="1"> </xs:element> @@ -567,6 +567,8 @@ </xs:element> <xs:element name="level" type="educationalLevel" minOccurs="0"> </xs:element> + <xs:element name="degreeThesisCourse" type="xs:boolean" minOccurs="1"> + </xs:element> <xs:element name="eduInstDesignation" type="xs:string" minOccurs="1"> </xs:element> </xs:sequence> From 7504c267c5a3f6faefd590f770347d6ce38b97fe Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Tue, 18 Mar 2025 08:34:21 +0100 Subject: [PATCH 15/16] Delete forum replies (#137) Allows deleting (your own) forum replies. Fixes #90 ## How to test 1. Log in as `eric@example.com` (supervisor) or as `sture@example.com` (author) 2. Open the forum in project "Putting the it in supervising" 3. Create a new thread 4. Post some replies as the different users 5. Delete the replies Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/137 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 | 6 +- .../dsv/scipro/forum/BasicForumService.java | 8 ++ .../scipro/forum/BasicForumServiceImpl.java | 60 +++++++++++++- .../scipro/forum/dataobjects/ForumThread.java | 9 -- .../forum/BasicForumServiceImplTest.java | 2 + .../BasicForumServiceIntegrationTest.java | 83 +++++++++++++++++++ .../se/su/dsv/scipro/test/SpringTest.java | 27 +++++- .../forum/panels/threaded/ForumPostPanel.html | 5 +- .../forum/panels/threaded/ForumPostPanel.java | 30 +++++++ .../panels/threaded/ThreadsOverviewPanel.java | 16 ++-- .../panels/threaded/ViewForumThreadPanel.java | 16 +++- .../threaded/ThreadsOverviewPanelTest.java | 5 +- 12 files changed, 241 insertions(+), 26 deletions(-) create mode 100644 core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceIntegrationTest.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 352c43456a..bdcf367030 100644 --- a/core/src/main/java/se/su/dsv/scipro/CoreConfig.java +++ b/core/src/main/java/se/su/dsv/scipro/CoreConfig.java @@ -349,14 +349,16 @@ public class CoreConfig { ForumPostReadStateRepository readStateRepository, AbstractThreadRepository threadRepository, FileService fileService, - EventBus eventBus + EventBus eventBus, + CurrentUser currentUser ) { return new BasicForumServiceImpl( forumPostRepository, readStateRepository, threadRepository, fileService, - eventBus + eventBus, + currentUser ); } diff --git a/core/src/main/java/se/su/dsv/scipro/forum/BasicForumService.java b/core/src/main/java/se/su/dsv/scipro/forum/BasicForumService.java index 6a680ad26f..44ff9f407a 100644 --- a/core/src/main/java/se/su/dsv/scipro/forum/BasicForumService.java +++ b/core/src/main/java/se/su/dsv/scipro/forum/BasicForumService.java @@ -23,4 +23,12 @@ public interface BasicForumService extends Serializable { ForumThread createThread(String subject); long countUnreadThreads(List<ForumThread> forumThreadList, User user); + + ForumPost getLastPost(ForumThread forumThread); + + boolean hasAttachments(ForumThread forumThread); + + boolean canDelete(ForumPost forumPost); + + void deletePost(ForumPost post); } diff --git a/core/src/main/java/se/su/dsv/scipro/forum/BasicForumServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/forum/BasicForumServiceImpl.java index c48e3cab91..c74a6fecab 100644 --- a/core/src/main/java/se/su/dsv/scipro/forum/BasicForumServiceImpl.java +++ b/core/src/main/java/se/su/dsv/scipro/forum/BasicForumServiceImpl.java @@ -10,6 +10,7 @@ import se.su.dsv.scipro.file.FileService; import se.su.dsv.scipro.forum.dataobjects.ForumPost; import se.su.dsv.scipro.forum.dataobjects.ForumPostReadState; import se.su.dsv.scipro.forum.dataobjects.ForumThread; +import se.su.dsv.scipro.system.CurrentUser; import se.su.dsv.scipro.system.User; public class BasicForumServiceImpl implements BasicForumService { @@ -19,6 +20,7 @@ public class BasicForumServiceImpl implements BasicForumService { private final ForumPostReadStateRepository readStateRepository; private final FileService fileService; private final EventBus eventBus; + private final CurrentUser currentUserProvider; @Inject public BasicForumServiceImpl( @@ -26,13 +28,15 @@ public class BasicForumServiceImpl implements BasicForumService { final ForumPostReadStateRepository readStateRepository, AbstractThreadRepository threadRepository, final FileService fileService, - final EventBus eventBus + final EventBus eventBus, + final CurrentUser currentUserProvider ) { this.postRepository = postRepository; this.readStateRepository = readStateRepository; this.threadRepository = threadRepository; this.fileService = fileService; this.eventBus = eventBus; + this.currentUserProvider = currentUserProvider; } @Override @@ -66,7 +70,7 @@ public class BasicForumServiceImpl implements BasicForumService { @Override public boolean isThreadRead(User user, ForumThread forumThread) { - for (ForumPost post : forumThread.getPosts()) { + for (ForumPost post : getPosts(forumThread)) { if (!getReadState(user, post).isRead()) { return false; } @@ -133,4 +137,56 @@ public class BasicForumServiceImpl implements BasicForumService { return post; } + + @Override + public ForumPost getLastPost(ForumThread forumThread) { + return Collections.max( + getPosts(forumThread), + Comparator.comparing(ForumPost::getDateCreated).thenComparing(ForumPost::getId) + ); + } + + @Override + public boolean hasAttachments(ForumThread forumThread) { + for (ForumPost post : getPosts(forumThread)) { + if (!post.getAttachments().isEmpty()) { + return true; + } + } + return false; + } + + @Override + public boolean canDelete(ForumPost forumPost) { + ForumPost initialPost = forumPost.getForumThread().getPosts().get(0); + if (forumPost.equals(initialPost)) { + // The initial post in a thread can never be deleted + return false; + } + + User user = currentUserProvider.get(); + // Current user can be null meaning the call came from the system + if (user == null) { + // Allow the system to delete any post + return true; + } + return Objects.equals(forumPost.getPostedBy(), user); + } + + @Override + @Transactional + public void deletePost(ForumPost post) { + if (!canDelete(post)) { + throw new PostCantBeDeletedException(); + } + post.setDeleted(true); + postRepository.save(post); + } + + private static final class PostCantBeDeletedException extends IllegalArgumentException { + + public PostCantBeDeletedException() { + super("User is not allowed to delete post"); + } + } } diff --git a/core/src/main/java/se/su/dsv/scipro/forum/dataobjects/ForumThread.java b/core/src/main/java/se/su/dsv/scipro/forum/dataobjects/ForumThread.java index 1c61c2c563..f1933e89c0 100644 --- a/core/src/main/java/se/su/dsv/scipro/forum/dataobjects/ForumThread.java +++ b/core/src/main/java/se/su/dsv/scipro/forum/dataobjects/ForumThread.java @@ -116,13 +116,4 @@ public class ForumThread extends LazyDeletableDomainObject { public User getCreatedBy() { return getPosts().get(0).getPostedBy(); } - - public boolean hasAttachments() { - for (ForumPost post : posts) { - if (!post.getAttachments().isEmpty()) { - return true; - } - } - return false; - } } diff --git a/core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceImplTest.java b/core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceImplTest.java index de6288d6f4..6846f54411 100644 --- a/core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceImplTest.java +++ b/core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceImplTest.java @@ -121,6 +121,8 @@ public class BasicForumServiceImplTest { ForumThread forumThread = new ForumThread(); forumThread.addPost(post); + when(postRepository.findByThread(forumThread)).thenReturn(List.of(post)); + when(readStateRepository.find(eq(goodUser), isA(ForumPost.class))).thenReturn(readState); when(readStateRepository.find(eq(badUser), isA(ForumPost.class))).thenReturn(notReadState); diff --git a/core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceIntegrationTest.java b/core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceIntegrationTest.java new file mode 100644 index 0000000000..ec5d815af7 --- /dev/null +++ b/core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceIntegrationTest.java @@ -0,0 +1,83 @@ +package se.su.dsv.scipro.forum; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import jakarta.inject.Inject; +import java.util.Set; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import se.su.dsv.scipro.forum.dataobjects.ForumPost; +import se.su.dsv.scipro.forum.dataobjects.ForumThread; +import se.su.dsv.scipro.system.User; +import se.su.dsv.scipro.test.IntegrationTest; + +public class BasicForumServiceIntegrationTest extends IntegrationTest { + + @Inject + BasicForumService basicForumService; + + private User op; + private User commenter; + + @BeforeEach + public void setUp() { + User op = User.builder().firstName("Bill").lastName("Gates").emailAddress("bill@example.com").build(); + this.op = save(op); + + User commenter = User.builder().firstName("Steve").lastName("Jobs").emailAddress("steve@example.com").build(); + this.commenter = save(commenter); + } + + @Test + public void can_not_delete_original_post() { + ForumThread thread = basicForumService.createThread("Test thread"); + ForumPost originalPost = basicForumService.createReply(thread, op, "Test post", Set.of()); + + setLoggedInAs(op); + + assertFalse(basicForumService.canDelete(originalPost)); + assertThrows(IllegalArgumentException.class, () -> basicForumService.deletePost(originalPost)); + } + + @Test + public void can_delete_reply_to_original_post() { + ForumThread thread = basicForumService.createThread("Test thread"); + ForumPost originalPost = basicForumService.createReply(thread, op, "Test post", Set.of()); + ForumPost reply = basicForumService.createReply(thread, commenter, "Test reply", Set.of()); + + setLoggedInAs(commenter); + + assertTrue(basicForumService.canDelete(reply)); + assertDoesNotThrow(() -> basicForumService.deletePost(reply)); + } + + @Test + public void can_not_delete_someone_elses_reply() { + ForumThread thread = basicForumService.createThread("Test thread"); + ForumPost originalPost = basicForumService.createReply(thread, op, "Test post", Set.of()); + ForumPost reply = basicForumService.createReply(thread, commenter, "Test reply", Set.of()); + + setLoggedInAs(op); + + assertFalse(basicForumService.canDelete(reply)); + assertThrows(IllegalArgumentException.class, () -> basicForumService.deletePost(reply)); + } + + @Test + public void system_can_delete_all_replies() { + ForumThread thread = basicForumService.createThread("Test thread"); + ForumPost originalPost = basicForumService.createReply(thread, op, "Test post", Set.of()); + ForumPost reply = basicForumService.createReply(thread, commenter, "Test reply", Set.of()); + ForumPost secondReply = basicForumService.createReply(thread, op, "Test post", Set.of()); + + setLoggedInAs(null); + + assertTrue(basicForumService.canDelete(reply)); + assertDoesNotThrow(() -> basicForumService.deletePost(reply)); + assertTrue(basicForumService.canDelete(secondReply)); + assertDoesNotThrow(() -> basicForumService.deletePost(secondReply)); + } +} 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 2d52bd0d36..363fc8b80c 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,6 +1,7 @@ package se.su.dsv.scipro.test; import com.google.common.eventbus.EventBus; +import jakarta.inject.Inject; import jakarta.persistence.EntityManager; import jakarta.persistence.EntityManagerFactory; import jakarta.persistence.EntityTransaction; @@ -28,6 +29,7 @@ import se.su.dsv.scipro.RepositoryConfiguration; import se.su.dsv.scipro.profiles.CurrentProfile; import se.su.dsv.scipro.sukat.Sukat; import se.su.dsv.scipro.system.CurrentUser; +import se.su.dsv.scipro.system.User; @Testcontainers public abstract class SpringTest { @@ -38,6 +40,9 @@ public abstract class SpringTest { @Container static MariaDBContainer<?> mariaDBContainer = new MariaDBContainer<>("mariadb:10.11"); + @Inject + private TestUser testUser; + private CapturingEventBus capturingEventBus; @BeforeEach @@ -64,6 +69,8 @@ public abstract class SpringTest { annotationConfigApplicationContext.getBeanFactory().registerSingleton("entityManager", this.entityManager); annotationConfigApplicationContext.refresh(); annotationConfigApplicationContext.getAutowireCapableBeanFactory().autowireBean(this); + + testUser.setUser(null); // default to system } @AfterEach @@ -83,6 +90,10 @@ public abstract class SpringTest { } } + protected void setLoggedInAs(User user) { + this.testUser.setUser(user); + } + protected List<Object> getPublishedEvents() { return capturingEventBus.publishedEvents; } @@ -108,7 +119,7 @@ public abstract class SpringTest { @Bean public CurrentUser currentUser() { - return () -> null; + return new TestUser(); } @Bean @@ -129,4 +140,18 @@ public abstract class SpringTest { super.post(event); } } + + private static class TestUser implements CurrentUser { + + private User user; + + @Override + public User get() { + return user; + } + + private void setUser(User user) { + this.user = user; + } + } } diff --git a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.html b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.html index eba931f634..b09f0405ca 100644 --- a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.html +++ b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.html @@ -7,9 +7,10 @@ <body> <wicket:panel> <div class="messageWrap"> - <div class="forumBlueBackground"> + <div class="forumBlueBackground d-flex justify-content-between"> <!-- DATE ROW--> - <wicket:container wicket:id="dateCreated"/> + <span wicket:id="dateCreated"></span> + <button wicket:id="delete" class="btn btn-sm btn-outline-danger ms-auto">Delete</button> </div> <div class="forumGrayBackground"> <div class="vertAlign"> diff --git a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.java b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.java index 004d99561a..225240d8e1 100644 --- a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.java @@ -1,7 +1,9 @@ package se.su.dsv.scipro.forum.panels.threaded; +import jakarta.inject.Inject; import org.apache.wicket.Component; import org.apache.wicket.markup.html.WebMarkupContainer; +import org.apache.wicket.markup.html.link.Link; import org.apache.wicket.markup.html.panel.Panel; import org.apache.wicket.model.IModel; import org.apache.wicket.model.LambdaModel; @@ -11,9 +13,11 @@ import se.su.dsv.scipro.components.ListAdapterModel; import se.su.dsv.scipro.components.SmarterLinkMultiLineLabel; import se.su.dsv.scipro.data.enums.DateStyle; import se.su.dsv.scipro.file.FileReference; +import se.su.dsv.scipro.forum.BasicForumService; import se.su.dsv.scipro.forum.dataobjects.ForumPost; import se.su.dsv.scipro.profile.UserLinkPanel; import se.su.dsv.scipro.repository.panels.ViewAttachmentPanel; +import se.su.dsv.scipro.session.SciProSession; public class ForumPostPanel extends Panel { @@ -22,6 +26,9 @@ public class ForumPostPanel extends Panel { public static final String CONTENT = "content"; public static final String ATTACHMENT = "attachment"; + @Inject + private BasicForumService basicForumService; + public ForumPostPanel(String id, final IModel<ForumPost> model) { super(id); add(new UserLinkPanel(POSTED_BY, LambdaModel.of(model, ForumPost::getPostedBy, ForumPost::setPostedBy))); @@ -62,5 +69,28 @@ public class ForumPostPanel extends Panel { } } ); + + add( + new Link<>("delete", model) { + @Override + public void onClick() { + ForumPost post = getModelObject(); + basicForumService.deletePost(post); + onPostDeleted(); + } + + @Override + protected void onConfigure() { + super.onConfigure(); + setVisible(allowDeletion() && basicForumService.canDelete(getModelObject())); + } + } + ); } + + protected boolean allowDeletion() { + return false; + } + + protected void onPostDeleted() {} } diff --git a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanel.java b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanel.java index 02a9d00bd5..d0e7e1c55d 100644 --- a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanel.java @@ -1,8 +1,7 @@ package se.su.dsv.scipro.forum.panels.threaded; +import jakarta.inject.Inject; import java.io.Serializable; -import java.util.Collections; -import java.util.Comparator; import java.util.List; import org.apache.wicket.markup.html.WebMarkupContainer; import org.apache.wicket.markup.html.basic.Label; @@ -15,6 +14,7 @@ import org.apache.wicket.model.LambdaModel; import org.apache.wicket.model.LoadableDetachableModel; import se.su.dsv.scipro.components.DateLabel; import se.su.dsv.scipro.data.enums.DateStyle; +import se.su.dsv.scipro.forum.BasicForumService; import se.su.dsv.scipro.forum.Discussable; import se.su.dsv.scipro.forum.dataobjects.ForumPost; import se.su.dsv.scipro.forum.dataobjects.ForumThread; @@ -23,6 +23,9 @@ import se.su.dsv.scipro.system.User; public class ThreadsOverviewPanel<A> extends Panel { + @Inject + private BasicForumService basicForumService; + public ThreadsOverviewPanel( final String id, final IModel<List<A>> model, @@ -41,7 +44,7 @@ public class ThreadsOverviewPanel<A> extends Panel { @Override protected void onConfigure() { super.onConfigure(); - setVisibilityAllowed(discussion.getObject().hasAttachments()); + setVisibilityAllowed(basicForumService.hasAttachments(discussion.getObject())); } } ); @@ -80,7 +83,7 @@ public class ThreadsOverviewPanel<A> extends Panel { BookmarkablePageLink<Void> newThreadLink(String id, IModel<A> thread); } - private static class LastPostColumn extends WebMarkupContainer { + private class LastPostColumn extends WebMarkupContainer { public LastPostColumn(String id, final IModel<ForumThread> model) { super(id); @@ -110,10 +113,7 @@ public class ThreadsOverviewPanel<A> extends Panel { return new LoadableDetachableModel<>() { @Override protected ForumPost load() { - return Collections.max( - model.getObject().getPosts(), - Comparator.comparing(ForumPost::getDateCreated).thenComparing(ForumPost::getId) - ); + return basicForumService.getLastPost(model.getObject()); } }; } diff --git a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ViewForumThreadPanel.java b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ViewForumThreadPanel.java index 9c62f77122..06b89d7e14 100644 --- a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ViewForumThreadPanel.java +++ b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ViewForumThreadPanel.java @@ -58,7 +58,21 @@ public class ViewForumThreadPanel<A> extends GenericPanel<A> { new ListView<>(POST_LIST, new PostProvider()) { @Override protected void populateItem(ListItem<ForumPost> item) { - item.add(new ForumPostPanel(POST, item.getModel())); + ListView<ForumPost> listView = this; + item.add( + new ForumPostPanel(POST, item.getModel()) { + @Override + protected boolean allowDeletion() { + return true; + } + + @Override + protected void onPostDeleted() { + // Refresh the list of posts + listView.detach(); + } + } + ); } } ); diff --git a/view/src/test/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanelTest.java b/view/src/test/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanelTest.java index 8c99db6c4f..614f92e7cd 100644 --- a/view/src/test/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanelTest.java +++ b/view/src/test/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanelTest.java @@ -9,6 +9,7 @@ import org.apache.wicket.model.Model; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import se.su.dsv.scipro.SciProTest; import se.su.dsv.scipro.forum.Discussable; @@ -20,11 +21,13 @@ import se.su.dsv.scipro.system.User; public class ThreadsOverviewPanelTest extends SciProTest { private List<ForumThread> threads; + private ForumPost post; @BeforeEach public void setUp() throws Exception { ForumThread forumThread = createThread(); threads = Arrays.asList(forumThread); + Mockito.when(basicForumService.getLastPost(forumThread)).thenReturn(post); } @Test @@ -54,7 +57,7 @@ public class ThreadsOverviewPanelTest extends SciProTest { private ForumThread createThread() { User bob = User.builder().firstName("Bob").lastName("the Builder").emailAddress("bob@building.com").build(); - ForumPost post = new ForumPost(); + post = new ForumPost(); post.setPostedBy(bob); ForumThread groupForumThread = new ForumThread(); groupForumThread.addPost(post); From 2ac30fa98041fcbc29a31e9d93132c31bd7358c9 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Tue, 18 Mar 2025 09:12:41 +0100 Subject: [PATCH 16/16] Add Checkstyle checking during Maven build (#138) So far no rules are activated and it just puts the infrastructure in place. Rules can be added in separately after discussion among the developers, along with fixing any violations of the rules. Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/138 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> --- Dockerfile | 1 + checkstyle.xml | 7 +++++++ pom.xml | 22 ++++++++++++++++++++++ 3 files changed, 30 insertions(+) create mode 100644 checkstyle.xml diff --git a/Dockerfile b/Dockerfile index acf3c889a3..71ff8ba3a5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -24,6 +24,7 @@ COPY test-data/src/ test-data/src/ RUN ./mvnw package \ --define skipTests \ --activate-profiles branch,DEV \ + --define checkstyle.skip=true \ --define skip.npm \ --define skip.installnodenpm diff --git a/checkstyle.xml b/checkstyle.xml new file mode 100644 index 0000000000..bd24b31e31 --- /dev/null +++ b/checkstyle.xml @@ -0,0 +1,7 @@ +<?xml version="1.0"?> +<!DOCTYPE module PUBLIC + "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN" + "https://checkstyle.org/dtds/configuration_1_3.dtd"> + +<module name="Checker"> +</module> diff --git a/pom.xml b/pom.xml index 3473242939..8567faf79d 100755 --- a/pom.xml +++ b/pom.xml @@ -211,6 +211,11 @@ <artifactId>maven-war-plugin</artifactId> <version>3.4.0</version> </plugin> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-checkstyle-plugin</artifactId> + <version>3.6.0</version> + </plugin> </plugins> </pluginManagement> <plugins> @@ -312,6 +317,23 @@ </execution> </executions> </plugin> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-checkstyle-plugin</artifactId> + <configuration> + <configLocation>checkstyle.xml</configLocation> + <failOnViolation>true</failOnViolation> + </configuration> + <executions> + <execution> + <id>validate</id> + <phase>process-sources</phase> + <goals> + <goal>check</goal> + </goals> + </execution> + </executions> + </plugin> </plugins> </build> </project> \ No newline at end of file