The lines to calc due date are doubled
ModDateLastSeen is not needed as we set the item not lost and we can set
the date as we do for date last borrowed and save a DB store
Largely this is reading the code and confirming the changes make sense
To test:
1 - Apply patch
2 - Check out items, confirm it works as before
3 - prove -v t/db_dependent/Circulation.t
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Staff can create smart rules with empty (undefined) fine grace period.
In Overdues.pm undefined firstremind means 0, do the same in Circulation.pm.
To test:
1. Apply these patches
2. Run:
kshell
prove t/db_dependent/Circulation.t
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch simply passes skip_record_index calls to Koha:Item:store
for all the changes done in AddReturn.
Testing is really verifiying that items are still correctly indexed at the end
For both search engines
To test:
1 - Find or create a record with multipel items
2 - Populate both:
UpdateItemLocationOnCheckin
UpdateNotForLoanStatusOnCheckin
3 - Confirm that checking in an item correctly updates the item status in search results
4 - Test with items issued, and items not issued
5 - Test when generating a transfer (checkin at different branch)
6 - Test when item was marked lost
7 - Test when filling transfer
Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
It came to light that it's not clear to all users that a checkin results
in the completion of a transfer if one exists for the item being checked
in. This patch adds such a notification to the error messages loop to
highlight that the item has been recieved from it's sending brnach.
To test
1/ Setup a transfer from branch A to branch B
2/ Check the item in at branch B
3/ Note that a new message appears in the 'Check in message' alert box
saying "Item recieved from branch A"
4/ Signoff
Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Some libraries would like to not only block the circulation of a guarantor based on fines owed by guarantees, but would also like to block circulation for all guarantees as well. Basically, if a family as a whole reaches a certain threshold of fines, the entire family will be blocked from checking out items.
Test Plan:
1) Apply this patch
2) Set NoIssuesChargeGuarantorsWithGuarantees to $14
3) Create a family of four ( 2 guarantors, 2 guarantees ) where the parents guarantee both children
4) Give 3 of the 4 a $5 fine
5) None of them should be able to check out items
Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
t/db_dependent/Koha/Items.t .. 6/12 Use of uninitialized value in string eq at /kohadevbox/koha/C4/Circulation.pm line 1460.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
When an item that has triggered a hold is in transit to a library,
if the item is accidently shelved instead of scanned at the pickup library,
and then checked out to another patron, the holds shows the item still in transit
in the status, but says that it is waiting at the library, and the since date has vanished!
Test plan:
1) Place an item at Library A on hold for Patron 1 of Library B with pickup library at Library B
2) Check in the item at Library A to trigger the hold transfer
3) At Library B, check out the item to Patron 2 (click Yes)
=> The hold is not marked as in transit and is back to priority 1
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.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 capitalizes the 'ADDITIONAL_MATERIALS' key in the
needsconfirm hash return value for CanBookBeIssued to restore
consistency with previous code and ensure SIP Checkout works as
expected.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Use build_sample_item in tests
Simplify tests for the confirmation
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Whilst QAing bug 13547 it was highlighted to me the at the 952$3 field,
and thus the item.materials field, may contain arbitrary notes about the
material rather than just numeric values. As such we need to check for
the field being defined as aposed to greater than '0'.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This clarifies the preference name to make it clear we are talking about
the 'parts' that make up an 'item'. 'Part' is a well known term in
british english libraries and I think perhaps 'Materials' may be
confused with other terms?
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
With this follow-up patch, the intended confirmation of checkin based on
CircConfirmParts value is effective.
Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/]
Sponsored-by: PTFS Europe [https://ptfs-europe.com/]
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan
1/ Catalogue an item to contain multiple parts by populating 'Materials
specified (bound volume or other part)'
2/ Enable the new system preference 'CircConfirmParts'
3/ Attempt to checkout the item created in step 1 to a user and note
that confirmation is now required.
4/ Checkout the item
5/ Attempt to checkin the item you have just checked out and note that
confirmation is required.
6/ Signoff
Sponsored-by: Royal College of Music [https://www.rcm.ac.uk/]
Sponsored-by: PTFS Europe [https://ptfs-europe.com/]
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 19014 altered CanBookBeRenewed to return (auto_)too_soon over on_reserve
For cron purposes this is the correct behaviour.
For display purposes we wish to see on_reserve over too_soon
This patchset adds a switch to 'CanBookBeRenewed' to alter the priority of these statuses
To test:
1 - set NoRenewalBeforePrecision to date only
2 - set a circ rule to auto-renewal=yes, no renewal before=0, checkout period to 7 days
3 - check item out
4 - confirm item shows Scheduled For Automatic Renewal in issues table
5 - place a hold on the item for another patron
6 - reload issues table for patron 1, confirm checkout still shows "scheduled for automatic renewal" rather than "on hold"
7 - change No Renewal Before value to 7
8 - reload issues table for patron 1, confirm checkout now shows "on hold"
9 - Apply patch
10 - restart_all
11 - Reload the issues table - confirm 'on_hold' still shows
12 - Change No Renewal Before to 0
13 - Refresh issues table, still shows 'On hold'
14 - perl misc/cronjobs/automatic_renewals.pl -v
15 - Result shows 'auto_too_soon'
16 - prove -v t/db_dependent/Circulation.t
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
When batch editing, 2 reindex calls are sent to ES/Zebra.
We can easily avoid that reusing the skip_modzebra_update (renamed skip_record_index)
Additionally we should only send one request for biblio, and we should
only do it if we succeed
As the whole batch mod is in a transaction it is possible to fail in which case
Zebra queue is reset, but ES indexes have already been set
In addition to the skip param this patchset moves Zebra and Elasticsearch calls to
Indexer modules and introduces a generic Koha::SearchEngine::Indexer so that we don't
need to check the engine when calling for index
The new index_records routine takes an array so that we can reduce the calls to
the ES server.
The index_records routine for Zebra loops over ModZebra to avoid affecting current behaviour
Test plan:
General tests, under both search engines:
1 - Add a biblio and confirm it is searchable
2 - Edit the biblio and confirm changes are searchable
3 - Add an item, confirm it is searchable
4 - Delete an item, confirm it is not searchable
5 - Delete a biblio, confirm it is not searchable
6 - Add an authority and confirm it is searchable
7 - Delete an authority and confirm it is not searchable
Batch mod tests, under both search engines
1 - Have a bib with several items, none marked 'not for loan'
2 - Do a staff search that returns this biblio
3 - Items show as available
4 - Click on title to go to details page
5 - Edit->Item in a batch
6 - Set the not for loan status for all items
7 - Repeat your search
8 - Items show as not for loan
9 - Test batch deleting items
a - Test with a list of items, not deleting bibs
b - Test with a list of items, deleting bibs if no items remain where all items are only item on a biblio:
SELECT MAX(barcode) FROM items GROUP BY biblionumber HAVING COUNT(barcode) IN (1)
c - Test with a list of items, deleting bibs if no items remain where some items are the only item on a biblio:
SELECT MAX(barcode) FROM items GROUP BY biblionumber HAVING COUNT(barcode) IN (1,2)
10 - Confirm records are update/deleted as appropriate
Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch
* sets one check for reserves and another for old_reserves in
atomic update
* Adds a message below the checkbox and adds detail when a hold is non
priority
* Fixes issue when there are more than one hold, but the first is non
priority
* Adds test case for this last scenario
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch implements necesary code to implement non priority feature
To test:
1) Apply all patches.
2) Run updatedatabase.
3) Checkout a specific item for patron1.
4) Place a hold on the same item for patron2 (do not check non priority
hold checkbox).
5) Try to renew the item for patron1.
CHECK => in checkouts table, there is a message that the item could not
be renewed because there was a hold.
6) Cleanup all checkouts and holds.
7) repeat steps 3 to 5, but this time check the non priority checkbox.
SUCCESS => item was renewed
8) prove t/db_dependent/Holds.t
Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
After discussion with Martin we decided that it could be the correct way
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
It was not used
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This code was duplicated and we are going to need it.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Remove duplicated logic for searching circulation rules.
This can be replaced with get_effective_rules().
To test:
1. prove t/db_dependent/Circulation/GetHardDueDate.t
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
1 - Go to Circulation->Transfer
2 - Note your signed in branch
3 - Find an item from your branch and create a transfer to branch B
4 - Confirm the item is marked as held at your current branch and is being transferred to B
5 - Find an item from a third branch, branch C
6 - Transfer that item to brnach B
7 - Confirm the item is held at your current branch and is being transferred to B
8 - prove -v t/db_dependent/Circulation.t
9 - prove -v t/db_dependent/Koha/Items.t
10 - prove -v t/db_dependent/RotatingCollections.t
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 26078: (follow-up) used validTransfer instead of checking WrongTransfer message
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
- Have 2+ branches
- Set your default return policy to Item returns to issuing library
- Find an item with holding branch of Branch A
- Set your library to Branch B
- Check item in
- Koha generates a transfer to Branch A. In doing so, it sets the item holding branch to Branch B
- Set your library to Branch A
- Check item in
- Koha generate a transfer to Branch B. In doing so, it sets the item holding branch to Branch A
- Set you library to Branch A
- Check the item in
. . . Repeat indefinitely.
Apply patch
Repeat
No more loop
Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The original code assumed every codepath should be trated as a
successful check-in. Such is not the case and the regression tests
verify that the code acknowledges this.
To test:
1. Apply the regression tests patch
2. Run:
$ kshell
k$ prove t/db_dependent/Illrequests.t
=> FAIL: Tests fail
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This is only a perltidy of _check_max_qty to remove some space
inconsistencies, like:
if( $max_checkouts_allowed eq '' ){ return;}
if ( $checkout_count - $onsite_checkout_count >= $max_checkouts_allowed ) {
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We only need to prefetch items if CircControl is set to ItemHomeLibrary
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Same as before but for patron_maxissueqty
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Making use of Koha::Checkouts make the code much more readable here.
It fixes 2 flaws:
* $type was not quote escaped
* the effective itemtype was not used which could lead to wrong
calculation (for instance item-level_itypes is set but the item does
not have the itype defined)
However there is something to note, we are going to make things a bit
less effective as we are now fetching the items to get their effective
itemtype (vs a SUM done at DB level)
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The different calls to _check_max_qty can be improved to highlight what
is really changing between each of them.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch replaces all calls to RefundLostItemFeeRules with
Koha::CirculationRules->get_lostreturn_policy and removes the module it
makes redundant.
Test plan
1/ Confirm that there are no longer any uses of RefundLostItemFeeRules
in the codebase
2/ Confirm circulation tests still all pass
3/ Confirm you can still set and unset the lost return rules
4/ Signoff
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes the hooks be called only if the checkin is effective
(i.e. if it is not rejected for some reason.
To test:
1. Apply the regression tests patch
2. Run:
$ kshell
k$ prove t/db_dependent/Koha/Plugins/Circulation_hooks.t
=> FAIL: Tests fail
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
AddReturn can be called on an item even if it's not checked out (to
trigger holds for instance).
The hook should (?) not be called in that situation
Test plan:
Confirm the above and that the following tests are now passing:
t/db_dependent/SIP/Message.t
t/db_dependent/Reserves.t
t/db_dependent/Circulation/issue.t
t/db_dependent/SIP/Transaction.t
t/db_dependent/Circulation.t
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch gets rid of a helper method used for calling the plugin
hooks.
To test:
1. Run:
$ kshell
k$ prove t/db_dependent/Koha/Plugins/Circulation_hooks.t
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests still pass!
4. Sign off :-D
Sponsored-by: ByWater Solutions
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch simplifies the payload as suggested on bug 25855. It also
keeps some specific params that cannot be deduced from the passed
checkout object, (e.g. if it is an onsite checkout).
Tests are cleared and added for this special exceptions.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds a call to _after_circ_actions into AddIssue and passes
an appropriate payload for plugins to utilise after issuing items.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes the hook be passed the Koha::Checkout object instead
of a hand-crafted list of attributes.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch generalizes the hook so it can be used by other circulation
actions.
Tests are also simplified by mocking some of the (extensive) plugin
hooks.
To test:
1. Repeat the test plan on the original patch
=> SUCCESS: All good
2. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nick Clemens
Didier Gautheron
Martin Renvoize
Kyle M Hall
Jonathan Druart
Frédéric Demians
Agustin Moyano
Lari Taskula
Tomas Cohen Arazi