GetIssuingCharges uses its own private sub to choose the correct rental discount rule, specifying the rules returned in order: 1 - same library, same category, same itemtype 2 - all libraries, same category, same itemtype 3 - same library, same category, all itemtypes 4 - all libraries, same category, all itemtypes It doesn't match what the circ rules page says, and is missing several options: same library, same patron category, same item type same library, same patron category, all item types same library, all patron categories, same item type same library, all patron categories, all item types default (all libraries), same patron category, same item type default (all libraries), same patron category, all item types default (all libraries), all patron categories, same item type default (all libraries), all patron categories, all item types
Hmm, I'm wondering if there was a deliberate reason behind this or whether it's just a historical mistake.
Created attachment 113823 [details] [review] Bug 26593: Remove _get_discount_from_rule This patch remove the private sub used in GetIssuingCharges in favor of get_effective_rule It corrects the wrong precedence for rules and adds tests to cover this subroutine NOTE: the 'branch' for the discount will be determined by the signed in branch, this is a bug to be fixed in the future To test: 1 - Define a rentalcharge for an itemtype 2 - Define a 10% discount for library A, category A, all itemtypes 3 - Define a 50% discount for all libraries, category A, same itemtype 4 - Attempt to checkout an item from library A of the matching itemtype 5 - The 50% discount is applied 6 - Apply patches 7 - Attempt to checkout an item from library A of the matching itemtype 8 - The 10% discount is applied prove -v t/db_dependent/Circulation.t
Created attachment 114552 [details] [review] Bug 26593: Remove _get_discount_from_rule This patch remove the private sub used in GetIssuingCharges in favor of get_effective_rule It corrects the wrong precedence for rules and adds tests to cover this subroutine NOTE: the 'branch' for the discount will be determined by the signed in branch, this is a bug to be fixed in the future To test: 1 - Define a rentalcharge for an itemtype 2 - Define a 10% discount for library A, category A, all itemtypes 3 - Define a 50% discount for all libraries, category A, same itemtype 4 - Attempt to checkout an item from library A of the matching itemtype 5 - The 50% discount is applied 6 - Apply patches 7 - Attempt to checkout an item from library A of the matching itemtype 8 - The 10% discount is applied prove -v t/db_dependent/Circulation.t Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 115013 [details] [review] Bug 26593: Remove _get_discount_from_rule This patch remove the private sub used in GetIssuingCharges in favor of get_effective_rule It corrects the wrong precedence for rules and adds tests to cover this subroutine NOTE: the 'branch' for the discount will be determined by the signed in branch, this is a bug to be fixed in the future To test: 1 - Define a rentalcharge for an itemtype 2 - Define a 10% discount for library A, category A, all itemtypes 3 - Define a 50% discount for all libraries, category A, same itemtype 4 - Attempt to checkout an item from library A of the matching itemtype 5 - The 50% discount is applied 6 - Apply patches 7 - Attempt to checkout an item from library A of the matching itemtype 8 - The 10% discount is applied prove -v t/db_dependent/Circulation.t Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
I could have sworn I'd already QA'd this a couple of months back! All works as expected.. Passing QA
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.02
Cannot replicate issue on 20.05. Discounts are being applied correctly based on library when following test plan without . Not backporting.
Reverted a different patch and retested this one. Pushed to 20.05.x for 20.05.08
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.
*** Bug 25115 has been marked as a duplicate of this bug. ***