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.
This commit is contained in:
Andreas Svanberg 2024-12-02 13:31:29 +01:00
parent 03ad12f435
commit d2e490bdd8
3 changed files with 21 additions and 41 deletions
view/src
main/java/se/su/dsv/scipro/applicationperiod
test/java/se/su/dsv/scipro/applicationperiod

@ -16,10 +16,7 @@
<fieldset class="mb-3">
<legend><wicket:message key="projectTypes"/></legend>
<div class="form-check" wicket:id="projectTypes">
<input type="checkbox" wicket:id="checkbox" class="form-check-input"/>
<label class="form-check-label" wicket:for="checkbox"><span wicket:id="name"></span></label>
</div>
<div wicket:id="projectTypes"></div>
</fieldset>
<div class="form-row">

@ -1,16 +1,13 @@
package se.su.dsv.scipro.applicationperiod;
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;
@ -38,7 +35,6 @@ public class AdminEditApplicationPeriodPage extends AbstractAdminMatchPage imple
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;
@ -57,13 +53,16 @@ public class AdminEditApplicationPeriodPage extends AbstractAdminMatchPage imple
RequiredTextField<String> title = new RequiredTextField<>(TITLE, LambdaModel.of(getModel(), ApplicationPeriod::getName, ApplicationPeriod::setName));
add(new ComponentFeedbackPanel(TITLE_FEEDBACK, title));
add(title);
add(new ListView<>(PROJECT_TYPES, availableProjectTypes()) {
@Override
protected void populateItem(ListItem<ProjectType> item) {
item.add(new CheckBox(CHECKBOX, new ProjectTypeSelectionModel(item.getModel())).setOutputMarkupId(true));
item.add(new Label("name", item.getModel().map(ProjectType::getName)));
}
});
add(new BootstrapCheckBoxMultipleChoice<>(
PROJECT_TYPES,
LambdaModel.of(
getModel(),
ApplicationPeriod::getProjectTypes,
ApplicationPeriod::setProjectTypes
),
availableProjectTypes(),
new LambdaChoiceRenderer<>(ProjectType::getName, ProjectType::getId)
));
final FormComponent<LocalDate> startDate = addDateField(START_DATE, LambdaModel.of(getModel(), ApplicationPeriod::getStartDate, ApplicationPeriod::setStartDate));
final FormComponent<LocalDate> endDate = addDateField(END_DATE, LambdaModel.of(getModel(), ApplicationPeriod::getEndDate, ApplicationPeriod::setEndDate));
final FormComponent<LocalDate> courseStartDate = addDateField(COURSE_START_DATE, LambdaModel.of(getModel(), ApplicationPeriod::getCourseStartDate, ApplicationPeriod::setCourseStartDate));
@ -108,29 +107,6 @@ public class AdminEditApplicationPeriodPage extends AbstractAdminMatchPage imple
getRootForm().error(getString("overlapping"));
}
}
private class ProjectTypeSelectionModel extends AbstractCheckBoxModel {
private final IModel<ProjectType> model;
public ProjectTypeSelectionModel(IModel<ProjectType> 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<ApplicationPeriod> getLoaded(final PageParameters pp){

@ -1,7 +1,9 @@
package se.su.dsv.scipro.applicationperiod;
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;
@ -38,6 +40,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();
@ -49,8 +52,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<ProjectType> choice = (CheckBoxMultipleChoice<ProjectType>) component;
Assertions.assertEquals(projectTypeService.findWithModule(ProjectModule.MATCH), choice.getChoices());
}
@Test
@ -99,7 +106,7 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest {
when(applicationPeriodService.doesPeriodOverlap(ArgumentMatchers.any(ApplicationPeriod.class))).thenReturn(NOT_OVERLAPPING);
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<ApplicationPeriod> captor = ArgumentCaptor.forClass(ApplicationPeriod.class);