[Bug 26506] New: Koha::Account::pay will fail if $userenv is not set
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 Bug ID: 26506 Summary: Koha::Account::pay will fail if $userenv is not set Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Fines and fees Assignee: koha-bugs@lists.koha-community.org Reporter: matthias.meusburger@biblibre.com QA Contact: testopia@bugs.koha-community.org Koha::Account::pay will fail if $userenv is not set. The context would be a payment plugin for the opac, in which the payment system will call a callback URL to confirm the payment has been made. In this situation, no user is logged in when the plugin uses Koha::Account::pay. The problem is that the following line in Koha::Account::pay: my $manager_id = $userenv ? $userenv->{number} : 0; will result in an SQL error, since $userenv->{number} is not set, and 0 is not an acceptable value because of foreign key constraint: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_hkr`.`accountlines`, CONSTRAINT `accountlines_ibfk_borrowers_2` FOREIGN KEY (`manager_id`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE) By the way, if you have a look at C4/Accounts.pm, in the manualinvoice function: my $manager_id = C4::Context->userenv ? C4::Context->userenv->{'number'} : undef; $manager_id will be set to undef, which will not fail, since undef is an acceptable value regarding to SQL foreign key constraints. It's hard to provide a test plan, since it relies on a plugin, sorry, but I will try to answer to any question :) -- 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=26506 --- Comment #1 from Matthias Meusburger <matthias.meusburger@biblibre.com> --- Created attachment 110552 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=110552&action=edit Bug 26506: Fix Koha::Account::pay if called when no user is logged-in Currently, Koha::Account::pay will fail if no user is logged-in when it is called. In this case, this patch will use undef instead of 0 for $manager_id in order to comply with the foreign key constraint on manager_id. -- 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=26506 Matthias Meusburger <matthias.meusburger@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|koha-bugs@lists.koha-commun |matthias.meusburger@biblibr |ity.org |e.com -- 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=26506 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch 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=26506 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #110552|0 |1 is obsolete| | --- Comment #2 from Magnus Enger <magnus@libriotech.no> --- Created attachment 110581 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=110581&action=edit Bug 26506: Fix Koha::Account::pay if called when no user is logged-in Currently, Koha::Account::pay will fail if no user is logged-in when it is called. In this case, this patch will use undef instead of 0 for $manager_id in order to comply with the foreign key constraint on manager_id. Signed-off-by: Magnus Enger <magnus@libriotech.no> Biblibre and Libriotech have created a OPAC payment plugin that was having problems because of this bug. Applying the fix in this patch to a live server solved the problem. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@libriotech.no --- Comment #3 from Magnus Enger <magnus@libriotech.no> --- I have signed off the patch because I know it solves a problem. However, I have been involved in the work that Biblibre has done to uncover and fix the bug, so if my signoff is considered to be "not independent enough" please set the status back to NSO. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Exactly the same patch as attached to bug 26332. As this one is signed off, I think I can safely QA this version even though I wrote the duplicate. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |22008 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22008 [Bug 22008] accountlines.manager_id is missing a foreign key constraint -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |14825 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14825 [Bug 14825] Accounts Rewrite Omnibus -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Whilst I agree with this patch and will PQA it once back at a computer, I do feel it perhaps highlight some issues with the plugin code in question (as I highlighted on my version of the bug and it prompted me to fix my own payments plugin). Lots of Koha functions still exist that expect a userenv to always be present so in general I feel developers should always attempt to set at least the minimum of branch code and interface in a skeleton userenv if they can't catch an existing environment. Just some food for though Matthias 😉. For an example of this see my commit on one of the payment plugins we maintain: https://github.com/PTFS-Europe/koha-plugin-wpm-online_payments/commit/0a0ee1... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Don't forget to write a test. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- *** Bug 26332 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Good catch, it needs a unit test. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #110581|0 |1 is obsolete| | --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 111045 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111045&action=edit Bug 26506: Fix Koha::Account::pay if called when no user is logged-in Currently, Koha::Account::pay will fail if no user is logged-in when it is called. In this case, this patch will use undef instead of 0 for $manager_id in order to comply with the foreign key constraint on manager_id. Signed-off-by: Magnus Enger <magnus@libriotech.no> Biblibre and Libriotech have created a OPAC payment plugin that was having problems because of this bug. Applying the fix in this patch to a live server solved the problem. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 111046 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111046&action=edit Bug 26506: (QA follow-up) Add Unit Test Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 Martin Renvoize <martin.renvoize@ptfs-europe.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=26506 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |20.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=26506 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 20.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|20.11.00 |20.11.00, 20.05.06 released in| | CC| |lucas@bywatersolutions.com --- Comment #12 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 20.05.x for 20.05.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26506 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aleisha@catalyst.net.nz --- Comment #13 from Aleisha Amohia <aleisha@catalyst.net.nz> --- tests fail on 19.11.x. not backported. $ prove t/db_dependent/Koha/Account.t t/db_dependent/Koha/Account.t .. 7/11 # Looks like you planned 5 tests but ran 4. # Failed test 'pay() tests' # at t/db_dependent/Koha/Account.t line 724. Can't use string ("71") as a HASH ref while "strict refs" in use at t/db_dependent/Koha/Account.t line 720. # Looks like your test exited with 255 just after 8. t/db_dependent/Koha/Account.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 4/11 subtests Test Summary Report ------------------- t/db_dependent/Koha/Account.t (Wstat: 65280 Tests: 8 Failed: 1) Failed test: 8 Non-zero exit status: 255 Parse errors: Bad plan. You planned 11 tests but ran 8. Files=1, Tests=8, 3 wallclock secs ( 0.02 usr 0.01 sys + 2.42 cusr 0.84 csys = 3.29 CPU) Result: FAIL if needed, please fix. There also doesn't seem to be a test plan here so I'm not sure how to test this. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org