Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The account offsets table should be used to track increments and
decrements of fines via payments and credits, as well as fine accruals.
It should be able to match fees to payments and visa versa, so we can
know which fee was paid by a given payment, and which payments applied
to a given fee.
Test Plan:
1) Apply this patch
2) Run updatedatabase
3) Note the table accountoffsets has been renamed to account_offsets
4) Ensure fine generation creates offsets
5) Ensure creating a manual invoice creates an offset
6) Ensure a lost item charge creates an offset
7) Ensure Reverse Payment creates an offset
8) Ensure a payment creates an offset
9) Ensure a payment for multiple fees creates an offset for each
10) Ensure writeoffs create offsets
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This has been raised by failures on t/db_dependent/Circulation/issue.t
(thanks tests!)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To recreate:
1 - Manually add a lost fine to a ptron and include a barcode
2 - Attempt to write off the fine
3 - Internal server error
4 - Checkout an item and mark lost to checkin and fine
5 - Attempt to write off line
6 - Internal server error
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch moves some code and prevents checking for too many checkouts
when performing a renewal via checkout
To test:
1 - Set a rule to limit checkouts to a single issue, allowing renewal
2 - Issue an item to a patron
3 - Issue the same item
4 - In staff client you get a confirm to renew and a notice of Too Many
checkouts, don't confirm
5 - VIA Sip - you get a renewal response, but in logs the renewal fails
as a 'too
6 - Apply patch
7 - Via staff client you shoudl get renewal confirm with no too many
error
8 - SIP checkout should renew
Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nothing important here, but this line should have been removed by bug
18966:
2177 # Update the fines
2178 $dbh->do(q|UPDATE accountlines SET issue_id = ? WHERE
issue_id = ?|, undef, $old_checkout->issue_id, $issue->issue_id);
The issue_id is now the same when moved from issues to old_issues. We do
not need to update the accountlines table.
No test plan here, you need to understand previous changes to validate
this patch.
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 17829 must have been handle this specific case: GetMember set
category_type, but now $borrower is a Koha::Patron unblessed and does
not contain the category_type.
The fix is to call ->category->category_type on the Koha::Patron object
to be able to know if they are a statistic patrons.
Test plan:
Run the tests
Tests pass, as does QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The statement for head3 NB ('nota bene'?) looks like a hash key
in the list of possible return values for $needsconfirmation.
Moved it up and prefixed it with IMPORTANT.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The returns from C4::Circulation::CanBookBeIssued used
to be structured as a hashref of entries like
REASON => {
data => 'foo',
moredata => 'bar',
};
Some entries still are. But many are now
REASON => 1,
data => 'foo',
moredata => 'bar',
The sip Checkout routine still assumed the former, as it
reports any causes it was not aware of (to maintain support for
a changing api) The data fields could leak into the screen message
field of the response. e.g. the borrowernumber or surname of the
borrower who has a hold on an issued title. Some real messages were
getting obscured by this
This patch sanatizes the return from from CanBookBeIssued
by removing keys which are not all uppercase
It also fixes a case where the key's data element was used
for the screen message when we should use the key itself
Updated the documentation of CanBookBeIssued to flag up
the assumption re case and the fact that 3 elements rather
than two may be returned
The loop through the returned keys was a bit bogus
so we now explicitly jump out if noerror is unset
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested quite extensively. Test results put on Bugzilla.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Currently AddIssue tests if renewal, but logs an issue even if so. This
patch moves the logging into the conditional so a log entry is only
added if we aren't renewing (as renewals are logged separately)
To test:
1 - prove t/db_dependent/Circulation.t - one test should fail
2 - Enable both issue and renewal logs
3 - Checkout an item to a patron
4 - View the logs - the issue is captured
5 - Checkout the item to the patron again and confirm renewal
6 - Both an issue and a renewal are logged
7 - Apply patch
8 Repeat 1-6, tests should pass and only renewal should be logged
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
As Jonathan pointed out, GetItem already called effective_itemtype.
So we can just use $item->{itype} here.
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>
In the regular situation, you can get itemtype via biblio and then
biblioitem as well as via biblioitem (at least when item-level_itypes
is set to biblio).
But since Koha unfortunately defined two relations in item, one for
biblioitemnumber (the good one) and one for biblionumber (redundant),
TestBuilder (correctly) builds one biblioitem and two biblios.
If you item-level_itypes to biblio record, this will result in failing tests
when calling AddReturn (in this case Koha/Patrons.t).
It will crash on:
Can't call method "itemtype" on an undefined value at C4/Circulation.pm line 1826.
Cause: AddReturn goes via the biblionumber to biblio and than to
biblioitems, and it does not find a biblioitem. (Not a fault from TestBuilder
but a database design problem.)
This patch makes a small change in AddReturn to retrieve the itemtype via
biblioitem. It actually is a shorter road than items->biblio->biblioitems.
Note: I do not test the Biblioitems->find call, since we already checked
the GetItem call before and we have a foreign key constraint.
I did not call $item->effective_itemtype since we still use GetItem; this
could be done later.
Adjusted Circulation/Returns.t too: If we add an item with TestBuilder and
we called AddBiblio before, we should link biblioitemnumber as well.
Test plan:
[1] Do not apply this patch yet.
[2] Set item-level_itypes to biblio record.
[3] Run t/db_dependent/Koha/Patrons.t. (It should fail.)
[4] Apply this patch.
[5] Run t/db_dependent/Koha/Patrons.t again.
[6] Run t/db_dependent/Circulation/Returns.t
[7] Git grep on AddReturn and run a few other tests calling it.
Note: Bugs 19070/19071 address three tests that call AddReturn too.
[8] In the interface, check in a book.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Note: Bugs 19070 and 19071 are already pushed. The command in comment #4
has all the tests successful.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan:
1. ReturnBeforeExpiry is activated
2. useDaysMode is different from "circulation rules only"
3. Set expiry date of a patron to a near date
4. Set a closed day on calendar for this date
5. Do a checkout
Without patch, return date will be patron expiration date
With the patch, return date will be last open day before patron expiration day
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Koha suffers of big bugs due to its history: When data are deleted, they
are moved to another tables.
For instance issues and old_issues: when a checkin is done, it is moved
to the old_issues table.
That leads to a main problem that is described on
https://wiki.koha-community.org/wiki/DBMS_auto_increment_fix
However we tried first to fix the problem (for issues/old_issues) at
code level on bug 18242.
The goal was to prevent data lost.
Data lost may happens in this case:
Check an item out (issue_id = 1)
Check an item in (issue_id = 1)
Restart MySQL (reset auto increment for issue_id to 1)
Check an item out (issue_id = 1)
Check an item in => BOOM, the issue_id is a PK in old_issues and the
move fails.
Before bug 18242 the data were lost, we inserted the value into
old_issues, which fails silently (because of RaiseError set to 0 in
Koha::Database), then delete the row from issues.
That has been fixed using a transaction.
This patch introduced a regression we tried to fix on bug 18651 comment
0, the patron was charged even if the checkin was rejected.
A good way to fix that would have been to LOCK the tables:
1- Start a transaction
2- LOCK the table to make sure nobody will read id and avoid race
conditions
3- Move the content from one table to the other, dealing with ids
4- UNLOCK the table
5- Commit the transaction
But there were problems using LOCK and DBIx::Class (See commit
905572910b - Do no LOCK/UNLOCK the table).
Finally the solution implemented is not acceptable for several reasons:
- batch checkins may fail
- issue_id will always stay out of sync (between issues and old_issues)
See 18651 comment 66.
Since the next stable releases are very soon, and we absolutely need to
fix this problem, I am suggesting to:
1- Execute the move in a transaction to avoid data lost and reject the
checkin if we face IDs dup
=> It will only reject 1 checkin (max is 1 * MySQL restart), no need to
deal with race conditions,
2- Display a warning on the checkin page and link to a
solution/explanation
3- Communicate as much as we can on the proper fix: Update auto
increment values when the DBMS is restarted -
https://wiki.koha-community.org/wiki/DBMS_auto_increment_fix
4- Display a warning on the about page for corrupted data (see bug
18931)
5- Write and make available a maintenance script to fix corrupted data
(TODO LATER)
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
1 - Set 'OpacRenewalBranch' to various settings
2 - Renew an item for a ptron under each setting
3 - Confirm action_log entries reflect the correct branch for each
secnario
4 - prove t/db_dependent/Circulation/issue.t
Signed-off-by: David Kuhn <techservspec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Most of the time C4::Biblio::GetBiblioData is used to retrieve the title
and/or the author of a bibliographic record.
This patch replaces the easy occurrences of GetBiblioData, the ones
where the 2 joins are needed, but only data from biblio and biblioitems
table are.
Test plan:
It will be hard to test everything, I'd suggest a QAer to review this
patch and confirm that the difference occurrences of GetBiblioData have
been correctly replaced by calling Koha::Biblios->find or
$biblio->bibioitem
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
GetMember returned a patron given a borrowernumber, cardnumber or
userid.
All of these 3 attributes are defined as a unique key at the DB level
and so we can use Koha::Patrons->find to replace this subroutine.
Additionaly GetMember set category_type and description.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
- Item does not have a title attribute, it comes from biblio
- There is an additional call to effective_itemtype done on AddReturn,
so we need to catch both warnings
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The subroutine C4::Biblio::GetBiblioFromItemNumber was wrong for several
reasons:
- badly named, we can get biblio info from a barcode
- SELECT * from items, biblio and biblioitems
makes things hard to follow and debug, we never know where do come from
the value we display
- sometimes called only for trivial information such as biblionumber,
author or title
This patchset suggests to replace it with calls to:
- Koha::Items->find for item's info
- $item->biblio for biblio's info
- $item->biblio->biblioitem for biblioitem's info
Test plan:
Item's info should correctly be displayed on the following pages:
- circulation history
- transfer book
- checkin
- waiting holds
QA will check the other changes reading the code, it's trivial
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
$issue is now a Koha::Checkout, not a hashref
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
is_overdue must be called even if there is not dropbox date (!)
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
There are a few calls to GetItemIssue where it's not as easy to make
sure everything will be fine just replacing the calls with a
Koha::Issues->find
- In AddReturn the overdue flag is used (that's why this patch depends
on bug 17689)
- In CanBookBeRenewed, as well as the overdue flag the dates converted
to DateTime were used. It's now our job to convert them when we need
them.
- Same in AddRenewal but we also call _CalculateAndUpdateFine, so we
need to update the variables in this subroutine.
Note that, prior to this patch, AddReturn returned the GetItemIssue
hashref in the $iteminformation. Most of the time this variable is not
used, I have found only 1 place where it's used: circ/returns.pl
TODO: In this script we should call ->is_overdue instead of the
DateTime->compare calls
Test plan:
All the circulation tests must pass (it's how I have caught the specific
cases).
Do some checkins/checkouts/renewal and focus on the due date
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>
This patch simply removes the C4::Circulation::GetItemIssue subroutine
Test plan:
At this point, `git grep GetItemIssue` should not return any occurrence
in the codebase
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
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>
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>
We cannot LOCK the old_issues table here, other tables are accessed and DBIx::Class rename it with "me":
DBD::mysql::st execute failed: Table 'me' was not locked with LOCK
TABLES [for Statement "SELECT `me`.`issue_id`, `me`.`borrowernumber`,
`me`.`itemnumber`, `me`.`date_due`, `me`.`branchcode`,
`me`.`returndate`, `me`.`lastreneweddate`, `me`.`renewals`,
`me`.`auto_renew`, `me`.`auto_renew_error`, `me`.`timestamp`,
`me`.`issuedate`, `me`.`onsite_checkout`, `me`.`note`, `me`.`notedate`
FROM `old_issues` `me` WHERE ( `me`.`issue_id` = ? )" with ParamValues:
0='2'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832.
Consequence: We could have a checkin refused if there is a race, but
this is the simplest and safest way to fix it.
Found this by inserting the same issue_id in old_issues before checkin:
The call to ->search( )->get_column is in scalar context and will
return the number of results, i.e. always 1.
If you have an issue_id 2 in old_issues, it will crash:
DBIx::Class::Storage::DBI::_dbh_execute(): Duplicate entry '2' for key 'PRIMARY'
The fix is fairly simple: Put get_column in list context and pick the first
array entry.
NOTE: Using DBIx's get_column()->max here might look simpler here.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Variable $original_issue_id is not used. The id is retrieved later from
$issue when updating accountlines.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
For more info, see:
commit be156d9ad9
Bug 15854: Use a READ and WRITE LOCK on message_queue
and
commit b40456f7dd
Bug 18364: Do not LOCK/UNLOCK tables from tests
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
If the "max(issue_id) from old_issue + 1" already exists in issues, the
move fails.
For instance we have
1, 2, 3, 4 in issues
checkin 4
1, 2, 3 in issues (AI=5)
4 in old_issues
Restart mysql => AI is reset to MAX(issue_id) => 4
checkout a new one
1, 2, 3, 4 in issues (AI=5)
4 in old_issues
checkin 4 (will get id 5 in old_issues)
1, 2, 3 in issues (AI=5)
4, 5 in old_issues
=> This works with and without this patch
Now we have
1, 2, 3 in issues (AI=5)
4, 5 in old_issues
Restart mysql => AI is reset to MAX(issue_id) => 4
checkout 2 new ones
1, 2, 3, 4, 5 in issues (AI=7)
4, 5 in old_issues
checkin 4 (4 becomes 6 in old_issues)
1, 2, 3, 5 in issues (AI=6)
4, 5, 6 in old_issues
=> This did not work without with patch
The update of the issue_id was made before the move (so in the issues
table), the PK did not allow it
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2. If the move fails for whatever reason (see
https://lists.katipo.co.nz/pipermail/koha/2017-May/048045.html for an
example), fines can be charged. It should not
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
1. AddReturn returns a $issue hashref with the old issue_id value
=> At first glance it does not affect anything, but would be good to fix
it for future uses.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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 set the lang parameter when C4::Letters::GetPreparedLetter is
called to generate the notice.
Note that we do not need to pass it if want_librarian is set.
TODO: I do not know what to do with TransferSlip
Sponsored-by: Orex Digital
Signed-off-by: Hugo Agud <hagud@orex.es>
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>
The test in SendCirculationAlert is extended by adding an env var
called KOHA_NO_TABLE_LOCKS. If this var is set to a true value,
the table locking is skipped too.
This is useful when running a test without prove. The variable could be
set in a shell profile.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
From the MySQL doc:
"LOCK TABLES is not transaction-safe and implicitly commits any active transaction before attempting to lock the tables."
If the LOCK/UNLOCK statements are executed from tests, the current transaction will be committed.
To avoid that we need to guess if this code is execute from testsa or not (yes it is a bit hacky)
Better ideas are welcome!
Another fix would have been to revert
commit be156d9ad9
Bug 15854: Use a READ and WRITE LOCK on message_queue
but theorically a race is still possible.
Existing tests seem to be safe, to test this patch you will need new
tests from bug 17964.
Test plan:
prove t/db_dependent/Letters/TemplateToolkit.t
twice, and notice that changes have been comitted.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Silly error here, the $switch_onsite_checkout is not correctly
calculated, it should not only be set to the value of the syspref, but
meet all the conditions.
Test plan:
Set a circ rule with number of on-site checkout = 2 and number of
checkouts = 2.
Switch on both ConsiderOnSiteCheckoutsAsNormalCheckouts and SwitchOnSiteCheckouts
Do a regular checkout
Do another regular checkout
At this point you have reach the maximum number of checkouts allowed.
Not try another one.
=> Without this patch you will not get the warning
=> With this patch you will see it
Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The AddRenewal subroutine currently uses the circulation rules for the branch
stored in the Issues table (which is the holding branch) when calculating the new due date.
This patch replaces using the branch from the Issues table with the branch
specified by the HomeOrHoldingBranch syspref.
To test:
1. Set up 2 branches, Branch1 and Branch2
2. Set up a loan rule in Branch1 for DVDs with a 21 day loan period and a
21 day renewal period.
3. Set up a loan rule in Branch2 for DVDs with a 14 day loan period and a
14 day renewal period.
4. Checkout a DVD belonging to Branch1 while logged into Branch2. It will
receive the correct 21 day loan period.
5. Renewing the same DVD while logged into either Branch1 or Branch2
will give a 14 day due date, rather than 21 days.
6. Checkout a DVD belonging to Branch2 while logged into Branch1. It will
receive the correct 14 day loan period.
7. Renewing the same DVD while logged into either Branch1 or Branch2
will give a 21 day due date, rather than 14 days.
8. Apply the patch and repeat steps 4-7. The correct due date should be given
when the item is renewed, regardless of where it is checked out or renewed.
This update removes reassignment of $branch variable.
Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
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>
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 code was a bit weird and this patch cleans it a bit by renaming
variables and adding a variable.
Sponsored-by: Orex Digital
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
