Bug 22456 added a "waiting_hold_cancellation" rule but it doesn't look it's being installed correctly, which is causing fatal errors when you view an OPAC user account with waiting holds...
I don't know how the MySQL database is being set up in koha-testing-docker, but if "installer/data/mysql/db_revs/220600023.pl" is involved, it's clearly not working. While I can see the "hold_cancellation_requests" table in the database, the "waiting_hold_cancellation" circulation_rules entry is not getting created.
I reckon koha-testing-docker is created from kohastructure.sql and then uses the koha-testing-docker scripts to populate from there. So really... this just means that any new Koha instance won't work because it's missing this circulation rule. Koha shouldn't need this circulation rule to be set, to the problem is with Koha::Hold::cancellation_requestable_from_opac which uses method chaining when it shouldn't. I'd say the fix here is probably to just remove the method chaining from Koha::Hold::cancellation_requestable_from_opac. Should be easy peasy lemon squeezy.
Created attachment 138771 [details] [review] Bug 31303: Prevent fatal errors when missing "waiting hold cancellation" rule This patch removes method chaining which didn't account for the fact that the "waiting hold cancellation" circulation rule might not exist. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/reserve/request.pl?biblionumber=29 2. Search for "koha" user 3. Choose the specific item and click "Place hold" 4. Check in "39999000001310" and click "Confirm hold (Y)" 5. Go to http://localhost:8080/cgi-bin/koha/opac-user.pl#opac-user-holds 6. Rejoice in Koha not exploding 7. Go to http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl 8. Go to "Default waiting hold cancellation policy" and change "Cancellation allowed" to "Yes" and click "Add" 9. Go to http://localhost:8080/cgi-bin/koha/opac-user.pl#opac-user-holds 10. Note the "Cancel" button appears 11. Rejoice again that the "Cancel" button appeared
Please use the depends on, that way the author of the patch that might have caused the issue will also be informed and we can avoid backporting with open bugs (not in this case, but in general :))
(In reply to Katrin Fischer from comment #4) > Please use the depends on, that way the author of the patch that might have > caused the issue will also be informed and we can avoid backporting with > open bugs (not in this case, but in general :)) I see the comments now and the patch! (still early here...), still useful to link :)
(In reply to Katrin Fischer from comment #4) > Please use the depends on, that way the author of the patch that might have > caused the issue will also be informed and we can avoid backporting with > open bugs (not in this case, but in general :)) Nice one! I was so focused on bug 15516 that I forgot to link it to bug 22456 too! Thanks :)
(In reply to David Cook from comment #1) > I don't know how the MySQL database is being set up in koha-testing-docker, > but if "installer/data/mysql/db_revs/220600023.pl" is involved, it's clearly > not working. You can do: $ git checkout v22.05.00 -b test $ reset_all $ git checkout master $ updatedatabase and check yourself if it works or not.
Created attachment 138835 [details] [review] Bug 31303: Prevent fatal errors when missing "waiting hold cancellation" rule This patch removes method chaining which didn't account for the fact that the "waiting hold cancellation" circulation rule might not exist. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/cgi-bin/koha/reserve/request.pl?biblionumber=29 2. Search for "koha" user 3. Choose the specific item and click "Place hold" 4. Check in "39999000001310" and click "Confirm hold (Y)" 5. Go to http://localhost:8080/cgi-bin/koha/opac-user.pl#opac-user-holds 6. Rejoice in Koha not exploding 7. Go to http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl 8. Go to "Default waiting hold cancellation policy" and change "Cancellation allowed" to "Yes" and click "Add" 9. Go to http://localhost:8080/cgi-bin/koha/opac-user.pl#opac-user-holds 10. Note the "Cancel" button appears 11. Rejoice again that the "Cancel" button appeared Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 138836 [details] [review] Bug 31303: (follow-up) Use get_effective_rule_value instead Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Thanks for taking care, David. I think my follow-up highlights what we should be doing in the codebase, in general.
Pushed to master for 22.11. Nice work everyone, thanks!
(In reply to Tomás Cohen Arazi from comment #10) > Thanks for taking care, David. I think my follow-up highlights what we > should be doing in the codebase, in general. Ah yes. Makes sense to me!
Patchset does not apply cleanly to 22.05.x branch. If needed please rebase