From db0ea51ed9a0d9b5d117ac28a9e7fcc02aeed721 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg Date: Mon, 2 Dec 2024 13:31:29 +0100 Subject: [PATCH 1/2] Fix project type selection being lost on validation error while creating application periods If you have FormComponents in a ListView you need to call setReuseItems(true) on the ListView. Otherwise the ListItems will be recreated before rendering which results in them losing their "converted input" (what Wicket calls the submitted value). Instead of simply calling setReuseItems(true) on the ListView, which would've solved the problem, it was instead replaced by a proper FormComponent for dealing with this exact case (a CheckboxMultipleChoice component). This reduces the amount of code required and more clearly communicates intent. The change required some minor test refactoring. --- .../AdminEditApplicationPeriodPage.html | 5 +- .../AdminEditApplicationPeriodPage.java | 47 +++---------------- .../AdminEditApplicationPeriodPageTest.java | 11 ++++- 3 files changed, 17 insertions(+), 46 deletions(-) diff --git a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html index 04de12622c..4581c0f65b 100644 --- a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html +++ b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html @@ -16,10 +16,7 @@
-
- - -
+
diff --git a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java index 2409d13d83..51a31f8c7c 100644 --- a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java +++ b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java @@ -4,17 +4,14 @@ import jakarta.inject.Inject; import java.time.LocalDate; import java.time.LocalTime; import java.util.List; -import org.apache.wicket.extensions.model.AbstractCheckBoxModel; import org.apache.wicket.feedback.FencedFeedbackPanel; -import org.apache.wicket.markup.html.basic.Label; import org.apache.wicket.markup.html.form.*; -import org.apache.wicket.markup.html.list.ListItem; -import org.apache.wicket.markup.html.list.ListView; import org.apache.wicket.markup.html.panel.ComponentFeedbackPanel; import org.apache.wicket.model.IModel; import org.apache.wicket.model.LambdaModel; import org.apache.wicket.model.LoadableDetachableModel; import org.apache.wicket.request.mapper.parameter.PageParameters; +import se.su.dsv.scipro.components.BootstrapCheckBoxMultipleChoice; import se.su.dsv.scipro.components.BootstrapDatePicker; import se.su.dsv.scipro.components.BootstrapTimePicker; import se.su.dsv.scipro.components.DatesValidator; @@ -39,7 +36,6 @@ public class AdminEditApplicationPeriodPage public static final String TITLE = "title"; public static final String FEEDBACK = "Feedback"; public static final String TITLE_FEEDBACK = "titleFeedback"; - public static final String CHECKBOX = "checkbox"; @Inject private ProjectTypeService projectTypeService; @@ -63,17 +59,12 @@ public class AdminEditApplicationPeriodPage add(new ComponentFeedbackPanel(TITLE_FEEDBACK, title)); add(title); add( - new ListView<>(PROJECT_TYPES, availableProjectTypes()) { - @Override - protected void populateItem(ListItem item) { - item.add( - new CheckBox(CHECKBOX, new ProjectTypeSelectionModel(item.getModel())).setOutputMarkupId( - true - ) - ); - item.add(new Label("name", item.getModel().map(ProjectType::getName))); - } - } + new BootstrapCheckBoxMultipleChoice<>( + PROJECT_TYPES, + LambdaModel.of(getModel(), ApplicationPeriod::getProjectTypes, ApplicationPeriod::setProjectTypes), + availableProjectTypes(), + new LambdaChoiceRenderer<>(ProjectType::getName, ProjectType::getId) + ) ); final FormComponent startDate = addDateField( START_DATE, @@ -139,30 +130,6 @@ public class AdminEditApplicationPeriodPage getRootForm().error(getString("overlapping")); } } - - private class ProjectTypeSelectionModel extends AbstractCheckBoxModel { - - private final IModel model; - - public ProjectTypeSelectionModel(IModel model) { - this.model = model; - } - - @Override - public boolean isSelected() { - return getModelObject().getProjectTypes().contains(model.getObject()); - } - - @Override - public void select() { - getModelObject().addProjectType(model.getObject()); - } - - @Override - public void unselect() { - getModelObject().removeProjectType(model.getObject()); - } - } } private LoadableDetachableModel getLoaded(final PageParameters pp) { diff --git a/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java b/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java index 7b68730909..04809af0c2 100644 --- a/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java +++ b/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java @@ -7,8 +7,10 @@ import java.io.Serializable; import java.time.LocalDate; import java.util.Collections; import java.util.List; +import org.apache.wicket.Component; import org.apache.wicket.Page; import org.apache.wicket.feedback.FeedbackMessage; +import org.apache.wicket.markup.html.form.CheckBoxMultipleChoice; import org.apache.wicket.markup.html.form.RequiredTextField; import org.apache.wicket.request.mapper.parameter.PageParameters; import org.apache.wicket.util.tester.FormTester; @@ -36,6 +38,7 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest { @BeforeEach public void setUp() throws Exception { bachelor = new ProjectType(DegreeType.BACHELOR, "Bachelor", "Bachelor"); + bachelor.setId(8L); bachelor.addModule(ProjectModule.MATCH); when(projectTypeService.findWithModule(ProjectModule.MATCH)).thenReturn(Collections.singletonList(bachelor)); startPage(); @@ -47,8 +50,12 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest { } @Test + @SuppressWarnings("unchecked") public void contains_project_type_selection() { - tester.assertModelValue(path(FORM, PROJECT_TYPES), projectTypeService.findWithModule(ProjectModule.MATCH)); + tester.assertComponent(path(FORM, PROJECT_TYPES), CheckBoxMultipleChoice.class); + Component component = tester.getComponentFromLastRenderedPage(path(FORM, PROJECT_TYPES)); + CheckBoxMultipleChoice choice = (CheckBoxMultipleChoice) component; + Assertions.assertEquals(projectTypeService.findWithModule(ProjectModule.MATCH), choice.getChoices()); } @Test @@ -105,7 +112,7 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest { ); FormTester formTester = tester.newFormTester(FORM); fillInForm("Title", 0, 1, 2, formTester); - formTester.setValue(path(PROJECT_TYPES, 0, CHECKBOX), true); + formTester.setValue(path(PROJECT_TYPES), String.valueOf(bachelor.getId())); formTester.submit(); ArgumentCaptor captor = ArgumentCaptor.forClass(ApplicationPeriod.class); -- 2.39.5 From d387e8002e311875728b8321bfeceda7cf40f0d2 Mon Sep 17 00:00:00 2001 From: Andreas Svanberg Date: Mon, 2 Dec 2024 14:50:29 +0100 Subject: [PATCH 2/2] Require at least one project type to be selected when creating/editing application periods --- .../AdminEditApplicationPeriodPage.html | 1 + .../AdminEditApplicationPeriodPage.java | 15 ++++++++------- .../AdminEditApplicationPeriodPage.properties | 3 ++- .../AdminEditApplicationPeriodPageTest.java | 14 ++++++++++++++ 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html index 4581c0f65b..8c5597de96 100644 --- a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html +++ b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html @@ -17,6 +17,7 @@
+
diff --git a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java index 51a31f8c7c..8781578a73 100644 --- a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java +++ b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java @@ -58,14 +58,15 @@ public class AdminEditApplicationPeriodPage ); add(new ComponentFeedbackPanel(TITLE_FEEDBACK, title)); add(title); - add( - new BootstrapCheckBoxMultipleChoice<>( - PROJECT_TYPES, - LambdaModel.of(getModel(), ApplicationPeriod::getProjectTypes, ApplicationPeriod::setProjectTypes), - availableProjectTypes(), - new LambdaChoiceRenderer<>(ProjectType::getName, ProjectType::getId) - ) + BootstrapCheckBoxMultipleChoice projectTypeChoice = new BootstrapCheckBoxMultipleChoice<>( + PROJECT_TYPES, + LambdaModel.of(getModel(), ApplicationPeriod::getProjectTypes, ApplicationPeriod::setProjectTypes), + availableProjectTypes(), + new LambdaChoiceRenderer<>(ProjectType::getName, ProjectType::getId) ); + projectTypeChoice.setRequired(true); + add(projectTypeChoice); + add(new FencedFeedbackPanel("projectTypesFeedback", projectTypeChoice)); final FormComponent startDate = addDateField( START_DATE, LambdaModel.of(getModel(), ApplicationPeriod::getStartDate, ApplicationPeriod::setStartDate) diff --git a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.properties b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.properties index f5095052d4..dc945a5788 100644 --- a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.properties +++ b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.properties @@ -14,4 +14,5 @@ success= Application period saved. overlapping= Overlapping application period already exists. date.Required= You need to specify a valid date. hours.Required= Hours field is required. -minutes.Required= Minutes field is required. \ No newline at end of file +minutes.Required= Minutes field is required. +projectTypes.Required=You must select at least one project type. diff --git a/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java b/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java index 04809af0c2..a6bc599bd9 100644 --- a/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java +++ b/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java @@ -120,12 +120,25 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest { MatcherAssert.assertThat(captor.getValue().getProjectTypes(), Matchers.hasItem(bachelor)); } + @Test + public void requires_at_least_one_project_type_to_be_selected() { + FormTester formTester = tester.newFormTester(FORM); + fillInFormWithValidValues(formTester); + formTester.setValue(path(PROJECT_TYPES), ""); + formTester.submit(); + tester.assertErrorMessages(tester.getLastRenderedPage().getString("projectTypes.Required")); + } + private void submitForm(String title, int startDate, int endDate, int courseStartDate) { FormTester formTester = tester.newFormTester(FORM); fillInForm(title, startDate, endDate, courseStartDate, formTester); formTester.submit(); } + private void fillInFormWithValidValues(FormTester formTester) { + fillInForm("Title", 0, 1, 2, formTester); + } + private void fillInForm(String title, int startDate, int endDate, int courseStartDate, FormTester formTester) { formTester.setValue(TITLE, title); final LocalDate now = LocalDate.now(); @@ -133,6 +146,7 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest { formTester.setValue(END_DATE, now.plusDays(endDate).toString()); formTester.setValue(COURSE_START_DATE, now.plusDays(courseStartDate).toString()); formTester.setValue("courseStartTime", "08:00"); + formTester.setValue(PROJECT_TYPES, String.valueOf(bachelor.getId())); } private void startPage() { -- 2.39.5