C4::Circulation::GetItemIssue returned all the issue and item
informations for a given issue. Moveover it also did some date
manipulations. Most of the time this subroutine was called, there
additional information were useless as the caller usually just needed
the basic issue's infos 'from the issue table).
This first patch updates the simple calls, ie. the ones that just need
the issue's infomations.
Test plan:
The following operations should success:
- transfer a book
- create a rule for on-site checkouts and confirm that a patron cannot
check more items out that it's defined in the rule.
- Renew an issue using ILSDI
- Using SIP confirm that you are able to see your issues
Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Circulation.t is failing randomly on our CI
https://jenkins.koha-community.org/job/Koha_Master_D8/192/console
# Failed test at t/db_dependent/Circulation.t line 1147.
# got: '1'
# expected: '0'
# Failed test at t/db_dependent/Circulation.t line 1152.
# got: '1'
# expected: '0'
# Failed test at t/db_dependent/Circulation.t line 1156.
# got: '1'
# expected: '0'
# Failed test at t/db_dependent/Circulation.t line 1170.
# got: '1'
# expected: '0'
# Failed test at t/db_dependent/Circulation.t line 1184.
# got: '1'
# expected: '0'
# Looks like you failed 5 tests of 23.
Sometimes one of the alert or impossible flags is set.
This patch guesses that it's because of the 'restricted' value of the item that is evaluated to 1.
If it is not fixed by this patch, we will have more info next time (at least know
if alert or impossible is set).
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
The fix is trivial. Using random data will lead to this situations. The good thing is that
it lets us spot places in which tests need more fine-grained data.
If a patron owes more than the OPACFineNoRenewals value, the issue won't
be auto renewed anymore (driven by the new pref OPACFineNoRenewalsBlockAutoRenew).
Test plan:
Note: You will have to manually change data in your DB, make sure you
have access to the sql cli.
1/ Set the OPACFineNoRenewals to 5 (for instance)
2/ Set OPACFineNoRenewalsBlockAutoRenew to block
3/ Check an item out to a patron and mark is as an auto renewal
4/ Make sure the patron does not have any fees or charges.
5/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl)
Confirm that the issue has been renewed
6/ Create an invoice for this patron with a amount > OPACFineNoRenewals (6
for instance)
7/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl)
Confirm that the issue has not been renewed.
8/ Set OPACFineNoRenewalsBlockAutoRenew to allow
9/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl)
Confirm that the issue has been renewed
Sponsored-by: University of the Arts London
Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>
Signed-off-by: Janet McGowan <janet.mcgowan@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch adds a new circulation rule (no_auto_renewal_after_hard_limit) to block/allow
auto renewals after a given date.
The idea is to stop renewals at a given date. That way the library will have
time to send overdues and get the books back before the students do on holiday.
Test plan:
0/ Execute the update DB entry
1/ Define a rule with no_auto_renewal_after_hard_limit set to tomorrow
2/ Modify the issues.issuedate, to simulate a checkout in the past:
UPDATE issues
SET issuedate = "yyyy-mm-dd hh:mm:ss"
WHERE itemnumber = YOUR_ITEMNUMBER;
with issuedate = 2 days before for instance
3/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl)
Confirm that the issue has been renewed
4/ Modify the no_auto_renewal_after_hard_limit and set it to yesterday
5/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl)
Confirm that the issue has not been renewed
Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>
Signed-off-by: Janet McGowan <janet.mcgowan@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Updating to use they/them and skipping the ones changed to it
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comments throughout the Koha codebase assume that
all librarians or borrowers are male by using the
pronoun 'he' universally. This patch changes to
'he or she' / 'him or hers'.
Testing plan:
- ensuring modifying tests still pass:
+ C4/SIP/t/06patron_enable.t
+ t/db_dependent/Circulation.t
+ t/db_dependent/Koha/Patrons.t
+ t/db_dependent/Reserves.t
Sponsored-By: California College of the Arts
No code changes detected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan:
prove all these tests, they must all pass
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Sounds more appropriate and consistent with existing action logs.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
For an unknown reason, the use_ok('Circulation') does not work as
intended (see 3660c451a3).
With the new use of C4::Log, the trick does no longer work.
It does not make sense to add the use_ok('C4::Log') in Circulation.t,
removing it.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
20/02/17 : added the syspref RenewalLog
24/20/17 : added a test for the syspref Renewal Log
test plan
1 - Chose a Borrower and have him renewing an item
2 - Check the renew logs : they should be empty
3 - Apply patch and set the syspref RenewalLog to 1
4 - Have the Borrower renewing a new item
5 - Check the renew logs : there should be your renew
I called the function logaction, which is in charge of modifying the
logs, within the function which adds a new renewal at the list.
Signed-off-by: Julien Comte <julien.comte@u-psud.fr>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
the new code skip testing for more borrowers if the item can't be renewed
this require more tests for the case where 2+ items are reserved.
I tried to add 1 more reserve to the main test suit, but too many other tests
rely on specific holds, and I couldn't get around it.
Instead, I added a subtest that consider the specific simple case and leave the
other test cases exactly has they were designed.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Amended-patch: Just fix 3 indendations
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Koha::Issues and Koha::Checkouts have been added to the codebase to
represent the same thing.
In ODLIS the word Issue is never used in the sense we use it. Another
problem with Issue is it has so many meaning in English (such as
problem/bug)
The word Checkout *is* in ODLIS, closer to what we use:
http://www.abc-clio.com/ODLIS/odlis_c.aspx#checkoutslip
Test plan:
git grep Koha::Issue
should not return any occurrences and the tests must still pass
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Sponsored-by: ByWater Solutions
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
To test:
- Run:
$ prove t/db_dependent/Circulation.t
=> FAIL: Lots of warnings
- Apply the patch
- Run:
$ prove t/db_dependent/Circulation.t
=> SUCCESS: All green and no itemtype warnings
- Sign off :-D
Sponsored-by: ByWater Solutions
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
If an issue marked as auto_renew is renewed manually, we want to display
the latest auto renew date possible.
Test plan:
1/ Define circ rules as in the previous patch.
2/ Check a item out, mark it as an auto renewal
3/ Back date the issuedate and make sure it will be too late to renew it
4/ Use the Circulation > renew page (circ/renew.pl) to manually renew
this issue.
You should get a warning "You barcode has been scheduled for automatic renewal
and cannot be renewed anymore since DATE."
If the pref AllowRenewalLimitOverride is set, you will be allowed to
renew it anyway.
Sponsored-by: University of the Arts London
Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch adds a new circulation rule (no_auto_renewal_after) to block/allow
auto renewals after a given delay.
For instance, if the issue date is 10 days before today, and
no_auto_renewal_after is set to 10, tomorrow the issue won't be auto
renewed.
Test plan:
0/ Execute the update DB entry
Note: You will have to manually change data in your DB, make sure you
have access to the sql cli.
1/ Define a rule with no_auto_renewal_after (10 for instance) and
norenewalbefore (5 for instance).
(This new rule will behave the same as norenewalbefore: the unit depends
on the lengthunit value).
The automatic renewals will be done from 5 to 10 days ahead.
2/ Modify the issues.issuedate, to simulate a checkout in the past:
UPDATE issues
SET issuedate = "yyyy-mm-dd hh:mm:ss"
WHERE itemnumber = YOUR_ITEMNUMBER;
with issuedate = 2 days before for instance
3/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl)
Confirm that the issue has not been renewed (too soon)
4/ Repeat step 2 with a due date set as 11 days before
5/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl)
Confirm that the issue has not been renewed (too late)
6/ Repeat step 2 with a due date set as 7 days before
7/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl)
Confirm that the issue has been renewed (issues.renewals has been
incremented and date_due has been updated according your circ rules).
Sponsored-by: University of the Arts London
Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
[1] Renames the dbrev version to XXX; adding a my for $sth, although
not strictly necessary it feels better.
[2] Circulation.t stumbles over:
Undefined subroutine &C4::Circulation::GetItem called at ... line 1283.
Somehow this is related to use_ok instead of a regular use.
Worked around this by require_ok and adding a regular use.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Unless in t/db_dependent/Context.t where we want to test the cache
behaviors.
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
Right now, fines are updated based on the fine description. There are a
number of areas where this can go wrong ( date or time format changing,
title being modified, etc ). Now that issues has a unique
identifier, we should use that for selection and updating of fines.
Test Plan:
1) Apply this patch
2) Test creating and updating fines via fines.pl
and checking in overdue items. No changes should be noted.
3) prove t/db_dependent/Circulation.t
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
This patch adds test cases for the new syspref NoRenewalBeforePrecision.
Tests if GetSoonestRenewDate returns correct values for both 'date' and
'exact_time' settings.
Also adds a few comments related to bugs 7413 and 14101.
To test:
1) prove t/db_dependent/Circulation.t
Sponsored-by: Hochschule für Gesundheit (hsg), Germany
Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
This patch adds new test cases to check if CanBookBeRenewed provides
correct return values and error codes for premature renewals. Both manual
and automatic renewals and different settings for 'No renewal before'
are tested.
To test:
1) prove t/db_dependent/Circulation.t
Sponsored-by: Hochschule für Gesundheit (hsg), Germany
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Tests assume that the branchcodes CPL/MPL/etc. already exist in the DB.
If they need them, they should create them.
Test plan:
Execute the differente test files on a DB without any branchcode or
at least without CPL/MPL branches.
Confirm that the tests pass.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
At the opac, the renew checkbox should not be displayed if it's an
on-site checkout (same on the intranet).
On the way, this patch adds a specific message to the intranet if the
librarian try to renew an on-site checkout.
Indeed before this patch a renew was allowed if the barcode was scanned.
Test plan:
1/ Create an on-site checkout for a patron
2/ Confirm that the checkbox 'renew' is not displayed on the checkout
list tables
3/ At the OPAC, the renew should not be allowed (no checkbox)
4/ Try to check the item out to the same patron, confirm that you get a
specifig message to inform you the renew is not allowed for on-site
checkouts.
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Changed 'issue' to 'item' in the error message.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
The reserves.constrainttype does not exist anymore, it has been removed
by bug 9809.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Tested both unit test patches together.
Without main patch applied: do not pass as expected / with main patch pass OK as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
It seems that many librarians find it disconcerting to have no feedback
with the new checkouts table. It seems that many of them wait for it to
fully load, check to verify the item was checked out, and only then
check out the next item.
To help alleviate this issue, we can have the checkouts page give
feedback about the item that was just checked out.
Test Plan:
1) Apply this patch
2) Check an item out
3) Note the message "$title ($barcode) due on $date_due"
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
This works well and fixes a very problematic issue with the new AJAX
circ. I will be submitting a follow-up which I think is an improvement
to the display.
Signed-off-by: Jason Burds <jburds@dubuque.lib.ia.us>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Jen DeMuth <jdemuth@roseville.ca.us>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
C4::Reserves:
* Added OnShelfHoldsAllowed() to check issuingrules
* Added OPACItemHoldsAllowed() to check issuingrules
* IsAvailableForItemLevelRequest() changed interface, now takes
$item_record,$borrower_record; calls OnShelfHoldsAllowed()
opac/opac-reserve.pl and opac/opac-search.pl:
* rewrote hold allowed rule to use OPACItemHoldsAllowed()
* also use OnShelfHoldsAllowed() through
* IsAvailableForItemLevelRequest()
templates:
* Removed AllowOnShelfHolds and OPACItemHolds global flags, they now
only have meaning per item type
Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
I have tested this patch left, right and upside down for the last
several months. All tests have passed.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
There were multiple calling conventions for C4::Context's
set_userenv routine. So the following commands were used to
find discrepancies:
grep "::set_userenv" `find .`
grep "\->set_userenv" `find .`
The first grep demonstrated that the smaller change is from
:: to -> as only C4/Auth.pm, installer/InstallAuth.pm, and
t/db_dependent/Circulation.t would need to be modified. This
patch corrects C4::Context's set_userenv routine to be object
call based (use ->) by using a shift to ignore the first
parameter, and modify the three files found with :: calls.
As the result of trying to roll a distribution,
t/Circulation_barcodedecode.t was discovered to be faulty. The
cause being incorrect parameters! This was hidden when there
was no shift in the set_userenv routine. However, with its
correction, the test broke.
This led me to read the POD documentation for the function
set_userenv in C4::Context and realize it was outdated as
well. It has been revised to match the current version of
the function.
Then intentionally bad parameters passed to the set_userenv
routine in C4::Context were hunted down. The biggest problems
were missing surnames or branch names.
Rebase required because of shibboleth change in C4/Context.pm
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
* Allow on shelf holds needed to be enabled
* Added some error supression code for undefined string comparison
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
These new unit tests will fail due to the fact that Koha::Database
uses a separate dbh handle than C4::Context->dbh
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
This patch fixes two unit tests that broke because of the new feature.
Also adds some new test cases.
To test:
1) prove t/db_dependent/Circulation.t
2) prove t/db_dependent/Circulation_Issuingrule.t
Sponsored-by: Hochschule für Gesundheit (hsg), Germany
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
This patch adds a direct unit test of GetSoonestRenewDate()
and verifies that CanBookBeRenewed() returns a 'too_soon'
error as expected.
To test:
[1] Verify that prove -v t/db_dependent/Circulation.t passes.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This patch corrects a typo that broken ModReserveFill(). This
patch also adds a unit test that (via two levels of indirection)
exercises ModReserveFill().
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Fixing some things the QA script pointed out this morning:
- whitespace
- variable declaration
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Changes tests for GetUpcomingDueIssues:
days_in_advance should be inclusive:
2 should find items due in 2 days from now.
1 should find items due tomorrow.
0 should only find items due today.
Adds new tests for Bug 11218:
As the advance_notices.pl, the only script using GetUpcomingDueIssues
is not only used for PREDUE notices, but also for
DUE notices, we need to also select items due today.
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests and QA script pass.
I fixed a redundant 'my' that the QA script pointed out.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Some unit tests related to Bug 9362.
Sponsored-by: Universidad Nacional de Cordoba
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
http://bugs.koha-community.org/show_bug.cgi?id=10606
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
New and old tests pass.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Follow-up fixing some and adding more unit tests.
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Initial work on unit tests for CanBookBeRenewed. More scenarios are needed.
Also made Circulation.t use autocommit=0.
Sponsored-by: Universidad Nacional de Córdoba
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests and qa script pass.
Tests done:
- created a .koc file with return, issue and fine payments.
- queued that file into Koha
- created some transactions using the Firefox plugin
- queued that into Koha
- processed files and checked outcome was ok
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
