Commit graph

457 commits

Author SHA1 Message Date
cae45797b8
Bug 30856: Remove C4::Reserves::CanReserveBeCanceledFromOpac
This subroutine can easily be replaced and is not really needed.

Test plan:
No changes expected, try to suspend/resume holds from the OPAC

Note that you cannot affect somebody's else holds.

Note for QA: The extra fetch of Koha::Hold will be removed on bug 37868.

Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-09-16 13:47:07 +02:00
860d3967ae
Bug 17729: Replace IsItemOnHoldAndFound
This subroutine can easily be replaced with
$item->holds->filter_by_found->count \o/

Test plan:
Confirm that the old sub and $item->holds->filter_by_found->count
produce the same query

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-09-16 10:41:02 +02:00
Julian Maurice
c951b02182
Bug 35959: Fix C3 merge of Koha::Old::Hold
Test plan:

Run `perl -c Koha/Old/Hold.pm`
It should print 'syntax OK'

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-07-05 15:48:04 +02:00
975a869c49
Bug 37155: Remove use of unblessed patron
This removes the unblessing of the patron object and uses fields form the patron

To test:
Confirm tests still pass:
prove -v t/db_dependent/Holds.t t/db_dependent/Circulation.t t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t t/db_dependent/Reserves.t t/db_dependent/api/v1/holds.t

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-07-01 18:55:51 +02:00
11c5abda48
Bug 37155: Refactor GetAgeRestrictions
This routine currently takes the agerestriction value from biblioitems and an unblessed borrower object
and uses the date of birth to calculate whether the ptrons DOB is before or after the minimum value required
against the age restriction

We have a routine in the patron object to get the patron's age - we cna use this against the parsed agerestriction
value in a simple comparison and remove the need to unbless and pass the patron.

FIXME: We should move this to a biblioitems or biblio object method

To test:
0 - In Admin -> Koha to MARC mapping, set biblioitems.agerestriction to 521,a
1 - Set syspref AgeRestrictionMarker to 'Age'
2 - Edit a record and set 521$a to 'Age 14'
3 - Add an item or copy the barcode of the item on that record
4 - Attempt to checkout item to Lisa Charles in sample data, or a 15 year old patron
5 - It should checkout fine
6 - Check in item
7 - Edit patron  Joyce Gaines to set age to 13 DOB:06/20/2011, or create a 13 year old patron
8 - Attempt to checkout item
9 - Item is blocked
10 - Apply patch
11 - Repeat tests, confirm no change

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-07-01 18:55:50 +02:00
Emily Lamancusa
dc00e55a32
Bug 34972: Remove GetOtherReserves
GetOtherReserves attempts to set the waiting/transit status for the next
hold on the list when applicable, but in practice it either leaves the
hold state unchanged, or sets the itemnumber without setting the found
status (erroneously converting bib-level holds to item-level holds).

The latter situation only occurs when the user has been prompted to
confirm, cancel, or revert the hold, and is able to ignore the prompt.
In those situations, the hold's state should not change.

GetOtherReserves does not need to change the hold state, and it does not
do so correctly. Besides that, it does not do much other than call
CheckReserves, and is only used in 3 places.

This patch removes GetOtherReserves, and refactors returns.pl and
C4::Reserves::ModReserveCancelAll to call CheckReserves directly instead.

To test:
1. Place 2 bib-level holds for 2 different patrons (Patron A and Patron
    B) on the same record, both for pickup at the logged-in library
2. Check in an item from that record to fill Patron A's hold
3. Set the hold's expiration date to yesterday by accessing the database
    in the command line:
    - In a ktd shell prompt, open the db client with koha-mysql kohadev
    - UPDATE reserves
        SET expirationdate = DATE_SUB(CURDATE(), INTERVAL 1 DAY)
        WHERE borrowernumber = <Patron A's borrowernumber>
4. Go to Circulation > Holds Awaiting Pickup, and find the hold in the
    "holds waiting past their expiration date" tab
5. Click the "Cancel hold" button in the Actions column next to the hold
   (do not check in the book)
6. Return to the bib record and look at Patron B's hold
--> Note that Patron B's hold is now an item-level hold and does not
    have a waiting status

7. Cancel Patron B's hold
8. Place 2 new holds on the record: one for Patron A at the logged-in
    library, and one for Patron B at a different library
9. Check in an item to fill Patron A's hold
10. Repeat steps 3-5 to expire and cancel Patron A's hold
11. Return to the Holds tab of the bib record and look at Patron B's hold
--> Note that Patron B's hold is now an item-level hold, and there is no
    "Revert transit status" button

12. Place 2 bib-level holds for 2 different patrons (Patron A and Patron
    B) on the same record, both for pickup at the logged-in library
13. Check in an item from that record to fill Patron A's hold
14. Check in the same item again. A modal will pop up, saying that the
    hold is already waiting
15. In the modal, choose a cancellation reason and click "Cancel hold"
--> A new modal will pop up to fill Patron B's hold
16. Click "Ignore" on the modal for Patron B's hold
17. Return to the bib record and look at Patron B's hold
--> Note that Patron B's hold is now an item-level hold and does not
    have a waiting status

18. Apply patch
19. Repeat steps 1-6
--> Note that Patron B's hold is still a bib-level/"next available" hold
20. Repeat steps 7-11
--> Note that Patron B's hold is still a bib-level/"next available" hold
21. Repeat steps 12-17
--> Note that Patron B's hold is still a bib-level/"next available" hold

Make sure correct behavior is unchanged:

22. Cancel Patron B's hold
23. Place 2 new holds on the record: one for Patron A at the logged-in
    library, and one for Patron B at a different library
24. Check in an item from that record to fill Patron A's hold
25. Check in the same item again. A modal will pop up, saying that the
    hold is already waiting
26. In the modal, choose a cancellation reason and click "Cancel hold"
--> A new modal will pop up to fill Patron B's hold
27. Click "Print slip, transfer, and confirm" on the modal for Patron B's hold
--> Confirm that the information on the slip is correct
--> Confirm that the hold is correctly put in transit

22. Set HoldsAutoFill and HoldsAutoFillPrintSlip to "Do"

23. Place a bib-level hold for the logged-in library
24. Check in an item from that bib
--> Confirm the information on the slip is correct
--> Confirm the hold is correctly assigned and set to waiting
25. Place a bib-level hold for a different library
26. Check in an item from that bib
--> Confirm the information on the slip is correct
--> Confirm the hold is correctly put in transit
27. Change the logged-in branch to match the hold pickup location
28. Check the item in
--> Confirm the information on the slip is correct
--> Confirm the hold is correctly assigned and set to waiting

29. Repeat steps 22-26
--> Confirm a correct hold slip pops up for Patron B's hold
--> Confirm that Patron B's hold is correctly put in transit
30. Cancel Patron B's hold
31. Place 2 bib-level holds for 2 different patrons (Patron A and Patron
    B) on the same record, both for pickup at the logged-in library
33. Repeat steps 24-26
--> Confirm a correct hold slip pops up for Patron B's hold
--> Confirm Patron B's hold is correctly set to Waiting

34. Prove t/db_dependent/Circulation.t
35. Prove t/db_dependent/Koha/Holds.t
--> Tests pass

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-05-07 15:53:42 +02:00
4576eb73da
Bug 25159: Implement diffs in action logs for holds
Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Restart all the things!
4) Enable HoldsLog
5) Perform various hold related actions
6) Observe the diff column is populated by a JSON string
   of the diff format generated by Struct::Diff

Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-05-02 16:47:43 +02:00
1fe9180c41
Bug 34032: (QA follow-up) Tidy code
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-04-29 09:35:38 +02:00
Emmi Takkinen
03efe5ba76
Bug 34032: Set new expirationdate if waiting status is reverted
When one reverts holds waiting status holds expiration
date is not set even if DefaultHoldExpirationdate
syspref is enabled. This patch adds new param
hold_reverted to be used when RevertWaitingStatus is
used to determine if expiration date should be set again.

To test:
1) Make sure you have DefaultHoldExpirationdate syspref enabled.
2) Find hold with status "Waiting".
3) Revert waiting status.
=> Note that hold has no expiration date set.
4) Apply this patch.
5) Repeat steps 2 and 3.
=> Expiration date should now be set based on reserve
date.

Also prove t/db_dependent/Hold.t.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Esther <esther@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-04-29 09:35:37 +02:00
4e42f1182d
Bug 35491: Add logging to RevertWaitingStatus
This patch simply adds a logaction line to RevertWaitingStatus

To test:
1 - Enable HoldsLog
2 - Place a hold
3 - Fill the hold
4 - Revert the waiting status
5 - Note there is no action log added
6 - Apply patch
7 - Repeat 2-4
8 - Confirm you now have a MODIFY action logged for the reversion

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2023-12-19 13:42:13 +01:00
Aleisha Amohia
30c375ae8b
Bug 17617: Confirmation email to patron when hold is placed
This enhancement adds a new notice HOLDPLACED_PATRON that will be sent to a patron when a hold is placed for them. It depends on the new system preference EmailPatronWhenHoldIsPlaced to be enabled.

To test:
1) Update database and restart services
2) Go to Koha Administration -> System preferences and search for the new EmailPatronWhenHoldIsPlaced syspref. It should be disabled by default - leave it disabled for now.
3) Search for a record and go to the Holds tab. Place a hold for your patron.
4) Go to your patron's account and go to the Notices tab. Confirm the HOLDPLACED_PATRON notice was NOT queued.
5) Enable the EmailPatronWhenHoldIsPlaced syspref.
6) Repeat steps 3 and 4. Confirm the HOLDPLACED_PATRON notice WAS generated and queued.
7) Confirm tests pass t/db_dependent/Holds.t

Sponsored-by: Fire and Emergency New Zealand
Signed-off-by: Kelly <kelly@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-11-08 11:41:27 -03:00
3ec73d80e2
Bug 17798: Confirm hold when printing slip from another patron's account
This patch adds a few pieces of information to the print slip button
and makes the code confirm the hold

As we are printing before the confirm, we also add the ability to pass
in the itemnumber to 'ReserveSlip'

This is slightly hacky, however, I don't see another way to allow
printing without an additional page reload.

To test:
 1 - Place a title level hold for patron A, for delivery to library B
 2 - Attempt to checkout an item from the record above to Patron B from
     library A
 3 - You receive an alert about the hold
 4 - Click "Don't check out, confirm hold, and print slip"
 5 - Confirm the slip looks correct and has item info
 6 - Confirm that item is in transit to fill hold
 7 - Revert transit status
 8 - Attempt to checkout the item to Patron B from Library B
 9 - Click "Don't check out, confirm hold, and print slip"
10 - Confirm slip is correct
11 - Confirm item is marked waiting

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-11-06 08:42:41 -03:00
fde91e178d
Bug 8838: Add digest option for HOLD notice
Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Restart all the things!
4) Enable the new digest option for "Hold filled" messages
5) Trap multiple holds for a patron
6) Note a single notices is generated for all the trapped holds!

Signed-off-by: George Williams <george@nekls.org>
Signed-off-by: Laura ONeil <laura@bywatersolutions.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-10-25 11:07:48 -03:00
795c60e577
Bug 35027: Add 'hold' to patron activity triggers
This patch adds 'hold' to the list of triggers available for tracking
patron activity.

Test plan
1) Select 'Placing a hold on an item' in the
   TrackPatronLastActivityTriggers system preference
2) As a staff member, place a hold on any item for a test user
3) Confirm that the borrowers.lastseen field is updated for that test
   borrower

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-10-24 10:05:25 -03:00
e9d8cc08c5
Bug 30825: Move holds_control_library to Koha::Policy::Holds
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-09-22 12:35:46 -03:00
752fb21b47
Bug 30825: Remove GetReservesControlBranch in favour of Koha::Item->holds_control_library
This patch removes the GetReservesControlBranch method, and replaces its
uses with the newly introduced method.

To test:
1. Apply this patch
2. Verify that placing holds from the OPAC works
=> SUCCESS: Things work as expected
3. Run:
   $ kshell
  k$ prove t/db_dependent/Reserves* \
           t/db_dependent/Hold* \
           t/db_dependent/Koha/Hold* \
           t/db_dependent/Koha/Biblio.t
=> SUCCESS: Tests pass!
4. 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: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-09-22 12:35:46 -03:00
72bfb416d3
Bug 34666: Combine queries in _Findgroupreserve
The queries here are the same except for 2 differences:
1 - They check if the hold was on a particular item
2 - The latter confirms that the reserve item group matches the item's item group

For 1, it doesn't matter - only 1 item can be mapped ot a reserve, itemnumber is the primary key
for hold_fill_targets - so we are either matching it in the first query or the second, either way we get the same
reserve - the returns are the same so we don't care which query it came from

For 2, this has already been checked when the queue was built. We don't need to verify the match because
it wouldn't be in the targets if they didn't match

To test:
1 - Apply second unit test patch
2 - prove t/db_dependent/Reserves.t
3 - It should pass
4 - Apply this patch
5 - prove t/db_dependent/Reserves.t
6 - It continues to pass

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-09-08 11:54:53 -03:00
fe3872f628
Bug 34666: Allow item_group to be null and still match
The current logic requires that the grop ids match, but this eliminates null matches
from the group.

The fallout essentially is that the queue won't be checked to fill holds in cases of title level matches
where holds don't have an item group id. The queue checks the transport cost matrix while the check reserves
check does not, so this may have an impact on holds costs and delivery times

To test:
0 - Apply unit test patch
1 - prove -v t/db_dependent/Reserves.t
2 - It fails
3 - Apply this patch
4 - prove -v t/db_dependent/Reserves.t
5 - It passes!

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-09-08 11:54:52 -03:00
Hammat Wele
a09a926458
Bug 30846: 'If any unavailable' doesn't consider negative notforlan values as unavailable
When we set up a circulation rule where 'On shelf holds allowed' is 'If any unavailable' and we have a record with one 'Ordered' item, we cannot place this item on hold.

This patch allows placing hold on item with negative not for loan values, when using rule with 'On shelf holds allowed' set to 'If any unavailable'

To test:

1. Set up a circulation rule where on shelf holds are not allowed and force the choosing of an item (to facilitate the test)
    1.1. Go to Administration > Circulation and fines rules
    1.2. In the matrix, add a circulation like this
          - Patron category: All
          - Item type: Books
          - Current checkouts allowed: 10
          - Current on-site checkouts allowed: 10
          - Loan period: 21
          - Holds allowed (total): 10
          - Holds allowed (daily): 10
          - Holds per record (count): 10
          - On shelf holds allowed: If any unavailable
          - OPAC item level holds: Force
    1.3. Click Save
2. Create a record with one 'Ordered' item (or any negative value not for loan status)
    2.1. Go to Cataloging
    2.2. Click New record
    2.3. Fill out the mandatory fields (by default in MARC21: 000, 003, 005, 008,  040, 245, and 942 (942 should be set to Books))
    2.4. Click Save
    2.5. Fill out the following item fields
          - Not for loan: Ordered
          - Koha item type: Books
    2.6. Click Add item
    2.7. Click Normal to go to the detailed record
3. Try to place a hold on the 'Ordered' item
    3.1. From the detailed record, click OPAC view: Open in new window.
    --> Note that the 'Place hold' option is not present
4. Add a second 'Available' item
    4.1. Back in the staff interface tab with the detailed record, click New > New item
    4.2. Make sure the item type is set to Books
    4.3. Add a barcode in p
    4.4. Click Add item
5. Try again to place a hold on the 'Ordered' item
    5.1. Go back to the OPAC tab and refresh the page
    --> Note that the 'Place hold' option is still not present
6. Check out the available item to a patron
    6.1. In the staff interface tab, copy the barcode from the available item
    6.2. Go to Patrons
    6.3. Click on Search
    6.4. Click Check out next to one of the patrons
    6.5. Paste the barcode in the box and click Check out
7. Try again to place a hold on the 'Ordered' item
    7.1. Go back to the OPAC tab and refresh the page
    --> Note that the 'Place hold' option is now present
    7.2. Click Place hold
    --> Note that only the checked out item is available to place on hold, if you click Show unholdable items, it will show the Ordered item, but you can't place a hold on it.
8. Apply the patch
9. Go to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
        --> Note that the 'Place hold' option is still present
        9.1. Click Place hold
        --> Note that you can now place a hold on the 'Checked out' or the 'Ordered' item.
10. Check in the item to make it available again
    10.1. In the staff interface tab, click on 'Show checkouts' button
    10.2. Select the Checked out item and click on 'Renew or check in selected items' button.
11. Try again to place a hold on the 'Ordered' item
    11.1. Go back to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
    --> Note that the 'Place hold' option is still present
    11.2. Click Place hold
    --> Note that only the 'Ordered' item is available to place on hold, if you click Show unholdable items, it will show the Available item and you can't place a hold on it.
12. Delete the available item to keep only the Ordered item
    12.1 in the staff interface tab, click on 'Search catalog' and search for the record
    12.2 click on 'Edit' then 'Edit items'
    12.3 Delete the available item
13. Try to place a hold on the remain 'Ordered' item
    13.1 Go back to the OPAC tab and click on the book title right next to 'Place a hold on' checkbox to go back to the record details.
        --> Note that the 'Place hold' option is present
    13.2. Click Place hold
    --> Note that you can place a hold on the Ordered item.

Signed-off-by: Amaury GAU <amaury.gau@bulac.fr>
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-08-15 15:14:11 +03:00
08913eeda1
Bug 34178: (QA follow-up) Tidy
Tidy the relevant lines to pass the new QA rules

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-07-19 13:00:44 -03:00
105750acb1
Bug 34178: Cache ItemsAnyAvailableAndNotRestricted in memory and don't precalculate
There are several places in the code where we precalculate ItemsAnyAvailableAndNotRestricted to avoid
looping on this routine when calling IsAvailableForItemLevelRequest on a list of items form a biblio

The value of ItemsAnyAvailableAndNotRestricted is only used when there is a circulation rule for
'onshelfholds' with a value of '2' (If all unavailable)

Rather than calculate a value that may never be used, let's cache this value per request when we do
calculate it - and reuse the cached value

To test:
 1 - Apply patch
 2 - Set circulation rule 'On shelf holds allowed' as 'If all unavailable'
    make sure the rule applies to all of the items/patrons you test with
 3 - Find a record with two items that are available
 4 - Try to place a hold for a patron - not allowed
 5 - Check out one item to another patron
 6 - Attempt hold - still not allowed
 7 - Check out second item to another patron
 8 - Attempt hold - allowed!
 9 - Apply patch
10 - Cancel and replace hold - it is allowed!
11 - Check in one item, and cancel hold
12 - Place hold - not allowed!
13 - Check in second item
14 - Place hold - not allowed!
15 - prove -v t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-07-19 13:00:42 -03:00
45852c950e
Bug 30860: Cache CanItemBeReserved return value
This patch caches the return value of CanItemBeReserved that could
be then returned *on
demand*
We don't want to introduce side-effects hard to catch from this simple
change, so let's return the cache value only from the 2 scripts we are
dealing with.

This patch requests all item values from CanBookBeReserved on request.pl

Before this we either:
- Looped every item to find out that book could not be reserved
- Looped until we found an item that could be reserved, then looped all items to get statuses

In the worst case we avoid double processing a single item, in the best case we avoid double
processing all items (if only last on record is holdable)

To test:
1 - Find a record in staff client with several items
2 - Set AllowHoldsOnDamagedItems  to 'Dont allow'
3 - Add a damaged item to record
4 - Set a hold rule to only allow holds form homebranch and ensure record has items from other branches
5 - Setup things to prevent more items from being held
6 - Attempt hold for patron
7 - Note item statuses
8 - Apply patch
9 - Confirm statuses are as they were before

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-06-23 10:01:07 -03:00
d0195cef6c
Bug 33791: (QA follow-up) Stick to 'item_id' for the parameter name
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-05-22 14:46:30 -03:00
bd5c5eaa38
Bug 33791: Pass itemnumber to $hold->fill
Test plan:

Without this patch:
Place next available level on some book for patron A.
Checkout this book directly to patron A.
Check old_reserves table for this reserve; does not have itemnumber.

With this patch:
Do the same.
In old_reserves the itemnumber should be saved.
Run again t/db_dependent/Koha/Hold.t. Should pass.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-05-22 14:46:30 -03:00
David Gustafsson
ddc2906b77
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines
To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.t
  t/db_dependent/Koha/Template/Plugin/CirculationRules.t
  t/db_dependent/Letters/TemplateToolkit.t
  t/db_dependent/Members/GetAllIssues.t
  t/db_dependent/Members/IssueSlip.t
  t/db_dependent/Patron/Borrower_Discharge.t
  t/db_dependent/Patron/Borrower_PrevCheckout.t
  t/db_dependent/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/SIP/ILS.t
  t/db_dependent/Holds.t
  t/db_dependent/Holds/LocalHoldsPriority.t
  t/db_dependent/Holds/HoldFulfillmentPolicy.t
  t/db_dependent/Holds/HoldItemtypeLimit.t
  t/db_dependent/Circulation/transferbook.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-05-12 12:40:21 -03:00
Janusz Kaczmarek
e4b94e4d82
Bug 33210: (Bug 31963 follow-up) No hold fee message on OPAC should be displayed when there is no fee
After resolving Bug 31963 everything works as expected when there is
hold fee defined (!= 0).  But in case when the fee for given patron
category is set to 0.00, the user will always see the message "You will
be charged a hold fee of 0,00 ...", which is obviously not intended.

This is because categories.reservefee is returned from database as
'0.000000' and as such, without type casting, is interpreted as string
in Perl. Prior to Bug 31963 the result was compared to 0 before sending
anything to the template, so the casting was done, now it is not.

To test:
========
1. Go to Administration -> Patron categories
2. Edit your patron category and give a hold fee of 0.
3. HoldFeeMode does not matter - you can set it to any value.
4. In another tab, open the OPAC.
5. Search the OPAC for any record with an item.
6. Go to place a hold on this record.  You will see "You will be
   charged a hold fee of 0,00" --> This is a bug.
7. Apply patch and restart services.
6. Repeat step 6.
8. You should NOT see the hold fee message.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-04-21 10:36:27 -03:00
ed30887924
Bug 32455: Use from_email_address for 'from' field for hold notices
This patch updates two occurrences where the inbound library email is
used as the from address

To test:
 1 - Set a unique 'Email' and 'Reply to' address for a library
 2 - Find a patron of that library, ensure they have an email
 3 - Ensure their messaging preference for holds is 'email'
 4 - Set system preference ReservesMaxPickupDelay to -1
 5 - Set system preference  ExpireReservesMaxPickUpDelay to Allow
 6 - Set system preference ExpireReservesAutoFill to Do
 7 - Place and fill a hold for that patron at that library
 8 - Check the patron's notification tab, confirm the from address is the 'Reply to'
 9 - Place a hold for another patron on the same item
10 - Run the expired holds cronjob:
     perl misc/cronjobs/holds/cancel_expired_holds.pl --reason=whatever
11 - Check the message_queue - notice the from address is the 'reply to'
12 - Apply patch
13 - Repeat 1-11, confirm the from addresses are correct now

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2023-02-07 10:26:02 -03:00
8cc9c705b1
Bug 24860: Skip non-matching item group holds in CheckReserves
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-11-04 19:39:56 -03:00
8efe22b770
Bug 24860: Add ability to select an item group when placing a hold
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-11-04 19:39:55 -03:00
9670e71b00
Bug 24860: Implement reserves.item_group_id
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-11-04 19:39:55 -03:00
0f9fcb1087
Bug 29102: Remove ignore_found_holds
If not counting patrons holds, found or unfound, we no longer need this option
introduced by bug 28078

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-11-04 19:20:25 -03:00
Aleisha Amohia
e1a02dde8f
Bug 31963: Only show hold fee msg on OPAC if patron will be charged
This patch ensures HoldFeeMode is considered when displaying a message
to patrons on the OPAC that says they'll be charged a hold fee when
placing or collecting the hold.

When HoldFeeMode is set to not_always or "only if all items are checked
out and the record has at least one hold already" then the hold fee
message should only show if all items on the record are checked out, AND
the record has at least one hold already - both of these conditions must
be met.

To test:
1. Go to Administration -> Patron categories
2. Edit your patron category and give a hold fee of $1.
3. Go to Administration -> System preferences and search for
HoldFeeMode. Set to 'only if all items are checked out and the record
has at least one hold already' if not already set. Keep this tab open.
4. In another tab, open the OPAC.
5. Search the OPAC for a record with one item which is NOT checked out.
6. Go to place a hold on this record. Confirm you see a message saying
that you will be charged a hold fee, even though not all items are
checked out and the record does not have a hold --> This is the bug.

7. Apply patch and restart services.

Items available, no holds placed

8. Repeat steps 5-6. This time, you should NOT see the hold fee message.

Items available, holds placed

9. In your staff interface tab, find the same record.
10. Place a hold for a different patron on this record.
11. In your OPAC tab, find this record again and go to place a hold. You
should NOT see the hold fee message.

No items available, no holds placed

12. In your staff interface tab, cancel the hold placed on this record.
13. Check out the item to a different patron.
14. In your OPAC tab, find this record again and go to place a hold. You
should NOT see the hold fee message.

No items available, holds placed

15. In your staff interface tab, keep the item checked out to another
patron.
16. Place a hold for a third patron on this record.
17. In your OPAC tab, find this record again and go to place a hold. You
SHOULD see the hold fee message.

Multiple holds

18. Search the OPAC for a record. Make sure your search will return more
than one result, including our test record.
19. Check the checkbox for our test record, plus another record where
the item is not checked out.
20. Click the Place hold button to place holds on all of our selected
records. You should only see the hold fee message above our test record.

21. In your staff interface tab, test setting HoldFeeMode to the other
values and confirm the hold message shows on the OPAC as expected.
22. Confirm tests pass t/db_dependent/Reserves/GetReserveFee.t

Sponsored-by: Horowhenua Libraries Trust

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-11-02 19:40:23 -03:00
b30d2363b6
Bug 30718: Fix imports
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-08-19 10:12:06 -03:00
48bf9b1d91
Bug 30718: Use flatpickr's altInput
The idea rely on the KohaDates TT plugin for the date formatting. We
should not have any output_pref calls in pl or pm (there are some
exceptions, for ILSDI for instance).

Also flatpickr will deal with the places where dates are inputed. We
will pass the raw SQL value (what we call 'iso' in Koha::DateUtils), and
the controller will receive the same value, no need to additional
conversion.
Note that DBIC has the capability to auto-deflate DateTime objects,
which makes things way easier. We can either pass the value we receive
from the controller, or pass a DT object to our methods.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-08-19 08:26:31 -03:00
482f3f1630
Bug 14364: (follow-up) Cleanup duplicate code and adjust notices and prefs
This patch removes a duplicated stanza left form moving routine
Changes the routines to use inbound_library_address
Improves the display if the system preferences
Updates the update file
Moves smaple notice

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-07-29 16:00:26 -03:00
6c75695542
Bug 14364: (QA follow-up) Generate message for transfers as well
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-07-29 16:00:22 -03:00
Kyle M Hall
6a770ace34
Bug 14364: Allow automatically canceled expired waiting holds to fill the next hold
Right now, if a library automatically cancels expired waiting holds, a
librarian must still re-checkin an item to trap the next available hold
for that item. It would be better if the next hold was automatically
trapped and the librarians receive an email notification so they can
make any changes to the item if need be ( hold area, hold slip in item,
etc ).

Test Plan:
 1) Apply this patch
 2) Run updatedatabase.pl
 3) Create a record with one item
 4) Place two holds on that record
 5) Check in the item and set it to waiting for the first patron
 6) Set ReservesMaxPickUpDelay to 1
 7) Enable ExpireReservesMaxPickUpDelay
 8) Enable ExpireReservesAutoFill
 9) Set an email address in ExpireReservesAutoFillEmail
10) Modify the holds waitingdate to be in the past
11) Run misc/cronjobs/holds/cancel_expired_holds.pl
12) Note the hold is now waiting for the next patron
12) Note a waiting hold notification email was sent to that patron
13) Note a hold changed notification email was sent to the library

Signed-off-by: Victoria Faafia <vfaafia29@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-07-29 16:00:17 -03:00
a1739ea43b
Bug 12630: Rebase tests and cover CheckReserves
It turns out we do honor reservedate in CheckReserves, so a hold with a lower priority will
fill before a hold in the future. I add tests to cover this and fix the old tests to pass again

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-06-13 10:24:50 -03:00
0ca445fca8
Bug 12630: (QA follow-up) - Rename _ShiftPriorityByDateAndPriority to _ShiftPriority
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-06-13 10:24:50 -03:00
Olli-Antti Kivilahti
05ac7e0d4a
Bug 12630: Prioritizing "Hold starts on date" -holds causes all other holds to be prioritized as well!
-------------------------
-- REPLICATE LIKE THIS --
-------------------------

0. Enable AllowHoldDateInFuture-system preference!

1. Select a biblio with some holds.
2. Place a hold with the "Hold starts on date"-attribute set to future.
3. More the specific hold up on the priority queue.
4. Add another normal hold, observe how it is prioritized with the "Hold starts on date"-hold, leaving old holds to the prioritization queue tail.

Unfair eh?

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

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-06-13 10:24:45 -03:00
81bdce5c43
Bug 28529: Make biblio-level hold itemtype count against max rules
The current situation is that biblio-level holds can be assigned an item
type, so they can only be fulfilled by items matching that specified
item type (be it item-level itype or the fallback to biblio-level).

But there's the situation in which max holds limits for a specific item
type can be overridden by using biblio-level holds with item type
selection (AllowHoldItemTypeSelection) enabled.

To test:
1. Have a patron of category 'Staff' (S)
2. Have 3 records with items with the 'BK' item type, and maybe others
3. Enable AllowHoldItemTypeSelection
4. Set a limit of 2 max holds for that category+item type
5. In the OPAC. Place bibio-level holds, with item type contraint to 'BK' on those 3 records
=> FAIL: You can place the 3 holds
6. Cancel the holds
7. Apply this patch and restart all
8. Repeat 5
=> SUCCESS: You can only place 2 holds
9. Run:
   $ kshell t/db_dependent/Reserves.t
=> SUCCESS: Tests pass!
10. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-06-01 16:26:44 -03:00
af1f8d34db Bug 30728: Only trigger real-time holds queue update if enabled
This patch makes the places in which Koha enqueues holds queue for
real time updates verify the feature is enabled.

To test:
1. Apply this patches up to the unit tests
2. Run:
   $ updatedatabase
   $ kshell
  k$ git diff origin/master --name-only | grep -e '\.t$' | xargs prove
=> FAIL: tests fail, the code doesn't care about the syspref
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Be happy!
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-05-12 22:17:46 -10:00
Aleisha Amohia
50d917dbfc Bug 30291: Changes to controller scripts
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-05-05 11:17:36 -10:00
c0f3f21416 Bug 29346: (follow-up) Build holds queue when AlterPriority or RevertWaitingStatus is called
To test:
1 - Place 3 holds on a bib with a single item
2 - Confirm bib shows in holds queue
3 - Check in item and cnofirm hold
4 - Bib is no longer in queue
5 - Revert the waiting status
6 - The hold is in the queue again
7 - Move top hold to bottom
8 - Confirm queue selects hold for new top priorty patron

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-05-05 11:17:36 -10:00
b09d93b4e9 Bug 29346: Hold actions triggers
This patch makes several holds related actions schedule the background
job for real-time update of the holds queue.

This actions are:

- place (C4::Reserves::AddReserve)
- fill (Koha::Hold->fill)
- cancel (Koha::Hold->cancel)
- suspend (Koha::Hold->suspend)
- resume (Koha::Hold->resume)

The cancel() action is added a *skip_holds_queue* parameter to skip
triggering the background job entirely. It targets cases like
C4::Biblio::DelBiblio in which all biblio holds are cancelled in a loop.
In that case, we just want to cancel them and let a single backgroung
job take care of the holds queue, once the biblio is deleted.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Hold.t \
           t/db_dependent/Reserves.t
=> SUCCESS: Tests pass! Triggers are triggered
3. Sign off :-D

Sponsored-by: Montgomery County Public Libraries
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-05-05 11:17:35 -10:00
e7a2705c5d Bug 30180: Add 'placed' after_hold_hook
This patch adds a new 'after_hold_action' hook, that is called with the
'placed' action parameter.

To test:
1. Apply the unit tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Plugins/Holds_hooks.t
=> FAIL: Boo, the hook is not called
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: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-05-04 14:29:24 -10:00
821897bbff Bug 29517: Check if agerestriction field is mapped before fetching biblio
This patch simply adds a check of cached values to see if agerestriction enabled before
fetching the biblio object

To test:
1 - prove -v t/db_dependent/Holds.t

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-04-04 16:23:45 +02:00
514cbb809a Bug 19532: (QA follow-up) Simplify resultset accessors
This patch makes the different ->recalls accessors implemented on this
bug be more standard. This means:
- They don't do special things like default sorting or stripping out
  special parameters. That's all left to the caller and the methods are
  clean: they just return the related objects
- Useful filtering methods for Koha::Recalls resultsets are added. The
  only used one (in the end) was ->filter_by_current. It seems like a
  better approach, because it gives devs more control on how they want
  to chain things, and there's a single place in which to maintain the
  criteria of what is 'current' or 'finished'. This clearly makes the
  'old' column obsolete IMHO, at least in the use cases I found. This is
  covered by tests as well.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-14 22:45:52 -10:00
Aleisha Amohia
1ddde85181 Bug 19532: (follow-up) Fixes along recall workflow
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-14 22:45:51 -10:00
Aleisha Amohia
b800c2e68d Bug 19532: Other objects used in recalls feature
- biblio->recalls
- biblio->can_be_recalled
- item->recall
- item->can_be_recalled
- item->can_set_waiting_recall
- item->check_recalls
- patron->recalls
- Biblio.RecallsCount

and relevant tests
- t/db_dependent/Stats.t
- t/db_dependent/Koha/Item.t
- t/db_dependent/Koha/Biblio.t
- t/db_dependent/Koha/Patron.t
- t/db_dependent/XSLT.t
- t/db_dependent/Search.t
- t/db_dependent/Holds.t
- t/db_dependent/Circulation/transferbook.t
- t/db_dependent/Circulation.t

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-14 22:45:51 -10:00