Bug 2000 - separation of issuing & fines rules breaks defaults propagation
Summary: separation of issuing & fines rules breaks defaults propagation
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: PC All
: PATCH-Sent (DO NOT USE) blocker (vote)
Assignee: Galen Charlton
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 20227
  Show dependency treegraph
 
Reported: 2008-04-07 11:54 UTC by Chris Cormack
Modified: 2019-06-27 09:24 UTC (History)
0 users

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 00:37:03 UTC


---- Reported by rch@liblime.com 2008-04-07 11:54:02 ----

I've submitted a patch for Circulation.pm that reproduces GetLoanLength's issuingrule determination in a new fcn: GetIssuingRule().
This is intended to replace Overdues.pm's GetIssuingRules(), 
which ignores both defaults and branches.
However, this reveals a bug:
By separating the interfaces for Issuing and Fines, the following condition arises:
(1) set any branch, any borrower category, and just default fines for itemypes.
(2) set Issuing rule for specific branch, specific categorycode.

This creates an entry with null fine that overrides the intended default behavior for the fines interface.

From the state of current code, I don't imagine that any currently live/testing 3.0 installs are actually using the Fines Rules interface outside of Sanop , so I suggest a simple fix:

If fines are used, require that smart-rules.pl  is the issuingrules interface.
Else, allow use of issuingrules.pl.
and, remove finesrules.pl 

Thoughts ?



---- Additional Comments From jmf@liblime.com 2008-04-10 06:43:18 ----

Re-assigning to Galen



---- Additional Comments From joe.atzberger@liblime.com 2008-05-27 15:42:10 ----

Confirming fines2.pl did not work unless I used smart-rules.pl



---- Additional Comments From joe.atzberger@liblime.com 2008-05-30 19:45:25 ----

The question is how to determine that a system is using fines or not.  Hopefully we don't need a syspref, and can just take the full-population of any fines line as proof enough?



---- Additional Comments From rch@liblime.com 2008-05-30 22:31:30 ----


Actually, this reveals a further issue with Paul's recent patch:
http://lists.koha.org/pipermail/koha-patches/2008-May/000585.html
Here we are using '*' in the branch and categorycode columns of the issuingrules table to mean 'default', but it means 'total' for itemtypes.
But that is meaningless for fines, so a single entry in the issuingrules table then means 'total' for maxissueqty, but 'default' for fines columns.

Thus, relabelling 'Any' to 'Total' in smartrules.pl for itemtype 
(as I suggest in http://lists.koha.org/pipermail/koha-patches/2008-May/000619.html )
Will not work.
I think we need to add a 'default' for itemtype, which should be '*' in issuingrules table (i.e., don't push Paul's patch), and if we want to enforce a 'total', we need to test against another character, say '~' or '+' for a rule indicating total (only applicable to itemtype).
I think it would be easier to code that than to write the documentation to explain the differences in issuingrules.pl and smartrules.pl and how 'Any' means one thing for maxissueqty but another thing for fines settings.

If that is done, then I think it's sufficient to simply say 'use smartrules if you charge fines, if not, you can use issuingrules.pl', so there's no need to test for fines use. 



(In reply to comment #3)
> The question is how to determine that a system is using fines or not. 
> Hopefully we don't need a syspref, and can just take the full-population of any
> fines line as proof enough?
> 





---- Additional Comments From paul.poulain@biblibre.com 2008-06-05 06:44:59 ----

I think it would be better to have * for total and ~ for default.
Otherwise, I agree with the idea.

OTH, I think it's better, from far, to have only 1 method to fill issuing & fines rules.
what could be the problem/limit of using only the smartrule one ?

If I don't mind, the problem of the "old" one, is the very large array you can get if you have many itemtypes/patron categories.



---- Additional Comments From gmcharlt@gmail.com 2008-06-09 15:28:46 ----

Working on this now.  The total number of loans across all item types is really an attribute of the combination of patron categories and branches, and doesn't IMO opinion belong in issuing rules, but in a different table, e.g.,

borrower_loan_limit:
  branchcode
  categorycode
  maxloans

If this table were added (or if we absolutely must, simulated in issuingrules using a '+' itemtype), then '*' would become the default selector for branch, item type, and category.



---- Additional Comments From gmcharlt@gmail.com 2008-06-20 06:12:15 ----

Patches submitted.



---- Additional Comments From jmf@liblime.com 2008-06-20 06:57:05 ----

patch pushed, thanks Galen!



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 00:37 UTC  ---

This bug was previously known as _bug_ 2000 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=2000

Actual time not defined. Setting to 0.0
The original reporter of this bug does not have
   an account here. Reassigning to the person who moved
   it here: chris@bigballofwax.co.nz.
   Previous reporter was rch@liblime.com.