1. APPLY PATCH, updatedatabase, restart_all
2. Search for the new AutoDeleteFromCartWhenHoldPlaced system preference
3. Turn it on for both staff and OPAC
4. From the staff interface put some items in your cart.
5. Place a hold on all of them.
6. Once the hold(s) have been successfully done those items should no longer be in your cart.
7. Put more items into your staff cart, place a hold on some of them but not all.
8. Only the items you have placed a hold on should be removed.
9. Go to the OPAC and put some items in your cart.
10. Place a hold on all of them, those items should now be deleted from the cart.
11. Place more items into your OPAC cart, place a hold on some of them but not all.
12. Only the items you have placed a hold on should be removed.
13. Switch the AutoDeleteFromCartWhenHoldPlaced to only be for 'staff'.
14. Try steps 4-8 again.
15. Switch the AutoDeleteFromCartWhenHoldPlaced to only be for 'opac'.
16. Try steps 9 -12 again.
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Sarah Fletcher <sfletcher@ncwlibraries.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Standardizes error code spelling across the codebase:
- Changes 'too_much_oweing' to 'too_much_owing'
- Changes 'auto_too_much_oweing' to 'auto_too_much_owing'
The word "owing" is the correct spelling in English.
Files updated:
- C4/Circulation.pm
- C4/SIP/Sip/MsgType.pm
- circ/renew.pl
- koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt
- koha-tmpl/intranet-tmpl/prog/js/checkouts.js
- koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
- koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
- misc/cronjobs/automatic_renewals.pl
- opac/opac-reserve.pl
- opac/opac-user.pl
- t/db_dependent/Circulation.t
All tests should pass successfully.
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
We have different implemented solution to select a tab on opac-user:
* a "tab" variable (from bug 37972), it's never used
* several variables passed from the controller to the template, depending on the tab to select (it works and is used)
* use of anchors (it does not work and is used)
We should fix and standardize it.
I've linked to several reports that seem to conflict with each others.
This patch suggests to have a "tab" variable
Test plan:
Confirm that the different links to tab to opac-user are not using a
query parameter to pass the tab to select, e.g. from the opac-main page.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit completely modernizes the hold fee system by migrating from
the deprecated categories.reservefee column to the circulation rules framework
and implementing modern Koha::Objects patterns.
WHAT CHANGED:
1. MIGRATION TO CIRCULATION RULES:
- Moved hold fees from categories.reservefee to circulation_rules table
- Added database migration script with atomic update
- Maintains existing fee amounts during migration
- Provides rule hierarchy: category+itemtype+branch specific rules
2. NEW SYSTEM PREFERENCE: TitleHoldFeeStrategy
- Options: highest, lowest, most_common
- Addresses inconsistent title-level hold fees when items have different fees
- Default: 'highest' (safest approach for library revenue)
3. MODERN Koha::Objects API:
- Koha::Item->holds_fee($patron): Calculate fee for specific item
- Koha::Hold->calculate_hold_fee(): Enhanced item/title-level logic
- Koha::Hold->should_charge($stage): Centralized charging logic
- Koha::Hold->charge_hold_fee(): Modern fee charging with proper accounting
- Removed C4::Reserves::GetReserveFee and ChargeReserveFee
4. ENHANCED FUNCTIONALITY:
- Proper ReservesControlBranch logic (patron home vs item home library)
- Complete HoldFeeMode implementation (placement/collection/not_always)
- Clear OPAC messaging with "You will be charged..." commitment
- Comprehensive test coverage for all fee scenarios
5. TECHNICAL IMPROVEMENTS:
- Object-oriented design following Koha patterns
- Proper error handling and edge case management
- Modern test structure with comprehensive coverage
- Clean separation between fee calculation and charging
TEST PLAN:
A) MIGRATION AND SETUP:
1. Apply all patches and run database update
2. Verify existing hold fees migrated to circulation rules:
- Administration > Circulation and fine rules
- Previous categories.reservefee values now in "Hold fee" column
3. Confirm old categories.reservefee column no longer used
B) BASIC FEE CALCULATION:
1. Set up patron categories with different hold fees in circulation rules
2. Create items with different item types, each with different hold fees
3. Test item-level holds charge correct fees based on specific item
4. Test title-level holds use TitleHoldFeeStrategy preference
C) TITLE-LEVEL HOLD STRATEGIES:
1. Set TitleHoldFeeStrategy to "highest":
- Create title with items having fees $1.00, $2.00, $3.00
- Verify title-level holds charge $3.00
2. Set TitleHoldFeeStrategy to "lowest":
- Verify same title charges $1.00
3. Set TitleHoldFeeStrategy to "most_common":
- Add multiple items with $2.00 fee
- Verify title-level holds charge $2.00 (most common)
D) HOLDFEEMODE PREFERENCES:
1. Set HoldFeeMode to "any_time_is_placed":
- Place holds and verify immediate fee charging
- Check patron account for correct hold fee debits
2. Set HoldFeeMode to "any_time_is_collected":
- Place holds, verify no immediate charges
- Check out items to patron, verify fee charged on collection
3. Set HoldFeeMode to "not_always":
- With available items: verify no fee charged
- With all items checked out but no other holds: verify no fee
- With all items checked out and other holds exist: verify fee charged
E) RULE HIERARCHY TESTING:
1. Set different fees for different category/itemtype/branch combinations
2. Verify most specific rule applies:
- Branch + Category + ItemType (most specific)
- Category + ItemType
- Category only (most general)
3. Test with different patron categories and item types
F) USER INTERFACE TESTING:
1. OPAC interface:
- Verify "You will be charged..." messages display correct amounts
- Test both item-level and title-level hold fee messages
- Confirm fees match actual charges in patron account
2. Staff interface:
- Place holds for patrons and verify fee calculations
- Check Administration > Circulation rules interface works correctly
G) EDGE CASES AND ERROR HANDLING:
1. Test $0.00 fees (no charge scenarios)
2. Test patrons with no applicable fee rules (should default to $0.00)
3. Test items with no holdable copies (should return $0.00)
4. Test invalid/missing itemtypes or categories
5. Test holds on biblios with no items
H) REGRESSION TESTING:
1. Verify existing hold placement workflows unchanged
2. Check patron account charging remains accurate
3. Confirm OPAC and staff interfaces display correctly
4. Test that existing holds continue to work properly
I) UNIT TESTS:
Run the following test suites to verify functionality:
- prove t/db_dependent/Koha/Hold.t
- prove t/db_dependent/Koha/Item.t
- prove t/db_dependent/Reserves.t
- prove t/db_dependent/Circulation.t
BACKWARDS COMPATIBILITY:
- All existing functionality preserved
- Hold fee amounts maintained during migration
- Existing workflows continue unchanged
- Legacy code updated to use new API seamlessly
Sponsored-by: OpenFifth <https://openfifth.co.uk>
Signed-off-by: Jackie Usher <jackie.usher@westsussex.gov.uk>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Refactor: Update controller logic to make use of patron->create_hold_group
This ensures consistency when creating a new hold group for a patron,
regardless if on the Staff UI or REST API
Signed-off-by: Anneli <anneli.osterman@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Bug 15516: Add ability to create and view hold groups from OPAC
This patch adds a "Treat as hold group" to opac-reserve.pl,
which lets OPAC users create a hold group which can include
1+ bibs and either bib-level holds, or item-level holds, or both.
This patch shows the item-level and bib-level holds in a hold group
for the user's holds in the OPAC. This includes both the opac-user.pl
page and the self-checkout hold display.
Bug 15516: (follow-up) Only show hold groups for multi hold in OPAC
Bug 15516: (follow-up) Only show hold groups for multi hold in staff interface
Bug 15516: Add 'DisplayAddHoldGroups' system preference to toggle ability to add titles to hold groups
These patches add the ability to create hold groups across multiple bibs in the staff interface or OPAC.
These hold groups are fulfilled by the first item checked in across the hold group.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
It can be useful, for instance, if a library has the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.
Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
and click on 'Place hold' button at the top of the results.
2. Search for a patron
3. Check the 'Treat as hold group' checkbox
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are deleted.
The other holds will also be removed from the holds queue.
9. Confirm tests pass
- t/db_dependent/Koha/Holds.t
- t/db_dependent/Reserves/HoldGroup.t
Note: the "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
- circ/view_holdsqueue.pl
Note: A hold group is counted as only one hold
Sponsored-by: Catalyst IT
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
When OPACHoldsIfAvailableAtPickup preference is set to Don't allow and circulation rules allow on-shelf holds only if any unavailable, and we have a record with a checked out copy at Centerville and an available copy at Midway, a Midway patron should not be able to place a hold for pickup at Midway since the Midway copy is available.
This is the setting:
- circulation rules allow on-shelf holds only if any unavailable
- OPACHoldsIfAvailableAtPickup is set to Don't allow
To test:
1. Set up the circulation rule so that on-shelf holds are allowed ('Yes')
2. Create a record with two items, one at Centerville and one at Midway
3. Check out the Centerville item
4. Log into the OPAC with a Midway patron (I used Henry Acevedo)
5. Try to place a hold on the record
--> Midway is not a valid pickup location (correct behaviour)
6. Cancel
7. Change the circulation rules so that on-shelf holds are only allowed 'if any unavailable'
8. Try to place a hold on the record from the OPAC again
--> The system allows you to place the hold for pickup at Midway (incorrect behaviour)
9. Optionally, check the Holds to pull for Midway (the hold is there, it shouldn't)
10. Optionnaly, check in the Midway item (it is trapped for Henry's hold)
11. Apply the patch
12. Try again to place a hold on the record from the OPAC
--> it is impossible to select Midway, Midway is not a valid pickup location
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>
This commit is generated using:
% perl misc/devel/tidy.pl
*within* ktd, to get the same version of perltidy than what will be used
by our CI (currently v20230309).
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Check for 'renew' when appropriate
Check for 'hold' when appropriate
The following command must return nothing before this is pushed to
master:
git grep "\beffective_BlockExpiredPatronOpacActions\b"
Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch removes the Date_from_syspref recently added and replaces it
with the correct parseDate call as per bug 35559. We also clean up the
code around setting the input value in the first place and use iso which
is what the rest of flatpickr expects and now is handled correctly in
futuredate pickers.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This patch sets the holddatefrom date to today by default on the opac.
Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
1 - Place a hold on the opac
2 - Note after placing the hold the URL is:
http://localhost:8080/cgi-bin/koha/opac-user.pl?#opac-user-holds
3 - Note the holds tab is not active
4 - APPLY PATCH and restart_all
5 - Try steps 1 - 3 again, this time the holds tab should be opened.
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
It adds unnecessary complexity and information.
Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
So that pickup delay can have a different value per patron category,
item type or branch.
To test:
1) Update database, restart services
2) Set ReservesMaxPickUpDelay syspref (if not already set)
3) Edit your circulation rules and set a value under 'Holds pickup
period (day) that is DIFFERENT from ReservesMaxPickUpDelay. Set a few
different numbers for different branches as well.
4) Place a hold on a biblio from the staff client.
5) Check in an item from that biblio and confirm the hold as waiting
6) Confirm the expiration date is calculated using the 'Holds pickup
period' value instead of the ReservesMaxPickUpDelay syspref
7) Revert the waiting status and delete the hold
8) Re-place the hold on the biblio on the OPAC. Notice that when you
change the pick up location, the number of days in the pickup message
below the dropdown changes based on the circ rules.
9) Create a holiday with a date that will overlap with the 'Holds pickup
period'
10) Check in an item from that biblio and confirm the hold as waiting
11) Confirm the expiration date is calculated using the 'Holds pickup
period' value AND considers the special holiday
12) Confirm tests pass t/db_dependent/Holds/WaitingReserves.t
13) Test Talking Tech:
13a) Enable TalkingTechItivaPhoneNotification
13b) Go to Tools -> Notices & slips. Add content to the HOLD phone
(itiva) notice.
13c) In your terminal, run perl
/path/to/koha/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl -o
~/itiva.tmp -w 0 --type=RESERVE
Sponsored-by: Catalyst IT
Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
if OPACHoldsIfAvailableAtPickup is set to "Don't allow" (i.e. if patrons cannot place holds on items that are available at their library), patrons cannot place holds on items that are on order (or any negative not for loan value) for pickup at their library.
Technically, those items are not "available", so they shouldn't be affected by this system preference.
To replicate:
Prerequisites :
- Have at least two libraries (I used the sample libraries)
- Have a patron in one of the libraries (I used Henry Acevedo), make sure you know the user id and password for that patron
- Make sure the circulation rules allow holds for the patron category
1. Set OPACHoldsIfAvailableAtPickup to "Don't allow"
1.1. Go to Administration > Global system preference
1.2. Search for OPACHoldsIfAvailableAtPickup
1.3. Change the value to "Don't allow"
1.4. Click "Save all Circulation preferences"
2. Make an item "On order" at the patron's library (Henry Acevedo is at Midway)
2.1. If needed, create a bibliographic record and item
2.2. Edit the item to assign the "On order" not for loan status
2.3. Change the home and holding library to the patron's library (Midway)
2.4. Click "Save changes"
3. Try to place a hold on the item through the OPAC
3.1. In the OPAC, log in as the patron
3.2. Search for the item
3.3. Click "Place hold"
--> Notice the patron's library is greyed out in the pickup location drop-down menu"
4. Apply the patch
5. Repeat step 3
--> Notice you can select the patron's library in the pickup location drop-down menu and place a hold with it
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Test plan:
1. Set system preferences:
- Disable OPACAllowUserToChooseBranch
- Set ReservesControlBranch to "item's home library"
- Enable UseBranchTransferLimits
- Set BranchTransferLimitsType to "item type"
- Enable canreservefromotherbranches
- Disable IndependantBranches
2. Create two libraries: ITEM_LIB and PATRON_LIB
3. Set circulation rules:
- Remove all circulation rules (DELETE FROM circulation_rules)
- Set a default rule that allow some holds and with "OPAC item level
holds" = "force". Allow "on shelf" holds for easier testing.
- In the rules for ITEM_LIB, under "Default checkout, hold and return
policy",
- set "Hold policy" to "From any library"
- set "Hold pickup library match" to "item's home library"
- In the rules for PATRON_LIB, under "Default checkout, hold and
return policy",
- set "Hold policy" to "From any library",
- set "Hold pickup library match" to "any library"
4. In "Library transfer limits" disable all transfers from ITEM_LIB to
PATRON_LIB for an item type (let's say "BOOK")
5. Create a biblio with an item:
- item type must be the same as in previous step ("BOOK")
- home branch and holding branch must be ITEM_LIB
6. Create a patron at PATRON_LIB, give it a password so it can log in.
7. Go to OPAC, and login with this patron
8. Try to place a hold for the new item. You should be allowed to place
a hold, but when doing it, no holds will be created.
9. Apply patch, restart Koha
10. Try to place a hold for the new item. This time the hold should be
created
Signed-off-by: Jessie Zairo <jzairo@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Merged tidy patch and switch to Koha::Policy::Holds here.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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>
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>
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>
This patch changes opac-reserve.pl to return the error(s) when placing
a hold as a pipe delimited list which is then translated to a message for the
user
To test:
1 - Find a record with items available on the opac
2 - Click 'place hold' and set things up, but do not confirm
3 - In staff client, do something to make hold invalid:
- Make item damaged
- Make library not a pickup location
- Place other holds for patron up to limit
- etc.
4 - Confirm hold on OPAC
5 - You are sent to patron's account, hold is not placed
6 - There is little or no message to explain why
7 - Apply patch
8 - Repeat
9 - Now errors are clear
Signed-off-by: Andrew Auld <andrew.auld@ptfs-europe.com>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Placing a hold at the OPAC explodes with a 500 when the record is a
serial
DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Column 'biblionumber' in where clause is ambiguous at /usr/share/koha/lib/Koha/Objects.pm line 394
Test plan:
* Create a serial subscription for an existing bibliographic record
* Place a hold on this record at the OPAC
Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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>
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>
This never really made it. So let's remove all traces.
Test plan:
Look at code changes and git grep reqholdnotes.
Bonus: Place a hold with/without hold notes.
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>
This will save us fetching more info if we are denying the holds
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
The only bit we get form the hash is the rank and title
We can simply fetch the biblio once to calculate
no need to fetch all the info as we don't display or use it
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
New holds have found set to undef - we can do this directly
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Here the items biblionumber is either equal to the biblionumber passed
in or it is not and should be - we can just assign directly from the
item in all cases
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Search for the variables, they aren't used!
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This is defined but never used
To test:
1 - git grep noreserve
2 - only one ocurrence
3 - Apply patch
4 - repeat, no ocurrence
5 - Note the plural has a few uses
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Currently we loop over all the holds on the record to get a count of the reserves not
waiting and a rank for the upcoming hold
We can simply count the number of nont waiting holds and add 1
Note: Before this patch the count of holds included the new hold we were going to place
which meant rank and count were the same. I believe countaing existing holds and our
upcoming rank is the correct behavior
To test:
1 - Set OPACShowHoldQueueDetails system preference to 'Show holds and their priorty level'
2 - Place a few holds on a bib on the staff side
3 - Mark 1 waiting
4 - Attempt to place hold on the bib on the opac
5 - Note 'Your priority' and 'Number of holds' are the same number
6 - Place another hold, confirm both increase
7 - Set another hold waiting, confirm both decrease
8 - Apply patch
9 - Confirm 'Your priority' is now 1 greater than Number of holds
10 - Place a hold, set a hold waiting, confirm numbers adjust as appropriate
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This patch replaces the call to GetTransfers in opac-reserve with the
get_transfer method available from the Koha::Item object
Test plan
1/ Place a reserve on an item at a different branch
2/ Trigger the transfer of the item
3/ Check the opac-reserves page for the display of the current transfer
status
4/ Apply patch
5/ Repeat step 3
Rebased-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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>
This patch adds an explanation when a hold cannot be placed, in some situation:
tooManyReserves
tooManyHoldsForThisRecord
tooManyReservesToday
itemAlreadyOnHold
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This reduces lookups for the related items that we now fetch in the templates
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
To get the host items we need to search all items, not those limited to the
same biblionumber
To test:
1 - Enable EasyAnalytics
2 - Attach an item to a record
3 - Confirm it is listed in the items list on opac reserve
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Bug 27272 is going to remove C4::Items::GetItemsInfo in favour of Koha::Items->search_ordered.
Here we are going to deal with opac-reserve
Test plan:
List items on the modified view and confirm that all the info is
displayed correctly
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
$itemLoopIter->{reservedate} = $first_hold->reservedate,
$itemLoopIter->{ExpectedAtLibrary} = $first_hold->branchcode;
1st line ends with a comma instead of a semicolon.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Before this patch the date formatting for the due date was done
in the .pl instead of the template and the branchcode would show
instead of the library's name in the list of items when placing
an item level hold in the OPAC.
To test:
* Find or create a record with mulitple items
* Check out one of the items
* Place a record level hold
* Return an item to set to waiting
* Place an item level hold
* Go to the OPAC and start placing an item level hold
* Verify the messages in the item table show the branchcode
* Apply patch
* Only visible change should be the library name replacing the branchcode
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Lucas Gass
Martin Renvoize
Jonathan Druart
Pedro Amorim
Tomas Cohen Arazi
David Cook
Aleisha Amohia
Hammat Wele
Koha Development Team
Pedro Amorim
Martin Renvoize
Marcel de Rooy
Julian Maurice
Nick Clemens
Kyle Hall
Joonas Kylmälä
Katrin Fischer