[Bug 35969] New: opac-sendbasket: Improve error message, remove some logging
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Bug ID: 35969 Summary: opac-sendbasket: Improve error message, remove some logging Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: OPAC Assignee: oleonard@myacpl.org Reporter: m.de.rooy@rijksmuseum.nl QA Contact: testopia@bugs.koha-community.org There is no need for log lines like: [WARN] Error sending mail: empty basket at /usr/share/perl5/CGI/Compile.pm line 151. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|oleonard@myacpl.org |m.de.rooy@rijksmuseum.nl Severity|enhancement |minor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 161713 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161713&action=edit Bug 35969: Add error code in opac-sendbasket.pl No need to carp about empty basket or no reply address. Just add a simple explanation on the form. Note that it makes no sense to show the form if the patron has no email address. We need it as reply-to. Test plan: Try opac-sendbasket.pl?bib_list=null Remove email address from your account, try again. 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=35969 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|minor |trivial -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |BLOCKED CC| |jonathan.druart+koha@gmail. | |com --- Comment #2 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- This will conflict with bug 34478. Please keep in mind that we have something HUGE in the queue that MUST be high priority for everybody and pushed as soon as possible. Those kind of patches should definitely be delayed until 34478 is pushed. Marked as blocked. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #2)
This will conflict with bug 34478. Please keep in mind that we have something HUGE in the queue that MUST be high priority for everybody and pushed as soon as possible. Those kind of patches should definitely be delayed until 34478 is pushed.
Marked as blocked.
No reason to block every patch for something In discussion, right? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 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=35969 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161713|0 |1 is obsolete| | --- Comment #4 from David Nind <david@davidnind.com> --- Created attachment 161795 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161795&action=edit Bug 35969: Add error code in opac-sendbasket.pl No need to carp about empty basket or no reply address. Just add a simple explanation on the form. Note that it makes no sense to show the form if the patron has no email address. We need it as reply-to. Test plan: Try opac-sendbasket.pl?bib_list=null Remove email address from your account, try again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=35969 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #5 from David Nind <david@davidnind.com> --- I've signed off, as I think having better messages is great. Some suggestions: There was an error sending the cart: No valid biblio records found.
There was an error sending the cart: no valid records were found.
Since we do not have your email address, we cannot send a mail. We need your email address as reply-to address.
There was an error sending the cart: we cannot send an email as there is no email address recorded for your account (your email address is used for the reply to address).
Testing notes (using KTD): 1. Go to http://127.0.0.1:8080/cgi-bin/koha/opac-sendbasket.pl?bib_list=null 2. Log in as koha/koha 3. Add an email address to the form and click 'Send': me@example.com 4. Error message displayed: "There was an error sending the cart." 5. Note the warning in the log: /var/log/koha/kohadev/plack-opac-error.log 6. Apply the patch 7. Repeat steps 1-3 8. Note that the error displayed is now: "Since we do not have your email address, we cannot send a mail. We need your email address as reply-to address." -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 161804 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161804&action=edit Bug 35969: (follow-up) Improve wording As suggested by David Nind. Thanks! 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=35969 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply CC| |kyle@bywatersolutions.com --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Needs a CSRF rebase. Also, the template has an else block that will never display because the script only sets two possible error values. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161795|0 |1 is obsolete| | --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 163773 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163773&action=edit Bug 35969: Add error code in opac-sendbasket.pl No need to carp about empty basket or no reply address. Just add a simple explanation on the form. Note that it makes no sense to show the form if the patron has no email address. We need it as reply-to. Test plan: Try opac-sendbasket.pl?bib_list=null Remove email address from your account, try again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=35969 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161804|0 |1 is obsolete| | --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 163774 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163774&action=edit Bug 35969: (follow-up) Improve wording As suggested by David Nind. Thanks! 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=35969 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Kyle M Hall from comment #7)
Needs a CSRF rebase. Also, the template has an else block that will never display because the script only sets two possible error values.
Rebased. Yes you are right about this ELSE + [% ELSE %] + <p>There was an error sending the cart.</p> Just added for completeness. Things change over time and this catches the rest.. Doesnt hurt imo to keep it? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163773|0 |1 is obsolete| | --- Comment #11 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 164635 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164635&action=edit Bug 35969: Add error code in opac-sendbasket.pl No need to carp about empty basket or no reply address. Just add a simple explanation on the form. Note that it makes no sense to show the form if the patron has no email address. We need it as reply-to. Test plan: Try opac-sendbasket.pl?bib_list=null Remove email address from your account, try again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163774|0 |1 is obsolete| | --- Comment #12 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 164636 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164636&action=edit Bug 35969: (follow-up) Improve wording As suggested by David Nind. Thanks! Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emily.lamancusa@montgomeryc | |ountymd.gov Status|Signed Off |Passed QA --- Comment #13 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- I agree with keeping the ELSE as a fallback - it doesn't hurt and it does make the code more robust. I squashed a capitalization fix into the follow-up, but otherwise it looks good, and passes the QA script. Setting to PQA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|opac-sendbasket: Improve |Improve error message, |error message, remove some |remove some logging when |logging |sending a cart from the | |OPAC -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.05.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=35969 --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |emily.lamancusa@montgomeryc |y.org |ountymd.gov -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|24.05.00 |24.05.00,23.11.06 released in| | Status|Pushed to main |Pushed to stable CC| |fridolin.somers@biblibre.co | |m --- Comment #15 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|24.05.00,23.11.06 |24.05.00,23.11.06,23.05.12 released in| | CC| |lucas@bywatersolutions.com Status|Pushed to stable |Pushed to oldstable --- Comment #16 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 23.05.x for upcoming 23.05.12 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35969 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED CC| |wainuiwitikapark@catalyst.n | |et.nz Status|Pushed to oldstable |RESOLVED --- Comment #17 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org