[Bug 10543] New: false mandatory warning in additem
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Bug ID: 10543 Summary: false mandatory warning in additem Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Cataloging Assignee: gmcharlt@gmail.com Reporter: fridolyn.somers@biblibre.com CC: m.de.rooy@rijksmuseum.nl When editing a item, we get a popup warning : "1 mandatory fields empty (highlighted)". But all mandatory fields are filled. This is because there is a field with a huge content (more than 100 characters), it it displayed in a textarea instead of input. This braked javascript form check. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Fridolyn SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|gmcharlt@gmail.com |fridolyn.somers@biblibre.co | |m --- Comment #1 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- Created attachment 19412 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19412&action=edit Proposed patch See commit message. Very tricky bug :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Fridolyn SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Could you please provide a test plan for this patch? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz --- Comment #3 from Chris Cormack <chris@bigballofwax.co.nz> --- Kyle the test plan is on the patch (in the commit message as it should be) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- Test plan from patch: Test plan : - In a framework, make sure you have in item field : An optionnal subfield (without authorised value) then a mandatory subfield then an optional subfield next to mandatory subfield For example : $i (optional), $r (mandatory) and $s (optional) - Open a biblio record and create a new item - Enter more than 100 characters in $i, fill $r and $s - Click "Add item" - Edit this item => You get a textarea for $i - Empty $s and Save => Without patch, you get a warning and form is not saved and you see that $s is yellow while it should be $r => With patch, form is saved - Re-edit this item - Empty $r and save => You get a warning and form is not saved -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |srdjan@catalyst.net.nz --- Comment #5 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Maybe we should consider putting that function in a separate js while we are at it? Having it tripled does not feel right. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 --- Comment #6 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> ---
Maybe we should consider putting that function in a separate js while we are at it? Having it tripled does not feel right. It is only double and it is quite short. In acquisition there is a template condition : [% IF (AcqCreateItemReceiving) %]
-- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gmcharlt@gmail.com --- Comment #7 from Galen Charlton <gmcharlt@gmail.com> --- (In reply to Fridolyn SOMERS from comment #6)
Maybe we should consider putting that function in a separate js while we are at it? Having it tripled does not feel right. It is only double and it is quite short. In acquisition there is a template condition : [% IF (AcqCreateItemReceiving) %]
No, there are three: serials-edit.tt includes a version of that same check; the serials receiving form should be checked to see whether the same bug applies there as well. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Fridolyn SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19412|0 |1 is obsolete| | --- Comment #8 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- Created attachment 19572 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19572&action=edit Bug 10543 - false mandatory warning in additem Revised patch using a JS method in cataloging.js -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 --- Comment #9 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- Created attachment 19574 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19574&action=edit Bug 10543 - Unify item mandatory subfields check Followup 1 : Use of CheckMandatorySubfields method in other item forms than additem.tt : - acqui/neworderempty.tt : was an old-fashioned code - acqui/orderreceive.tt : was the same code as additem.tt - serials/serials-edit.tt : was a code similar to CheckMandatorySubfields but yet slower because each() function loops on every subfield, not only mandatories. Also did not add "missing" class. Note that check is only performed on visible item forms. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 --- Comment #10 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- Created attachment 19576 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19576&action=edit Bug 10543 - add mandatory subfield check to addorderiso2709.pl addorderiso2709.tt was missing item form check. Adds it with a call to CheckMandatorySubfields of course. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 --- Comment #11 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- (In reply to Galen Charlton from comment #7)
No, there are three: serials-edit.tt includes a version of that same check; the serials receiving form should be checked to see whether the same bug applies there as well.
Thanks for your help, you where totally right. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 --- Comment #12 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Fridolyn, The last one, shouldn't it be just CheckMandatorySubfields()? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 --- Comment #13 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- (In reply to Srdjan Jankovic from comment #12)
Fridolyn, The last one, shouldn't it be just CheckMandatorySubfields()?
The last patch ? No, the CheckMandatorySubfields() has an argument which is the parent object to use for searching subfield lines. This is necessary for the serial edition where there are hidden subfield lines, one must not check all line of the form. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19572|0 |1 is obsolete| | --- Comment #14 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Created attachment 19704 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19704&action=edit [SIGNED-OFF] Bug 10543 - false mandatory warning in additem When editing a item, you get a popup warning : "1 mandatory fields empty (highlighted)". But all mandatory fields are filled. This is because there is a field with a huge content (more than 100 characters), it it displayed in a textarea instead of input. This patch corrects javascript form check by using siblings(). Also adds a more precise filter to find mandatory inputs because looking in full page may bring unwanted results. Test plan : - In a framework, make sure you have in item field : An optionnal subfield (without authorised value) then a mandatory subfield then an optional subfield next to mandatory subfield For example : $i (optional), $r (mandatory) and $s (optional) - Open a biblio record and create a new item - Enter more than 100 characters in $i, fill $r and $s - Click "Add item" - Edit this item => You get a textarea for $i - Empty $s and Save => Without patch, you get a warning and form is not saved and you see that $s is yellow while it should be $r => With patch, form is saved - Re-edit this item - Empty $r and save => You get a warning and form is not saved Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19576|0 |1 is obsolete| | --- Comment #15 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Created attachment 19705 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19705&action=edit [SIGNED-OFF] Bug 10543 - Unify item mandatory subfields check Use of CheckMandatorySubfields from cataloging.js everywhere an item cataloging form is checked for mandatories Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 --- Comment #16 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Created attachment 19706 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19706&action=edit [SIGNED-OFF] Bug 10543 - add mandatory subfield check to addorderiso2709.pl When adding orders from staged file, you can create one order per record using item form in the bottom of addorderiso2709.pl. This form was missing check. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19574|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19704|0 |1 is obsolete| | Attachment #19705|0 |1 is obsolete| | Attachment #19706|0 |1 is obsolete| | --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 19980 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19980&action=edit [PASSED QA] Bug 10543 - false mandatory warning in additem When editing a item, you get a popup warning : "1 mandatory fields empty (highlighted)". But all mandatory fields are filled. This is because there is a field with a huge content (more than 100 characters), it it displayed in a textarea instead of input. This patch corrects javascript form check by using siblings(). Also adds a more precise filter to find mandatory inputs because looking in full page may bring unwanted results. Test plan : - In a framework, make sure you have in item field : An optionnal subfield (without authorised value) then a mandatory subfield then an optional subfield next to mandatory subfield For example : $i (optional), $r (mandatory) and $s (optional) - Open a biblio record and create a new item - Enter more than 100 characters in $i, fill $r and $s - Click "Add item" - Edit this item => You get a textarea for $i - Empty $s and Save => Without patch, you get a warning and form is not saved and you see that $s is yellow while it should be $r => With patch, form is saved - Re-edit this item - Empty $r and save => You get a warning and form is not saved Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Notes on last patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 --- Comment #18 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 19981 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19981&action=edit [PASSED QA] Bug 10543 - Unify item mandatory subfields check Use of CheckMandatorySubfields from cataloging.js everywhere an item cataloging form is checked for mandatories Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 19982 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19982&action=edit [PASSED QA] Bug 10543 - add mandatory subfield check to addorderiso2709.pl When adding orders from staged file, you can create one order per record using item form in the bottom of addorderiso2709.pl. This form was missing check. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All 3 patches pass tests and QA script. Some additional notes on the test plan from the first patch: It took me a while to figure out a combination of subfields that would trigger the bug. I am not sure about the condition that transforms the input field into a textbox as it does not seem to happen for every field. What worked in the end was (MARC21): - $g cost, normal purchase price = mandatory - $h serial enumeration/chronolgy = optional, filled with text over 100 characters Checked for: - Adding items in cataloguing Was able to reproduce the problem, mandatory check works as expected. - Adding items from staged file in acq Was not able to trigger the problem, because we don't edit the item here, but mandatory check works as expected. - Addings items for serial subscriptions Was not able to trigger the problem, because we don't edit the items here, but mandatory check works better as before as missing mandatory fields are now highlighted and weren't before. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #20 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Fridolyn! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |tomascohen@gmail.com --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- This patch has been pushed to 3.12.x, will be in 3.12.5. Thanks Fridolyn! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10543 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bgkriegel@gmail.com --- Comment #22 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Pushed to 3.10.x, will be in 3.10.11 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org