[Bug 24010] New: Number of issues to display to staff accepts non-integer values
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 Bug ID: 24010 Summary: Number of issues to display to staff accepts non-integer values Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Serials Assignee: koha-bugs@lists.koha-community.org Reporter: joonas.kylmala@helsinki.fi QA Contact: testopia@bugs.koha-community.org CC: colin.campbell@ptfs-europe.com When adding a new serial subscription in the page /cgi-bin/koha/serials/subscription-add.pl one can add to the field "Number of issues to display to staff" a non-integer value like "f" and then when you save this new subscription and go to the "Subscriptions" tab in /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXXX you get the following error in plack-error.log:
DBD::mysql::st fetchrow_hashref failed: fetch() without execute() [for Statement "SELECT serialid,serialseq, status, planneddate, publisheddate, notes FROM serial WHERE subscriptionid = ? AND status IN (2,4,41,42,43,44) ORDER BY publisheddate DESC LIMIT 0,f " with ParamValues: 0=2] at /home/koha/kohaclone/C4/Serials.pm line 784.
-- 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=24010 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=22812 CC| |jonathan.druart@bugs.koha-c | |ommunity.org Assignee|koha-bugs@lists.koha-commun |jonathan.druart@bugs.koha-c |ity.org |ommunity.org --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Same as bug 22812, we must use Koha::Subscription here. -- 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=24010 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 95610 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95610&action=edit Bug 24010: Use Koha::Subscription in ModSubscription We must use Koha::Subscription instead of raw SQL. It will fix issue with default and integer values. Test plan: Edit a subscription and set number of issues = "f" Save => Without this patch there is a SQL error in the log: Incorrect integer value: 'f' for column 'numberlength' => With this patch the other changes are effective. Note: We also could change the type attribute of the input to "number", to have a client-side check Also, the return value of ModSuggestion is never used, so we are safe with that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 --- Comment #3 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- This fix doesn't solve the original problem reported plus I'm not able to reproduce the error described in the patch:
Incorrect integer value: 'f' for column 'numberlength'
When I edit "Subscription length" field (I assume this is the "number of issues") to "f" Regardless whether this patch is applied I only get in plack-error.log
Use of uninitialized value $sub_on in concatenation (.) or string at /home/koha/kohaclone/serials/subscription-add.pl line 113. Argument "f" isn't numeric in numeric ne (!=) at /home/koha/kohaclone/serials/subscription-add.pl line 279.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95610|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=24010 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED Assignee|jonathan.druart@bugs.koha-c |koha-bugs@lists.koha-commun |ommunity.org |ity.org --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Sorry, I read the description too quickly! -- 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=24010 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW -- 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=24010 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=23064 -- 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=24010 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |jonathan.druart@bugs.koha-c |ity.org |ommunity.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=24010 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 95717 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95717&action=edit Bug 24010: DB Changes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 95718 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95718&action=edit Bug 24010: Enforce integer values for staffdisplaycount and opacdisplaycount -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |oleonard@myacpl.org --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- The second patch does not work as expected. I am struggling to make the validation work on the form. It seems that the validate jQuery function collides with the validated class put on the form. Owen, is it clear for you what we should do in this case? I have also tried type="number" but it does not block form submission. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 --- Comment #8 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Instead of failing the DB change could we just make the non-integer values NULL? The non-integer values never worked so it should not be a problem to modify them to NULL? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Joonas Kylmälä from comment #8)
Instead of failing the DB change could we just make the non-integer values NULL? The non-integer values never worked so it should not be a problem to modify them to NULL?
I can use an SQL regex to set to NULL non-numeric values, yes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Owen, you suggested me to use the new version of jQuery Validate (from bug 24089), but it still does not work for me.
From IRC: <Joubu> oleonard: does it work with the new version? <oleonard> Yes. I suggest you favor the "validated" class and add 'data-rule-number="true"' to the inputs
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95718|0 |1 is obsolete| | --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 95847 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95847&action=edit Bug 24010: Enforce integer values for staffdisplaycount and opacdisplaycount (second try) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 --- Comment #12 from Owen Leonard <oleonard@myacpl.org> --- It works for me if I change the input type to "number." I wouldn't have expected that to be necessary, but it's not an incorrect change to make. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Does it block the form submission? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95847|0 |1 is obsolete| | --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 95935 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95935&action=edit Bug 24010: Enforce integer values for staffdisplaycount and opacdisplaycount (second try) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Jonathan Druart from comment #14)
Created attachment 95935 [details] [review] Bug 24010: Enforce integer values for staffdisplaycount and opacdisplaycount (second try)
Should this be NSO? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW Assignee|jonathan.druart@bugs.koha-c |koha-bugs@lists.koha-commun |ommunity.org |ity.org --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- No, it does not work. -- 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=24010 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95935|0 |1 is obsolete| | -- 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=24010 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- 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=24010 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95717|0 |1 is obsolete| | --- Comment #17 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 134985 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134985&action=edit Bug 24010: DB Changes -- 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=24010 --- Comment #18 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 134986 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134986&action=edit Bug 24010: Number of issues to display to staff accepts non-integer values This patch modifies the subscription entry form so that it will perform a check on the staffdisplaycount and opacdisplaycount fields before proceding to the second step. It verifies that the values are numeric. The changes are made in the style of the existing form validation, which should be rewritten to either use the validation plugin or to peform checks in a way that all checks are run before warning the user. However, this smaller change will work in the meantime. To test, apply the patch and go to Serials -> New subscription. - Fill out the form with at least the required fields, but put something other than a number if the "Number of issues to display to staff" and "Number of issues to display to the public" with non-numeric characters. - When you click the "Next" button you should get an error message, "Number of issues to display to staff must be a number." - Correct the issues to display to staff field and submit again. - You should get a different error message, "Number of issues to display to the public must be a number." - Correct this field and you should be able to proceed to the next step. -- 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=24010 --- Comment #19 from Owen Leonard <oleonard@myacpl.org> --- The atomicupdate file needs to be updated to use our current syntax, and I think it needs this addition: If you've got existing subscriptions but you didn't specify a number for staffdisplaycount and opacdisplaycount, those columns will contain an empty string. The database update checks to see if the columns can be updated to INT and aborts if not. But it should be reasonable to start by updating empty entries to make them NULL isn't it? update subscriptions set staffdisplaycount = NULL where staffdisplaycount = ''; update subscriptions set opacdisplaycount = NULL where opacdisplaycount = ''; Then the db update can proceed automatically even if there are existing blank values. If that's correct, can someone take a crack at updating the atomicupdate file? -- 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=24010 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134985|0 |1 is obsolete| | --- Comment #20 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 135092 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135092&action=edit Bug 24010: DB Changes Amended-patch: adjusted to new atomic update format -- 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=24010 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|koha-bugs@lists.koha-commun |oleonard@myacpl.org |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=24010 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=24010 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134986|0 |1 is obsolete| | --- Comment #21 from David Nind <david@davidnind.com> --- Created attachment 136538 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136538&action=edit Bug 24010: Number of issues to display to staff accepts non-integer values This patch modifies the subscription entry form so that it will perform a check on the staffdisplaycount and opacdisplaycount fields before proceding to the second step. It verifies that the values are numeric. The changes are made in the style of the existing form validation, which should be rewritten to either use the validation plugin or to peform checks in a way that all checks are run before warning the user. However, this smaller change will work in the meantime. To test, apply the patch and go to Serials -> New subscription. - Fill out the form with at least the required fields, but put something other than a number if the "Number of issues to display to staff" and "Number of issues to display to the public" with non-numeric characters. - When you click the "Next" button you should get an error message, "Number of issues to display to staff must be a number." - Correct the issues to display to staff field and submit again. - You should get a different error message, "Number of issues to display to the public must be a number." - Correct this field and you should be able to proceed to the next step. 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=24010 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135092|0 |1 is obsolete| | --- Comment #22 from David Nind <david@davidnind.com> --- Created attachment 136539 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136539&action=edit Bug 24010: DB Changes Amended-patch: adjusted to new atomic update format 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=24010 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Text to go in the| |This adds validation to the release notes| |subscription entry form to | |check that the values for | |these fields are numbers: | |- | |Number of issues to display | |to staff | |- Number of issues | |to display to the public -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=24010 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #136538|0 |1 is obsolete| | Attachment #136539|0 |1 is obsolete| | --- Comment #23 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 137060 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=137060&action=edit Bug 24010: Number of issues to display to staff accepts non-integer values This patch modifies the subscription entry form so that it will perform a check on the staffdisplaycount and opacdisplaycount fields before proceding to the second step. It verifies that the values are numeric. The changes are made in the style of the existing form validation, which should be rewritten to either use the validation plugin or to peform checks in a way that all checks are run before warning the user. However, this smaller change will work in the meantime. To test, apply the patch and go to Serials -> New subscription. - Fill out the form with at least the required fields, but put something other than a number if the "Number of issues to display to staff" and "Number of issues to display to the public" with non-numeric characters. - When you click the "Next" button you should get an error message, "Number of issues to display to staff must be a number." - Correct the issues to display to staff field and submit again. - You should get a different error message, "Number of issues to display to the public must be a number." - Correct this field and you should be able to proceed to the next step. 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=24010 --- Comment #24 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 137062 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=137062&action=edit Bug 24010: DB Changes Amended-patch: adjusted to new atomic update format 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=24010 --- Comment #25 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Thanks, Owen! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 --- Comment #26 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 137175 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=137175&action=edit Bug 24010: (QA follow-up) Correct COMMENT syntax Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 --- Comment #27 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 22.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |lucas@bywatersolutions.com Version(s)|22.11.00 |22.11.00, 22.05.04 released in| | --- Comment #28 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 22.05.x for 22.05.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |arthur.suzuki@biblibre.com Version(s)|22.11.00, 22.05.04 |22.11.00, 22.05.04, released in| |21.11.11 Status|Pushed to stable |Pushed to oldstable --- Comment #29 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- pushed to 21.11.x for 21.11.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24010 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED CC| |victor@tuxayo.net Resolution|--- |FIXED --- Comment #30 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document, marking resolved. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org