Bug/vspc 271 - Exhibition Configuration: if not exhibition language should be required #348
Conversation
…ound" This reverts commit 552a29c.
|
Can one of the admins verify this patch? |
|
There should also be server-side validation. |
| @@ -99,6 +99,13 @@ public RedirectView createOrUpdateExhibition(HttpServletRequest request, | |||
| exhibition.setStartSpace(startSpace); | |||
| exhibition.setTitle(title); | |||
| exhibition.setMode(exhibitMode); | |||
|
|
|||
| if(languages.isEmpty()) { | |||
There was a problem hiding this comment.
I believe you can simply make the RequestParam exhibitLanguage required instead. Have you tried that?
There was a problem hiding this comment.
yes, I checked that - the RequestParam exhibitLanguage is required by default and it was giving the server error.
Can I make it required = false and add a null check here to give out an error message in the ui or remove this if block and keep it as before?
There was a problem hiding this comment.
Given that there are quite a few request parameter by now, I would create a class the paramters and validate that. Similar to how it's done here:
https://www.baeldung.com/spring-mvc-form-tutorial
|
|
||
| private String defaultExhibitLanguage; | ||
|
|
||
| private String exhibitionParam; |
There was a problem hiding this comment.
what's that? what's a exhibition param?
| private ExhibitionModes exhibitMode; | ||
|
|
||
| @NotEmpty | ||
| private String spaceParam; |
There was a problem hiding this comment.
it was passed from the form, removed it from here now
vspace/src/main/java/edu/asu/diging/vspace/web/staff/forms/ExhibitionConfigurationForm.java
Show resolved
Hide resolved
| private List<String> exhibitLanguage; | ||
|
|
||
| @NotNull(message = "Please select exhibit mode") | ||
| private ExhibitionModes exhibitMode; |
There was a problem hiding this comment.
there is not really a reason to save 3 characters, just write out "exhibition" (and in the message it definitely need to be written out)
|
Make it so, Jenkins. |
|
after that should be good |
pooja-thalur
changed the title
Guidelines for Pull Requests
If you haven't yet read our code review guidelines, please do so, You can find them here.
Please confirm the following by adding an x for each item (turn
[ ]into[x]).Please provide a brief description of your ticket
(you can copy the ticket if it hasn't changed)
Exhibition Configuration: if not exhibition language should be required
...Put description here...
The user should select at least one exhibition language from the list (right now there is a tomcat error). There should also be a field help “To select multiple languages, hold down the shift key when selecting.”
Anything else the reviewer needs to know?
... describe here ...