Keep and validate project type selection when creating/editing application periods (#47)
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. Fixes #33 --- Now requires at least one project type to be selected before saving. Fixes #34 --- ## How to test 1. Go to "Admin" / "Match" / "Application periods" 2. Click create new 3. Submit without selecting any types 4. See that there's proper feedback 5. Leave name blank and select some types 6. Submit 7. See that the project type selection sticks around Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Reviewed-on: #47 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>
This commit is contained in:
parent
c6bd17d9ad
commit
f67f37ecdd
view/src
main/java/se/su/dsv/scipro/applicationperiod
AdminEditApplicationPeriodPage.htmlAdminEditApplicationPeriodPage.javaAdminEditApplicationPeriodPage.properties
test/java/se/su/dsv/scipro/applicationperiod
@ -16,10 +16,8 @@
|
||||
|
||||
<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>
|
||||
<div wicket:id="projectTypesFeedback"></div>
|
||||
</fieldset>
|
||||
|
||||
<div class="form-row">
|
||||
|
||||
@ -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;
|
||||
@ -62,19 +58,15 @@ public class AdminEditApplicationPeriodPage
|
||||
);
|
||||
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)));
|
||||
}
|
||||
}
|
||||
BootstrapCheckBoxMultipleChoice<ProjectType> 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<LocalDate> startDate = addDateField(
|
||||
START_DATE,
|
||||
LambdaModel.of(getModel(), ApplicationPeriod::getStartDate, ApplicationPeriod::setStartDate)
|
||||
@ -139,30 +131,6 @@ public class AdminEditApplicationPeriodPage
|
||||
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) {
|
||||
|
||||
@ -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.
|
||||
minutes.Required= Minutes field is required.
|
||||
projectTypes.Required=You must select at least one project type.
|
||||
|
||||
@ -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<ProjectType> choice = (CheckBoxMultipleChoice<ProjectType>) 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<ApplicationPeriod> captor = ArgumentCaptor.forClass(ApplicationPeriod.class);
|
||||
@ -113,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();
|
||||
@ -126,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() {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user