[Koha-bugs] [Bug 4530] Circulation & fines rules aren't always applied from most specific to less specific

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=4530

Paul Poulain <paul.poulain at biblibre.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|PATCH-Sent (DO NOT USE)     |P2
             Status|Signed Off                  |Failed QA
                 CC|                            |francois.charbonnier at biblib
                   |                            |re.com,
                   |                            |gaetan.boisson at biblibre.com
                   |                            |, paul.poulain at biblibre.com

--- Comment #21 from Paul Poulain <paul.poulain at biblibre.com> 2012-02-17 17:18:48 UTC ---
QA comments/questions

=== point 1 (and a question) ===
(In reply to comment #3)
> More comprehensive description of rule selection:
> * Rules are chosen in order of preference borrower category, item type,
> branch
> * If value for certain rule is null, next rule in order is chosen
> * 0 is treated as null

does it mean that, if I set the general rule to 7 documents for 21 days, and
set a rule for 
CHILDREN / DVD to 0 it will be treated as NULL and the general rule will be
applied ?
If yes, it's a big change in the behaviour, and, imo, a big mistake = libraries
want to define such rules.
0 should mean 0. I agree we should handle "empty" as "rule unused at this
level"

=== point 2 ===
If I understand well your code, the GetIssuingRules relies on
            ." order by branchcode desc, categorycode desc, itemtype desc";
and the rule returned is the result ordered 1st in this query, right.
Reading what's written on smart-rules.pl:
    same library, same patron type, same item type
    same library, same patron type, all item types
    same library, all patron types, same item type
    same library, all patron types, all item types
    all libraries, same patron type, same item type
    all libraries, same patron type, all item types
    all libraries, all patron types, same item type
    all libraries, all patron types, all item types

it's consistent with the order.

=== point 3 ===
In CanItemBeReserved, you removed the SQL that was
-                               categorycode DESC, 
-                               itemtype     DESC, 
-                               branchcode   DESC;"
in favor of GetIssuingRuleRecord, that is 
            ." order by branchcode desc, categorycode desc, itemtype desc";

It mean that the behaviour will change depending on how you have setup your
rules, isn't it ?
I don't say I disagree, I say it must be clearly stated/advertised, and this
patch should not be pushed in 3.6.
(however, I think it's more consistent, so it's good)

=== point 4 ===
small point : SQL must have reserved words in capital
so 
            ." ORDER BY branchcode DESC, categorycode DESC, itemtype DESC";
and not 
            ." order by branchcode desc, categorycode desc, itemtype desc";

=== point 5 ===
This patch also change the fines/hold/renewal behaviour, isn't it ? More tests
must be done in this area

Conclusion:
* marking failed QA for point 4 and until point 1 is addressed. I'll also ask
in // for one of BibLibre librarians to investigate this bug/patch

-- 
Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.
You are watching all bug changes.