[Bug 25408] New: CanBookBeReserved should check "opacitemholds" policy
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Bug ID: 25408 Summary: CanBookBeReserved should check "opacitemholds" policy Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: arthur.suzuki@biblibre.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com how to reproduce : Set an issuing rule with opacitemholds set to "Force" Make a search and select multiple records Place a hold on those records (using multi-hold records) Verify hold is placed on the record (next item available) where it shouldn't! -- 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=25408 --- Comment #1 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 104488 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104488&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved test plan: 1) set issuingrule with opacitemholds set to "Force" 2) try to add a new hold using multi-hold 3) hold is placed on the record where it shouldn't 4) apply patch 5) retry to place same hold 6) success: hold cannot be placed since record-level hold is not available) -- 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=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104488|0 |1 is obsolete| | --- Comment #2 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 104496 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104496&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved test plan: 1) set issuingrule with opacitemholds set to "Force" 2) try to add a new hold using multi-hold 3) hold is placed on the record where it shouldn't 4) apply patch 5) retry to place same hold 6) success: hold cannot be placed since record-level hold is not available) -- 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=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=21563 -- 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=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=16547 -- 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=25408 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Please add tests to cover this change. -- 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=25408 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal Assignee|koha-bugs@lists.koha-commun |arthur.suzuki@biblibre.com |ity.org | -- 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=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net Status|Needs Signoff |Failed QA --- Comment #4 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Aside from the QA issue:
1) set issuingrule with opacitemholds set to "Force" 2) try to add a new hold using multi-hold 3) hold is placed on the record where it shouldn't
Can't reproduce. To detail: - went to circ rules - there is only one general rule, for all sites, all item types, all categories - set "OPAC item level holds" for "Force" - went to the OPAC - used the search - ticked two result - "Place hold" - placed a hold without touching anything. (on record had 1 item, the other 2) - went back to the staff interface to each record's hold page - both hold are listen as "Only item" That's not expected right? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #5 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Well, seems this bug also impact holds placed with WebServices. CanItemBeReserved also miss this check... patch definitely need rework. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|CanBookBeReserved should |CanBookBeReserved & |check "opacitemholds" |CanItemBeReserved should |policy |check "opacitemholds" | |policy --- Comment #6 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Well, seems this bug also impact holds placed with WebServices. CanItemBeReserved also miss this check... patch definitely need rework. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #7 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 105988 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105988&action=edit Bug 25408: Added unit tests for opacitemholds policy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #8 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 105989 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105989&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved test plan: 1) apply tests patch 2) run tests 3) verify not green 4) apply patch 5) run tests (again) 6) success: green :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104496|0 |1 is obsolete| | Attachment #105989|0 |1 is obsolete| | --- Comment #9 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 105990 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105990&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved test plan: 1) apply tests patch 2) run tests 3) verify not green 4) apply patch 5) run tests (again) 6) success: green :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |20985 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 [Bug 20985] CanBookBeReserved and CanItemBeReserved should check 'On shelf holds allowed' policy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105988|0 |1 is obsolete| | --- Comment #10 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 106413 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106413&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105990|0 |1 is obsolete| | --- Comment #11 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 106414 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106414&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved test plan: 1) apply tests patch 2) run tests 3) verify not green 4) apply patch 5) run tests (again) 6) success: green :) Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #12 from Victor Grousset/tuxayo <victor@tuxayo.net> --- It works! Are the additional warnings caused by the patch? # Only test patch applied kohadev-koha@ea2bce8f4ac7:/kohadevbox/koha$ prove t/db_dependent/Reserves.t t/db_dependent/Reserves.t .. 14/62 Use of uninitialized value in string eq at /kohadevbox/koha/C4/Reserves.pm line 558. Use of uninitialized value in string eq at /kohadevbox/koha/C4/Reserves.pm line 558. t/db_dependent/Reserves.t .. 59/62 # Failed test 'record-level holds should not be possible with opacitemholds set to "Force"' # at t/db_dependent/Reserves.t line 1013. # got: 'OK' # expected: 'RecordHoldNotAllowed' # Failed test 'item-level holds should not be possible with opacitemholds set to "No"' # at t/db_dependent/Reserves.t line 1049. # got: 'OK' # expected: 'notReservable' # Looks like you failed 2 tests of 6. # Failed test 'test opacitemholds rules' # at t/db_dependent/Reserves.t line 1079. # Looks like you failed 1 test of 3. t/db_dependent/Reserves.t .. 61/62 # Failed test 'reserves.item_level_hold' # at t/db_dependent/Reserves.t line 1080. t/db_dependent/Reserves.t .. 62/62 # Looks like you failed 1 test of 62. t/db_dependent/Reserves.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/62 subtests Test Summary Report ------------------- t/db_dependent/Reserves.t (Wstat: 256 Tests: 62 Failed: 1) Failed test: 61 Non-zero exit status: 1 Files=1, Tests=62, 7 wallclock secs ( 0.04 usr 0.01 sys + 5.90 cusr 0.83 csys = 6.78 CPU) Result: FAIL # Applied the implementation kohadev-koha@ea2bce8f4ac7:/kohadevbox/koha$ prove t/db_dependent/Reserves.t t/db_dependent/Reserves.t .. 20/62 Use of uninitialized value in string eq at /kohadevbox/koha/C4/Reserves.pm line 577. Use of uninitialized value in string eq at /kohadevbox/koha/C4/Reserves.pm line 577. Use of uninitialized value in string eq at /kohadevbox/koha/C4/Reserves.pm line 329. Use of uninitialized value $opacitemholds in string eq at /kohadevbox/koha/C4/Reserves.pm line 425. Use of uninitialized value in string eq at /kohadevbox/koha/C4/Reserves.pm line 329. Use of uninitialized value in string eq at /kohadevbox/koha/C4/Reserves.pm line 329. Use of uninitialized value $opacitemholds in string eq at /kohadevbox/koha/C4/Reserves.pm line 425. t/db_dependent/Reserves.t .. ok All tests successful. Files=1, Tests=62, 8 wallclock secs ( 0.05 usr 0.01 sys + 5.97 cusr 0.71 csys = 6.74 CPU) Result: PASS kohadev-koha@ea2bce8f4ac7:/kohadevbox/koha$ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #13 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 106521 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106521&action=edit Bug 25408: QA followup -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #14 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Hi Tuxayo, Hope you are doing well, you seem to be doing fine! I've made a small qa patch to solve the warn :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106413|0 |1 is obsolete| | --- Comment #15 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 106535 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106535&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106414|0 |1 is obsolete| | --- Comment #16 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 106536 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106536&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved test plan: 1) apply tests patch 2) run tests 3) verify not green 4) apply patch 5) run tests (again) 6) success: green :) Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106521|0 |1 is obsolete| | --- Comment #17 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 106537 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106537&action=edit Bug 25408: QA followup Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #18 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (In reply to Arthur Suzuki from comment #14)
I've made a small qa patch to solve the warn :)
It works and no additional warnings :) (In reply to Arthur Suzuki from comment #14)
Hi Tuxayo, Hope you are doing well, you seem to be doing fine!
Doing okay, best wishes for you :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #19 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- yay, thanks! :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Status|Signed Off |Failed QA --- Comment #20 from Nick Clemens <nick@bywatersolutions.com> --- Needs a minor rebase I am not sure this is the best way to go. With these patches we fetch every item from the db and check the hold rule - but this doubles work we do in CanItemBeReserved and feels unnecessary Currently the loop in CanBookBeReserved returns as soon as it finds one 'OK' - you loop over all items to check the holds policy first - I think if we return the holds policy too from CanItemBeReserved we can return if one item is OK and item level holds not forced. I believe this would allow removal of the caller check too The code on the patches returns 'RecordHoldNotAllowed' if any items have item level holds forced, but shouldn't it only return that if all items on the biblio are forced? You add an extra fetch for the item which has already been fetched in the code: 429 $item = Koha::Items->find( $itemnumber ); -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106535|0 |1 is obsolete| | Attachment #106536|0 |1 is obsolete| | Attachment #106537|0 |1 is obsolete| | --- Comment #21 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 107777 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107777&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #22 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 107778 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107778&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved test plan: 1) apply tests patch 2) run tests 3) verify not green 4) apply patch 5) run tests (again) 6) success: green :) Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #23 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 107779 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107779&action=edit Bug 25408: (QA follow-up) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #107779|0 |1 is obsolete| | --- Comment #24 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 107780 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107780&action=edit Bug 25408: (QA follow-up) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #25 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- (In reply to Nick Clemens from comment #20)
Needs a minor rebase
done \o/
I am not sure this is the best way to go. With these patches we fetch every item from the db and check the hold rule - but this doubles work we do in CanItemBeReserved and feels unnecessary
Currently the loop in CanBookBeReserved returns as soon as it finds one 'OK' - you loop over all items to check the holds policy first - I think if we return the holds policy too from CanItemBeReserved we can return if one item is OK and item level holds not forced. I believe this would allow removal of the caller check too
Good advice, applied :)
The code on the patches returns 'RecordHoldNotAllowed' if any items have item level holds forced, but shouldn't it only return that if all items on the biblio are forced?
Erfff... you mean different items belonging to a single biblio could have different rules? daaaaaamn...
You add an extra fetch for the item which has already been fetched in the code: 429 $item = Koha::Items->find( $itemnumber );
This has been corrected in the qa patch :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #26 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Afaik they can have different rules, but afaik the most strict is enforced in the GUI (which makes sense to me). So if you have one item that has enforced item holds, you will get that. -- You are receiving this mail because: You are watching all bug changes.
afaik the most strict is enforced in the GUI (which makes sense to me) I talked this over with Andrew, there doesn't seem to be a perfect way to do
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #27 from Nick Clemens <nick@bywatersolutions.com> --- This seems to be affecting the staff side as well To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place hold on staff side 3 - Hold is not allowed 4 - Change rule to 'force' 5 - Hold can be placed I worry about this line: return { status => 'OK' } if ( $canReserve->{status} eq 'notReservable' && $opacitemholds eq 'N' ); as it short circuits some other checks, but I think the scripts end up double checking and it is not a problem. Other that that all the login in the scripts should be in the modules ;-) this, but as long as this patch isn't changing current rules I think we are good Can we get a more detailed test plan covering the cases on opac and staff side? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #28 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hm, do we need an interface parameter maybe to distinguish between the call coming from OPAC or staff? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #29 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- (In reply to Nick Clemens from comment #27)
I worry about this line: return { status => 'OK' } if ( $canReserve->{status} eq 'notReservable' && $opacitemholds eq 'N' ); as it short circuits some other checks, but I think the scripts end up double checking and it is not a problem. Other that that all the login in the scripts should be in the modules ;-)
Checks not made in ILS-DI, at least when I first looked at the bug ;) Moving checks in this modules will helps maintaining coherence between interfaces (so i think...) but any opinion is welcome :) @Katrin : I see that there is now a "params" which might help passing data to these function, i'll try to get previous behaviour using this argument. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #30 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- maybe i can try to remove double checks of this rule in other places in the code and see if tests passes as well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #31 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 111681 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111681&action=edit Bug 25408: (QA follow-up) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #32 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 111682 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111682&action=edit Bug 25408: More test (with staff context) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #33 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Hi Katrin, I've added a mechanism so that the context (staff, opac, api) is passed to the Can(Item|Book)BeReserved. Nick, I couldn't see double checks in the pl's. (made a grep on opacitemholds in the pl's...) you are right and i didn't try to remove anything :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #34 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Arthur, your commit messages are a little sparse, the first commit should have a description of the problem and the test plan. Can you please add some more inofrmation? (see also comment#27?) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #107778|0 |1 is obsolete| | --- Comment #35 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 111693 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111693&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place hold on staff side 3 - Hold is still allowed 4 - Attempt to place hold from opac or api 5 - Hold is not allowed 6 - Change rule to 'force' 7 - Hold can be placed in all interfaces Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111693|0 |1 is obsolete| | --- Comment #36 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 111696 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111696&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place hold on staff side 3 - Hold is still allowed 4 - Attempt to place hold from opac or api 5 - Hold is not allowed 6 - Change rule to 'force' 7 - Hold can be placed in all interfaces Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111696|0 |1 is obsolete| | --- Comment #37 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 111716 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111716&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api) To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place hold on staff side 3 - Hold is still allowed 4 - Attempt to place hold from opac or api 5 - Hold is not allowed 6 - Change rule to 'force' 7 - Hold can be placed in all interfaces Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #107777|0 |1 is obsolete| | Attachment #107780|0 |1 is obsolete| | Attachment #111681|0 |1 is obsolete| | Attachment #111682|0 |1 is obsolete| | Attachment #111716|0 |1 is obsolete| | --- Comment #38 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 111717 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111717&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api) To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place hold on staff side 3 - Hold is still allowed 4 - Attempt to place hold from opac or api 5 - Hold is not allowed 6 - Change rule to 'force' 7 - Hold can be placed in all interfaces Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #39 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 111718 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111718&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #40 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 111719 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111719&action=edit Bug 25408: (QA follow-up) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #41 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 111720 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111720&action=edit Bug 25408: (QA follow-up) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #42 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 111721 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111721&action=edit Bug 25408: More test (with staff context) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #43 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- re-uploaded everything in the proper order to ease application of the patch with git-bz -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #44 from Victor Grousset/tuxayo <victor@tuxayo.net> ---
5 - Hold is not allowed
«There are no items that can be placed on hold.» Looks ok I guess.
7 - Hold can be placed in all interfaces
Hold in staff interface silently fails. I go back to the hold list page but no hold listed. No logged error. Hold in OPAC fails with «This title cannot be requested.» -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111717|0 |1 is obsolete| | Attachment #111718|0 |1 is obsolete| | Attachment #111719|0 |1 is obsolete| | Attachment #111720|0 |1 is obsolete| | Attachment #111721|0 |1 is obsolete| | --- Comment #45 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 115125 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115125&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #46 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Hi, I've rebased the patch and have added an error message on staff interface -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #115125|0 |1 is obsolete| | --- Comment #47 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 115126 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115126&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #48 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 115127 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115127&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api) To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place hold on staff side 3 - Hold is still allowed 4 - Attempt to place hold from opac or api 5 - Hold is not allowed 6 - Change rule to 'force' 7 - Item-level hold can be placed in all interfaces 8 - Record-level hold cannot be placed in all interfaces Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #49 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 115128 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115128&action=edit Bug 25408: (QA follow-up) Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #50 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 115129 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115129&action=edit Bug 25408: More test (with staff context) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #51 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 115130 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115130&action=edit Bug 25408: (QA follow-up) Added handling of calling context (staff or else) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=24841 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #52 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- 1. Not sure I understand, CanItemBeReserved is called from other C4::Reserves subroutines where the context is not passed. Why only passing it from direct calls from controller scripts? 2. The following change is not needed: sub CanBookBeReserved{ my ($borrowernumber, $biblionumber, $pickup_branchcode, $params) = @_; + my $patron = Koha::Patrons->find( $borrowernumber ); + my $borrower = $patron->unblessed; 3. Commit messages "(QA follow-up)" are not meaningful. Add more info or squash. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #53 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Can you please take a look at the last comment, Arthur? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|20985 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 [Bug 20985] CanBookBeReserved and CanItemBeReserved should check 'On shelf holds allowed' policy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #115126|0 |1 is obsolete| | Attachment #115127|0 |1 is obsolete| | Attachment #115128|0 |1 is obsolete| | Attachment #115129|0 |1 is obsolete| | Attachment #115130|0 |1 is obsolete| | --- Comment #54 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 121524 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121524&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #55 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 121525 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121525&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api) Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place hold on staff side 3 - Hold is still allowed 4 - Attempt to place hold from opac or api 5 - Hold is not allowed 6 - Change rule to 'force' 7 - Item-level hold can be placed in all interfaces 8 - Record-level hold cannot be placed in all interfaces Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #56 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Hi Joubu, points 2 and 3 from last comment has been addressed. "Context" variable name is missused, I'm uploading a new version of the patch with use of C4::Context->interface. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121524|0 |1 is obsolete| | Attachment #121525|0 |1 is obsolete| | --- Comment #57 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 121526 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121526&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api) Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place hold on staff side 3 - Hold is still allowed 4 - Attempt to place hold from opac or api 5 - Hold is not allowed 6 - Change rule to 'force' 7 - Item-level hold can be placed in all interfaces 8 - Record-level hold cannot be placed in all interfaces Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121526|0 |1 is obsolete| | --- Comment #58 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 121527 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121527&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #59 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 121528 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121528&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api) Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place hold on staff side 3 - Hold is still allowed 4 - Attempt to place hold from opac or api 5 - Hold is not allowed 6 - Change rule to 'force' 7 - Item-level hold can be placed in all interfaces 8 - Record-level hold cannot be placed in all interfaces Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121528|0 |1 is obsolete| | --- Comment #60 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 121530 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121530&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api) Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place hold on staff side 3 - Hold is still allowed 4 - Attempt to place hold from opac or api 5 - Hold is not allowed 6 - Change rule to 'force' 7 - Item-level hold can be placed in all interfaces 8 - Record-level hold cannot be placed in all interfaces Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #61 from Victor Grousset/tuxayo <victor@tuxayo.net> ---
6 - Change rule to 'force' 7 - Item-level hold can be placed in all interfaces
Odd, in staff I get «Too many holds: Jane Doe has too many holds.» So no holds at all can be placed. OPAC: «This title cannot be requested.» Also
4 - Attempt to place hold from opac or api 5 - Hold is not allowed
I can't place at all an OPAC hold when "OPAC item level holds" : don't allow I get «There are no items that can be placed on hold.» Did it work for you? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #62 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 128505 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128505&action=edit Bug 25408: (QA follow-up) This rule should not apply when placing a hold from staff interface -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #63 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Salut Victor :) Sorry for the late reply... I made a follow-up patch to make sure things won't be changed on the staff interface. Can you retry the patch-set please? Thanks in advance! All the best, Arthur -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #64 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Oh damn it, I see what you meant by "I can't place a hold at all from Opac". forget the "re-test please", I will re-work things (opac interface) and re-upload a patch. Things are working from staff and ils-di though. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #65 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- (In reply to Arthur Suzuki from comment #64)
Oh damn it, I see what you meant by "I can't place a hold at all from Opac". forget the "re-test please", I will re-work things (opac interface) and re-upload a patch. Things are working from staff and ils-di though.
Ok, i got the issue while applying this patch to 20.11, but on master it seems ok, I can place a hold at the record level on a koha-testing-docker. Or maybe it's late here, buzzy day and i'm not sure about anything anymore... I guess another test from another person won't hurt. Best, Arthur Suzuki for BibLibre Support Team -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121527|0 |1 is obsolete| | Attachment #121530|0 |1 is obsolete| | Attachment #128505|0 |1 is obsolete| | --- Comment #66 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 130180 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130180&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #67 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 130181 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130181&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api) Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place hold on staff side 3 - Hold is still allowed 4 - Attempt to place hold from opac or api 5 - Hold is not allowed 6 - Change rule to 'force' 7 - Item-level hold can be placed in all interfaces 8 - Record-level hold cannot be placed in all interfaces Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #68 from Fridolin Somers <fridolin.somers@biblibre.com> --- Main patch makes a cosmetic change in t/db_dependent/Reserves.t : diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index c4afba6424..5c4b0b64df 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -1083,7 +1083,7 @@ subtest 'reserves.item_level_hold' => sub { subtest 'test opacitemholds rules in staff context' => sub { plan tests => 2; - C4::Context->interface('intranet'); + C4::Context->interface('intranet'); Avoid this to ease rebases. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #69 from Fridolin Somers <fridolin.somers@biblibre.com> --- Patch does not apply :
git bz apply 25408
Bug 25408 - CanBookBeReserved & CanItemBeReserved should check "opacitemholds" policy 130180 - Bug 25408: Added unit tests for opacitemholds policy 130181 - Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 25408: Added unit tests for opacitemholds policy Applying: Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved error: sha1 information is lacking or useless (t/db_dependent/Reserves.t). error: could not build fake ancestor Patch failed at 0001 Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-25408-Add-opacitemholds-checks-in-CanBookBeRes-2ODFm7.patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130180|0 |1 is obsolete| | Attachment #130181|0 |1 is obsolete| | --- Comment #70 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 132658 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=132658&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #132658|0 |1 is obsolete| | --- Comment #71 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 132660 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=132660&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #72 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 132661 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=132661&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api) Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place hold on staff side 3 - Hold is still allowed 4 - Attempt to place hold from opac or api 5 - Hold is not allowed 6 - Change rule to 'force' 7 - Item-level hold can be placed in all interfaces 8 - Record-level hold cannot be placed in all interfaces Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Owen Leonard <oleonard@myacpl.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=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff --- Comment #73 from Victor Grousset/tuxayo <victor@tuxayo.net> --- There has been a confusion. My last testing was from before the last changes to fix issues found afterwards. So my signoff isn't valid anymore and this patch is still waiting for me or someone else to test it again. Switching back to need SO -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30845 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30845 [Bug 30845] OMNIBUS: Move holds checks into CanBookBeReserved/CanItemBeReserved -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30851 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30851 [Bug 30851] Improve performance of request.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |david@davidnind.com --- Comment #74 from David Nind <david@davidnind.com> --- I'm not sure I understand exactly what is going on, but I have changed the status to Failed QA after: - working through the test plan, and - working through all the options (Allow, Don't allow, Force) for the OPAC, staff interface, and ILS-DI. If I've totally misunderstood what this is about and what is required, please feel free to change the status back! Questions: 1. Should the circulation rule "OPAC item level holds" apply for the staff interface? If it should, then the setting name in the staff interface needs changing. 2. By API I'm assuming ILS-DI. If it needs testing using the REST API, can you provide some instructions on how to do this? (I have Postman available + the RESTer addon for Firefox, but APIs are not something I'm familiar with). I only tested with a circulation rule of All All - I didn't try different combinations of libraries, patron types, and item types. I used koha-testing-docker with the default configuration provided - I didn't change any system preferences apart from enabling ILS-DI. Test plan results ~~~~~~~~~~~~~~~~~ Step 1: OPAC (setting = Don't allow) - can't place any holds either at record level or item level, message for each record is "There are no items that can be placed on hold." Step 2: Staff interface (setting = Don't allow) - can still place holds (as expected), when placing multiple holds from the search don't get to select item-level holds; when placing holds from the record can select and place item level holds Step 4: . OPAC (setting = Don't allow) - can't place ANY holds (either record level or item level), message for is "There are no items that can be placed on hold." . ILS-DI (setting = Don't allow) - record level hold placed (as expected), item level hold NOT placed (as expected), get response <code>recordHoldsOnly</code> Step 7: . OPAC (setting = Force) - can't place ANY holds (either record level or item level), message is "This title cannot be requested." . Staff interface (setting = Force) - no change in behaviour from before the patch was applied: can still pace a hold when on the record detail page (either record level hold or item level hold); for records with multiple items and from search cannot select individual items to place a hold on . ILS-DI (setting = Force) - record level hold NOT placed (as expected), get response <code>recordHoldNotAllowed</code>; item level hold place (as expected) Before patch applied ~~~~~~~~~~~~~~~~~~~~ OPAC - Before patch applied - single record: all PASS, works as expected for Allow, Don't allow, and Force - Before patch applied - multiple records from search, records with one item and multiple items: all PASS, works as expected for Allow, Don't allow, and Force Staff interface - Didn't expect to work as circulation rule setting is for OPAC - Force = doesn't work - With any of the settings for records with multiple items and from search cannot select individual items to place a hold on Record level hold using ILS_DI http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=49&bib_id=146&request_location=127.0.0.1 - OPAC item level holds = Allow = PASS (hold placed) - OPAC item level holds = Don't allow = FAIL (hold placed) - OPAC item level holds = Force = FAIL (hold placed) Item level hold using ILS_DI http://127.0.0.1:8080/:8080/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=49&bib_id=146&item_id=315 - OPAC item level holds = Allow = PASS (item level hold placed) - OPAC item level holds = Don't allow = FAIL (item level hold placed) - OPAC item level holds = Force = PASS (item level hold placed) Tests pass: prove t/db_dependent/Reserves.t After patch applied ~~~~~~~~~~~~~~~~~~~ OPAC - After patch applied - single record: - Allow: can place a record level or item level hold - Don't allow: can't place a hold, message is "There are no items that can be placed on hold." - Force: can't place a hold, message is "This title cannot be requested" - After patch applied - multiple records from search, one record with one item and two records with multiple items: - Allow: can place record level and item level holds - Don't allow: can't place any holds, message for all records is "There are no items that can be placed on hold." - Force: can't place any holds, message for all records is "This title cannot be requested." Staff interface: - No change in behaviour from before the patch was applied - With any of the settings for records with multiple items and from search cannot select individual items to place a hold on Record level hold using ILS-DI http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=49&bib_id=146&request_location=127.0.0.1 - OPAC item level holds = Allow = PASS (hold placed) - OPAC item level holds = Don't allow = PASS (hold placed) - OPAC item level holds = Force = PASS (hold NOT placed, get response <code>recordHoldNotAllowed</code>) Item level hold using ILS-DI http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=49&bib_id=146&item_id=315 - OPAC item level holds = Allow = PASS (item level hold placed) - OPAC item level holds = Don't allow = PASS (item level hold NOT placed, get response <code>recordHoldsOnly</code>) - OPAC item level holds = Force = PASS (item level hold placed) Tests pass: prove t/db_dependent/Reserves.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=15404 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #75 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- (In reply to David Nind from comment #74)
I'm not sure I understand exactly what is going on, but I have changed the status to Failed QA after: - working through the test plan, and - working through all the options (Allow, Don't allow, Force) for the OPAC, staff interface, and ILS-DI.
If I've totally misunderstood what this is about and what is required, please feel free to change the status back!
Questions:
1. Should the circulation rule "OPAC item level holds" apply for the staff interface? If it should, then the setting name in the staff interface needs changing.
It doesn't, it is only supposed to be effective for OPAC actions, hence the name should be kept.
2. By API I'm assuming ILS-DI. If it needs testing using the REST API, can you provide some instructions on how to do this? (I have Postman available + the RESTer addon for Firefox, but APIs are not something I'm familiar with).
It is mostly for ILS-DI, which is supposed be an interface for discovery tools so, very close or supposed to work as an OPAC. In my opinion, API are to be used for automation as staff interface. if the api route is supposed to behave like OPAC, it is usually in the "/public" namespace. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #76 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- @David, I rebased the patch on latest master but reproduce what you've seen. I cannot place holds from OPAC with opacitemholds rule set to "Don't Allow" : should be able to place a record-level hold. Same with value "Force", although I should be able to place item-level holds. This patch needs rework... working on it -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #132660|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #132661|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #77 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 164680 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164680&action=edit Bug 25408: Added unit tests for opacitemholds policy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #78 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 164681 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164681&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #79 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 164682 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164682&action=edit Bug 25408: (QA follow-up) using perltidy to resolve complaining QA scripts -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164680|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164681|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164682|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #80 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 164684 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164684&action=edit Bug 25408: Added unit tests for opacitemholds policy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #81 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 164685 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164685&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api) Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place item-level hold on staff interface 3 - Hold is still allowed (rule do not apply to staff) 4 - Attempt to place item-level hold from opac or ilsdi 5 - Hold is not allowed (rule apply from opac or ilsdi) 6 - Change rule to 'force' 7 - Item-level hold can still be placed from the staff interface 8 - Record-level hold cannot be placed from opac or ilsdi -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #82 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 164686 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164686&action=edit Bug 25408: (QA follow-up) using perltidy to resolve complaining QA scripts -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #83 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Hello Guys, I've made a rework of that one so I thought it would be better to have it back to Need-Signoff status. So, it's been rebased, tests provided. I made a separate patch for perltidy because it introduce a lot of cosmetic changes (but no logic changes, which is in the previous patch). Hopefully it will pass the signoff and qa this time :) Best, Arthur -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164684|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164685|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164686|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #84 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 164693 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164693&action=edit Bug 25408: Added unit tests for opacitemholds policy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #85 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 164694 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164694&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api) Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place item-level hold on staff interface 3 - Hold is still allowed (rule do not apply to staff) 4 - Attempt to place item-level hold from opac or ilsdi 5 - Hold is not allowed (rule apply from opac or ilsdi) 6 - Change rule to 'force' 7 - Item-level hold can still be placed from the staff interface 8 - Record-level hold cannot be placed from opac or ilsdi -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@ptfs-europe.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 pierre.genty@biblibre.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pierre.genty@biblibre.com --- Comment #86 from pierre.genty@biblibre.com --- Patch tested for ILS-DI with two rules : - books : OPAC item level holds = Allow - DVDs : OPAC item level holds = Force Before applying patch : - book reserve (biblionumber = 7) : /cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=11&bib_id=7&request_location=127.0.0.1 = hold allowed => OK - DVD reserve (biblionumber = 296) : /cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=11&bib_id=296&request_location=127.0.0.1 = hold allowed => KO After applying patch : - book reserve (biblionumber = 7) : /cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=11&bib_id=7&request_location=127.0.0.1 = hold allowed => OK - DVD reserve (biblionumber = 296) : /cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=11&bib_id=296&request_location=127.0.0.1 = "recordHoldNotAllowed" => OK Patch seems to work well with ILS-DI ! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Pedro Amorim <pedro.amorim@ptfs-europe.com> 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=25408 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164693|0 |1 is obsolete| | --- Comment #87 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 164878 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164878&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Pedro Amorim <pedro.amorim@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=25408 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164694|0 |1 is obsolete| | --- Comment #88 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 164879 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164879&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api) Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place item-level hold on staff interface 3 - Hold is still allowed (rule do not apply to staff) 4 - Attempt to place item-level hold from opac or ilsdi 5 - Hold is not allowed (rule apply from opac or ilsdi) 6 - Change rule to 'force' 7 - Item-level hold can still be placed from the staff interface 8 - Record-level hold cannot be placed from opac or ilsdi Signed-off-by: Pedro Amorim <pedro.amorim@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=25408 --- Comment #89 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- This looks like it's a good change i.e. moving the check logic to a lower layer. The test plan works as described and all tests pass. I can't find a bug or issue this introduces, but others more experienced in the holds logic than me may disagree. Also tested the following ILSDI endpoint: /cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=11&bib_id=76&item_id=167 It respects the opacitemholds config as expected, I believe, as ILSDI is an OPAC interface and context is 'opac' when using ILSDI Some thoughts related to code specifically: - Personally I'd prefer this to be unfolded in more commits to ease the review process, with each having a short explanation of why the changes are required: Something like this, (only a suggestion): -- Move get_opacitemholds_policy check into Reserves.pm::CanBookBeReserved -- Update CanItemBeReserved to return recordHoldsOnly status if interface is OPAC -- Update warning labels in opac-reserve.tt accordingly. - A new unnecessary empty new line is added in opac-reserve.pl - Some lines that have not been changed have been tidied and included - Unnecessary changes are made e.g. $patron->borrowernumber to $patron->id (correct me if I'm wrong here, please!) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #90 from Victor Grousset/tuxayo <victor@tuxayo.net> ---
- Unnecessary changes are made e.g. $patron->borrowernumber to $patron->id (correct me if I'm wrong here, please!)
Indeed, in bunch of lines that are already changed, it's a good opportunity to enforce terminology without adding more conflict possibilities. But here it's in lines that are not changed, so it's adding new conflicts possibilities with other submitted patches and when backporting.
- Some lines that have not been changed have been tidied and included
Yes, also that.
- Personally I'd prefer this to be unfolded in more commits to ease the review process, with each having a short explanation of why the changes are required:
At least having the full commit message list these points. Not sure about splitting the commit after the work is done. Maybe I missed the main point, but in this case the 3 changes look 95% likely to have been done together and not as iteration with a different logic on each step. That's a nice thing to keep in mind for other reviews or my own patches, thanks :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164878|0 |1 is obsolete| | --- Comment #91 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 165111 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165111&action=edit Bug 25408: Added unit tests for opacitemholds policy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164879|0 |1 is obsolete| | --- Comment #92 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 165112 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165112&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api). Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold What this patch does : -- Move get_opacitemholds_policy check into Reserves.pm::CanBookBeReserved -- Update CanItemBeReserved to return recordHoldsOnly status if interface is OPAC -- Update warning labels in opac-reserve.tt accordingly. To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place item-level hold on staff interface 3 - Hold is still allowed (rule do not apply to staff) 4 - Attempt to place item-level hold from opac or ilsdi 5 - Hold is not allowed (rule apply from opac or ilsdi) 6 - Change rule to 'force' 7 - Item-level hold can still be placed from the staff interface 8 - Record-level hold cannot be placed from opac or ilsdi -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #93 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- (In reply to Victor Grousset/tuxayo from comment #90)
- Unnecessary changes are made e.g. $patron->borrowernumber to $patron->id (correct me if I'm wrong here, please!)
Indeed, in bunch of lines that are already changed, it's a good opportunity to enforce terminology without adding more conflict possibilities. But here it's in lines that are not changed, so it's adding new conflicts possibilities with other submitted patches and when backporting.
I've fixed this in the latest version of the patch :)
- Some lines that have not been changed have been tidied and included
Yes, also that.
Also fixed :)
- Personally I'd prefer this to be unfolded in more commits to ease the review process, with each having a short explanation of why the changes are required:
At least having the full commit message list these points. Not sure about splitting the commit after the work is done. Maybe I missed the main point, but in this case the 3 changes look 95% likely to have been done together and not as iteration with a different logic on each step. That's a nice thing to keep in mind for other reviews or my own patches, thanks :)
I've added the relevant points to the commit message, next time I will try to better split the commits. I don't know if the patch can move to "Passed QA" or if further work is needed? Thanks for the review anyway :) Best, Arthur -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #94 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- This looks great. You've addressed the reasons Victor FQA so it should be set back to Signed-off so that he can look again, I believe. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |victor@tuxayo.net |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #95 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- (In reply to Pedro Amorim from comment #94)
This looks great. You've addressed the reasons Victor FQA so it should be set back to Signed-off so that he can look again, I believe.
Great, hope that will pass QA soon :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #96 from Victor Grousset/tuxayo <victor@tuxayo.net> ---
it should be set back to Signed-off
Indeed, there are no changes to the code that would invalidate the signoff. (minor ones wouldn't invalidate it, but here, it's even zero actual changes :) ) Thanks Pedro for giving a look and thanks Arthur for the fixes :) Will look back asap see if I understand enough the code to QA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165111|0 |1 is obsolete| | --- Comment #97 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 165442 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165442&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165112|0 |1 is obsolete| | --- Comment #98 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 165443 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165443&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api). Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold What this patch does : -- Move get_opacitemholds_policy check into Reserves.pm::CanBookBeReserved -- Update CanItemBeReserved to return recordHoldsOnly status if interface is OPAC -- Update warning labels in opac-reserve.tt accordingly. To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place item-level hold on staff interface 3 - Hold is still allowed (rule do not apply to staff) 4 - Attempt to place item-level hold from opac or ilsdi 5 - Hold is not allowed (rule apply from opac or ilsdi) 6 - Change rule to 'force' 7 - Item-level hold can still be placed from the staff interface 8 - Record-level hold cannot be placed from opac or ilsdi Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165442|0 |1 is obsolete| | --- Comment #99 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 165444 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165444&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165443|0 |1 is obsolete| | --- Comment #100 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 165445 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165445&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api). Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold What this patch does : -- Move get_opacitemholds_policy check into Reserves.pm::CanBookBeReserved -- Update CanItemBeReserved to return recordHoldsOnly status if interface is OPAC -- Update warning labels in opac-reserve.tt accordingly. To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place item-level hold on staff interface 3 - Hold is still allowed (rule do not apply to staff) 4 - Attempt to place item-level hold from opac or ilsdi 5 - Hold is not allowed (rule apply from opac or ilsdi) 6 - Change rule to 'force' 7 - Item-level hold can still be placed from the staff interface 8 - Record-level hold cannot be placed from opac or ilsdi Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #101 from Victor Grousset/tuxayo <victor@tuxayo.net> --- It works :) (readded Pedro's SO line that was lost in Comment 91 changes) It functionally makes sense, found nothing out of order after some exploratory testing on opac, staff and ILS-DI. Thanks David for the earlier testing and notes :) REST API doesn't seem to have a way to place holds so nothing to see there. Like Pedro, I'm not confident in understanding alone enough the completeness and correctness of the many changes in C4/Reserves.pm and opac-reserve.pl to fully QA. But let's try going with this since we both found no obvious issue in the code. The RM is free to request more review. Works, makes sense, QA script happy, code looks good, passing QA :) -- @Arthur: For releases notes, it would be clearer if the name of the ticket was something like: «"OPAC item level holds" circulation rules are not followed by ILS-DI» Do you confirm that's the fix in a nutshell? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #102 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am sure everyone here has tested well, but I'd feel a little bit more confident if we could maybe get Kyle or another hold expert to look at this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #103 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I don't have a status for "request additional review". Hmpf. :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #104 from Nick Clemens <nick@bywatersolutions.com> --- I am still unsure about one of the lines here: In CanBookBeReserved, during the item loop: return { status => 'recordHoldNotAllowed' } if $opacitemholds eq 'F'; If one of the items is type 'new' and doesn't allow record level holds, but another is 'book' and does allow it - we will return 'recordHoldNotAllowed' if we hit the 'new' item first. You also call 'get_opacitemholds_policy' in CanBookBeReserved and also add a call to get_effective_rules in CanItemBeReserved - these should be getting the same rule - why do we need to check at both levels? The check in CanItemBeReseerved should handle returning the necessary status -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #105 from Nick Clemens <nick@bywatersolutions.com> --- see comment 20 and comment 25 - I think the problem with multiple item types on a single bib is still the issue -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #106 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Nice catch, indeed that part of comment 20 wasn't addressed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |baptiste.wojtkowski@biblibr | |e.com --- Comment #107 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Hi ! I rebased and tried to go through the test plan but I could not manage to reproduce the first issue, is it normal ? My tests on "OPAC item level holds" on the opac BEFORE applying patch: 1 - Force -> I cannot place hold on record, only on item 2 - Don't allow -> I cannot place hold on item 3 - Allow -> I can place on both I always can place an hold on opac. (Applying the patch breaks this but I might have improperly rebased) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #108 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- (In reply to Baptiste Wojtkowski (bwoj) from comment #107)
Hi ! I rebased and tried to go through the test plan but I could not manage to reproduce the first issue, is it normal ?
My tests on "OPAC item level holds" on the opac BEFORE applying patch: 1 - Force -> I cannot place hold on record, only on item 2 - Don't allow -> I cannot place hold on item 3 - Allow -> I can place on both
I always can place an hold on opac.
(Applying the patch breaks this but I might have improperly rebased)
Hello Baptiste, the rule applies well to any operation done from the UI because it is checked in the perl scripts. The idea was to move this check to C4/Reserves to have a consistent behavior over any interfaces (opac, staff, ils-di, api) with a single point of entry in case we need to change it in the future. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165444|0 |1 is obsolete| | --- Comment #109 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Created attachment 176115 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176115&action=edit Bug 25408: Added unit tests for opacitemholds policy Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165445|0 |1 is obsolete| | --- Comment #110 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Created attachment 176116 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176116&action=edit Bug 25408: Add "opacitemholds" checks in CanBookBeReserved & CanItemBeReserved The rule "opacitemholds" seemed not to be controlled in some cases. This patch adds a control for this issuingrule in such a way it is checked across all interfaces (staff, opac, api). Added error message when trying to hold on the record and opacitemhold is set to "Force" to item-level hold What this patch does : -- Move get_opacitemholds_policy check into Reserves.pm::CanBookBeReserved -- Update CanItemBeReserved to return recordHoldsOnly status if interface is OPAC -- Update warning labels in opac-reserve.tt accordingly. To test: 0 - Apply patches 1 - Set rule to "Don't allow" item specific holds on opac 2 - Attempt to place item-level hold on staff interface 3 - Hold is still allowed (rule do not apply to staff) 4 - Attempt to place item-level hold from opac or ilsdi 5 - Hold is not allowed (rule apply from opac or ilsdi) 6 - Change rule to 'force' 7 - Item-level hold can still be placed from the staff interface 8 - Record-level hold cannot be placed from opac or ilsdi Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #111 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Rebased on main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32702 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32702 [Bug 32702] Item statuses that block holds should be checked in CanItemBeReserved -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|32702 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32702 [Bug 32702] Item statuses that block holds should be checked in CanItemBeReserved -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 --- Comment #112 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Is this patch still failed qa?(In reply to Katrin Fischer from comment #26)
Afaik they can have different rules, but afaik the most strict is enforced in the GUI (which makes sense to me). So if you have one item that has enforced item holds, you will get that.
In that case then comment #20 and comment #25 part about multiple items with different rules for a single record is already addressed? In the current state of the patch, the most restrictive rule is applied. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|victor@tuxayo.net | Status|Failed QA |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org