This patch clarifies the logic for determining if a given item to be
checked out would be the second (or third, etc.) loan on the same bib.
As a conseqence, if the item is already on loan to the patron, the
circ staffer won't see the multiple-loans-on-a-bib warning, just
the confirmation to renew the loan or the warning that no more
renewals are lest.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This patch adds a new system preference, AllowMultipleIssuesOnABiblio.
If this system preference is OFF, an alert is raised if a patron
tries to check out an item even when they already have a different
item checked out from that bib.
The librarian can force the checkout anyway.
It doesn't alert the librarian if the biblio is a subscription
Test plan:
1. Create a biblio with at least 2 items
2. Checkout the first item for a borrower
3. Set syspref AllowMultipleIssuesOnABiblio to OFF.
4. Try to checkout the second item with the same borrower. A message
should appear telling you that this borrower already borrowed an item
from this biblio.
If you have the permission 'force_checkout' You should also see two
buttons to confirm (or not) the checkout
5. Click on 'No'. The checkout is not done
6. Repeat step 4 and click 'Yes', the checkout is done.
7. Return the second item.
8. Set syspref AllowMultipleIssuesOnABiblio to ON
9. Try to checkout the second item with the same borrower. This time
the checkout is done without warnings.
Followed test plan. Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests and QA script pass, works well. Tested:
* Permission to override
* check out a second item from a record with subscriptions works
* check out a second item from a 'normal' record is warned about,
but can be done
* No permission to override
* subscription item: can be checked out
* normal item: can't be checked out
* Feature turned off
* Check out never warns/blocks
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This patch tightens up the initialization of the $restriction_age
variable and uses $bibvalues throughout.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This patch makes the parsing of AgeRestrictionMarker values consider
the case where the marker is immediately followed by the age, e.g.
"K16" in Finland.
How I tested:
[1] Configure Age Restricition (see Syspref AgeRestrictionMarker) and
have a biblio record with e.g. PEGI 99 in age restriction field.
[2] Try to check out to a patron with age < 99
[3] Check out should be blocked
[4] Change entry in age restriction field to PEGI99
[5] Checkout should be possible
[6] Apply patch
[7] Checkout schould now be blocked
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
C4::Circulation::CanBookBeRenewed called C4::Members::GetMemberDetails to
retrieve categorycode and branchcode.
- categorycode is used to retrieve the issuing rule
- the borrower information is passed to
C4::Circulation::_GetCircControlBranch. Which only uses the branchcode
parameter.
GetMemberDetails does a lot of calls/queries (patronflags,
account, etc.) that are not needed by CanBookBeRenewed.
This patch replaces it with a call to C4::Members::GetMember.
Note: I presented this small optimisation during a quick introduction to
NYTProf (hackfest 14 in Marseille).
Test plan:
- launch member unit tests
- check the code
Checking the code resulted in the following:
CanBookBeRenewed builds a hash reference from the borrowernumber
(2482). Note it is only used in this function and not passed in.
_GetCircControlBranch (2485) requires that hashreference to
have a branchcode key. As stated above.
The following line (2486) requires it have a categorycode key.
As such, C4::Members::GetMemberDetails is confirmed to be
overkill, and C4::Members::GetMember is sufficient.
Testing Done
------------
0) Back up DB
1) Make sure MPL is in the list of libraries.
2) Apply the patch.
3) run the koha qa test tool
4) prove -v t/db_dependent/Circulation.t
Patch applies cleanly. QA Test tool was all OK. All tests ran successfully.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This patch modifies GetSoonestRenewDate() so that it returns
undef if the patron, item, or loan cannot be found. This
better reflects the usage of this routine GetSoonestRenewDate(),
as none of its callers tried to check the second return
value containing an error code.
This patch also updates the POD to match.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This patch modifies CanBookBeRenewed, so that based on
issuingrules.norenewalbefore a new error "too_soon" can be returned.
Also adds a new subroutine GetSoonestRenewDate.
To test:
1) Create an issuing rule with "No renewal before" set to value X
and "Unit" set to days.
2) Test the following steps for both:
Home > Patron > Patron details
Home > Circulation > Checkouts
3) On the checkout page, test for today's issues as well as previous
issues. (Check something out on one day and something else on the
next day, then do the testing.)
4) Confirm that items can't be renewed if current date is more than
X days before due date.
5) Confirm that the date and time of the soonest possible renewal are
displayed in the format specified by global sysprefs "dateformat"
and "TimeFormat".
6) Confirm that items can be renewed if "No renewal before" is
undefined or current date is X or less days before due date.
7) Confirm that if the number of allowed renewals is exceeded
"Not renewable" is displayed, no matter what "No renewal before"
is set to.
8) Test the same things with "Unit" set to hours.
Sponsored-by: Hochschule für Gesundheit (hsg), Germany
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
The method of checking the logged in user for superlibrarian privileges
is obtuse ( $userenv && $userenv->{flags} % 2 != 1 ) to say the least.
The codebase is littered with these lines, with no explanation given. It
would be much better if we had one subroutine that returned a boolean
value to tell us if the logged in user is a superlibrarian or not.
Test Plan:
1) Apply this patch
2) Verify superlibrarian behavior remains unchanged
Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comments on second patch.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
To test:
- Check out at least 3 items, due dates should be today, sometime in
the past and one day after tomorrow.
- Edit the message options - activate DUE and PREDUE notices with
days in advance = 2
- Run the advance_notices.pl script with -v -c
Result: Only a PREDUE notice is generated
- Run the advance_notices.pl script with -v -c -m 2
Result: Only the PREDUE message is generated correctly.
- Run t/db_dependent/Circulation.t
- without first patch: all tests pass.
- with first patch: some tests will fail.
- Apply patch.
- Rerun script, now PREDUE and DUE notices should be
generated.
- Run t/db_dependent/Circulation.t again, all tests should pass.
Add more items with different due dates, rerun and check results.
Run t/Circulation.t to confirm all tests pass.
- Apply the patch
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Passes functional tests and automated tests.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
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>
[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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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.
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>
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>
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>
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>
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.
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
