[Bug 42398] New: Form validation does not work when a dedicated validation handler is set and form has class validated
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 Bug ID: 42398 Summary: Form validation does not work when a dedicated validation handler is set and form has class validated Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Staff interface Assignee: koha-bugs@lists.koha-community.org Reporter: bibliothek@th-wildau.de QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com When a form has a class="validated" and gets a dedicated validation handler like $("#add_additional_content").validate({ submitHandler: function(form){ ... } }); this dedicated handler gets overwritten by staff-global.js as there each form[class=validated] is called with $(this).validate(); So each form with a dedicated validation handler must not have a class "validated". So far this appears to: - koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt <form method="get" id="modal-item-group-create-form" class="validated"> - koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt <form method="get" id="modal-item-group-edit-form" class="validated"> - koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt <form method="get" id="modal-item-group-set-form" class="validated"> - koha-tmpl/intranet-tmpl/prog/en/modules/tools/additional-contents.tt <form id="add_additional_content" method="post" action="/cgi-bin/koha/tools/additional-contents.pl" class="validated"> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 Jan Kissig <bibliothek@th-wildau.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 --- Comment #1 from Jan Kissig <bibliothek@th-wildau.de> --- Created attachment 198076 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198076&action=edit Bug 42398: Remove class validated on forms with dedicated validate handlers This patch fixes form validation for forms that have a dedicated validation handler set via .validate({...}). It removes the class validated from these form elements as theses would otherwise be validated by a call via staff-gobal.js which would overwrite the dedicated handler. Test plan: 1. Item groups: a) enable sys pref EnableItemGroups b) search a biblio with several items f.e. /cgi-bin/koha/catalogue/detail.pl?biblionumber=146 c) go to the item groups tab and add a new item group d) you are being forwarded to a strange url like /cgi-bin/koha/catalogue/detail.pl?description=foo&display_order=0# instead of having an item group created 2. Additional contents 'news' e) go to /cgi-bin/koha/tools/additional-contents.pl?category=news (Tools -> News) f) click either 'New entry' or 'New entry using text editor' g) enter a required publication date h) save the form. You will not be required to enter a default title or content. apply patch followed by a restart_all Item groups: i) redo c) and try also j) - editing an item group k) adding items to an item group via Holdings tab -> checking items -> Add/move items to an item group Additional contents 'news': l) redo e) til h) and check that you are asked to enter a title for 'Default' and content for 'Default' -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #2 from David Nind <david@davidnind.com> --- I attempted to test, but I am leaving to someone else who understands the issue and what the change is that is required (and what is actually changed that is noticeable). The only comment I'd make is if fields are required, then they should marked as such in the user interface. Testing notes (using KTD): 1. Step 1d: - For the first group, I end up back on holdings table - For the second group I create, I end up on the item groups tab - I don't see the URL described (even looking at the dev tools, network options) 2. Step 2h: neither the default title or content are marked as required 3. After the patch: - For holds group, no difference noted - For adding news items, browser pop-up windows say that values are required for fields: note - none of the fields are marked as required -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |bibliothek@th-wildau.de |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 --- Comment #3 from Jan Kissig <bibliothek@th-wildau.de> --- (In reply to David Nind from comment #2)
I attempted to test, but I am leaving to someone else who understands the issue and what the change is that is required (and what is actually changed that is noticeable).
The only comment I'd make is if fields are required, then they should marked as such in the user interface.
Testing notes (using KTD): 1. Step 1d: - For the first group, I end up back on holdings table - For the second group I create, I end up on the item groups tab - I don't see the URL described (even looking at the dev tools, network options) 2. Step 2h: neither the default title or content are marked as required 3. After the patch: - For holds group, no difference noted - For adding news items, browser pop-up windows say that values are required for fields: note - none of the fields are marked as required
Hey David, seems problem 1 (Item groups) got solved with Bug 41566: Restore item groups ft There the validate() function call was moved outside the '$(document).ready()' block whichs seems to fix the problem. Interestingly this solution would also fix the issue on 2 (Additional contents 'news'). I will check on this and also fix the missing 'required' flags you mentioned. The origin of the problem seems to be the order when validate is called: this does not work: - staff-global.js calls validate() on all items class 'validated' - $(document).ready -> $(form).validate({submitHandler}) but calling in this order works: - $(form).validate({submitHandler}) - staff-global.js calls validate() on all items class 'validated' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 --- Comment #4 from David Nind <david@davidnind.com> --- Thanks Jan! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 --- Comment #5 from Jan Kissig <bibliothek@th-wildau.de> --- Created attachment 198802 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198802&action=edit Bug 42398: Reimplement form validation to news This patch reimplements form validation on news (additional contents). The form.validate() call which adds a submitHandler is moved outside the document ready block in order to work again. Markup like class=required is added to meet JS10 guidelines. JS alerts (when default title or default content was empty) were replaced by setting focus the required field. Test plan: a) go to /cgi-bin/koha/tools/additional-contents.pl?category=news (Tools -> News) b) click either 'New entry' or 'New entry using text editor' c) enter a required publication date d) save the form. You will not be required to enter a default title or content. e) apply patch followed by a restart_all f) go to /cgi-bin/koha/tools/additional-contents.pl?category=news (Tools -> News) g) click either 'New entry' or 'New entry using text editor' h) check that title and content in the 'Default' tab are marked as required i) try to send form f.e. when only title and content in 'English' tab are set j) check if default title or content is missing the default tab is shown and the field gets focused -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 Jan Kissig <bibliothek@th-wildau.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Form validation does not |Form validation does not |work when a dedicated |work on additional content |validation handler is set |news |and form has class | |validated | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 --- Comment #6 from Jan Kissig <bibliothek@th-wildau.de> --- (In reply to David Nind from comment #2) ...
The only comment I'd make is if fields are required, then they should marked as such in the user interface.
...
2. Step 2h: neither the default title or content are marked as required
Hi David, from the code side the default title and default content were already required and this was checked via JS on form submission. Unfortunately this JS check did not work because it was called in the wrong order (see last 6 lines in comment 3). With the last patch I moved the order and added some markup to highlight the required fields. Why especially these fields are required and not the default fields in Additional content Pages f.e. is only known by the original author of these scripts. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 --- Comment #7 from David Nind <david@davidnind.com> --- Thanks Jan. With both patches applied, the test plan for the second patch works as expected. Is the first (original patch) still required? Or does the test plan need adjusting? David -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 Jan Kissig <bibliothek@th-wildau.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198076|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 --- Comment #8 from Jan Kissig <bibliothek@th-wildau.de> --- (In reply to David Nind from comment #7)
Thanks Jan.
With both patches applied, the test plan for the second patch works as expected.
Is the first (original patch) still required? Or does the test plan need adjusting?
David
Hi David, thanks for the info. I forgot to mark patch #1 obsolete. Only test plan for in comment #5 is required -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198802|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 --- Comment #9 from David Nind <david@davidnind.com> --- Created attachment 198917 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198917&action=edit Bug 42398: Reimplement form validation to news This patch reimplements form validation on news (additional contents). The form.validate() call which adds a submitHandler is moved outside the document ready block in order to work again. Markup like class=required is added to meet JS10 guidelines. JS alerts (when default title or default content was empty) were replaced by setting focus the required field. Test plan: a) go to /cgi-bin/koha/tools/additional-contents.pl?category=news (Tools -> News) b) click either 'New entry' or 'New entry using text editor' c) enter a required publication date d) save the form. You will not be required to enter a default title or content. e) apply patch followed by a restart_all f) go to /cgi-bin/koha/tools/additional-contents.pl?category=news (Tools -> News) g) click either 'New entry' or 'New entry using text editor' h) check that title and content in the 'Default' tab are marked as required i) try to send form f.e. when only title and content in 'English' tab are set j) check if default title or content is missing the default tab is shown and the field gets focused Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This fixes adding or release notes| |editing news (Tools > | |Additional tools > News) so | |that a default title and | |content is required for a | |news item. --- Comment #10 from David Nind <david@davidnind.com> --- Thanks Jan! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |jonathan.druart@gmail.com |y.org | Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198917|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 --- Comment #11 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 198944 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198944&action=edit Bug 42398: Reimplement form validation to news This patch reimplements form validation on news (additional contents). The form.validate() call which adds a submitHandler is moved outside the document ready block in order to work again. Markup like class=required is added to meet JS10 guidelines. JS alerts (when default title or default content was empty) were replaced by setting focus the required field. Test plan: a) go to /cgi-bin/koha/tools/additional-contents.pl?category=news (Tools -> News) b) click either 'New entry' or 'New entry using text editor' c) enter a required publication date d) save the form. You will not be required to enter a default title or content. e) apply patch followed by a restart_all f) go to /cgi-bin/koha/tools/additional-contents.pl?category=news (Tools -> News) g) click either 'New entry' or 'New entry using text editor' h) check that title and content in the 'Default' tab are marked as required i) try to send form f.e. when only title and content in 'English' tab are set j) check if default title or content is missing the default tab is shown and the field gets focused Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 --- Comment #12 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|26.05.00 |26.05.00,25.11.05 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42398 --- Comment #13 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Thanks all, pushed to 25.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org