|
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 Reverting status from BLOCKED to Needs Signoff and removing bug 33166 from the "Depends on" list Bug 33166 is a different bug that has nothing to do with this bug Followed the test plan from comment 13. Nothing happens when clicking on the circulation rules link. My pop-up blocker was off, so nothing in the browser should have prevented it from appearing. Would it be possible to change the link to a button and have it be a bit bigger? It took a minute for me to see and realize that was clickable text. Created attachment 173427 [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 Co-authored-by: Victor Grousset/tuxayo <victor@tuxayo.net> Co-authored-by: Arthur Suzuki <arthur.suzuki@biblibre.com> Rebased-by: Julian Maurice <julian.maurice@biblibre.com> Co-authored-by: Julian Maurice <julian.maurice@biblibre.com> Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 173428 [details] [review] Bug 23732: Fixes for Bootstrap 5 Thanks a lot Julian for the earlier rebase!!! :o I had started locally a while ago but never got to catch up with main. Here is another rebase and basic Bootstrap 5 fixes. Testing was very superficial given I just found about those Bootstrap 5 changes that made the modal not open at all ^^" I'll try to check the rest soonish. (In reply to Esther Melander from comment #22) > Followed the test plan from comment 13. Nothing happens when clicking on the > circulation rules link. My pop-up blocker was off, so nothing in the browser > should have prevented it from appearing. > > Would it be possible to change the link to a button and have it be a bit > bigger? It took a minute for me to see and realize that was clickable text. Thanks for trying to test. I suppose you where trying on a sandbox. So the old patches weren't applying and on sandbox creation it's not shown when that happens. That an unfortunate pitfall of the current sandbox UI, more detail in «Bug number: Don't fill it» https://wiki.koha-community.org/wiki/Sandboxes#Step_1:_Set_up_a_sandbox It's not ready yet for retesting, I'll notify when it is. |