[Bug 23888] New: Incorrect vendor id in subscription creation causes internal server error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 Bug ID: 23888 Summary: Incorrect vendor id in subscription creation causes internal server error Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor 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 subscription on the page /cgi-bin/koha/serials/subscription-add.pl and typing in the vendor field a non-existing vendor id "9999999999" and other fields correctly we get the following internal server error when clicking on the second page ”Save subscription”: DBIx::Class::Storage::DBI::_dbh_execute(): Out of range value for column 'aqbooksellerid' at row 1 at /kohadevbox/koha/Koha/Object.pm line 156 at /usr/share/perl5/DBIx/Class/Exception.pm line 77 When such incorrect vendor id is typed it should not allow us to go to the next page by clicking ”Next” but instead complain the vendor info is not correct. -- 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=23888 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #1 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I can't replicate the error on master - now it just saves and doesn't show a vendor name in the details. We should validate if the vendor id is valid before switching to the next page. -- 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=23888 --- Comment #2 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- (In reply to Katrin Fischer from comment #1)
I can't replicate the error on master - now it just saves and doesn't show a vendor name in the details. We should validate if the vendor id is valid before switching to the next page.
That happens to me also when I type some other invalid vendor id. If I type exactly "9999999999" then the error happens. Did you type that exactly? -- 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=23888 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=23888 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 95253 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95253&action=edit Bug 23888: Do not allow invalid vendor id on creating a subscription It will avoid crash and invalid data when creating/updating a subscription. This could have been done with a AJAX query but seems more convenient this way. Test plan: - Create or update a subscription - In the "Vendor" input try an empty string, a valid vendor's id, and invalid one. => With an empty string you get the existing alert message => With a valid id you do not get any messages => With an invalid id you are not allowed to go to page 2 -- 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=23888 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Assignee|koha-bugs@lists.koha-commun |jonathan.druart@bugs.koha-c |ity.org |ommunity.org Severity|minor |normal -- 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=23888 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #4 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- If I don't have any vendors in Koha then this error comes up to the browser's JavaScript console:
ReferenceError: BOOKSELLER_IDS is not defined
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 95297 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95297&action=edit Bug 23888: Fix JS error if no vendor defined
ReferenceError: BOOKSELLER_IDS is not defined
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #6 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Jonathan, please squash that JS error follow-up to the original patch as both of these patches are yours. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Not a problem for testing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 --- Comment #8 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- The meeting notes in https://wiki.koha-community.org/wiki/Development_IRC_meeting_27_September_20... state that "follow-up patches must be squashed if not signed-off yet, and the history is not needed". It really makes reading the history a lot easier if we don't have unnecessary commits. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Joonas Kylmälä from comment #8)
The meeting notes in https://wiki.koha-community.org/wiki/ Development_IRC_meeting_27_September_2017 state that "follow-up patches must be squashed if not signed-off yet, and the history is not needed". It really makes reading the history a lot easier if we don't have unnecessary commits.
The history is needed for you, to know which changes I made. QA and RM can squash patches if needed. Feel free to squash when signing off this one. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 --- Comment #10 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 98963 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=98963&action=edit Bug 23888: Do not allow invalid vendor id on creating a subscription It will avoid crash and invalid data when creating/updating a subscription. This could have been done with a AJAX query but seems more convenient this way. Test plan: - Create or update a subscription - In the "Vendor" input try an empty string, a valid vendor's id, and invalid one. => With an empty string you get the existing alert message => With a valid id you do not get any messages => With an invalid id you are not allowed to go to page 2 Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 --- Comment #11 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 98964 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=98964&action=edit Bug 23888: (follow-up) Wrap English string in translation function This patch makes alert added in subscription-add.js translatable using the new __() function. To test, apply the patch and confirm that the alert still works correctly when submitting the subscription edit form with an invalid vendor id. Test that the string can be translated (using fr-Fr for example): 1. cd misc/translator && ./translate update fr-FR 2. Translate strings in misc/tranlator/po/fr-FR-messages-js.po 3. cd misc/translator && ./translate install fr-FR Switch to the fr-FR translation and confirm the the alert shows your translated string. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 --- Comment #12 from Owen Leonard <oleonard@myacpl.org> --- Notes: - I squashed the original 2 patches according to guidelines - The QA tool will give a false positive on untranslatable strings because it hasn't been updated along with the new JS-translation features. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #13 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- There are 2 patches : Bug 23888: Do not allow invalid vendor id on creating a subscription -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95253|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=23888 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95297|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=23888 Fridolin SOMERS <fridolin.somers@biblibre.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=23888 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #98964|0 |1 is obsolete| | --- Comment #14 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Created attachment 101826 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101826&action=edit Bug 23888: (follow-up) Wrap English string in translation function This patch makes alert added in subscription-add.js translatable using the new __() function. To test, apply the patch and confirm that the alert still works correctly when submitting the subscription edit form with an invalid vendor id. Test that the string can be translated (using fr-Fr for example): 1. cd misc/translator && ./translate update fr-FR 2. Translate strings in misc/tranlator/po/fr-FR-messages-js.po 3. cd misc/translator && ./translate install fr-FR Switch to the fr-FR translation and confirm the the alert shows your translated string. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 --- Comment #15 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- (In reply to Fridolin SOMERS from comment #13)
There are 2 patches : Bug 23888: Do not allow invalid vendor id on creating a subscription
Ok fixed thanks ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 --- Comment #16 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- New translation sysptem in JS files is great ! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED CC| |m.de.rooy@rijksmuseum.nl --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QAing -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Joonas Kylmälä from comment #8)
The meeting notes in https://wiki.koha-community.org/wiki/ Development_IRC_meeting_27_September_2017 state that "follow-up patches must be squashed if not signed-off yet, and the history is not needed". It really makes reading the history a lot easier if we don't have unnecessary commits.
Joonas: This is not day to day experience. When I want to read a larger patch set, I can squash it myself. But quite exceptional, I will submit the squashed patch in QA. Theory and practice? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Fridolin, you forgot signoff lines. No big deal -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #98963|0 |1 is obsolete| | --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 101967 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101967&action=edit Bug 23888: Do not allow invalid vendor id on creating a subscription It will avoid crash and invalid data when creating/updating a subscription. This could have been done with a AJAX query but seems more convenient this way. Test plan: - Create or update a subscription - In the "Vendor" input try an empty string, a valid vendor's id, and invalid one. => With an empty string you get the existing alert message => With a valid id you do not get any messages => With an invalid id you are not allowed to go to page 2 Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #101826|0 |1 is obsolete| | --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 101968 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101968&action=edit Bug 23888: (follow-up) Wrap English string in translation function This patch makes alert added in subscription-add.js translatable using the new __() function. To test, apply the patch and confirm that the alert still works correctly when submitting the subscription edit form with an invalid vendor id. Test that the string can be translated (using fr-Fr for example): 1. cd misc/translator && ./translate update fr-FR 2. Translate strings in misc/tranlator/po/fr-FR-messages-js.po 3. cd misc/translator && ./translate install fr-FR Switch to the fr-FR translation and confirm the the alert shows your translated string. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Still seeing a false negative in qa tools: FAIL Untranslatable string found (The vendor does not exist) - See coding guideline JS5 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> ---
This could have been done with a AJAX query but seems more convenient this way.
Or just lazy programming? :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 --- Comment #23 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #22)
This could have been done with a AJAX query but seems more convenient this way.
Or just lazy programming? :)
Not only, I think it's much more efficient this way. I did it in AJAX for the biblio, as we do not want to embed an array of all biblionumbers. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23888 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #25 from Joy Nelson <joy@bywatersolutions.com> --- QA failure when applying to 19.11.x not backported FAIL koha-tmpl/intranet-tmpl/prog/js/subscription-add.js OK git manipulation FAIL untranslatable_strings Untranslatable string found (The vendor does not exist) - See coding guideline JS5 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org