Summary: | Hold rules checker: show matched rules and syspref values to help understand why a hold is possible or not | ||
---|---|---|---|
Product: | Koha | Reporter: | Victor Grousset/tuxayo <victor> |
Component: | Hold requests | Assignee: | Victor Grousset/tuxayo <victor> |
Status: | BLOCKED --- | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | arthur.suzuki, bgkriegel, dcook, gmcharlt, julian.maurice, koha, laurence.rault, magnus |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
URL: | https://gitlab.com/tuxayo/Koha/-/tree/hold-rules-checker-master/ | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35195 | ||
Change sponsored?: | --- | Patch complexity: | Medium patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Bug Depends on: | 33166 | ||
Bug Blocks: | |||
Attachments: |
Bug 23732: Hold rules checker
Bug 23732: Add unit tests Screen capture - Check this out :) Screen capture - position of "hold rule checker" button Bug 23732: Add unit tests Bug 23732: Renamed TODOs and added translation capabilities Bug 23732: Hold rules checker Screenshot |
Description
Victor Grousset/tuxayo
2019-10-03 13:32:08 UTC
Created attachment 95435 [details] [review] Bug 23732: Hold rules checker == Test plan == - apply - yarn build - generate translations - try to place a hold, on the Hold details page you should see a "Holding rules" blue button - click it - check that the info if consistent with the holding rules, the syspref and the current and future number of holds of the patron - place the hold - set maxreserves syspref to 1 - try to place a hold - see in the holds rules checker that the maxreserves is highlighted in red (it the only thing supporting highlighting) - restore maxreserves syspref - test that sabotaging the matched circulation rule or hold policy is reflected (the limits) in the hold rules checker Created attachment 95436 [details] [review] Bug 23732: Add unit tests Created attachment 95443 [details]
Screen capture - Check this out :)
Screen capture of our nice hold rules checker tool :)
Created attachment 95444 [details]
Screen capture - position of "hold rule checker" button
Created attachment 95445 [details] [review] Bug 23732: Add unit tests == test plan == 1. prove -r t/db_dependent/Reserves 2. Apply bugzilla 3. prove t/db_dependent/Reserves/HoldRulesChecker.t 4. prove -r t/db_dependent/Reserves screenshots: Not that "Check this out" is the name of the record. A very great also not so great name for examples ^_^ *Note that Overall this looks really good--and really useful! I see a few instances of "TODO" in the changes. Are these things which need a follow-up? Indeed I spotted these, I though I had fixed these but can't find the commit anywhere :o Planned to take care of these, at the same time of 18.11.x rebase. Created attachment 95934 [details] [review] Bug 23732: Renamed TODOs and added translation capabilities When backporting on 18.11 I got this error (when only circulation used): Template process failed: undef error - Can't call method "description" on an undefined value at /home/koha/src/Koha/Template/Plugin/Categories.pm line 33. To debug with : https://wiki.koha-community.org/wiki/User:Victor_Grousset_-_tuxayo:Debugging#print_a_stack_backtrace Sorry, conflicts CONFLICT (content): Merge conflict in reserve/request.pl CONFLICT (content): Merge conflict in C4/Reserves.pm Created attachment 147501 [details] [review] Bug 23732: Hold rules checker Test plan: - apply patch - try to place a hold, on the Hold details page you should see a "Holding rules" button. Click on it - check that the info is consistent with the holding rules, the syspref and the current and future number of holds of the patron - place the hold - set maxreserves syspref to 1 - try to place a hold - see in the holds rules checker that the maxreserves is highlighted in red - restore maxreserves syspref - test that sabotaging the matched circulation rule or hold policy is reflected in the hold rules checker Created attachment 147504 [details]
Screenshot
Trying to revive this bug. There is one significant change: instead of a single circulation rule, the modal now shows all circulation rules that have been checked, in the order they have been checked. So if no hold is possible, the last rule should be the culprit. I tested with "On shelf holds allowed" set to "If all unavailable" and the modal didn't give that as a reason why. Are there some rules which are not checked by this feature? Tried to test on sandbox biblibre : When I choose a borrower during the hold process, it fails with : Can't call method "notforloan" on an undefined value at /kohadevbox/koha/C4/Reserves.pm line 1570 (In reply to Owen Leonard from comment #16) > I tested with "On shelf holds allowed" set to "If all unavailable" and the > modal didn't give that as a reason why. Are there some rules which are not > checked by this feature? Unfortunately yes. Only the rules checked by CanBookBeReserved are displayed. The 'on shelf holds' rule is checked in IsAvailableForItemLevelRequest so it's not displayed. I don't know how to fix that without a significant rewrite... :/ (In reply to Laurence Rault from comment #17) > Tried to test on sandbox biblibre : > When I choose a borrower during the hold process, it fails with : > Can't call method "notforloan" on an undefined value at > /kohadevbox/koha/C4/Reserves.pm line 1570 This error is not caused by this patch. I will open a new bug report > I will open a new bug report => Bug 33166 |