https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41917
Martin Renvoize (ashimema) <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #5 from Martin Renvoize (ashimema) <[email protected]> --- Hmm, there's certainly at least one QA Fail here.. but I'll add a follow-up for it. [Item level bookable = 0 now get's hard overridden by parent itemtype.. this would be a regression] But the bigger question regards the third commit and the change to get_effective_rule (Koha/CirculationRules.pm:298). There's lots of callers to this method and your changing the return so the regression scope it large. This change makes all circulation rules silently inherit from parent itemtype when no child-specific rule is found. This is a broad behavioural change, not scoped to bookings. A key concern is the interaction with C4/Circulation.pm::TooMany (lines 441–585), which already has its own explicit parent-type handling for maxissueqty. After the patch: - $maxissueqty_rule = get_effective_rule(itemtype => $child_type) can now return the parent's specific rule instead of the global rule. - This means $maxissueqty_rule->itemtype is no longer NULL (it's the parent type). - Line 575: if ($maxissueqty_rule->rule_value < $parent_maxissueqty_rule->rule_value && defined($maxissueqty_rule->itemtype)) — with the patch, both $maxissueqty_rule and $parent_maxissueqty_rule now hold the same parent rule. rule_value < rule_value is never true, so this branch is silently dead. This effectively changes checkout limit enforcement for child itemtypes. Tracing through with the existing TooMany.t scenario (branch=1, parent=2 rule), the tests appear to still pass — the counting and limit logic happen to converge on the same result. However, this relies on coincidental symmetry, not a deliberate test of the new code path. The TooMany.t parent-type subtest (t/db_dependent/Circulation/TooMany.t:864) should be run explicitly against this branch to verify, and ideally a test should be added for the affected $rule_itemtype path. Other callers that will silently change behaviour for child itemtypes include: - returnbranch — affects where items are routed on return - fine / lengthunit — affects fine calculation - opacitemholds — affects hold permissions Libraries with parent/child itemtype hierarchies and any parent-specific rules could see unexpected changes to all of these — not just bookings. So, I suppose the question is.. Was this a deliberate choice of the original implimentation or an oversight.. and in either case, is it likely to be in practical use anywhere in such a way that this would cause an active regression. -- 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/
