Commit graph

346 commits

Author SHA1 Message Date
Julian Maurice
d8c137a718 Bug 24031: Add plugin hook after_hold_create
It is called after a hold has been placed

Test plan:
1. Write a plugin that implements only after_hold_create (see
   `perldoc Koha::Plugins` for implementation details). Install it and
   enable it
2. Place a hold and verify that your plugin method has been called with
   the right parameters

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-07-30 17:30:23 +02:00
4b299f54aa Bug 23070: Pass no_triggers => 1 to Koha::Objects->update
To make sure we will update all the objects in one go (and no trigger
the ->set->store from Koha::Object->update)

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-07-20 16:16:37 +02:00
6f6f88105d Bug 23070: Increment all priorities in 1 query
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-07-20 16:16:37 +02:00
563a25d952 Bug 23070: Use Koha::Hold in C4::Reserves::RevertWaitingStatus
We are using raw SQL statements, we should use Koha::Hold instead.

This patch does not seem optimal, we would like to increment priority in
only 1 statement and without the need to fetch and loop all holds.

== Test plan ==
- apply patch
- place some holds on the same record
- check that the priorities look good
- mark one hold as waiting by doing a check-in
- revert the waiting status
- confirm that the priorities are recalculated correctly

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-07-20 16:16:37 +02:00
96a8710350
Bug 25566: Add option to ignore found holds and use it when checking high holds
To test:
 1 - Find or create a record with 10 items
 2 - Set sysprefs:
     decreaseLoanHighHolds - enable
     decreaseLoanHighHoldsDuration - 2
     decreaseLoanHighHoldsValue - 2
     decreaseLoanHighHoldsControl  - 'over the number of holdable items'/dynamic
 3 - Set circ rules to allow 1 hold per record on the relevant record
 4 - Place 3 holds on the record
 5 - Check one item in and confirm hold to set to waiting
 6 - Issue to the patron with the waiting hold
 7 - Get a notice that loan period is decreased
 8 - Don't confirm the checkout
 9 - Apply patch
10 - Restart all the things
11 - Repeat checkout, no decrease this time!

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-07-16 15:32:18 +01:00
326c0159a0 Bug 25232: Add ability to specify multiple notforloan values to skip
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>
2020-06-25 10:50:02 +02:00
465e5562fc Bug 25232: Add ability to skip trapping items with a given notforloan value
This is a companion/alternative to bug 25184, in that it allows an
explicit workflow for placing returned books into temporary storage for
a few days for decontamination purposes.

The idea here is to create a specific notforloan value for "In
Decontamination" or something along along those lines. This notforloan
value would never be trappable. At the end of decon,
UpdateNotForLoanStatusOnCheckin  could be used to remove the
notforloan status and allow checkins to be trapped to fill holds.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Give an item a negative notforloan value
4) Place a hold on the item
5) Check the item in
6) Note the item is trapped for hold
7) Set SkipHoldTrapOnNotForLoanValue to the same notforloan value
   you used in step 3
8) Check the item in again
9) Note Koha did not ask you to trap the item for hold!

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>
2020-06-25 10:50:02 +02:00
d864c5640a
Bug 24612: Make hold-transfer-slip take reserve_id
To make sure we are going to display the correct hold's info we need to
pass the reserve_id.

== Test plan ==
1. Add some content to HOLD_SLIP notice, e.g.

  <h2>[% branch.branchname %]</h2>
  <div>[% biblio.author %]<br>[% biblio.title %]<br>[% item.barcode %]
  <ul><li> Reserve ID:  [% hold.reserve_id %]</li>
  <li>Expiration date: [% hold.expirationdate %]</li></ul>

2. Add 2 holds for 1 patron to a single record
3. Check the reserve IDs in the reserves table - on a clean sandbox, they will be 1 and 2
4. Check in one of the items from the record and print the slip
5. Note that the reserve ID on the slip is 2 and the expiration date is blank
6. Repeated check ins do not change this
7. Check in a second item from the record
8. Note that the reserve ID for this hold is also 2, but this time the expiration date is filled in
9. Check in the first item again - the reserve ID stays as 2, but this time the expiration date is filled in
10. Apply patch
11. cancel the holds to come back to a clean state
    (and maybe ensure items aren't in transit)
12. redo the test and see the following differences
13. 1st checkin:
    1. expiration date ok
    2. the reserve ID is the one of the first hold
14. 2nd checkin:
    1. expiration date ok
    2. the reserve ID is the one of the second hold

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-22 09:33:16 +01:00
82e866fcb5
Bug 25184: Add syspref
It's entirely possible that some libraries are relying on the current
before for part of their workflow. Do to this possibility, it seems like
a good idea to control this behavior via a system preference.

Test Plan:
1) Apply this patch set
2) Run updatedatabase.pl
3) Set TrapHoldsOnOrder to "don't trap"
4) Set an item's notforloan value to -1
5) Place a hold on that item
6) Check in the item
7) Note the item is not trapped for hold
9) Set TrapHoldsOnOrder to "trap"
10) Check in the item
11) Koha should now ask if you'd like to trap the item for the hold!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-11 08:55:13 +01:00
cc938e6be1
Bug 25184: Items with a negative notforloan status should not be captured for holds
Negative notforloan statuses should allow holds to be placed but not captured.

Due to coronavirus, we have libraries setting all returned materials to a negative notforloan value of Quarantine for several days.
They're using UpdateNotForLoanStatusOnCheckin to set that status automatically. However, those items are still capturing for holds,
even though those items cannot be checked out until the notforloan status is removed.

In cases like an On Order item where we do want the hold to fill at checkin,
UpdateNotForLoanStatusOnCheckin should be used to clear that notforloan status so the hold can fill.

In master, if I set an item to a not for loan but holdable status ( < 0 ) I can place the hold,
capture the hold and set it to waiting, but *not* check it out to the patron!
This does not make sense. I should not be able to trap an item for checkout unless it can be checked out.

Test Plan:
1) Set an item's notforloan value to -1
2) Place a hold on that item
3) Check in the item
4) Trap the item for that hold
5) Attempt to check the item out to the patron, you will be unable to
   because it is notforloan
6) Apply this patch
7) Restart all the things!
8) Repeat steps 1-3
9) The screen should no longer ask if the item should be trapped
   to fill the hold!

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Catherine Ingram <Catherine.Ingram@cedarparktexas.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-11 08:55:05 +01:00
6ec2d13b9d
Bug 24769: (bug 14711 follow-up 2) Fix typo in parameter name for AddReserve
This patch fixes 2 other occurrences. The first one is in POD of
AddReserve, the other one fixes SIP code

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-05 11:03:41 +01:00
dcaba0ac4e
Bug 24620: Fix test, remove replaced code, use dt_from_string
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-04-14 17:24:41 +01:00
c132946cc0
Bug 24620: Close existing transfers when setting item to waiting
This patch adds a clause in ModReserveAffect to check if there
are existing transfers and close them when setting a hold to waiting

To test:
 1 - Set AutomaticItemReturn to Do
 2 - Checkin an item from Library B at Library A
 3 - Confirm item is in transfer (check the details page)
 4 - Place a item level hold for pickup at library A
 5 - Checkin the item at Library A
 6 - Confirm the hold
 7 - View the details page
 8 - Note the item is in transit and waiting
 9 - Apply patch
10 - Delete hold and repeat
11 - Confirm that transfer is closed when hold marked waiting

Signed-off-by: Sally <sally.healey@cheshirewestandchester.gov.uk>
Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-04-14 17:24:41 +01:00
6aee18111e
Bug 24547: Add more action logs for holds
It seems like ModReserveFill and ModReserveAffect should both produce action logs for holds.

Test Plan:
1) Apply this patch
2) Place a hold
3) Check in the item to trap the hold
4) Check out the item to fill the hold
5) Check the action logs for that reserve id
6) Note the new logs!

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-04-14 16:16:18 +01:00
Andrew Nugged
18ad5f5eea
Bug 24185: Make holds page fast when 'on shelf holds' set to 'If all unavailable'
When "reserve/request.pl -> C4/Reserves.pm::IsAvailableForItemLevelRequest" called many times with hundred of items and "on shelf holds" parameter set to "If all unavailable" for these items + patron, it goes slow.

It happens because in subloop it is checking if all items available so it is O(n^2) and it re-checks each time the same info for each item with repeating DB/data requests.

Fix:
The inner loop 1:1 picked out into separate subroutine and called outside of the loop, saving data in 'items_any_available' variable once, this variable passed to IsAvailableForItemLevelRequest to be used inside as the precalculated result.

This made algorithm O(n) instead of O(n^2) so there is noticeable speed increase.

How to reproduce:

1) on freshly installed kohadevbox create/import one book,
remember that biblionumber for later use it in down below,

2) add 100 items for that book for some library,

3) find some patron, that patron's card number we will
use as a borrower down below to open holds page,

4) check for the rule or set up a single circulation rule
in admin "/cgi-bin/koha/admin/smart-rules.pl",
that rule should match above book items/library/patron,
check that rule to have a non-zero number of holds (total, daily, count) allowed,
and, IMPORTANT: set up "On shelf holds allowed" to "If all unavailable",
("item level holds" doesn't matter).

5) open "Home > Catalog > THAT_BOOK > Place a hold on THAT_BOOK" page
("holds" tab), and enter patron code in the search field,
or you can create a direct link by yourself, for example, in my case it was:
/cgi-bin/koha/reserve/request.pl?biblionumber=4&findborrower=23529000686353

6) it should be pretty long page generation time on old code, densely increasing for every hundred items added. In the case of this solution, it's fast, and time increases a little only, linear.

I tested on my computer in VirtualBox for page generation times,
did 3-5 runs for same case to check if results are stable, and got such values:

(old code):
  100 items:    50 seconds
  200 items:   3.2 minutes
  300 items:   7.3 minutes

(version with fix):
  100 items:   4.4 seconds
  200 items:   7.5 seconds
  300 items:  10.4 seconds

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-25 09:40:58 +00:00
Andrew Nugged
d3a37911cf
Bug 24185: Make holds page faster: Preparatory refactoring
This is just refactoring. extracting logically independent code
to separate sub + tests update. No logic change yet.

Searching for "any_available" item among all biblionumber items was done
inside of "elsif on_shelf_holds == 2", and it is logically very independent
piece of code (this "@items" loop), it needs just biblionumber and patron
as parameters so it can be extracted into separate subroutine, and
later also called/reused from somewhere else.

This ability to call from another place also made for future patch
to remove O(n^2) problem with nested loops.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-25 09:40:39 +00:00
Andrew Nugged
edef63e5a8
Bug 24185: Make holds page faster - Call to subroutine moved below "if"
`$on_shelf_holds` was assigned before "return .. if" but not used in that code piece,
so sometimes it was useless. Moved assignment after "return .. if".

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-25 09:40:22 +00:00
8986310a60
Bug 24468: Remove unused C4::Reserves::_get_itype subroutine
Since bug 21206, C4::Reserves::_get_itype is not longer used and should
be removed.

  commit 31c29fd31f
  Bug 21206: Replace C4::Items::GetItem

UPDATE: In the meanwhile another occurrence was added to Reserves.t, but
easy to replace

Test plan:
  % git grep _get_itype
must not return any occurrences.

If one needs it, Koha::Item->effective_itemtype must be used instead.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-24 10:57:13 +00:00
9364ceba0c
Bug 24436: Add handling for reserve transfer triggers
To test:
1 - Place a hold for pickup at a different branch than you are signed in at on an item owned by current branch
2 - Check in the item
3 - Confirm the transfer
4 - Check the branchtransfers table and note the reason is 'Reserve'
5 - Set the preference CanMarkHoldsToPullAsLost to allow
6 - Place a hold on an item at branch you are singed in at
7 - Ensure you are not using strict sql modes (bug 22431)
8 - Go to Circulation->Holds to pull
9 - Mark hold as lost
10 - Check branchtransfers and confirm reason is "LostReserve"
11 - Check in item from step 1 at the correct branch
12 - Go to Circulation->Holds awaiting pickup
13 - Cancel the reserve
14 - Check branchtransfers and confirm reason is "CancelReserve"

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-20 15:59:45 +00:00
Aleisha Amohia
77c66abf7a
Bug 24688: Check the priority of the fulfilled hold when fixing priorities
Currently when fixing priorities of existing holds, priorities aren't
fixed when checking out a middle priority hold because the hold has been
fulfilled and deleted. This patch adds a check for, if the hold can't be
found, an old hold with the required reserve_id.

To test:
1. Place at least four holds on a biblio for different borrowers
2. Check out an item to the 3rd priority borrower
3. Check the existing holds table for the biblio. The priorities might be
out of order, or try clicking the dropdowns and there might be an empty
dropdown option, or open the browser developer tools and notice the option
values in the select are missing value=3.
4. Apply patch and restart memcached
5. Check out an item to the 2nd priority borrower
6. Check the existing holds table for the biblio. Confirm the priorities
have been fixed and are now consecutive with no gaps.

Sponsored-by: Chartered Accountants Australia and New Zealand
Signed-off-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-20 15:41:31 +00:00
04de40cdcf
Bug 24721: Update emailLibrarianWhenHoldIsPlaced to use inbound_email_address
This patch updates the emailLibrarianWhenHoldIsPlaced functionality to
use the inbound_email_address method to obtain the correct email address
for a branch.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-19 09:32:57 +00:00
515df43af8
Bug 21944: Improve efficiency of code
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-19 11:24:48 +00:00
76a4753dfe
Bug 24367: Remove warnings from Search.t
* Set defaults for index to 'kw'
* Check that priority is defined before using it.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-19 11:20:15 +00:00
11b44869d9
Bug 14711: Change prototype for AddReserve - pass a hashref
The number of parameters of AddReserve makes it hard to read and
maintain.
This patch replace it with a hashref, which will make the calls more
readable.

Moreover the bibitems has been removed as it was not used by the
subroutine.

Test plan:
- Make sure the tests pass
- Read the diff and search for typos
- Place a hold on few items

Note for QA: reservation_date and expiration_date do not match the DB column's names,
should we?

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-11 14:32:47 +00:00
585001c6d3
Bug 24529: Silence another uninitialised value warning
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-05 12:38:33 +00:00
1069bc2405
Bug 24529: Silence warnings for title level holds
This patch simply adds a test for itemnumber being defined in the
reserve before comparing it to silence the warning.

Test plan:
1) Prior to patch, run prove t/db_dependant/Circulation.t and note lots
   of `Use of uninitialized value in numeric eq (==) at
   /kohadevbox/koha/C4/Reserves.pm line 790` warnings.
2) Apply the patch, re-run the test and note the tests still pass but
   the warnings are now gone.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-05 12:38:25 +00:00
Joonas Kylmälä
0401d13d2c
Bug 18936: (follow-up) Make possible to have unlimited value for holds_per_day
Without this it is not possible to make for a specific patron category
or itemtype a more specific unlimited holds_per_day rule if you have a
catch all rule (categorycode = *, itemtype = *) with limited
holds_per_day value.

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-04 09:56:29 +00:00
25848e5af3
Bug 18936: Fix some more tests
* CanItemBeReserved
Prior to "Bug 18936: Convert issuingrules fields to circulation_rules",
GetHoldRule returned holds_per_record even if no reservesallowed was
defined. This change restores this behavior.
FIXME Note: In GetHoldRule we return itemtype only if reservesallowed is set,
not sure it is correct.

* t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
When setting returnbranch, holdallowed and hold_fulfillment_policy, we
should not provide categorycode.

* t/db_dependent/Holds.t
Prefer to keep the existing rules instead of removing them. It got quite
hard to understand what was going on here because of the mixup with
the rule reservesallowed that was in issuingrules, and the other rules
we used for the tests. Also, categorycode should not be passed to set
those 3 rules (holdallowed, hold_fulfillment_policy and returnbranch)

* t/db_dependent/Circulation.t
Setting lengthunit to 'hours', no need to make sure the rule has been
correctly be saved

* t/db_dependent/Circulation/CalcDateDue.t
It uses hardcoded data that is not in the sample data (categorycode=C).
Let use K that exists and postpone a refactore of the whole script (to
make it create the data it needs).

* t/db_dependent/Circulation/ReturnClaims.t
* t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t
Simple replace Koha::IssuingRule with Koha::CirculationRules

* t/db_dependent/Koha/Charges/Fees.t
=> FIXME Still failing, stuck here, need help

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-04 09:56:27 +00:00
72442dc8d2
Bug 18936: More fixes
Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-04 09:56:26 +00:00
Jesse Weaver
f2dfa17f0e
Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules
This necessitates moving the circ rules from using '*' to using
undef/NULL.

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-04 09:56:24 +00:00
58fda20e85
Bug 18936: Convert issuingrules fields to circulation_rules
Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-04 09:23:55 +00:00
Julian Maurice
0e73e723ef
Bug 19809: Re-allow to call Koha::Objects::find in list context
and remove 'scalar' keyword in calls where it's not needed.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-01-23 10:27:28 +00:00
1906a69e51
Bug 20567: Count found holds for holds limit
Signed-off-by: Hans Palsson <hans.palsson@hkr.se>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-01-15 13:48:10 +00:00
094f0623c2
Bug 24361: (bug 24217 follow-up) Fix several warnings in C4 modules
With bug 24217 pushed, lot of warnings appears during the tests are run.
Most of them are "Use of uninitialized value in "

Test plan:
Take a look at the output of run 1049
https://jenkins.koha-community.org/job/Koha_Master_D9/1049/consoleFull

Most of the warnings from this run will be removed by this patch

At least 2 are not fixed:
Use of uninitialized value in numeric eq (==) at /kohadevbox/koha/C4/Reserves.pm line 791.
t/db_dependent/Items_DelItemCheck.t => see 23463

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-01-07 13:59:51 +00:00
Agustin Moyano
3b25ea7c94
Bug 22284: Add "patron's hold group" as new hold_fulfillment_policy option
This patch adds "patron's hold group" as a new option to Hold pickup library match

To test:
1. Set ReservesControlBranch preference to item.
2. Create a hold group
3. Go to circulation and fines rules
SUCCESS => in 'Hold pickup library match' there is a new option called "patron's hold group"
4. In a library not in hold group set 'Hold policy' to "any" and 'Hold pickup library match' to "patron's hold group"
5. Search for a user in the hold group
6. 'Search to hold' for items of the library of step 4
7. Select an item and 'Place hold for [user]'
SUCCESS => in 'Pickup at' you should see patron's hold group as options
8. In OPAC sign in as the same user of step 5
9. Search for the item in step 7
SUCCESS => in 'Pick up locations' you should see patron's hold group as options
10. Sign off

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-01-03 12:58:05 +00:00
Agustin Moyano
da620fdf91
Bug 22284: (follow-up) Squash multiple follow-ups
* Bug 22284: (follow-up) Remove commented warn and address test failures
  * Bug 22284: (follow-up) fix test count after merge
  * Bug 22284: (follow-up) fixes after 15496
  * Bug 22284: (follow-up) fixes after 18936
  * Bug 22284: (follow-up) Remove HomeOrHolding from reserves

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-01-03 12:58:05 +00:00
Agustin Moyano
a998ba5714
Bug 22284: Control hold groups in C4::Reserves
This patch modifies C4::Reserves to control when hold group options where selected
in smart rules.
In CanItemBeReserved adds 2 new error status messages
1) branchNotInHoldGroup: when a patron's homebranch is not in item's hold group
2) pickupNotInHoldGroup: when a selected pickup location is not in item's hold group
Also CheckReserves is modified when item's priority is defined, to control by hold
group when required.
Finally, IsAvailableForItemLevelRequest was also modified to control by hold group when
required.

To test:
1) Apply this patch
2) prove t/db_dependent/Holds.t
SUCCESS => Result: PASS
3) Sign off

Sponsored-by: VOKAL
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-01-03 12:58:03 +00:00
dc5fe323d2
Bug 24217: use Modern::Perl for modules when strict is missing
We absolutely need that for modules.
We are enabling strict by using Modern::Perl.
Note that other modules from C4 use strict and warnings, instead of
Modern::Perl

Test plan: git grep 2505 **/*.pm should not return any results

And let's see later what needs to be fixed.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-01-03 10:45:46 +00:00
0f4bc4fd81
Bug 23964: (follow-up) We should check for only active holds when determining to set waiting
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-11-25 16:38:16 +00:00
f847cfdd7e
Bug 23964: (follow-up) Add comments and improve readability
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-11-21 11:27:12 +00:00
e083287048
Bug 23964: ReservesNeedReturn should only apply to available items
To test:
 1 - Checkout an item to a patron
 2 - Make sure 'ReservesNeedReturn' is set to 'Automatically'
 3 - Place an item level hold on the checked out item
 4 - Note the hold is marked waiting
 5 - Delete the hold
 6 - Apply patch
 7 - Place a new hold
 8 - Hold is not marked waiting
 9 - Please a hold on a different item
10 - Note it is marked waiting
11 - Test when item is damaged and hold not allowed on damaged items
12 - Test when item is in transit
13 - Test when item has another hold
14 - Only in the case where none of the above are true should the hold be marked waiting

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-11-21 11:26:53 +00:00
17d2211446
Bug 23985: (bug 21206 follow-up) Fix checkout list when "Hold pickup library match" not set
This patch restores the behavior prior to bug 21206.
If "Hold pickup library match" is "Not set", then
hold_fulfillment_policy equals an empty string.

Test plan:
1. Go to "Circulation and fines rules"
2. Under "Default checkout, hold and return policy", unset "Hold pickup
library match" and Save
3. Place a hold on one item for one patron
4. Try to checkout the same item with another patron

=> Without this patch you the checkout list do not show, and the logs
contain "The method Koha::Item-> is not covered by tests!"
=> With this patch applied you see the checkout list

QA will take care of comparing the statement with the one before bug 21206
and make sure they are equivalent

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-11-13 14:01:08 +00:00
950074fa7e
Bug 23049: Update debit_type_code 'Res' to 'RESERVE'
This patch updates all cases where debit_type_code was 'Res' to
'RESERVE' which is more informative and consistent with the new style
guidelines.

The patch also ensure all calls to add_debit with a type or 'reserve'
now pass the capitalised code instead.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-10-24 17:24:18 +01:00
Julian Maurice
95174eb36a
Bug 22922: Allow reservedate changes only if AllowHoldDateInFuture is on
Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-10-21 10:00:49 +01:00
Julian Maurice
8d047f2a33
Bug 22922: Allow to modify hold dates on reserve/request.pl
Test plan:
1. Place some holds
2. Go to the 'Holds' tab of the biblio record
3. Modify dates in the table and click on 'Update holds' button
4. Verify that dates have been correctly updated

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-10-21 10:00:36 +01:00
0a14f4009d
Bug 22610: Update SIP2 to use payment_type
The SIP2 interface has long used a set of accounttypes to denote payment
types. Now we have an authorised list of payment_types and a
payment_type field in accountlines this patch alters the logic to use
it.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-08-05 15:02:54 +01:00
3352d75bfe
Bug 14549: POD and variable cleanup
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-08-05 11:39:24 +01:00
36ee00ed95
Bug 14549: Check patrons reserves for any holds on a bib when checking out an item
MoveReserve uses CheckReserves to see if the current patron has any holds
on the title they are checking out, however, CheckReserves doesn't return
all holds on a biblio, it returns holds on the item from the holdsqueue
if they exist

This can create a condition where we check holds on an item, find we have it planned for
another borrower, confirm checkout to the current borrower, but don't fill their hold

To test:
  1) Find record
  2) place record level holds for 2 different patrons (record level)
  3) Run holds queue builder, check the queue to confirm an item was selected for patron 1
     Circulation->Holds queue->Library="All"
  4) Check out the item queued for patron with priority 1 to the second patron
  5) You should be asked to confirm, do so
  6) Note the item checks out, but both holds remain
  7) Apply patch
  8) Check in the item
  9) Don't confirm the hold
 10) Check the holds on the record and the holds queue
 11) Patron 1 should be priority 1 with an item selected from the holds queue
 12) Checkout to patron 2 as before
 13) Note the hold for patron 2 is filled this time
 14) Prove -v t/db_dependent/Reserves.t

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-08-05 11:39:15 +01:00
4f7f55e5fc
Bug 21181: Make cancel_expired_reserves.pl Cancel all holds not waiting if ExpireReservesMaxPickupDelay not set
To test:
0 - Make sure ExpireReservesMaxPickupDelay is set to 'Don't allow'
1 - Place a hold for a patron
2 - Check in at another library to set it in transit
3 - Update expirationdate of hold to be in the past
    UPDATE reserves SET expirationdate='1999-12-31'
4 - Run cancel_expired_holds.pl
5 - Hold is not canceled
6 - Apply patch
7 - Run again
8 - Hold is cancelled

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-07-05 08:39:23 +01:00
fd5d274d3a
Bug 9834: Preserve biblio level hold preference on reverting waiting status
When the waiting status of a hold is revert we want to preserve the
level hold preference of the user.

Test plan:
Place a hold at biblio level
Confirm the hold
Revert the waiting hold

=> The hold must still be considered as a biblio level hold ("next
available item")

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-06-15 08:00:28 +01:00