|
Description
Katrin Fischer
2013-09-09 15:37:04 UTC
Created attachment 28721 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC #1/2 (part #1: changes in opac-suggestions.* script + bootstrap template) This patch adds a configuration option which allows to define which fields (apart from the title) should be mandatory for a patron purchase suggestion form in OPAC. Supported fields that are configurable as mandatory/not mandatory for suggestions made via OPAC are: author|copyrightdate|isbn| publishercode|place|itemtype|patronreason. Note: this patch only applies to OPAC bootstrap theme. To test: 1/ Apply patch. 2/ Without 'OPACSuggestionMandatoryFields' option set, OPAC suggestion form should work as previously ("Only the title is required"). 3/ Set 'OPACSuggestionMandatoryFields' syspref to (e.g.) 'author|isbn|itemtype' 4/ Note that suggestion form in OPAC now requires 3 more fields to be entered and/or choosen (they should be labeled in red), and the user instruction changes slightly ("Form fields labeled in red are mandatory" ..). 5/ Try to submit some suggestions: for each field which is configured as mandatory, but is not filled with anything, there should be and extra message displayed regarding this field. 6/ Unless all mandatory fields are filled with something, form submission should not be possible. 7/ Repeat test 3-6 for all supported field names configurable as mandatory. Created attachment 28722 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC #2/2 (part #2: new syspref 'OPACSuggestionMandatoryFields' + DB updates) Created attachment 28792 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC #2/2 (part #2: new syspref 'OPACSuggestionMandatoryFields' + DB updates) Signed-off-by: Nick Clemens <nick@debian> First time using git-bz, both patches applied and tested and signing off on both Comment on attachment 28721 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC #1/2 Review of attachment 28721 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt @@ +39,5 @@ > <form action="/cgi-bin/koha/opac-suggestions.pl" method="post"> > <fieldset class="rows"> > <ol> > <li><label class="required" for="title">Title:</label><input type="text" id="title" name="title" maxlength="255" /></li> > + <li><label [% IF (mandatoryfields.author) %]class="required" [% END %]for="author">Author:</label><input type="text" id="author" name="author" maxlength="80" /></li> It is not allowed to add template toolkit directives into html tags (see http://wiki.koha-community.org/wiki/Coding_Guidelines#HTML1:_Template_Toolkit_markup_inside_HTML). @@ +319,5 @@ > _alertString += _("- You must enter a Title") + "\n"; > } > + [% IF (mandatoryfields.author) %]if (f.author.value.length == 0){ > + _alertString += _("- You must enter an Author") + "\n"; > + }[% END %] Maybe it could be better to use the required="required" html attribute on the input element, don't you think? Comment on attachment 28792 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC #2/2 Review of attachment 28792 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref @@ +534,5 @@ > + class: multi > + - "<br />Field names you can set here as mandatory for OPAC suggestions include: <code>author</code>, <code>copyrightdate</code>," > + - "<code>isbn</code>, <code>publishercode</code>, <code>place</code>, <code>itemtype</code> and <code>patronreason</code>." > + - "Note: <code>title</code> field would be allways implicitly required, no need to enter it here." > + - typo "allways" Created attachment 29091 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC #1/2 (part #1: changes in opac-suggestions.* script + bootstrap template) This patch: - adds a configuration option which allows to define which fields (apart from the title) should be mandatory for a patron purchase suggestion form in OPAC - impoves the method how required form fields are enforced, by utilising new HTML5 "required" attribute (old method would still be operational in old/legacy browsers which do not support "required" attribute properly). Allowed fields that are configurable as mandatory/not mandatory for suggestions made via OPAC are: author|copyrightdate|collectiontitle| isbn|publishercode|place|note|itemtype|patronreason. Note: this patch only applies to OPAC bootstrap theme. To test: 1/ Apply patch. 2/ Without 'OPACSuggestionMandatoryFields' option set, OPAC suggestion form should only require the title field. 3/ Set 'OPACSuggestionMandatoryFields' syspref to (e.g.) 'author|isbn|itemtype' 4/ Note that suggestion form in OPAC now requires 3 more fields to be entered and/or choosen (they should be labeled in red), and the user instruction changes slightly ("Form fields labeled in red are mandatory" ..). 5/ Try to submit some suggestions: for each field which is configured as mandatory, browser should enforce this field to be filled (in modern, HTML5 compilant browsers), or (as a fallback mechanism for legacy browsers) there should be an alert message displayed stating that some mandatory fields are missing. 6/ In both modern and legacy browsers, unless all mandatory fields are filled with something, form submission should not be possible. 7/ Repeat test 3-6 for all supported field names configurable as mandatory. If possible, "legacy" browser tests should ideally involve IE9 and (some older version of) Safari, which are rumored to have somehow questionable reputation regarding "required" HTML attribute support. Created attachment 29092 [details] [review] Bug 10848 [QA Followup] - Allow configuration of mandatory/required fields on the suggestion form in OPAC #2/2 QA follow-up for part #2: fixed OPACSuggestionMandatoryFields system preference description. (In reply to Jonathan Druart from comment #5) > Maybe it could be better to use the required="required" html attribute on > the input element, don't you think? Looks like a good idea, especially after I learned what it is and how it works ;). I've rewritten this patch to make use of this new attribute, I agree it does work a lot nicer that way, at least in modern browsers. I left Check() function still remaining as fallback mechanism for older/legacy browsers (not sure it's a best possible approach ?), also I think it may still come in handy e.g. in case when including some additional / more specific validity checks on the form contents may be desired in the future. On (not completely) unrelated subject: I think it would be quite beneficial to have some general-purpose utility function for: fetching + spliting + checking & (optionally) de-duplicating system preferences which are in 'value1|value2|...' form. Unless there already is such a function available somewhere, which I'm not aware of? (In reply to Jacek Ablewicz from comment #10) > On (not completely) unrelated subject: I think it would be quite beneficial > to have some general-purpose utility function for: fetching + spliting + > checking & (optionally) de-duplicating system preferences which are in > 'value1|value2|...' form. Unless there already is such a function available > somewhere, which I'm not aware of? Jacek, Thanks for your new patch! No this kind of function does not exist. But some syspref use '|', and others ',', ';'. So I am not sure that could be useful. Created attachment 34317 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC #2/2 (part #2: new syspref 'OPACSuggestionMandatoryFields' + DB updates) Signed-off-by: Nick Clemens <nick@debian> Created attachment 34318 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC #1/2 (part #1: changes in opac-suggestions.* script + bootstrap template) This patch: - adds a configuration option which allows to define which fields (apart from the title) should be mandatory for a patron purchase suggestion form in OPAC - impoves the method how required form fields are enforced, by utilising new HTML5 "required" attribute (old method would still be operational in old/legacy browsers which do not support "required" attribute properly). Allowed fields that are configurable as mandatory/not mandatory for suggestions made via OPAC are: author|copyrightdate|collectiontitle| isbn|publishercode|place|note|itemtype|patronreason. Note: this patch only applies to OPAC bootstrap theme. To test: 1/ Apply patch. 2/ Without 'OPACSuggestionMandatoryFields' option set, OPAC suggestion form should only require the title field. 3/ Set 'OPACSuggestionMandatoryFields' syspref to (e.g.) 'author|isbn|itemtype' 4/ Note that suggestion form in OPAC now requires 3 more fields to be entered and/or choosen (they should be labeled in red), and the user instruction changes slightly ("Form fields labeled in red are mandatory" ..). 5/ Try to submit some suggestions: for each field which is configured as mandatory, browser should enforce this field to be filled (in modern, HTML5 compilant browsers), or (as a fallback mechanism for legacy browsers) there should be an alert message displayed stating that some mandatory fields are missing. 6/ In both modern and legacy browsers, unless all mandatory fields are filled with something, form submission should not be possible. 7/ Repeat test 3-6 for all supported field names configurable as mandatory. If possible, "legacy" browser tests should ideally involve IE9 and (some older version of) Safari, which are rumored to have somehow questionable reputation regarding "required" HTML attribute support. Created attachment 34319 [details] [review] Bug 10848 [QA Followup] - Allow configuration of mandatory/required fields on the suggestion form in OPAC #2/2 QA follow-up for part #2: fixed OPACSuggestionMandatoryFields system preference description. patch rebased, small conflict on updatedb.pl Patch tested with a sandbox, by Valérie Bertrand <valerie.bertrand@univ-lyon3.fr> Created attachment 34324 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC #2/2 (part #2: new syspref 'OPACSuggestionMandatoryFields' + DB updates) Signed-off-by: Nick Clemens <nick@debian> Signed-off-by: Val Created attachment 34325 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC #1/2 (part #1: changes in opac-suggestions.* script + bootstrap template) This patch: - adds a configuration option which allows to define which fields (apart from the title) should be mandatory for a patron purchase suggestion form in OPAC - impoves the method how required form fields are enforced, by utilising new HTML5 "required" attribute (old method would still be operational in old/legacy browsers which do not support "required" attribute properly). Allowed fields that are configurable as mandatory/not mandatory for suggestions made via OPAC are: author|copyrightdate|collectiontitle| isbn|publishercode|place|note|itemtype|patronreason. Note: this patch only applies to OPAC bootstrap theme. To test: 1/ Apply patch. 2/ Without 'OPACSuggestionMandatoryFields' option set, OPAC suggestion form should only require the title field. 3/ Set 'OPACSuggestionMandatoryFields' syspref to (e.g.) 'author|isbn|itemtype' 4/ Note that suggestion form in OPAC now requires 3 more fields to be entered and/or choosen (they should be labeled in red), and the user instruction changes slightly ("Form fields labeled in red are mandatory" ..). 5/ Try to submit some suggestions: for each field which is configured as mandatory, browser should enforce this field to be filled (in modern, HTML5 compilant browsers), or (as a fallback mechanism for legacy browsers) there should be an alert message displayed stating that some mandatory fields are missing. 6/ In both modern and legacy browsers, unless all mandatory fields are filled with something, form submission should not be possible. 7/ Repeat test 3-6 for all supported field names configurable as mandatory. If possible, "legacy" browser tests should ideally involve IE9 and (some older version of) Safari, which are rumored to have somehow questionable reputation regarding "required" HTML attribute support. Signed-off-by: Val Created attachment 34326 [details] [review] Bug 10848 [QA Followup] - Allow configuration of mandatory/required fields on the suggestion form in OPAC #2/2 QA follow-up for part #2: fixed OPACSuggestionMandatoryFields system preference description. Signed-off-by: Val I followed the test plan, and changed the fields configuration as asked, both in IE 11 and Firefox 33.1. It works well. unfortunately I was not able ti test the patch on older browsers. Created attachment 34544 [details] [review] [PASSED QA] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC #1/2 (part #1: changes in opac-suggestions.* script + bootstrap template) This patch: - adds a configuration option which allows to define which fields (apart from the title) should be mandatory for a patron purchase suggestion form in OPAC - impoves the method how required form fields are enforced, by utilising new HTML5 "required" attribute (old method would still be operational in old/legacy browsers which do not support "required" attribute properly). Allowed fields that are configurable as mandatory/not mandatory for suggestions made via OPAC are: author|copyrightdate|collectiontitle| isbn|publishercode|place|note|itemtype|patronreason. Note: this patch only applies to OPAC bootstrap theme. To test: 1/ Apply patch. 2/ Without 'OPACSuggestionMandatoryFields' option set, OPAC suggestion form should only require the title field. 3/ Set 'OPACSuggestionMandatoryFields' syspref to (e.g.) 'author|isbn|itemtype' 4/ Note that suggestion form in OPAC now requires 3 more fields to be entered and/or choosen (they should be labeled in red), and the user instruction changes slightly ("Form fields labeled in red are mandatory" ..). 5/ Try to submit some suggestions: for each field which is configured as mandatory, browser should enforce this field to be filled (in modern, HTML5 compilant browsers), or (as a fallback mechanism for legacy browsers) there should be an alert message displayed stating that some mandatory fields are missing. 6/ In both modern and legacy browsers, unless all mandatory fields are filled with something, form submission should not be possible. 7/ Repeat test 3-6 for all supported field names configurable as mandatory. If possible, "legacy" browser tests should ideally involve IE9 and (some older version of) Safari, which are rumored to have somehow questionable reputation regarding "required" HTML attribute support. Signed-off-by: Valerie Bertrand <valerie.bertrand@univ-lyon3.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 34545 [details] [review] [PASSED QA] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC #2/2 (part #2: new syspref 'OPACSuggestionMandatoryFields' + DB updates) Signed-off-by: Nick Clemens <nick@debian> Signed-off-by: Valerie Bertrand <valerie.bertrand@univ-lyon3.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 34546 [details] [review] [PASSED QA] Bug 10848 [QA Followup] - Allow configuration of mandatory/required fields on the suggestion form in OPAC #2/2 QA follow-up for part #2: fixed OPACSuggestionMandatoryFields system preference description. Signed-off-by: Valerie Bertrand <valerie.bertrand@univ-lyon3.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 34547 [details] [review] Bug 10848 [QA Followup] - Title should behave the same as other required fields I like the feature, but failing because of the fact that being the available options a fixed list, I think it fits better with the 'multiple' syspref type, which is less error-prone, and user-friendly. If anyone objects failing it, please mark as 'In discussion' and add your comments. Created attachment 35830 [details] [review] Bug 10848: (QA followup) use the 'multiple' syspref type instead This patch makes the feature use the 'multiple' syspref type. It does so by changing the type in opac.pref and fixing the relevant files that are used to insert the data on the DB. It also changes opac-suggestions.pl to use ',0 as separator, as used by the 'multiple' syspref type. To test: - Verify there's no behaviour change besides the change in the sysprefs definition. Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> I put it back on the QA queue. Hope someone can take a look ASAP. The description of the pref is " (Note: title field would be always implicitly required, no need to enter it here).". But it's wrong, if the pref is empty, the title is mandatory. You can save and get "Your suggestion has been submitted.", but nothing has been saved. Marked as Failed QA. *** Bug 9846 has been marked as a duplicate of this bug. *** Could this be revived? Created attachment 54016 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC This patch adds a configuration option which allows to define which fields should be mandatory for a patron purchase suggestion form in OPAC. Test plan: 1/ Apply patch. 2/ Play with the new OPACSuggestionMandatoryFields system preference (select some fields as manadatory, select all, deselect all, try to submit some suggestions with mandatory fields filled and/or not filled etc.) to ensure that required fields (and only required fields) are enforced in the browser to be filled. 3/ With all options deselected, 'Title' field should still be mandatory (by default). Created attachment 54017 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC Add 'OPACSuggestionMandatoryFields' syspref definition plus an atomic DB update for the new preference. Created attachment 54018 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC This patch adds a configuration option which allows to define which fields should be mandatory for a patron purchase suggestion form in OPAC. Test plan: 1/ Apply patch. 2/ Play with the new OPACSuggestionMandatoryFields system preference (select some fields as manadatory, select all, deselect all, try to submit some suggestions with mandatory fields filled and/or not filled etc.) to ensure that required fields (and only required fields) are enforced in the browser to be filled. 3/ With all options deselected, 'Title' field should still be mandatory (by default). Created attachment 54019 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC Add 'OPACSuggestionMandatoryFields' syspref definition plus an atomic DB update for the new preference. Created attachment 54020 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC This patch adds a configuration option which allows to define which fields should be mandatory for a patron purchase suggestion form in OPAC. Test plan: 1/ Apply patch. 2/ Play with the new OPACSuggestionMandatoryFields system preference (select some fields as manadatory, select all, deselect all, try to submit some suggestions with mandatory fields filled and/or not filled etc.) to ensure that required fields (and only required fields) are enforced in the browser to be filled. 3/ With all options deselected, 'Title' field should still be mandatory (by default). Created attachment 54021 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC - part 2 Add 'OPACSuggestionMandatoryFields' syspref definition plus an atomic DB update for the new preference. In this revised version I dropped the old form validation handler entirely, in favour of the "required" attributes as a sole method of enforcement. Not sure if the special treatment for old / legacy / non-compliant browsers is still needed? If deemed necessary, I may try to re-add such an event handler which will be more on par with the current coding guidelines. I find that it's not possible to make the "library" selection mandatory. Is there a reason for that? Note that there is a JS validation plugin available in the OPAC if you want to use it. See opac-memberentry.tt for an example. The plugin can be set up to validate a form based on the markup (requiring fields which are , so it's not necessary to dynamically configure the fields in the JS if you don't want to. Created attachment 54083 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC This patch adds a configuration option which allows to define which fields should be mandatory for a patron purchase suggestion form in OPAC. Test plan: 1/ Apply patch. 2/ Play with the new OPACSuggestionMandatoryFields system preference (select some fields as manadatory, select all, deselect all, try to submit some suggestions with mandatory fields filled and/or not filled etc.) to ensure that required fields (and only required fields) are enforced in the browser to be filled. 3/ With all options deselected, 'Title' field should still be mandatory (by default). Signed-off-by: barbara johnson <barbara.johnson@bedfordtx.gov> Created attachment 54084 [details] [review] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC - part 2 Add 'OPACSuggestionMandatoryFields' syspref definition plus an atomic DB update for the new preference. Signed-off-by: barbara johnson <barbara.johnson@bedfordtx.gov> This seems to work well, but I'd like to address Owen's comment#38 The visibility of the library selection depends on AllowPurchaseSuggestionBranchChoice. It could be included in the mandatory list, maybe with a note about the dependency on the other pref in the text? I think it would be nice if the error messages were a bit more standard. If you are missing multiple fields, only the first will be highlighted. If I enter a wrong format into the year field, it says 'Please use the specified format' (translated from German), but gives no clue about the format. Would this be improved by Owen's suggestion of using the existing JS validation plugin? Jacek, could you please take a look? (In reply to Owen Leonard from comment #38) > I find that it's not possible to make the "library" selection mandatory. Is > there a reason for that? No, I just forgot about it. While it would not affect form submission itself (there will be always some non-empty value submitted in case when AllowPurchaseSuggestionBranchChoice is enabled), similar logic applies for itemtype, and yet our librarians found it vitally important that this field too could be marked as mandatory. And I agreed, good for consistency. Also still useful as an optional remainder for the user that a given field is relatively important and that they should pay a closer attention to set it accordingly. > Note that there is a JS validation plugin available in the OPAC if you want > to use it. See opac-memberentry.tt for an example. The plugin can be set up > to validate a form based on the markup (requiring fields which are , so it's > not necessary to dynamically configure the fields in the JS if you don't > want to. Hm, would it be as simple as adding jquery.validate.min.js include and one jquery call $('#add_suggestion_form').validate(); to the template? Let me see how it works ( please do not adjust your set).. (In reply to Katrin Fischer from comment #41) > Would this be improved by Owen's suggestion of using the existing JS > validation plugin? JQuery validation plugin (instead of - or as an addition to - HTML5 attributes alone) seems to be a better choice (more configurable, extensible, and probably more predictable with old browsers as well). However so far I have zero experience with this plugin (or any other jQuery plugins for that matter). > > Note that there is a JS validation plugin available in the OPAC if you want
> > to use it. See opac-memberentry.tt for an example. The plugin can be set up
> > to validate a form based on the markup (requiring fields which are , so it's
> > not necessary to dynamically configure the fields in the JS if you don't
> > want to.
>
> Hm, would it be as simple as adding jquery.validate.min.js include and one
> jquery call
>
> $('#add_suggestion_form').validate();
>
> to the template? Let me see how it works ( please do not adjust your set)..
So far so good, it's working (almost) fine, apart from two small issues:
1) for 'pattern' rule to work I guess additional include (additional-methods.js or pattern.js ?) may be needed, but neither seems to be anywhere in the code tree yet (?) - or maybe there is some other bug report waiting in the queue which includes one of them, that I can test and sign instead?
2) translateability - no matter what I try to add to the .po files etc., "This field is required" still appears only in english (but so is the "Please enter a valid email address." message in opac-memberentry.tt ?). Perhaps it's just my test install which is missing something, or in order for this message to be translateable some additional trickery may be needed..
Hi Jacek, how do you add to the po files? You should not need to change anything manually there. For a git installation try: - switch to misc/translator in your koha directory - perl translate update <langcode> - perl translate install <langcode> The first updates the templates the second installs them from the po file for testing. (In reply to Katrin Fischer from comment #45) > Hi Jacek, how do you add to the po files? > You should not need to change anything manually there. For a git > installation try: > - switch to misc/translator in your koha directory > - perl translate update <langcode> > - perl translate install <langcode> > > The first updates the templates the second installs them from the po file > for testing. That's how I've been trying to test it. Well, manually adding a few odd extra strings in the .po file and 'translate install' without 'translate update' is not below my dignity too ;) - I know it's not a proper way, but it should work just fine for testing purposes (usually does in similar circumstances). Problem is that messages embedded in the .js file ("This field is required", "Please enter a valid email address", ...) are not picked up for translation. This is most likely normal, I just wasn't aware of that. Looks like there are several ways to fix this issue, either globally for OPAC/bootstrap - like it was done for intranet/prog in Bug 6209, or locally in the given OPAC templates, e.g. using a combination of jQuery.extend(jQuery.validator.messages, { required: _("This field is required."), email: _("Please enter a valid email address."), ... for the "regular" mandatory fields, and/or jQuery.validator.addMethod("year", function(value, element) { ... }, _("Please enter a year in the valid format (YYYY, YY--)")); for a field with the pattern. I guess preferably it should be fixed globally for the OPAC (opac-memberentry.tt is affected by this too), but I'm a bit out of my depth here - I'll better open a separate bug report for that part. (In reply to Jacek Ablewicz from comment #46) > I'll better open a separate bug report for that part. I would be fine with having this form depend on browser-based validation using the "required" attribute for now, and to consider another layer of JS validation as a separate bug. Created attachment 54356 [details] [review] Bug 10848 - [QA Followup] Add missing branch option and fix pattern for copyrightdate - HTML5 input pattern for copyrightdate was not taking into an account the database field type, which is smallint(5) - added title="..." for the year field so the acceptable date format should be displayed as a hint/tooltip in the browser - added an missing option for configuring branch/library as mandatory (In reply to Katrin Fischer from comment #41) > If I enter a wrong format into the year field, it says 'Please use the specified > format' (translated from German), but gives no clue about the format. On the 2nd thought, previous pattern ("[12][0-9\-\?]{3}") wasn't making much sense - copyrightdate in the DB is just smallint(5). Added a hint for the user regarding the required format for that field as a title="", i.e. tooltip. Doesn't always work quite perfectly (depending on the browser), but hopefully it would be good enough for the time being / close enough for government work. The follow-up is small - I think it's safe to treat it as a QA follow-up. Looking at it again... having library mandatory appears a bit silly as it's currently not possible to 'unset' it - so I got not idea on how to trigger this :) Maybe something for a different bug: What about renaming copyright date to publication year? I think this could make the format more obvious too. Also... is collection title meant to be series? We have this showing up in some places in Koha and it seems related to UNIMARC - but not sure what it is supposed to be in MARC21. There are some things not super-perfect here, but I think no blockers. Making the library mandatory doesn't hurt - we can think about having it empty or to 'all libraries' separately. The new format check keeps you from submitting something that cannot be stored in the database - improving the GUI is also something we can do separately. Maybe we could just display a hint behind the field in a grey font as we do in other places. Passing QA. Created attachment 54430 [details] [review] [PASSED QA] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC This patch adds a configuration option which allows to define which fields should be mandatory for a patron purchase suggestion form in OPAC. Test plan: 1/ Apply patch. 2/ Play with the new OPACSuggestionMandatoryFields system preference (select some fields as manadatory, select all, deselect all, try to submit some suggestions with mandatory fields filled and/or not filled etc.) to ensure that required fields (and only required fields) are enforced in the browser to be filled. 3/ With all options deselected, 'Title' field should still be mandatory (by default). Signed-off-by: barbara johnson <barbara.johnson@bedfordtx.gov> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 54431 [details] [review] [PASSED QA] Bug 10848 - Allow configuration of mandatory/required fields on the suggestion form in OPAC - part 2 Add 'OPACSuggestionMandatoryFields' syspref definition plus an atomic DB update for the new preference. Signed-off-by: barbara johnson <barbara.johnson@bedfordtx.gov> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 54432 [details] [review] [PASSED QA] Bug 10848 - [QA Followup] Add missing branch option and fix pattern for copyrightdate - HTML5 input pattern for copyrightdate was not taking into an account the database field type, which is smallint(5) - added title="..." for the year field so the acceptable date format should be displayed as a hint/tooltip in the browser - added an missing option for configuring branch/library as mandatory Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Pushed to master for 16.11, thanks Jacek! *** Bug 17166 has been marked as a duplicate of this bug. *** |