Commit graph

336 commits

Author SHA1 Message Date
80897930b7 Bug 2720 - Overdues which debar automatically should undebar automatically when returned
This patch adds a more extensible and flexible debarments system to Koha. The fields
borrowers.debarred and borrowers.debarredcomment are retained for compatibility and
speed.

This system supports having debarments for multiple reasons. There are currently
three types of debarments:
OVERDUES    - Generated by overdue_notices.pl if the notice should debar a patron
SUSPENSION  - A punative debarment generated on checkin via an issuing rule
MANUAL      - A debarment created manually by a librarian

OVERDUE debarments are cleared automatically when all overdue items have been returned,
if the new system preference AutoRemoveOverduesRestrictions is enabled. It is disabled
by default to retain current default functionality.

Whenever a borrowers debarments are modified, the system updates the borrowers debarment
fields with the highest expiration from all the borrowers debarments, and concatenates
the comments from the debarments together.

Test plan:
 1) Apply patch
 2) Run updatedatabase.pl
 3) Verify the borrower_debarments table has been created and
    populated with the pre-existing debarments
 4) Run t/db_dependent/Borrower_Debarments.t
 5) Manually debar a patron, with an expiration date
 6) Verify the patron cannot be issued to
 7) Add another manual debarment with a different expiration date
 8) Verify the 'restricted' message lists the date farthest into the future
 9) Add another manual debarment with no expiration date
10) Verify the borrower is now debarred indefinitely
11) Delete the indefinite debarment
12) Verify the debarment message lists an expiration date dagain
13) Enable the new system preference AutoRemoveOverduesRestrictions
14) Set an overdue notice to debar after 1 day of being overdue
15) Check out an item to a patron and backdate the due date to yesterday
16) Run overdue_notices.pl
17) Verify the OVERDUES debarment was created
18) Return the item
19) Verify the OVERDUES debarment was removed
20) Disable AutoRemoveOverduesRestrictions
21) Repeat steps 15 though 18, verify the OVERDUES debarment was *not* removed
22) Add issuing rules so that an overdue item causes a temporary debarment
23) Check out an item to a patron and backdate the due date by a year
24) Return the item
25) Verify the SUSPENSION debarment was added to the patron

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-10-31 01:53:22 +00:00
Galen Charlton
b67654ecc2 Bug 10493: (follow-up) fix test errors
[1] Fix regression on bug 10663

Looks like the regression was introduced by a glitch during rebasing.

[2] Fix errors in Circulation_issue.t

The change in AddRenewal() turned up an issue with how the test
script issued one of the test items.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-10-23 04:25:34 +00:00
2e4411e77f Bug 10493: Add renewal script
This patch adds a renewal tool that functions similar to the returns where a
librarian can continuously scan items for renewal. This script blocks
renewals that are impossible, and allow the same renewal overrides
as circulation.pl

Test plan:
 1) Apply the patches for bug 8798
 2) Apply this patch
 3) Browse to /cgi-bin/koha/circ/renew.pl
 4) Enter an invalid barcode, you should get an error message
 5) Enter a valid, but not checked out barcode, you should get an error
    message.
 6) Enter a valid barcode that is checkout out and should be renewable,
    you should get a success message.
 7) Enable AllowRenewalLimitOverride
 8) Enter a barcode for an item that has been renewed too many times
 9) You should get a warning which you can override.
10) Disable AllowRenewalLimitOverride
11) Repeat steap 8
12) You should get a blocking error message
11) Enter a barcode for an item with unfilled holds on it,
    you should get an overridable warning

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Passes all tests and QA script, some issues have been
addressed in follow-ups.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-10-22 17:04:03 +00:00
Kenza Zaki
b8dbff0283 Bug 10795: Improvements for GetOpenIssue in C4::Circulation
This patch adds some improvements for the routine GetOpenIssue().
Now, it verifies if the parameter is given (if not it returns undef)
and it returns $sth->fetchrow_hashref() instead of a $issue.

To test:
prove t/db_dependent/Circulation_issue.t
t/db_dependent/Circulation_issue.t .. ok
All tests successful.
Files=1, Tests=16,  2 wallclock secs ( 0.06 usr  0.01 sys +  1.09 cusr  0.07 csys =  1.23 CPU)
Result: PASS

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Comment: Same situation as the one noted in comment of
Bug 10683, test fails unless there is an issuingrule
All, All with 1 as renewals allowed.

With that condition, it succeeds

No koha-qa errors

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-10-09 03:54:51 +00:00
Kenza Zaki
924c4f69e0 Bug 10698: give C4::Circulation::DeleteTransfer() a return value
This patch adds return values to DeleteTransfer:
Undef if no parameters are given
1 if a Transfer is deleted
0E0 if a wrong parameter is given

It also fixes some unit tests in t/db_dependent/Circulation_transfers.t

To test:
prove t/db_dependent/Circulation_transfers.t
t/db_dependent/Circulation_transfers.t .. ok
All tests successful.
Files=1, Tests=14, 20 wallclock secs ( 0.03 usr  0.00 sys +  0.39 cusr  0.02 csys =  0.44 CPU)
Result: PASS

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment: Applied 10681 and 10692 before 10698
Run prove t/db_dependent/Circulation_transfers.t without errors
No koha-qa errors on all 3 patches

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-10-09 03:46:53 +00:00
Bernardo Gonzalez Kriegel
24984fcb7b Bug 10711: (follow-up) removing tabs
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-10-09 03:43:42 +00:00
Kenza Zaki
c93e5b12f8 Bug 10711: make GetOfflineOperation() return $sth->fetchrow_hashref instead of $result
To test :
prove t/db_dependent/Circulation_OfflineOperation.t
t/db_dependent/Circulation_OfflineOperation.t .. ok
All tests successful.
Files=1, Tests=7, 19 wallclock secs ( 0.02 usr  0.01 sys +  0.24 cusr  0.02 csys =  0.29 CPU)
Result: PASS

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment: applied 10681 and 10710 before 10711
prove t/db_dependent/Circulation_OfflineOperation.t run without errors
koha-qa reports only 2 tabulations, fixed in a follow-up

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-10-09 03:43:03 +00:00
Kenza Zaki
e738f991b0 Bug 10681: remove inappropriate uses of finish() from C4::Circulation
This patch gets rid of finish().

From the man page

finish()
Indicate that no more data will be fetched from this statement handle
before it is either executed again or destroyed.
You almost certainly do not need to call this method.

Adding calls to "finish" after loop that fetches all rows is a common
mistake, don't do it, it can mask genuine problems like uncaught fetch errors.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-10-09 03:40:36 +00:00
ded520afdc Bug 9761: Make it possible to confirm future hold requests at checkin time
Description:

A new pref ConfirmFutureHolds is added. When confirming a hold at checkin time,
the number of days in this pref is taken into account when looking for reserves.
Note that this pref does not interfere with renewing, issuing or transferring
a book. For report Holds to pull, the default end date is calculated with this
new preference.
The use of ConfirmFutureHolds is useful only when future holds are allowed.

Test plan:
1) Enable future holds. Add a number of days into ConfirmFutureHolds.
2) Place a future hold within this number of days.
3) Run holds to pull report. Check default startdate and enddate.
4) Check this book in. Can you confirm the hold? Do not confirm.
5) Issue the book to another patron. You should not see a warning.
6) Renew the book for this patron via opac or staff. No warning either.
7) Check in again. Warning pops up again.
8) Transfer book. Switch branch. Check in. Hold found pops up. Do not confirm.
9) Back to first branch. Check in (with popup). Remove the hold. Add new future
hold past the number of days. Check in (no warn).

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-09-25 00:26:34 +00:00
Jonathan Druart
5ed940b18e Bug 10599: add option to unselect on loan items on the batch item modification tool
Sometimes when using the batch item modification tool, we would like to
automatically uncheck on loan items.

This patch also adds a new routine in C4::Circulation, IsItemIssued(),
which, when passed an itemnumber, returns whether the item is
currently on loan.

Test plan:
1/ Go to tools/batchMod.pl.
2/ Enter some barcode (at least 1 should be on loan).
3/ Click on the Continue button.
4/ Click on the "Clear on loan" link.
5/ Check that on loan items are unchecked.

Launch the unit test file:
prove t/db_dependent/Circulation/IsItemIssued.t

http://bugs.koha-community.org/show_bug.cgi?id=10572
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Works as expected, only modifies items that are checked (still). No regression noted.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Passes koha-qa.pl, works as advertised.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-09-18 16:45:24 +00:00
Paul Poulain
013726fda2 Bug 7639: tiny QA follow-up
no need to create a variable just to call to send it as parameter the line after

Signed off by: Alex Hatley <alexh@cctexas.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-09-17 15:21:23 +00:00
Meenakshi.R
0005f6bad3 Bug 7639: add option to forgive overdue fines when declaring items lost
This patch adds functionality to forgive overdue fine when an item is
set to lost status.  Fines are forgiven only when the syspref
WhenLostForgiveFine is set to yes. Item can be set to lost status from:

- catalogue/moredetail.pl
- cataloguing/additem.pl
- tools/batchMod.pl
- misc/cronjobs/longoverdue.pl

Changed subroutine C4::Circulation::LostItem to forgive fines on the
item depending on the value of syspref WhenLostForgiveFine. This
routine is currently used to return an item and charge a replacement
cost.

Also added a new syspref in C4::Circulation::LostItem -
WhenLostChargeReplacementFee.  The replacement fee will now be charged
only if this syspref is set to yes.  The default value of the
WhenLostChargeReplacementFee is yes, meaning that current behavior
will not change during upgrade.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Alex Hatley <alexh@cctexas.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Testing notes on last patch in series.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-09-17 15:10:46 +00:00
dbaefb626c Bug 10550: Fix database typo wthdrawn
This patch updates the wthdrawn field in items and deleteditems to be
withdrawn instead. No functional changes are made.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Comment: Save for translation files (that will be fixed on next release),
only occurrence of wthdrawn is on updatedatabase.pl
No koha-qa errors.

This touch many files, and I did not test everything,
but all seems normal. I think that any problem could
be fixed later.

Perhaps both entries in updatedatabase.pl could be joined
into one, but thats for QA.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-09-08 01:58:41 +00:00
Galen Charlton
ac11a5d9ff Bug 10699: (follow-up) fix parameter test in DeleteBranchTransferLimits()
Need to check for definedness, not Perl truth.

Also adds description of the return value to the POD.

To test:

Run prove -v t/db_dependent/Circulation_transfers.t and verify that
the tests pass.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-08-28 15:34:47 +00:00
Kenza Zaki
88f15db30a Bug 10699: add explicit return value for DeleteBranchTransferLimits()
This patch adds return values to DeleteBranchTransferLimits:
1 if a Transfer Limit is deleted
undef if no parameters is given
0E0 if a wrong parameter is given

More, it fixes and adds some tests in t/db_dependent/Circulation_transfers.t

To test :
prove t/db_dependent/Circulation_transfers.t
t/db_dependent/Circulation_transfers.t .. ok
All tests successful.
Files=1, Tests=14, 19 wallclock secs ( 0.02 usr  0.01 sys +  0.39 cusr  0.02 csys =  0.44 CPU)
Result: PASS

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Tested with patch for bug 10692 applied.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-08-28 15:23:51 +00:00
Galen Charlton
6751c5dc7c Bug 10693: (follow-up) fix parameter checking in CreateBranchTransferLimit()
There is nothing prevent '0' from being used as a library code.

To test:

Run prove -v t/db_dependent/Circulation_transfers.t and verify that
the tests pass.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-08-28 15:16:28 +00:00
Kenza Zaki
3c61f95fb4 Bug 10693: CreateBranchTransferLimit's return value in C4::Circulation.pm should be more explicit
This patch test if the parameters $toBranch and $fromBranch are given.
If not, CreateBranchTransferLimit now returns undef.
This patch also fixes and adds some regression tests in
t/db_dependent/Circulation_transfers.t

NOTE:
Currently, we can add a transferlimit to nonexistent branches because
in the database branch_transfer_limits.toBranch
and branch_transfer_limits.fromBranch aren't foreign keys.

To test:
prove t/db_dependent/Circulation_transfers.t
t/db_dependent/Circulation_transfers.t .. ok
All tests successful.
Files=1, Tests=15, 18 wallclock secs ( 0.02 usr  0.01 sys +  0.42 cusr  0.00 csys =  0.45 CPU)
Result: PASS

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests and QA script pass.
2013-08-28 15:15:10 +00:00
50ff4f4a94 Bug 10081: add library name to IndependentBranches error message
With IndependentBranches turned on, if you try to check out an item
which belongs to another library you will get an error message which is
missing the library name. This patch corrects the problem by passing the
necessary variable to the template and outputting the library name using
the KohaBranchName TT plugin.

To test, turn on IndependentBranches and try to check out an item
belonging to another library (note that you must test with a staff user
who is not a superlibrarian). The error message you see should include
the name of the library to which the item belongs:

"This item belongs to Nelsonville and cannot be checked out from this
location."

Checkouts of items belonging to the library should be unaffected.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-08-20 14:26:25 +00:00
3e387f72a3 Bug 10663: restore ability of active hold requests to block renewal
Sponsored-by: Universidad Nacional de Cordoba
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
This reverts changes made to CanBookBeRenewed by
patches from bug 9367.
GetReserveStatus is not suitable to recognize if an item
can fild a hold on return and CheckReserves is restored.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

This patch includes a squash of a follow-up authored by
Katrin Fischer <Katrin.Fischer.83@web.de>:

  CheckReserves returns '' when no reserve is found,
  so $resfound will always be defined and we need to
  check if it's true.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-08-15 22:31:52 +00:00
Fridolyn SOMERS
ff7f372029 Bug 9362: fix query that finds upcoming due loans
C4:Circulation:GetUpcomingDueIssues is used in the advance_notices.pl
script.  This patch corrects an error in its handling of the maxdays
parameter that resulted in it picking up *all* upcoming due loans and
recently overdue loans.

Test plan :
- Create an issue with a date due in the paste
- Create an issue with a date due in two days
- Launch advance notices with due date in max 2 days : perl misc/cronjobs/advance_notices.pl -c -n -v -m=2
=> You get a warn "found 0 issues"
- Launch advance notices with due date in max 3 days : perl misc/cronjobs/advance_notices.pl -c -n -v -m=3
=> You get a warn "found 1 issues"

Signed-off-by: Mathieu Saby <mathieu.saby@univ-rennes2.fr>
I did the following test :

- 1 book to check in 2 days
- 2 books to check in in the past

before applying the patch :

$perl ../misc/cronjobs/advance_notices.pl -c -n -v -m=2
getting upcoming due issues at ../misc/cronjobs/advance_notices.pl line 203.
found 1 issues at ../misc/cronjobs/advance_notices.pl line 205.

I changed the value of "-m" : 0, 1, 2, 3, 4
=> always 1 issue found (the book to check in in 2 days)

after applying the patch :

$perl ../misc/cronjobs/advance_notices.pl -c -n -v -m=2
found 0 issues
for m = 0, 1, 2 => 0 issues

$perl ../misc/cronjobs/advance_notices.pl -c -n -v -m=3
found 1 issues
for m = 3,4,5 => 1 issues (the book to check in in 2 days)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Passes koha-qa.pl, works as advertised.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-07-17 15:45:47 +00:00
835f745b55 Bug 9576: can now control whether max loans limit can be overriden
Adds a new system preference AllowTooManyOverride to control whether
a librarian can override the 'Too many checked out' message which is
currently always overridable.

Test Plan:
1) Apply patch
2) Run updatedatabase.pl
3) Attempt to check out 1 more item to a patron than the max issues
4) You should be allowed to override by default ( current behavior )
5) Set AllowTooManyOverride to "Don't allow"
6) Repeat step 3
7) You should be blocked from being able to issue the item

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>

The new system preference is activated by default, which mean there
will be no change in behaviour on update.

The system preference is correctly added to the database and .pref
files.

Test plan and QA script passes.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-07-17 15:07:42 +00:00
040eb4016f Bug 10262 - fine calculation at checkin not respecting CircControl
The fines.pl script uses the system preference CircControl to decide
what branches circ rules to use for fine generation.

Recently, code was added to the returns system to recalculate the fine
at checkin time ( to support hourly loans ). The problem is that this
code does not respect CircControl.

Test Plan:
1) Set circ control to "the library you are logged in at"
2) Set different fines rules for two different librarys
3) Check an item out at library A, backdate the due date so it's overdue
   and will have fines.
4) Check the item in at library B
5) Observe that the fines should be generated based on library A's rules,
   but the fines will be based on library B's rules instead!
5) Apply the patch
6) Repeat steps 3 and 4.
7) Observe now that the fines should reflect the fines rules for Library A

Note: it seems counter-intuitive for the fines system to behave this way
based on the preference being set to "the library you are logged in at"
but it does make sense. The rules used are from "the library you are
logged in at" when the item is first checked out.

If the fines system really did use the rules for the library the item was
returned to, it would be easy to exploit the library system. Some Koha
using systems have branches that charge fines, and others that don't, so
a patron could just return any overdue items to a non-charging branch
to avoid ever paying fines!

Furthermore, it would mean that the fines.pl script would be using one
set of rules to charge fines, and the returns system could possibly be
using another. Since fines.pl has been around far longer, it makes sense
to assume the fines.pl behavior is canonical.

Signed-off-by: Mickey Coalwell <mcoalwell@nekls.org>
Signed-off-by: George Williams <georgew@latahlibrary.org>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>

Merged with reservations; see comment on bug report for details.
2013-05-28 08:31:34 -07:00
fc61bd3018 Bug 10120: Adding a syspref to control if overdue charges are updated on return
On by default.

To Test

1/ Create an overdue item, that should get fines
2/ Return the item
3/ Check the borrowers record to see if the fine has been added/updated

Apply patch
1/ Make sure preference is set to do
Repeat steps 1-3 above

2/ Switch the preference to don't
Repeat stes 1-2
3/ Check the fine hasn't been added/updated

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests and QA script pass, works as described.
I would categorize this a bug fix for libraries that don't want
the new changed  behaviour that was introduced by recalculating
fines on return.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-05-24 08:47:12 -07:00
b00ec06968 Bug 10080 - Change system pref IndependantBranches to IndependentBranches
Test Plan:
1) Enable IndependantBranches
2) Apply this patch
3) Run updatedatabase.pl
4) Verify that the system preference still functions correctly

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-05-22 07:58:23 -07:00
Peter Crellan Kelly
571dab9c7c Bug 6506: When AnonymousPatron not set, deletion of issue history silently failed.
Remedied by:
- in Circulation.pm changing AnonymiseIssueHistory so that it returns ($rows, $err_history_not_deleted) instead of $rows
- consequential change to misc/cronjobs/batch_anonymise.pl to handle updated return value, and fail if there is an error
- consequential change to tools/cleanborrowers.pl although this still fails silently (raised as bug 9944)
- update of opac-privacy.pl to check return value and pass on error
- update of opac-privacy.tt to display error if appropriate

Note bug 9942 remains unfixed, which is a similar issue upon issue return.

To test:
1. OPAC
- enable privacy mode (preference OpacPrivacy)
- leave anonymous patron set to zero (preference AnonymousPatron)
- attempt to delete user history
- observe error
- check history - still there
- change anonymous patron to a valid user
- attempt to delete user history
- observe success message
- check history - gone

2. cleanborrowers.pl
- test it functions as before.  bug 9944 has been raised for it continuing to silently fail.

3. batch_anonymise.pl
- enable privacy mode (preference OpacPrivacy)
- leave anonymous patron set to zero (preference AnonymousPatron)
- run script (I use --days -1 for testing)
- script should fail with a Carp message
- change anonymous patron to a valid user
- run script as before
- script returns quietly
- check history - gone

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2013-05-01 08:44:11 -04:00
93f6053694 Bug 8770 - amount does not equal amountoutstanding on a fine with no payments or credits
If a lost item fee is owed and partially paid off when an item is
returned and a refund is processed, Koha tries to pay off existing
fees before adding any leftover balance as a credit on the account.

However, those fee payments aren't actually processed, due to a bug
where the accountnumber was quoted as a string literal, and thus the
for the fee payment would fail. This did not result in an DB error,
as the query was still valid SQL. Checking the return value of the
query would have revealed that the accountline had not been updated.

History:
This bug was introduced on April 23, 2007 with the commit
'reintroducing fixaccountforlostandreturned as requested by rosalie'.
Commit id 111d590e9c

On July 30, 2009 the error was flagged with a FIXME and remained
in that state until now.
Commit id 51e8fc2cb6

Test plan:
1) Create a test patron
2) Check out an item to that patron and give it a due date in the past
3) Run fines.pl to generate the fine for the item
4) Mark the item long overdue on the item tab (not in edit items)
5) Pay for the lost item
6) Check the item back in
7) Note the incorrect fines. Only the lost fee balance  due is refunded,
   not the entire lost fee, but no other fees are paid off.
8) Apply the patch
9) Repeat steps 1-6, then not the fines are paid correctly

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Totel due is correct after applying the patch.
All tests and QA script pass.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-04-07 19:56:03 -07:00
Jonathan Druart
5b4752e027 Bug 8365: FIX Return issue values instead of item values
Before this patch, the GetItemIssue routine returns items.renewals
instead of issues.renewals

Signed-off-by: Broust <jean-manuel.broust@univ-lyon2.fr>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests and QA script pass.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2013-03-22 07:57:02 -04:00
Jonathan Druart
574e2f9d60 Bug 8365: Followup Force DateTime type for the datedue variable
Signed-off-by: Broust <jean-manuel.broust@univ-lyon2.fr>
I tested one more time on a sandbox but I confirm the problem:
the due date doesn't change when you renew more than one time
with syspref renewalperiodbase turned on: "the old due date of
the checkout". The due date should change any time.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Added signed-off line and problem description from bugzilla.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2013-03-22 07:57:02 -04:00
Jonathan Druart
f35dbbf42e Bug 8365: Add unit tests and fix QA issues
This patch adds some unit tests for CalcDateDue and GetLoanLength

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests and QA script pass.

Tests done:
- Checked update works correctly for existing circulation rules.
- Adding, deleting and overwriting circulation rules works.
- Renewals work for different circulation rules and changes
  to the holiday calendar.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2013-03-22 07:57:02 -04:00
Jonathan Druart
8ad00bc02f Bug 8365: Add a renewal duration in the issuing rules
Renew an issue for a number of days (filled in the issuing rules).

Test if rules work for any i[item]types and if there is no regression.

- new column issuingrules.renewalperiod
- remove all occurrences of an already removed syspref (globalDueDate)
- remove an unused routine (Overdues::GetIssuingRules)

How it works:
- On existing installations, the issuingrules.renewalperiod =
  issuingrules.loanlength. So the behaviour is the same before and after
  this patch.
- when you add a rule, you can choose a renewal period (the unit value
  is the issuingrules.unit). So you can have a renewal period in hours
  or days.
- The default value for the renewal period is 21 days (same as
  loanlength)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Test comments on second patch.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2013-03-22 07:56:59 -04:00
7224e47dfe Bug 8220 - Allow koc uploads to go to process queue instead of being applied directly.
The primary advantage to the Firefox offline cirulation plugin when compared
to the offline circulation desktop application, is the ability to add offline
circulation actions to a queue so that multiple machines running offline
circ can have their circ actions combined and ordered chronologically before
being executed. This commit adds the ability to put actions from uploaded
KOC files into this queue. In this way, both the FF plugina and the desktop
application can be run side by side with no ill effects.

Signed-off-by: Bob Birchall <bob@calyx.net.au>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2013-03-21 20:35:37 -04:00
Adrien Saurat
7ceb9988e9 Bug 7241: circulation action logs record biblionumber instead of item number
Issue log and and circulation logs are incorrectly recording biblionumber instead of item number, creating incorrect logs.
This patch corrects this, and modifies the action_logs table in two ways:
- the timestamp is not updated anymore with UPDATES (but still automatically filled when INSERTing)
- to partially correct the broken logs, the log history is filled with the first found item for each biblio

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2013-03-21 11:42:56 -04:00
Fridolyn SOMERS
6cec57901a Bug 9423: Add notforloan value to issue confirmation or blocking message
When trying to checkout an item that is not for loan (due to
items.notforloan value or because itemtype is not for loan), you get a
confirmation or blocking message (depending on AllowNotForLoanOverride
syspref).

This message tells "Item is normally not for loan".

This patch adds some informations :
if itemtype is not for loan, tells "Item type is normally not for loan".
if item is not for loan due to items.notforloan value, adds notforloan
description to message.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Works as advertised: Not for loan authorized value is correctly shown.

Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2013-03-19 22:22:23 -04:00
Jonathan Druart
37b6ac0b14 Bug 9367: Followup: Code optimization: CheckReserves is too often called
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2013-03-16 11:49:43 -04:00
Jonathan Druart
fad1f44d42 Bug 9367: Code optimization: CheckReserves is too often called
This patch rewrites the GetReserveStatus routine in order to take in
parameter the itemnumber and/or the biblionumber.

In some places, the C4::Reserves::CheckReserves routine is called when
we just want to get the status of the reserve. In these cases, the
C4::Reserves::GetReserveStatus is now called.
This routine executes 1 sql query (or 2 max).

Test plan:
Check that there is no regression on the different pages where reserves
are used. The different status will be the same than before applying
this patch.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2013-03-16 11:49:43 -04:00
Fridolyn SOMERS
755decfd89 Bug 9392: confirmation does not appear for negative notforloan items issues
When checking-out an item marked "not for loan" with a positive value,
you get a message to forbid or confirm issue (depending on
AllowNotForLoanOverride syspref).  For an item with a negative value
(usually during acquisition process), the issue is made without forbid
or confirm message.

This patch simply changes the test notforloan > 0 into notforloan != 0.

Test plan :
- Checkout an item with items.notforloan = 0
=> The item is issued without message
- Checkout an item with items.notforloan > 0
=> A message forbids or asks confirmation to override
- Checkout an item with items.notforloan < 0 (e.g., on order)
=> A message forbids or asks confirmation to override

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Checked with AllowNotForLoanOverride on and off.
All tests and QA script pass.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2013-02-08 21:38:49 -05:00
Srdjan
95077493bd Bug 7243: Rewording and renaming
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Passed-QA-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2013-01-24 09:17:12 -05:00
Srdjan
1b4a266915 Bug 7243: Be selective when summing up charges for blocking issues
Added RentalsInNoissueCharges and ManInvlsInNoissueCharges sys prefs

Created C4::Members::cwGetMemberAccountBallance()
* A wrapper for GetMemberAccountRecords that gives info on non-issue and
  other charges
* Other charges are:
  'Res'
  'Rent' if RentalsInNoissueCharges is Mo
  authorised_values MANUAL_INV if ManInvlsInNoissueCharges is No

C4::Members::GetMemberAccountRecords() changes:
* Dropped input param $date, it is not used

Use split charges in C4::Circulation::CanBookBeIssued() and
C4::Members::patronflags(). That way only fines decide whether an item
can be issued, and not other non-fine charges

Signed-off-by: Marc Veron <veron@veron.ch>

Rebased (updatedatabase.pl)

ManInvInNoissueCharges and RentalsInNoissueCharges ar both included by default (= behaviour as before)

All variants tested: Both included, none included, manual invoice included, rentals included.
Works fine, blocks/does not blok as appropirate, messages appear as expected.

[Oct 12, 2012 marcelr:] Amended for updatedatabase.pl
Signed-off-by: M. de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2013-01-24 09:12:59 -05:00
Jonathan Druart
7fc4be502d Bug 9206: Followup QA FIX
- Fix indentation
- It is useless to loop on all items. If at least 1 item is already
  issued, we want to return 1.
2012-12-27 09:48:03 -05:00
e3f6a74660 Bug 9206 - Only allow place holds in records that the patron don't have in his possession - QA Followup
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
2012-12-27 09:48:03 -05:00
Vitor FERNANDES
63f2ceb603 Bug 9206 - Only allow place holds in records that the patron don't have in his possession
Added a system preference to turn on/off this feature.
By default the system allow the patron to place holds even if it is in his possession.
Script to place holds check if the system preference is off and if patron has at least one item to block holds.
Messages to say that are already in patron possession added to templates.
Method to check if patron has one issue from one record added to C4::Circulation

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2012-12-27 09:48:03 -05:00
Vitor FERNANDES
8048ba3201 BUG 9151 - Renewal notice according to patron alert preferences (Patch 2)
Added a system preferences (RenewalSendNotice - default no) to controll the renewal notice sending.
Added to installer languages the renewal notice template.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2012-12-21 07:41:25 -05:00
Vitor FERNANDES
a478c52e09 BUG 9151 - Renewal notice according to patron alert preferences
Modifications on AddRenewals and SendCirculationAlert in order to send a renewal notice to the patron according to the patron checkout alert preference.
Added a new notice template for renewals notices.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2012-12-21 07:41:24 -05:00
ca11d0ba29 Bug 7189 - preference to control if returning lost items gives refund
Right now when you return an item that was lost the patron's card is
credited with the lost fee, but not all libraries refund lost fees
and sometimes the fee is refunded after the patron has paid for it,
causing all kinds of financial issues.

Adds the syspref RefundLostItemFeeOnReturn to control whether
returning a lost item refunds the fee charged for losing that
item. Enabled by default to maintain Koha's current functionality.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Passed-QA-by: Paul Poulain <paul.poulain@biblibre.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2012-11-30 08:01:41 -05:00
7b18e2146c Bug 8800 - useDaysMode=Datedue wrong behaviour (revisited)
useDaysMode=Datedue wasn't used as advertised in the docs. Added
next_open_day and prev_open_day subs to Koha::Calendar and some tests for them.

- Koha::Calendar->addDate was rewritten in a more sane way (also split into
  addHours and addDays for convenience).
- Fixed a bug introduced in Bug 8966 regarding dt truncation and dtSets->contains
- Minor docs typos
- Use the passed Calendar mode or default to 'Calendar' in Koha::Calendar->_mockinit.
- Tests

I'm writing some db-dependent tests for is_holiday, and hopefully for CalcDateDue so any
rewrite/followup doesn't break things.

Regards
To+

Sponsored-by: Universidad Nacional de Córdoba
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passed-QA-by: Paul Poulain <paul.poulain@biblibre.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2012-11-22 09:11:19 -05:00
Mark Tompsett
c66270c02f Bug 8834 - Call to C4::Context::preference fails to pass $self in Circulation.pm
Changed:
    my $circcontrol = C4::Context::preference('CircControl');
To:
    my $circcontrol = C4::Context->preference('CircControl');
This will pass $self and 'CircControl' which will then prevent
a "Use of uninitialized value" error log entry.

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
2012-10-24 18:21:23 +02:00
Colin Campbell
1c4125cd0e Bug 8841 Duedate should be set to end of expiry day
When reducing duedate to patron's expiry date
it should be set to end of that day not 00:00
take account of the fact that the borrower
expiry date will have no hours/minutes

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
2012-10-24 18:17:06 +02:00
Paul Poulain
f58a6f4e10 Bug 3387 follow-up: fix perlcritic error 2012-10-01 17:35:45 +02:00
7a7e018ed3 Bug 3387 - WITHDRAWN items shouldn't check in
Adds a system preference BlockReturnOfWithdrawnItems to control
whether or not Koha should allow a withdrawn item to be returned
or not.

Also fixes the behavior where Koha will attempt to use a withdrawn
item to fill a hold.

Test Plan:
1) Set BlockReturnOfWithdrawnItems to "Block" ( this is the default )
2) Check an item out to a patron
3) Mark item as withdrawn
4) Attempt to return the item, you should still see the item on the
   borrower's record
5) Set BlockReturnOfWithdrawnItems to "Don't Block"
6) Attempt to return the item, you should see the item is no longer
   on the borrower's record.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
2012-10-01 17:07:53 +02:00
5da1298ac1 Bug 6679 - [SIGNED-OFF] fix 8 perlcritic violations in C4/Circulation.pm
- "return" statement with explicit "undef" at line 994, column 39.  See page 199 of PBP.  (Severity: 5)

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
2012-09-20 12:01:35 +02:00