[Bug 37881] New: guarantor code broken
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 Bug ID: 37881 Summary: guarantor code broken Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Packaging Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart@gmail.com QA Contact: testopia@bugs.koha-community.org CC: mtj@kohaaloha.com There are a lot of issues with this feature, but here now in main it's totally broken: 1. cannot save a patron when the relationship with the guarantor is not selected, it explodes with 500 "Invalid relationship passed, '' is not defined." => The UI does not block the form submission when the empty option is selected (wasn't it allowed before?) 2. If I select one, save. Then edit and save (without modifying anything): re 500 "There already exists a relationship for the same guarantor (5) and guarantee (35) combination" -- 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=37881 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |critical -- 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=37881 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emmi.takkinen@koha-suomi.fi --- Comment #1 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Did some testing, first issue seems to come from bug 14570 and has probably existed for a while. For second issue, indeed I'm unable to edit and save guarantee with existing guarantor relationship on main. However this works in version 24.05.01, so something must have changed recently? -- 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=37881 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|guarantor code broken |Guarantor code broken -- 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=37881 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |RM_priority -- 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=37881 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |regression -- 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=37881 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gmcharlt@gmail.com, | |kyle.m.hall@gmail.com Component|Packaging |Patrons -- 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=37881 --- Comment #2 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Jonathan Druart from comment #0)
2. If I select one, save. Then edit and save (without modifying anything): re 500 "There already exists a relationship for the same guarantor (5) and guarantee (35) combination"
Caused by bug 32530. -- 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=37881 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |32530 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32530 [Bug 32530] When duplicating child card, guarantor is not saved -- 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=37881 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |baptiste.wojtkowski@biblibr |ity.org |e.com CC| |baptiste.wojtkowski@biblibr | |e.com -- 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=37881 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> 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=37881 --- Comment #3 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Hi, Working on it but I have a few questions. Concerning point 1, I do not think it is related to the bug since it does not concern the same part of the code. Concerning point 2 Since we now always submit the old relationship to the user, it appears it is detected as a duplicate and an error is thrown while storing the object. I'm wondering why we throw an exception here since storing an existing relationship cannot have impact on data ? Could the fix be removing the exception thrown ? Maybe I can rework this part of the code to use no try/catch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 --- Comment #4 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Baptiste Wojtkowski (bwoj) from comment #3)
Hi, Working on it but I have a few questions. Concerning point 1, I do not think it is related to the bug since it does not concern the same part of the code.
Indeed, it could be moved to a separate bug report.
Concerning point 2 Since we now always submit the old relationship to the user, it appears it is detected as a duplicate and an error is thrown while storing the object.
I'm wondering why we throw an exception here since storing an existing relationship cannot have impact on data ? Could the fix be removing the exception thrown ? Maybe I can rework this part of the code to use no try/catch.
You cannot remove the exception, it is at the DB level: UNIQUE KEY `guarantor_guarantee_idx` (`guarantor_id`,`guarantee_id`), It ensures that we won't have a guarantor with different relationships on a given guarantee. All this code is weird to be honest, Koha::Patron->store takes guarantors in parameters but does not update them. We are passing the existing guarantors, (even if deleted!) and the new ones. We are removing and adding guarantors before and after the store. So if the store fails we have already removed the guarantors (you will be prompted with a "cannot save" error, but cancelling at this point will be confusing). Anyway, it seems that the logic is flaw and the code is buggy. What I would do is to simply test (in the controller, sub add_guarantors) if the guarantor already exists before trying to insert it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=37890 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 --- Comment #5 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- I created the bug for the 1. in BZ37890 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37890) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 --- Comment #6 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Note : since we fixed the add, and the add is performed after the remove without check, the checkbox "remove" is now broken -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch 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=37881 --- Comment #7 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Created attachment 171302 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171302&action=edit Bug 37881: Editing patron with guarantor won't crash This patch fixes issue introduced by BZ32530 Test Plan (on main): 1 - Edit a patron 2 - Do not change anything and submit -> you get an error 3 - Apply both patches 4 - Repeat 1&2 -> everything works fine 5 - Try and delete the guarantor -> it will be deleted -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 --- Comment #8 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Created attachment 171303 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171303&action=edit Bug 37881: Fix the delete button for guarantor This patch fixes issue introduced by BZ32530 Test Plan (on main): 1 - Edit a patron 2 - Do not change anything and submit -> you get an error 3 - Apply both patches 4 - Repeat 1&2 -> everything works fine 5 - Try and delete the guarantor -> it will be deleted -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |blawlor@clamsnet.org --- Comment #9 from Brendan Lawlor <blawlor@clamsnet.org> --- This patch works for editing a patron with a guarantor and deleting a guarantor. But when adding a new guarantor, if you don't select a relationship it still crashes with the error: Invalid relationship passed, '' is not defined. at /usr/share/perl5/Exception/Class/Base.pm line 88 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #10 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Brendan Lawlor from comment #9)
This patch works for editing a patron with a guarantor and deleting a guarantor.
But when adding a new guarantor, if you don't select a relationship it still crashes with the error:
Invalid relationship passed, '' is not defined. at /usr/share/perl5/Exception/Class/Base.pm line 88
Yes, it has been moved to bug 37890. It should not be considered blocker here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 Brendan Lawlor <blawlor@clamsnet.org> 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=37881 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #171302|0 |1 is obsolete| | --- Comment #11 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 171399 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171399&action=edit Bug 37881: Editing patron with guarantor won't crash This patch fixes issue introduced by BZ32530 Test Plan (on main): 1 - Edit a patron 2 - Do not change anything and submit -> you get an error 3 - Apply both patches 4 - Repeat 1&2 -> everything works fine 5 - Try and delete the guarantor -> it will be deleted Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #171303|0 |1 is obsolete| | --- Comment #12 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 171400 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171400&action=edit Bug 37881: Fix the delete button for guarantor This patch fixes issue introduced by BZ32530 Test Plan (on main): 1 - Edit a patron 2 - Do not change anything and submit -> you get an error 3 - Apply both patches 4 - Repeat 1&2 -> everything works fine 5 - Try and delete the guarantor -> it will be deleted Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #171399|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 171442 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171442&action=edit Bug 37881: Editing patron with guarantor won't crash This patch fixes issue introduced by BZ32530 Test Plan (on main): 1 - Edit a patron 2 - Do not change anything and submit -> you get an error 3 - Apply both patches 4 - Repeat 1&2 -> everything works fine 5 - Try and delete the guarantor -> it will be deleted Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> 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=37881 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #171400|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 171443 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171443&action=edit Bug 37881: Fix the delete button for guarantor This patch fixes issue introduced by BZ32530 Test Plan (on main): 1 - Edit a patron 2 - Do not change anything and submit -> you get an error 3 - Apply both patches 4 - Repeat 1&2 -> everything works fine 5 - Try and delete the guarantor -> it will be deleted Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> 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=37881 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 171444 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171444&action=edit Bug 37881: (QA follow-up) Correction to TT filters used We were missing a couple of TT filters and we were using a `$raw` where it was more appropriate to use `html`. 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=37881 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |martin.renvoize@ptfs-europe | |.com QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Apologies for not catching this earlier with bug 32530. This is all working and other issues have been put into other bugs. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |24.11.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=37881 --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.11! 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=37881 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|RM_priority | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |RESOLVED Resolution|--- |FIXED CC| |lucas@bywatersolutions.com --- Comment #18 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Missing dependencies for 24.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37881 Bug 37881 depends on bug 32530, which changed state. Bug 32530 Summary: When duplicating child card, guarantor is not saved https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32530 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org