If the input is not a number, display an appropriate error message.
Additionally, don't make an API call unless the input is a number.
Test by doing the following:
1) Step 19: Moving record level holds, the modal says 'Enter the biblionumber of new hold target', but if you enter 'xyz' and then search, the error shown is 'No item found with barcode: NaN'.
This patch fixes that.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Instead of filtering response on the client.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Test plan:
1. APPLY PATCH, updatedatabase, restart_all
2. Have two staff members. 1. Superlibrarian, 2. Staff with new permission to `alter_hold_targets`
3. Now place some holds on different record, you'll want some item level holds and some record level holds.
4. After placing some holds go to a record with some holds on it.
5. Go to the holds tab (reserve/request.p)
6. You should notice a disabled 'Move selected (0)' button, next to 'Cancel selected (0)'
7. Select some holds to move, ensure that the number updates accordingly. ( Move selected (2) )
8. When you click the button you should see 2 options, 'Item level holds to a different item' and 'Record level holds to a different record'
9. Clicking on either should bring up a modal with information about the holds you selected to move.
10. If you have item level holds and you select 'Item level holds to a different item' those holds should appear pre-checked.
11. If you have item level holds and you select 'Record level holds to a different record' those holds checkboxes should be disabled by default, you should see the 'Problem' as 'Cannot move a waiting or record level hold'
12. If you are moving a item level hold you should see a searchbar labeled 'Enter the item barcode of new hold target:'.
13. Enter a full or partial barcode and click search
14. You should see your item, if it exists.
15. Check 'Move all selected item level holds to this item', once you do so the yellow 'Move selected holds' button should be enabled.
16. Click 'Move selected holds', the hold should be moved.
17. You should see a message like " Moved the following holds:
Hold ID: 1854 - Moved from The Arden edition of the works of William Shakespeare. to The Arden edition of the works of William Shakespeare."
19. Do steps 12 - 17 again, but with a record level hold.
20. Now make or have many holds on a specific target record to test priority. Make note of the current priority
21. Now move a hold on to that record, testing both item and record level holds.
22. Now on the target record review the priority. The holds originally on the record should not have changed. The hold you move should now have the lowest priority on the record.
23. Check the original record, if a hold was moved all other priority should have been adjusted correctly. FOr example, if the hold you moved was 3 of 4 then the others should have adjusted priority of 1 2 3 now.
24. Turn on HoldsLog if it is disabled.
25. Move some holds, both item and record ones, and review the logs after each one. There should be a correct MODIFY entry for each move.
26. Make an item with a hold on it as damaged and turn on AllowHoldsOnDamagedItems. Try to move the hold, you should get an error telling you that the target item is damaged.
27. Now login as the non-superlibrarian patron from step 2. If they have the `alter_hold_targets` permission they should see the 'Move selected' button.
28. Revoke the `alter_hold_targets` permission. The button should not appear now.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds a booking_id field to the issues and old_issues tables
to create a linkage between checkouts and the bookings that generated
them. When a patron with a booking checks out the reserved item, the
checkout is linked to their booking and the booking status is updated
to 'completed'.
The booking_id is preserved during renewals and when checkouts are
moved to old_issues upon return.
Database changes:
- Add booking_id column to issues and old_issues tables
- Add foreign key constraints to maintain referential integrity
Test plan:
1. Apply the database update
2. Restart services
3. Create a booking for a patron
4. When the patron checks out the booked item:
a. Verify the checkout links to the booking (booking_id is set)
b. Verify the booking status changes to 'completed'
5. Renew the checkout
a. Verify the booking_id is preserved after renewal
b. Check the database: SELECT booking_id FROM issues WHERE issue_id = X
6. Return the item
a. Verify the checkout moves to old_issues
b. Verify the booking_id is preserved in old_issues
7. Test the new Koha::Booking methods:
a. For a booking with a checkout, verify $booking->checkout returns
the correct Koha::Checkout object
b. For a booking with an old checkout, verify $booking->old_checkout
returns the correct Koha::Old::Checkout object
8. Test the API:
a. Verify checkout endpoints include booking_id in responses
b. Verify the booking relationship can be embedded using
x-koha-embed: booking
9. Run the test suite:
prove t/db_dependent/Circulation.t
prove t/db_dependent/Koha/Booking.t
prove t/db_dependent/Koha/Checkout.t
NOTE: I also thought about the inverse: linking to a checkout from the
booking, but as Koha is a circulation centric system, this approach makes
more sense in my opinion. I also thought about bidirectional linking via
FKs but that is too much maintenance overhead (and potential error source)
that we don't need. The inverse lookups should still be performant through
the FKs.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch updates the database schema to add booking_id foreign key
columns to both issues and old_issues tables, enabling the linkage
between checkouts and the bookings that generated them.
Schema changes:
- Add booking_id column to issues and old_issues tables
- Add foreign key constraints referencing bookings table
- Preserve referential integrity with appropriate CASCADE actions
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
We build the the orders table depending on some conditions (sysprefs or if the
basket is closed/opened).
DataTables cannot correctly restore the state because the number of columns
stored in localStorage is different than the one we have one the table.
Test plan:
Open and close a basket
Confirm that the table is correctly displayed (no extra columns)
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The default value for stateDuration is 7200 (2h). Which means DataTables
will ignore the saved state if reused after this delay.
Additionally is causes problem on some tables where the default state is
not restored and all columns are displayed.
From https://datatables.net/reference/option/stateDuration
```
Please note that the value is given in seconds. The value 0 is a special value as it indicates that the state can be stored and retrieved indefinitely with no time limit.
```
We are going to set it to 0 so that the state is only lost/reset after a
logout
Test plan:
Ideally you can should edit this file and set saveState to 60 (seconds),
so it's easy to test.
Do not apply this patch and confirm that the state is not restored after
either 60s or 2h (if you modified the file or not).
It is a problem on the checkout list table as extra columns (like
"Today's checkouts") are displayed.
With this patch applied the state should be restored correctly.
If you logout the state will be lost however.
Ideally we might want to store user's preferences on the server.
Signed-off-by: Ludovic Julien <ludovic.julien@inLibro.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch makes the Booking module dependant of a new YesNo syspref: EnableBooking
To do this, I tried and get all changes using the graph of Bug 29002 and
got interrested in every tt change to make changes on this depend on the
new syspref. File inside the booking module should not be amended.
Here are the files I found relevant for each patch
Bug 37737:
- koha-tmpl/intranet-tmpl/prog/en/includes/patron-detail-tabs.inc
- koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
Bug 37736:
- koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation-home.tt
Bug 29002:
- koha-tmpl/intranet-tmpl/prog/en/includes/cat-toolbar.inc
- koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/ISBDdetail.tt
- koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/MARCdetail.tt
- koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
- koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/imageviewer.tt
- koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/labeledMARCdetail.tt
- koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt
TEST PLAN:
1 - Set "Bookable = 1" on a given item
2 - Make sure you know where are located
- The field bookable on exemplaries
- The button "Place booking" in item toolbae
- The link "Bookings (0)" in item menu
3 - Apply patch and update database
4 - Check that every field listed in 2 - is still present
5 - Set the syspref "EnableBooking" to "Disable"
6 - Check that every field listed in 2 - is still present
7 - Browse Norme, ISBD, Marc, Items tabs + pages imageviewer.pl, labeledMARCdetail.pl and check that nothing is broken
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test this on the UI, you can add the following line of code at the beginning on 'new' in SushiCounter.pm:
->{response}->{Report_Header}->{Release} = 6;
This will force the release to be '6'. Then you can perform a harvest using the UI. The background job will fail with the error message saying that counter release 6 is not supported
prove t/db_dependent/Koha/BackgroundJobs/ErmSushiHarvester.t
Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
QA script still fails but I believe these are false positives caused by the tests files being moved
Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Update original tests to consider COUNTER 5 specifically.
Add new test for COUNTER 5.1.
Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This moves the original SUSHI COUNTER logic to a new
Koha::ERM::EUsage::COUNTER::5 class that extends
Koha::ERM::EUsage::SushiCounter.
Adds a new Koha::ERM::EUsage::COUNTER::5_1 class with COUNTER 5.1
specific logic.
Keeps common logic in Koha::ERM::EUsage::SushiCounter.
This ensures we support both COUNTER 5.0 and COUNTER 5.1, but should
hopefully help us better maintain COUNTER 5.1 going forward, without having
to worry about COUNTER 5.0, as the code is seperated.
This should also better set us up for adding support for possible future COUNTER releases.
Test plan, k-t-d:
1) Enable ERMModule
2) Add a new usage data provider at:
<staff_url>/cgi-bin/koha/erm/eusage/usage_data_providers/add
3) Add the required credential parameters e.g. Customer ID and Requestor ID
4) Update the 'Report release' to '5.1', no single quote.
(If you add anything other than '5', it'll default to '5.1')
5) Visit the list of data providers and click the 'Run now' button for your data provider:
<staff_url>/cgi-bin/koha/erm/eusage/usage_data_providers
6) Click the background job link at the top. Verify the background job runs as expected.
7) After the successful harvest, run a custom report at, ensure the data is all correct:
<staff_url>/cgi-bin/koha/erm/eusage/reports
8) Visit the data provider at:
<staff_url>/cgi-bin/koha/erm/eusage/usage_data_providers/<provider_id>
9) Click the 'Import logs' tab, download the COUNTER 5.1 counter file.
Open the file and verify the data is correct.
Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
We are now considering more than one COUNTER release. Move test files accordingly.
prove t/db_dependent/Koha/ERM/EUsage/*
Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Also fixed the link on circ-nav and the breadcrumbs links.
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch reduces the number of click for the average librarian user.
To test:
1. Change your logged in branch to something other than Centerville
2. Navigate to the circulation home page
3. Click on the Holds queue link
4. Observe that you have to hit Submit to continue
5. Apply patch
6. Repeat steps 1 and 2
7. Observe that the holds queue comes up with your branch preselected
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch modifies the OPAC's DataTables JS so that any table
heading with a "dtr-control-col" will trigger the application of title
and aria-label attributes on the responsive controls column.
All OPAC tables with a responsive configuration have been modified with
the class.
To test, apply the patch and check all affected tables.
- Narrow the width of the browser window until the responsive
functionality is triggered.
- Check that the expand/collapse button has title and aria-label
attributes, "Expand or collapse row details"
- Authority search results
- Course reserves - List of courses
- Course reserves - List of titles for a course
- Bibliographic detail page:
- Holdsings
- Subscriptions
- Search history
- Bibliographic, past and present
- Authorities, past and present
- Most popular titles
- Logged in user's active clubs
- Logged in user's scheduled curbside pickups
- Logged in user's recalls history
- Logged in user's checkout history
- Logged in user's messaging settings
- Logged in user's submitted tags
Sponsored-by: Athens County Public Libraries
Signed-off-by: Jake Deery <jake.deery@openfifth.co.uk>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
CancelExpiredReserves had incorrect holiday logic that checked if **today** is a holiday instead of checking if the **hold's expiration date** was a holiday.
This caused:
* Holds that expired on non-holidays to remain active if cleanup script ran on holidays
* Holds that expired on holidays to be cancelled if cleanup script ran on non-holidays
The simple fix exposed additional edge cases around server downtime that could leave holds active indefinitely.
Changes:
* Implements comprehensive holiday logic when `ExpireReservesOnHolidays = 0`
* Same-day protection: Don't cancel holds that expire TODAY if today is a holiday
* Historical cleanup: Cancel holds that expired in the past when running on business days
* Retrospective cancellation: Handle server downtime by checking for missed business days
* Comprehensive test suite covering all scenarios including long-term expired holds
To test:
1. Apply the tests patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Reserves/CancelExpiredReserves.t
=> FAIL: Tests fail! The logic in the method is wrong!
3. Apply this patch
4. Repeat 2
=> SUCCESS: All tests pass, demonstrating correct behavior in all scenarios
5. Sign off :-D
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds regression tests that demonstrate the bug in
CancelExpiredReserves where the holiday logic checks if today is a
holiday instead of checking if the hold's expiration date was a holiday.
The bug causes:
1. Holds that expired on non-holidays to remain active if the cleanup
script runs on a holiday
2. Holds that expired on holidays to be cancelled if the cleanup script
runs on a non-holiday
Test scenarios added:
- Hold expired on Monday (non-holiday), script runs on Tuesday (holiday)
- Hold expired on Wednesday (holiday), script runs on Thursday (non-holiday)
- Hold expired on Friday (non-holiday), script runs on Sunday (holiday)
- Hold expired on Saturday (holiday), script runs on Sunday (holiday)
These tests currently fail, demonstrating the bug exists. When the bug
is fixed, all tests should pass.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds a new method to Koha::Calendar that returns a boolean indicating
whether there are any business days between two given dates (exclusive).
Implementation details:
* Returns 1 if any business days exist between start and end dates
* Returns 0 for consecutive days, same dates, or only holidays between
* Uses exclusive range (does not include start or end dates)
* Handles date comparison and timezone normalization
* Efficient early return when first business day is found
This is designed specifically for use cases like detecting missed
business days due to server downtime or scheduling issues.
To test:
1. Apply patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/Calendar.t
=> SUCCESS: All tests pass!
3. Sign off :-D
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
If you apply this patch before the other one:
prove t/db_dependent/Koha/Biblio.t
Will fail. Apply both patches. The above now passes.
This highlighted something. Following the test plan I can't reproduce
this anymore. In order to reproduce, the check-outs need to happen
before the bookings, otherwise when the check-outs happen on an existing
booking it'll 'complete' or 'cancel' the booking thus not causing the
error.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Test plan:
1) Have a record with 8 items.
2) Place a booking on 6 of the items (6 bookings total) for a full month
(e.g. August)
3) Check-out all 6 of those bookings
4) Attempt to place a new booking for one of the 2 free items for end of month, e.g. (Aug 26 -> Aug 28).
5) It will fail with error "Booking conflicts". This is because the code
is adding 6 checkouts with 6 bookings = 12 which is higher than 8
bookable items.
6) With this patch, check-outs that refer to an existing booking are
excluded, so the check becomes if 8 - 6 > 0 (which passes the check) rather than 8 - 12 > 0 (which fails the check)
Run existing tests:
prove t/db_dependent/Koha/Item.t
prove t/db_dependent/Koha/Biblio.t
prove t/db_dependent/Koha/Booking.t
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds unit tests.. but it also adds some FIXME and QUESTION
statements to those tests
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
1. In circ rules set a Default waiting hold cancellation policy to allow hold cancellation requests.
2. Add a hold for a patron and create a hold cancellation request.
3. Check the item in via SIP, I used the SIP emulator:
kohadevbox/koha/misc/sip_cli_emulator.pl -a localhost -p 6001 -l MPL -su term1 -sp term1 -m checkin --patron 23529000035676 --item 39999000001310
4. The hold remains.
5. APPLY PATCH, restart_all
6. Try 2 - 4 again, the hold should be cancelled
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Lucas Gass
Pedro Amorim
Paul Derscheid
Jonathan Druart
Tomas Cohen Arazi
Baptiste
Jason Robb
Owen Leonard
Martin Renvoize