https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408
--- Comment #89 from Pedro Amorim <[email protected]> --- 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. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
