When reviewing the different sysprefs that had "NULL" as default, I
only found those 2 occurrences that could be problematic. I think we
should default to 0 here.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit ca6cbc3cf9)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This follow-up addresses critical issues in the initial implementation:
1. **Patron Ownership Check**: Only mark bookings as 'completed' when
the patron checking out the item is the same patron who made the booking
2. **Smart Cancellation**: When another patron checks out an item that
conflicts with an existing booking:
- Cancel the booking only if checkout overlaps with actual booking period
- Respect lead/trail periods - don't cancel if conflict is only during
preparation periods (allows librarian override scenarios)
3. **Date Logic**: Proper overlap detection using start_date and end_date
rather than periods extended by lead time
This prevents incorrect booking completion and handles librarian override
scenarios appropriately.
Test plan:
1. Apply previous patch and this follow-up
2. Enable bookings and create a bookable item
Test A - Patron's own booking completion:
3. Create booking for Patron A: start = today, end = +7 days
4. Check out item to Patron A (due date = +5 days)
5. Verify booking status = 'completed'
Test B - Lead period override scenario:
11. Return item, set bookings_lead_period circulation rule = 3 days
12. Create booking for Patron A: start = +5 days, end = +8 days
13. Check out item to Patron B (due date = +4 days) - conflicts with lead period
14. Override the booking conflict warning
15. Verify Patron A's booking status remains 'new' (not cancelled)
16. This respects librarian's decision that item will return in time
Test C - No booking scenario:
17. Return item, ensure no active bookings exist
18. Check out item to any patron
19. Verify checkout works normally without errors
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 2f849fd79c)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch adds logic to automatically mark a patron's booking as
'completed' when they check out the item they have booked.
When AddIssue is called, the system now:
- Checks if there's a conflicting booking for the checkout period
- Marks the booking status as 'completed' if found
This ensures booking status accurately reflects when items are
actually checked out to fulfill bookings.
Test plan:
1. Enable bookings system preference
2. Create a bookable item (set bookable = 1 in items table or via item editor)
3. Create a booking for a patron for that item with start date = today,
end date = 7 days from now
4. Go to circulation and check out the item to the same patron who made
the booking
5. Check the booking status in the database or via the bookings interface
6. Verify the booking status has changed from 'new' to 'completed'
7. Confirm the checkout was successful and shows normally in patron's
checkouts list
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit a8fd9734c6)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 007d66e314)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch adds a catch during circulatoin actions and passes a message back to the caller
For checkouts and renewals we don't have a method for passing messages so we simply catch the exception
and warn - this seems reasonable, as we have now added a warning on the patron display that they are missing
a guarantor
To test:
1 - Create a Child type patron with no guarantor
2 - Create a patron in a non-child category that can_be_guarantee with no guarantor
3 - Check out an item to each patron above
4 - Enable system preference 'ChildNeedsGuarantor' and TrackLastPatronActivityTriggers - select all
5 - In mysql
UPDATE borrowers SET datelastseen = '2025-01-01';
6 - Restart all to clear cache (so update last seen will update)
7 - Attempt to checkout a book to either patron - 500 error
8 - Attempt to returnt he books the patrons have checked out - 500 error
9 - Attempt to renew their items - 500 error
10 - Apply patches, restart all
11 - Repeat checkout, renewal, checkin
12 - On checkout and renewal there should be a warning on the patron page, but actions succeed with no additional problems
13 - On return you get a warning letting you know the action succeeded but there is still a problem to resolve
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit bd32425271)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
**Problem:**
When using SIP checkout messages with the "no block" flag, the specified due date
was being ignored. Instead of honoring the provided due date, the system would:
1. Use the due date as the transaction timestamp (incorrect behavior)
2. Calculate a new due date based on circulation rules
3. Return an empty due date field (AH) in the SIP response
This affected SIP2 integrations where external systems needed to specify exact
due dates for checkouts, particularly for offline circulation scenarios.
**Root Cause:**
The `ProcessOfflineIssue` function in C4::Circulation was only returning a
success/failure message, but the SIP checkout transaction needed access to the
resulting checkout object to properly set the due date. The SIP Transaction::Checkout
module was calling `ProcessOfflineIssue` but couldn't retrieve the checkout details
to populate the transaction's due date field.
**Solution:**
1. Modified `ProcessOfflineIssue` to return both the status message AND the checkout
object: `return ( "Success.", $checkout );`
2. Updated `ProcessOfflineOperation` to handle the new return format by capturing
only the message: `( $report ) = ProcessOfflineIssue( $operation );`
3. Modified SIP Transaction::Checkout to capture both return values and use the
checkout object to set the proper due date via `duedatefromissue`
4. Fixed the timestamp parameter to use `dt_from_string` instead of the due date
**Test Plan:**
1. **Unit Tests:**
- Run `prove t/db_dependent/Circulation/OfflineCirculation.t` to verify
ProcessOfflineIssue returns checkout object and respects due_date parameter
- Run `prove t/db_dependent/SIP/Transaction.t` to verify SIP checkout honors
no_block_due_date parameter
2. **Manual SIP Testing:**
- Connect to SIP server via telnet on port 6001
- Send checkout message with no block flag and specific due date:
`111YN20250115 18000120241201 180001AP|AO|AC|AD|AB<barcode>|AA<cardnumber>|`
- Verify response contains correct due date in AH field
- Verify checkout record in database has the specified due date
3. **Regression Testing:**
- Verify normal SIP checkouts (without no block) still work correctly
- Verify offline circulation operations continue to function
- Verify ProcessOfflineOperation still processes queued operations correctly
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 4d069e5006)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
The `MoveReserve` method is called once, in `AddIssue`. The latter has the related `Koha::Item` object already fetched from the DB, but it passed the `itemnumber` which is used by `MoveReserve` to fetch the item from the DB. This should not happen.
To test:
1. Run:
$ ktd --shell
k$ prove t/db_dependent/Reserves* \
t/db_dependent/Koha/Hold* \
t/db_dependent/Hold* \
t/db_dependent/Circulation*
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests still pass!
4. Sign off :-D
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 1886165eae)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
In order to remove false positives with the QA script we should replace some tabs with spaces.
There are more but I wanted to make sure no regression would be
introduced
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 492e6814a1)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Set these sysprefs:
- FinesMode: Calculate and Charge
- WhenLostForgiveFine: Forgive
- WhenLostChargeReplacementFee: Charge
In circ rules:
- Have a rule that charges fines
- Set Refund lost item replacement fee to "Refund lost item charge and charge new overdue fine"
1 - Have/create item with replacement price
2 - Check item out to patron 1, due date in past
3 - Run fines.pl
4 - confirm fines on patron 1
5 - mark item lost
6 - confirm replacement cost generated and fine forgiven on patron 1
7 - check item in
8 - confirm replacement cost forgiven and new fine generated on patron 1
9 - check item out to patron 2, due date in future
10 - check item in
11 - mark item lost (no replacement fee generated because item is not checked out)
12 - in database, update date_due for checkout to patron 2, set to a date in the past
13 - check item in
14 - confirm patron 2 now has a fine
15 - Apply patches, restart all
16 - Repeat test plan with another item and confirm no fine is added for patron 2
Note: At step 12 we're pushing the date_due of the old issue into the past to make date_due<curdate(). In production, this comes up when there are days between steps 10 and 13.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 67c95addeb)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Typo in system preference call 'OPACFineNoRenewalsIncludeCredit'
=> 'OPACFineNoRenewalsIncludeCredits'
Test plan :
Run : prove t/db_dependent/Circulation.t
Run test plan of Bug 23293
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
If syspref CheckPrevCheckout is enabled, Koha checks if patron
has previously checked out item within timeperiod set in
syspref CheckPrevCheckoutDelay. This however doesn't take into
account cases where item is currently checked out for patron and
they are trying to check out another item from same record (can
be done if syspref AllowMultipleIssuesOnABiblio is enabled). We
should always check if item is currently checked out for the
patron if CheckPrevCheckout is enabled.
To test:
1. Enable syspref CheckPrevCheckout and set CheckPrevCheckoutDelay
as e.g. 10. Also enable syspref AllowMultipleIssuesOnABiblio.
2. Find a record that has been checked out for patron over 10 days ago.
3. Attempt to check out item from same record for same patron.
=> Note that you have to confirm if you want to check out item for
the patron.
4. Find record that is currently checked out for a patron.
5. Attempt to check out item from same record for same patron.
=> Note that item is checked out for patron without confirm message.
6. Apply this patch and restart services if needed.
7. Repeat steps 2. and 3.
=> Confirm that you still have to confirm if you want to check out
item for the patron.
8. Repeat steps 4. and 5.
=> New confirm message "Patron has this title currently checked out:..."
should be displayed and you have to confirm if you want to check out
item for the patron.
Also prove t/db_dependent/Patron/Borrower_PrevCheckout.t
Sponsored-by: Koha-Suomi Oy
Signed-off-by: Felicie <felicie.thiery@biblibre.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
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>
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>
Added head2 right before sub with missing POD
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This patch fixes a regression introduced by Bug 38505.
To test:
1. Enabled OpacTrustedCheckout system preference
2. Search for an item and get the barcode
3. Go to the OPAC
4. Click the Self-checkout button at the top
5. Enter the barcode and submit
6. Notice you are given an error UNKNOWN_BARCODE and the item is not checked out
7. Apply the patch and restart services
8. Repeat steps 4-6 and confirm you are able to successfully check out the item
9. Test submitting the modal with no barcode entered - confirm you are shown an appropriate error
10. Test with a fake barcode that does not exist - confirm you are shown an appropriate error
11. Run tests to confirm no regressions
prove t/db_dependent/Circulation*
prove t/db_dependent/DecreaseLoanHighHolds.t
prove t/db_dependent/rollingloans.t
prove t/db_dependent/api/v1/checkouts.t
prove t/db_dependent/Patron/Borrower_PrevCheckout.t
prove t/db_dependent/Koha/ILL/Requests.t
Sponsored-by: Reserve Bank of New Zealand
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised, including the tests.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
Before applying this patch, apply the CanBookBeIssued preparation: tests patch and run that,
Verify it fails, apply this patch. Run that same tests file again.
----- About this change -----
CanBookBeIssued wants to find one item given a barcode.
If a falsy (undef, empty) barcode is supplied, UNKNOWN_BARCODE should be returned.
Ensure this change does not introduce any regressions:
prove t/db_dependent/Circulation*
prove t/db_dependent/DecreaseLoanHighHolds.t
prove t/db_dependent/rollingloans.t
prove t/db_dependent/api/v1/checkouts.t
prove t/db_dependent/Patron/Borrower_PrevCheckout.t
Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
To test:
1. Apply patch, restart_all, updatedatabase
2. Check out an item to a patron
3. Query the database
- select return_branch from issues; should return NULL
- select branchcode from issues; should return the branchcode of the library you checkout the item out from
- select return_branch from old_issues; should match the branchcode from the previous query
Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
The Koha::Account::Line->checkout relationship accessor needed an update
to respect the new fields and foreign keys introduced here.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Update C4::Circulation::MarkIssueReturned to handle moving issue_id to
old_issue_id in accountlines.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This patch removes the last remaining use of updateWrongTransfer and the
method itself.
We replace it with a call to request_transfer that passes the 'replace'
reason of 'WrongTransfer' through to Koha::Item::Transfer->cancel.
Test plan
1/ git grep updateWrongTransfer to confirm there are no more uses of the
method.
2/ Confirm an incorrect transfer still behaves as expected at
circulation returns.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
1) Don't automagically always set a transfer to in transit on checkin..
wait for the user to actually confirm that's the case
2) New transfers triggered by a hold should take precidence, so hide
transfers for any other reason from display
3) Update get_transfer and get_transfers to ensure ordering isn't lost
when prefetch is used and add tests for this
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
Sponsored-by: Cuyahoga County Public Library <https://cuyahogalibrary.org/>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Sponsored-by: Cuyahoga County Public Library <https://cuyahogalibrary.org/>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This patch updates all instances where the current noissuescharge sysprefs are used. They will now use the is_patron_inside_charge_limits method to handle the patron category level limits
Sponsored-by: Cuyahoga County Public Library <https://cuyahogalibrary.org/>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
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>
Test plan using koha-testing-docker:
1) Make sure SIP is running. You may need to edit
/etc/koha/sites/SIPconfig.xml and remove the 8023 connector and
restart the SIP-server (koha-sip --restart kohadev)
2) Find a patron, say 23529000197047
3) Set a password by selecting "change password", set it to
"Password1234"
4) Find a book, say 39999000000856
5) Issue book to patron with sip-client:
sudo koha-shell -c "/usr/share/koha/bin/sip_cli_emulator.pl \
--address localhost --port 6001 -t cr \
--su term1 --sp term1 --message checkout \
--location CPL --item 39999000000856 \
--patron 23529000197047 --password Password1234"\
kohadev
6) Note the AH-header in the response which for example:
'AH20240619 235900'
7) Make a renewal with:
sudo koha-shell -c "/usr/share/koha/bin/sip_cli_emulator.pl \
--address localhost --port 6001 -t cr \
--su term1 --sp term1 --message renew \
--location CPL --item 39999000000856 \
--patron 23529000197047 --password Password1234"\
kohadev
8) Make sure the AH-header in the response is different from the
response to the checkout, for example: 'AH20240624 235900'
Signed-off-by: Tadeusz „tadzik” Sośnierz <tadeusz@sosnierz.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
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 updates the changed code to set the logged in librarian as
the person who resolved the return claim at checkin/checkout action.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
The new logic in AddReturn was flawed, we were missing return messages
due to moving the if statement too high and catching more code than
intended.
Mentored-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
To test:
1. Configure Claims returned
1. Go to Administration > Authorized Values > LOST
2. Add a new authorized value with value:6 and description:Claims returned
3. Go to Administration > System Preferences
4. Set ClaimReturnedLostValue to 6 and save
2. Check out an item to a patron.
1. Mark the item as claim returned
2. Check the item in.
3. A message stating that the item has been claimed as returned pops up with
an option to resolve.
4. Resolve the claim.
3. Check out the item to the patron again.
1. Mark the item as claim returned.
2. Check out the item to a new patron. Select “Yes, check out”
3. Go back to the previous patron. Notice that their claim was not resolved.
4. Apply the patch.
1. Updatedatabase
2. restart_all
3. Go to the system preferences and set the system preference
‘AutoClaimReturnStatusOnCheckin’ to ‘Returned by patron’
4. Set ‘AutoClaimReturnStatusOnCheckout’ to ‘Found in library’
5. Redo step 2
1. When checking the claim returned item in you will now see a message that
says “The previously claimed returned item has been found, automatically
resolving the associated claim.”
2. View the previous patron. Their claim has automatically been resolved
with a status of ‘Returned by patron’
6. Redo step 3
1. Upon checking the item out to another patron you will see a message that
says “The previously claimed returned item has been found, automatically
resolving the associated claim.”
2. View the previous patron. Their claim has automatically been resolved
with a status of ‘Found in library’.
7. Sign off and have a wonderful day!
Sponsored-by: Altadena Library District
Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Currenty, if we find only a defult rule for a checkout we are ignoring the parent rule.
We should not use the default if there is a parent rule
To test:
1: have an itype BK
2: create an itype BK2, set BK as the parent to BK2
3: create a circ rule for All Patrons, item type BK (displays as "Books (All)" in the circ rules interface), setting total checkouts to 1
4: do not create any rule specifically for the BK2 itype
5: have some BK items and some BK2 items
6: check a BK item out to a patron
7: check a BK2 item out to the same patron, observe you are not blocked from doing so
8: try to check out a second BK2 item to the same patron, get message "Too many checked out. 1 checked out, only 1 are allowed."
9: try to check out a second BK item to the same patrons, get message "Too many checked out. 2 checked out, only 1 are allowed."
10: return both items
11: check a BK2 item out to your patron
12: try to check a BK item out to your patron, get message "Too many checked out. 1 checked out, only 1 are allowed."
13: Apply patch
14: Repeat 7, you are now blocked
15: Other results should be the same
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Michael Adamyk <madamyk@ckls.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
On (at least) git installations of Koha checkouts and checkins fail on
error 500. Logs have following error:
Undefined subroutine &C4::Circulation::CheckReserves called...
Error happens also when one tries to open patrons checkouts from detail page.
Koha doesn't die but table just keeps loading. Solution is to add C4::Reserves
before CheckReserves when it's called from Circulation.pm.
To test:
1. Apply this patch.
2. Try to check out and check in item.
=> Confirm both operations are succesfull.
3. Attempt to open patrons checkouts from patron detail and checkout page.
=> Table should load
Also prove t/db_dependent/Circulation.t.
Sponsored-by: Koha-Suomi Oy
Signed-off-by: BabaJaga <babajagawgoglach@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Koha/Illrequests.pm -> Koha/ILL/Requests.pm
Merged:
t/db_dependent/Koha/Illrequests.t
t/db_dependent/Illrequests.t
Into:
t/db_dependent/Koha/ILL/Requests.t
ILL classes file structure is, for the most part, around 7 years old and doesn't follow a strict logic. It's so confusing that some test files exist redundantly.
This housekeeping should help future work in regards to ISO18626 to add Koha as a supplying agency instead of just requesting agency, as is now.
It should also help future housekeeping of moving backend related logic out of the Illrequest.pm into Illbackend.pm (now ILL/Request.pm and ILL/Backend.pm as of this patchset).
It should also help in structuring the addition of a master generic form (see bug 35570)
This patchset will require existing backends to be updated to match the new class names and structure, if they invoke them.
Test plan, k-t-d, run tests:
prove t/db_dependent/api/v1/ill_*
prove t/db_dependent/Koha/ILL/*
Test plan, k-t-d, manual:
1) Install FreeForm, enable ILL module, run:
bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
2) You'll have to switch the FreeForm repo to the one compatible with this work, like:
cd /kohadevbox/koha/Koha/Illbackends/FreeForm
git checkout reorganize_ILL
3) Do some generic ILL testing:
3.1) Create a request
3.2) Add a comment to a request
3.3) Edit a request
3.4) Edit a request's item metadata
3.5) Confirm a request
3.6) List requests
3.7) Filter requests list using left side filters
4) Install a metadata enrichment plugin:
https://github.com/PTFS-Europe/koha-plugin-api-pubmed
4.1) Create an ILL batch and insert a pubmedid like 123
4.2) Add the request and finish batch
5) Verify all of the above works as expected
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This patch fixes the overly extreme message that was designed to appeared
when someone tried to check out a booked item to the wrong user.
However, the message never appeared due to a logic error within
CanBookBeIssued which this patch also resolved.
Test plan
1) Make an item bookable
2) Add a booking for the item to patron A from tomorrow
3) In the database, update the booking start_date to today (or wait a
day before the next step)
4) Attempt to check the book out to patron B.
Note that you are displayed with the new message 'The item is booked
for another patron' and you are unable to check the item out.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
The code here was flawed, DateTime returns a 1-7 indexed array with the
first day being Monday, Koha expects an 0-6 indexed array with the first
day being Sunday.
Sponsored-by: PTFS Europe
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Sponsored-by: Catalyst IT
Sponsored-by: Auckland University of Technology
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
If ConsiderLibraryHoursWhenIssuing is set to shorten the loan period to
the closing time, if the loan period initially pushes the due date to
the following day, the day still gets added when calculating the due
date. We simply need to hardcode the due time here as the due day is the
same as the issue day.
We only need to calculate a due date if ConsiderLibraryHoursWhenIssuing
is set to extend the loan period to the next opening day, as we'll need
to consider holidays/closed days.
Sponsored-by: Catalyst IT
Sponsored-by: Auckland University of Technology
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Since days for branch hours are stored as 0-6 in the database, when it's
a Saturday (6) incrementing the date leads to an error when issuing an
hourly loan because no opening hours are found for the non-existent day
(7).
This patch fixes this by calculating the tomorrow day and setting it to
0 if it's greater than 6.
This patch also corrects the mappings for days, where local_day_of_week
caluclates the date with Sunday first, which put it out of sync with the
database opening hours days.
Sponsored-by: Catalyst IT
Sponsored-by: Auckland University of Technology
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This feature adds the ability to set opening and closing hours for your
library and for these hours to be considered when calculating due dates
for hourly loans. If the due date for an hourly loan falls after the
library closes, the library can choose for the due date to be shortened
to meet the close time, or extended to meet the open time the next day.
This feature adds a new table 'branch_hours' for storing the open and
close times per day for each library, and a new system preference
'ConsiderLibraryHoursInCirculation' to choose which behaviour should be
followed when calculating due dates.
To test:
1. Apply patches and update database. Upgrade schema if not applying
patch with schema changes. Restart services.
2. Go to Administration -> Libraries. Edit a library and scroll to the
bottom to find the 'opening hours' section. Test adding and removing
open and close times on various days. Confirm saving works as expected.
3. Add a new library and test adding open and close times works as
expected.
4. Edit your default library and save open and close times for each day.
5. Go to Administration -> Circulation and fine rules. Edit a rule, set
the unit to 'hours' and set the loan period to a number that would cause
a checkout to be due after the close time you just set, i.e. if you set
your close time to be 5pm and your system time is currently 1pm, set the
loan period to be 5 (5 hours) so the calculated due date would be 6pm.
6. Go to Administration -> system preferences. Search for
ConsiderLibraryHoursInCirculation. It should be under 'Checkout policy' in
the Circulation system preferences. Confirm the pre-selected option is
'ignore'. Keep this tab open.
6. In a new tab, get the barcode for an item that has an itemtype
matching the circulation rule you just set.
7. Go to the checkouts for a patron that has a categorycode matching the
circulation rule you just set.
8. Check out your item. Confirm that the checkout is due at the end of
the loan period, not taking closing hours into consideration. Return the
item.
9. Back in your other tab, set ConsiderLibraryHoursInCirculation to
'close', so the due date should be shortened to meet the close time.
10. Check out your item. Confirm the checkout is due when the library
closes. Return the item.
11. Back in your other tab, set ConsiderLibraryHoursInCirculation to
'open', so the due date should be extended to meet the opening time.
12. Check out your item. Confirm the checkout is due the next day when
the library opens.
13. Confirm tests pass t/db_dependent/Circulation/CalcDateDue.t
Sponsored-by: Catalyst IT
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Jonathan Druart
Martin Renvoize
Lucas Gass
Baptiste Wojtkowski
Nick Clemens
Lari Strand
Tomas Cohen Arazi
Fridolin Somers
Emmi Takkinen
Aleisha Amohia
Koha Development Team
Pedro Amorim
Eric Garcia
Martin Renvoize
Matt Blenkinsop
Emily Lamancusa
Andreas Jonsson
Laura Escamilla
Hayley Pelham