This patch adds comprehensive unit tests for the OAuth authentication
client, specifically covering the _get_data_and_patron method which
handles JWT token processing and UserInfo endpoint integration.
Test plan:
1. Apply patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/Auth/Client/OAuth.t
=> SUCCESS: Tests pass!
3. Tests cover:
- JWT id_token processing and patron matching
- UserInfo endpoint calls and data merging
- Non-existent user scenarios
- Combined JWT + UserInfo data sources
4. Sign off :-D
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
When starting SIP, Koha passes an incorrect value to the
Net::Server "group" parameter, which can generate an
error in the logs":
Couldn't become gid "<uid>": Operation not permitted
This change ensures that a valid numeric GID is passed instead.
Test plan:
1. Enter KTD shell.
2. To reproduce the error: sudo groupmod -g 8001 kohadev-koha
3. Restart SIP: sudo koha-sip --restart kohadev
4. Check SIP logs:
/var/log/koha/kohadev/sip-output.log
5. Observe the error:
Couldn't become gid "<uid>": Operation not permitted
6. Apply this patch.
7. Repeat steps 2 to 4.
8. Verify that the "Couldn't become gid" error is no longer logged.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Upon following original test plan after rebase, noticed that
OPACDisplay856uAsImage wasn't working for 'Details' or 'Results'.
The culprit was how each screen is invoking the template, passing the
string param in a wrong XSL way.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This work DRYs the 856 display logic to renderMARCOnlineResourceField.
This was previously written in detail and results pages.
Once this single function exists, 857 can also make use of it to be
displayed using the same logic/features already present for 856.
1) If testing k-t-d, add the test data for test plan (otherwise prep instructions below):
updatedatabase
2) Force reindex of this biblio by saving it, visit the biblio:
<staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=76
3) Click "Edit" -> "Edit record" -> "Save"
4) Open the following URL's, preferably on 4 different browser tabs (we're using biblio 76 for this test):
<staff_url>/cgi-bin/koha/catalogue/search.pl?q=music (first record [76] is the one we're testing)
<staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=76
<opac_url>/cgi-bin/koha/opac-search.pl?idx=&q=music (first record [76] is the one we're testing)
<opac_url>/cgi-bin/koha/opac-detail.pl?biblionumber=76
5) Verify that on all pages there are 3 instances of 856 (Online resources) and 3 instances of 857 (Online archive resources) with their respective link labels and URLs.
6) Click on the "Koha community no protocol" link for 856 and 857, notice the 'http://' protocol is added.
7) Change the values of OPACDisplay856uAsImage + Display856uAsImage and refresh the respective page. Verify it works as expected (respects the system preference). Note: This only applies for 856, not 857 (out of scope here).
8) Change the value of OPACURLOpenInNewWindow and click on the links before and after. Verify they respect the system preference.
If not on k-t-d and updatedatabase not available:
perl /kohadevbox/koha/installer/data/mysql/updatedatabase.pl
or
koha-shell -c "perl /kohadevbox/koha/installer/data/mysql/updatedatabase.pl" <instance_name>
If CLI not available (preparation on UI):
A) Add field 857 to the 'Default' framework
B) Edit the field:
<staff_url>/cgi-bin/koha/admin/marc_subfields_structure.pl?op=add_form&tagfield=857&frameworkcode=
C) Check the 'Editor' checkbox for subfields q, u and z (Add these if not there already)
D) Edit the record in question, visit:
<staff_url>/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=76
E) Click 'Settings' dropdown and pick 'Default' framework.
F) Add MARC data that corresponds to the following:
3 instances of 856 as below. 3 instances of 857 as below.
<datafield tag="856" ind1="4" ind2="2">
<subfield code="z">Koha community no protocol</subfield>
<subfield code="u">koha-community.org/</subfield>
</datafield>
<datafield tag="856" ind1="4" ind2="2">
<subfield code="z">Koha community with protocol</subfield>
<subfield code="u">https://koha-community.org/</subfield>
</datafield>
<datafield tag="856" ind1="4" ind2="2">
<subfield code="z">Koha logo image</subfield>
<subfield code="q">jpeg</subfield>
<subfield code="u">https://koha-community.org/files/2013/09/cropped-kohabanner3.jpg</subfield>
</datafield>
<datafield tag="857" ind1="4" ind2="2">
<subfield code="z">Koha community no protocol</subfield>
<subfield code="u">koha-community.org/</subfield>
</datafield>
<datafield tag="857" ind1="4" ind2="2">
<subfield code="z">Koha community with protocol</subfield>
<subfield code="u">https://koha-community.org/</subfield>
</datafield>
<datafield tag="857" ind1="4" ind2="2">
<subfield code="z">KohaCon 2024 logo image</subfield>
<subfield code="q">jpeg</subfield>
<subfield code="u">https://2024.kohacon.org/wp-content/uploads/2024/01/kohacon24_fr_verticale_vert_noir.png</subfield>
</datafield>
G) At this point you're ready to start the original test plan from step 4)
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Now that OPAC detail and search results are consitent, we can DRY this
logic to its own function to be also utilized by 857 in the following
commit
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The OPAC logic for 856 in results page and detail page has diverged.
This commit ensures the 856 in the OPAC detail and search results pages
are consistent, in preparation to become DRY in the following commits.
With this DRY refactor, the following divergences are now aligned:
- 856 block now calls AddMissingProtocol regardless of value OPACURLOpenInNewWindow (previously was only being called if OPACURLOpenInNewWindow was 0). Now it's in line with what exists in MARC21slim2OPACDetail.xsl
- substring(,1,6)='image/' is also now a consideration when OPACURLOpenInNewWindow is 1. (previously was only being called if OPACURLOpenInNewWindow was 0). This is now in line with that exists in MARC21slim2OPACDetail.xsl
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The FTP passive mode setting was not being saved due to a typo in the
parameter name ('passiv' instead of 'passive'). This fixes the typo
and adds display of passive mode status in the transports table.
Test plan:
1. Go to Administration > File transports
2. Click "New file transport"
3. Fill in the name (e.g., "Test FTP")
4. Change Transport to "FTP"
5. Set Passive mode to "Enabled (Recommended)"
6. Fill in required fields (host, username, etc.) and save
7. Edit the transport you just created
8. Verify Passive mode is still set to "Enabled" (not reverted to Disabled)
9. Return to the file transports list
10. Verify the Transport column shows "FTP (passive)" for this transport
11. Edit the transport and change Passive mode to "Disabled"
12. Save and return to the list
13. Verify the Transport column now shows just "FTP" (without the passive indicator)
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>
GetReserveFee was removed as part of the migration to circulation
rules-based hold fees, but reserve/request.pl still imported and
used it, causing compilation errors.
This patch replaces the two usages of GetReserveFee with the modern
approach using Koha::Item->holds_fee($patron), matching the
implementation in opac-reserve.pl.
The fee is calculated by iterating through available items and using
the first non-zero fee found, which provides backward compatibility
with the legacy reserve_charge template variable.
This implementation uses ReservesControlBranch to determine which
branch's circulation rules apply (either item home library or patron
home library), consistent with the existing Koha policy framework.
Test plan:
1. Apply patch
2. Verify reserve/request.pl compiles: perl -wc reserve/request.pl
3. Navigate to staff interface holds page with patron and biblio
4. Verify hold fee is displayed correctly if configured in
circulation rules
5. Place a hold and verify fee is charged correctly
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The TitleHoldFeeStrategy system preference was added in the database
update script (251200023.pl) but was missing from sysprefs.sql,
causing issues for fresh installations that don't run the update
script.
This preference likely disappeared during rebases. Adding it in the
correct alphabetical position between 'timeout' and
'todaysIssuesDefaultSortOrder'.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
- The test for circulation rules date calculations did not account for
the start date being bold, only the period endpoint dates
- place_booking.js explicitly adds the selected start date to boldDates
(boldDates = [new Date(startDate)]), giving it the "title" class
- The verification loop checking for unexpected bold dates then failed
when it encountered the start date, which was bold but not expected
- Add clearZoneStart to expectedBoldDates to match actual behavior
To test:
- yarn cypress run --spec t/cypress/integration/Circulation/bookingsModalDatePicker_spec.ts
- Verify that the "should handle circulation rules date calculations and visual feedback" test now passes
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Reorganize and expand Cypress tests into three focused spec files:
bookingsModalBasic_spec.ts:
- Modal initialization and field interactions
- Form validation and submission
- Edit booking functionality
- 'Any item' booking with server-side item selection
bookingsModalDatePicker_spec.ts:
- Flatpickr initialization and constraints
- Date range validation
- Circulation rules (issue length, renewals)
- Lead/trail period conflict detection
- 'Any item' mode date availability
- Event dots for existing bookings
bookingsModalTimezone_spec.ts:
- Cross-timezone date display
- Multi-day booking spans
- Conflict detection with timezone awareness
- API round-trip date preservation
- Month boundary handling
Add shared test utilities to e2e.js for date helpers.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Extract utility functions and reduce code duplication:
- Add toInt(), startOfDay(), datesOverlap(), isDateInRange() helpers
- Simplify containsAny() using Set and Array.some()
- Refactor isItemAvailableForPeriod() to use new utilities
- Refactor getAvailableItemsOnDate() and isDateDisabledForSpecificItem()
- Extract submit handler helpers: getAvailableItemsForPeriod(),
buildBookingPayload(), createTimelineItem(), showBookingError(),
showBookingSuccess(), refreshBookingsTable(), setPickerDates()
- Replace parseInt() calls with toInt() for consistency
- Simplify modal reset handler with jQuery method chaining
No functional changes - prepares codebase for future Vue conversion.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Fix date handling issues that caused bookings to shift by a day
when the browser timezone differs from the server timezone:
- Normalize dates to browser timezone for calendar display
- Use dayjs without UTC conversion for bookingsByDate keys
- Extract local date and send as explicit UTC day boundaries
- Preserve user's selected DATE regardless of browser timezone
Timeline display:
- Convert server dates to library timezone for display
- Add $timezone() and $toDisplayDate() helper functions
This ensures that a booking for "June 15" displays and saves as
June 15 regardless of the user's timezone offset.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
When booking "any item of type X", lead/trail period handling must
consider that multiple items may be available:
- Track per-item conflicts instead of global conflicts
- Only disable dates when ALL items of the type have conflicts
- Find closest "all items booked" dates for visual feedback
- Use mathematical search (180 days) to detect conflicts across
month boundaries
Visual feedback for 'any item' mode:
- Show lead/trail periods around "all items booked" dates
- These represent dates where no item of the type is available
Skip redundant class-based conflict checks since mathematical
detection already handles cross-month boundary cases.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Enhance the booking modal to detect and prevent lead/trail period
conflicts in both directions:
- New booking's lead period vs existing booking's trail period
- New booking's trail period vs existing booking's lead period
Visual feedback:
- Highlight existing booking's lead periods (before booking starts)
- Highlight existing booking's trail periods (after booking ends)
- Add CSS classes for start/end boundaries with rounded corners
User feedback:
- Show contextual messages explaining why dates are blocked
- Differentiate between lead period, trail period, and booking conflicts
- Display informational messages when hovering available dates
The conflict detection uses mathematical date range comparisons that
work across month boundaries, not just visible calendar dates.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds end-to-end Cypress tests for the booking modal improvements,
including smart window maximization and server-side optimal item selection.
New Tests:
1. "should successfully submit an 'Any item' booking with server-side optimal item selection"
- Tests the complete "any item" booking workflow
- Selects itemtype and "Any item" option (item_id = 0)
- Submits booking form
- Verifies modal closes without errors
- Validates booking created in database
- Confirms item was assigned by server
- Verifies server-side optimal selection completed
- Test status: PASSING
2. "should maximize booking window by dynamically reducing available items during overlaps"
- Creates 4 items with strategic booking patterns
- Tests "never re-add items to pool" algorithm
- Validates 5 scenarios:
a) Full window when all items available
b) Reduced window when Item 1 becomes unavailable
c) Further reduction when Items 1&2 unavailable
d) Minimal window with Items 1,2,3 unavailable
e) No availability when all items booked
- Demonstrates smart window maximization
- Test status: PASSING
3. "should handle edit mode date initialization correctly"
- Tests fix for edit mode race condition
- Verifies dates populate correctly when editing booking
- Ensures itemtype is set before date initialization
- Tests that flatpickr dates are set properly
- Test status: PASSING
Updated Tests:
- "should successfully submit a booking"
* Updated to use day+5 start date (accounts for 3-day lead period)
* Ensures test doesn't fail on past-date constraint
- Updated test comments and log messages to reflect server-side selection
* Changed from "client-side selectOptimalItem()" references
* Updated to "server-side optimal item selection"
* Clarified that server performs the optimal selection
Test Coverage:
- End-to-end "any item" booking submission
- Smart window maximization algorithm
- Edit mode date initialization
- Server-side optimal selection validation
- Database verification of assigned items
- Modal interaction and form submission
Test plan:
1. Start KTD environment
2. Ensure Cypress is installed: yarn install
3. Run: npx cypress run --spec "t/cypress/integration/Circulation/bookingsModalBasic_spec.ts"
4. Verify all 11 tests pass
5. Optionally run in interactive mode: npx cypress open
Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch implements client-side improvements for the bookings modal,
including smart window maximization and hybrid submission logic that
delegates optimal item selection to the server.
Smart Window Maximization:
- Implements "never re-add items to pool" algorithm
- Dynamic item pool reduction based on availability
- isDateDisabledForItemtype() calculates available dates
- isDateInMaximumWindow() validates end dates against item pool
- Provides real-time calendar feedback to users
Hybrid Submission Logic:
For "any item" bookings, the client now uses a smart hybrid approach:
1. Filters items by selected itemtype
2. Checks availability for the booking period
3. If 0 items available: Shows error (no suitable item)
4. If 1 item available: Sends item_id (optimization - skips server)
5. If 2+ items available: Sends itemtype_id (delegates to server)
This minimizes server round-trips while delegating complex optimal
selection logic to the server when multiple choices exist.
Test plan:
1. Navigate to biblio detail page with bookable items
2. Click "Place booking" and select patron/pickup location
3. Select itemtype (not specific item) - "any item" mode
4. Select start date - verify end dates are maximized
5. Verify calendar disables dates when no items available
6. Submit booking - verify it succeeds
7. Check that optimal item was selected (server-side)
8. Test edit mode with "any item" selection
9. Verify both create and edit use same logic
Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds unit tests for the server-side optimal item selection
functionality.
t/db_dependent/Koha/Booking.t:
- Added subtest for _select_optimal_item() method (7 tests)
* Tests with no items (returns undef)
* Tests with single item (returns that item)
* Tests with multiple items (selects item with longest future availability)
* Tests optimal selection across items with different booking patterns
* Tests handling of cancelled bookings (ignored in selection)
* Tests iterator reset after selection
- Added subtest for _assign_item_for_booking() with itemtype filtering (6 tests)
* Tests booking without item_id or itemtype_id (selects any available)
* Tests booking with itemtype_id (filters by type)
* Tests optimal selection within filtered itemtype
* Tests exception when no items of specified type available
- Added integration test for full optimal selection workflow (5 tests)
* Creates 3 items with different future booking patterns
* Verifies items selected in optimal order (B→C→A)
* Demonstrates preservation of constrained resources
* Validates complete algorithm chain
* Tests booking failure when all items exhausted
t/db_dependent/api/v1/bookings.t:
- Added subtest for itemtype_id parameter validation (16 tests)
* Tests successful booking creation with itemtype_id
* Tests that item is automatically assigned
* Tests validation: cannot specify both item_id and itemtype_id
* Tests validation: must specify either item_id or itemtype_id
* Tests clash detection with itemtype-based bookings
- Fixed duplicate booking_id test to use different dates
* Prevents clash detection from masking duplicate ID constraint
- Updated test count from 3 to 8 subtests in Booking.t
- Updated test count to match actual assertions (16 not 18)
Test Coverage:
- Internal method testing: _select_optimal_item()
- Integration testing: _assign_item_for_booking()
- API endpoint testing: add() with itemtype_id
- End-to-end workflow validation
- Edge cases: empty sets, single items, ties, cancelled bookings
Test plan:
1. prove t/db_dependent/Koha/Booking.t
2. prove t/db_dependent/api/v1/bookings.t
3. Verify all tests pass
Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch implements an algorithm for selecting the item that would
leave the largest gap after the proposed booking to maximize the
opportunity for future bookings.
Key Changes:
Koha::Booking:
- Added set_itemtype_filter() method for transient itemtype parameter
- Added _select_optimal_item() method using single aggregated query
* Groups bookings by item_id with MIN(start_date)
* Selects items with no future bookings (optimal)
* Among items with bookings, selects one with latest next booking
* O(1) complexity - single database query with GROUP BY
- Updated _assign_item_for_booking() to use optimal selection
* Filters by itemtype when itemtype_id is provided
* Replaces previous random selection (RAND())
API Changes:
- Added itemtype_id parameter support to add() endpoint
- Added itemtype_id parameter support to update() endpoint
- Validates exactly one of item_id or itemtype_id is provided
- itemtype_id is transient - not persisted to database
- Extracted and applied via set_itemtype_filter() before store()
Architecture:
- itemtype_id is NOT a database column (transient only)
- Used as selection criterion, then discarded
Test plan:
1. Apply patch and restart services
2. Create "any item" booking via staff interface
3. Verify item with longest future availability is selected
4. Test via API with itemtype_id parameter
5. Verify itemtype_id is not persisted to database
6. Run t/db_dependent/Koha/Booking.t
7. Run t/db_dependent/api/v1/bookings.t
Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
This commit integrates hold fee configuration into the main circulation
rules system, providing granular control over hold fees based on patron
category, item type, and branch combinations.
Changes:
- Add hold_fee as a new circulation rule type with full scope support
- Remove deprecated reservefee column mapping from Koha::Patron::Category
- Integrate hold fee input into the circulation rules admin interface
- Add hold fee column to the main circulation rules matrix
- Enable proper rule hierarchy resolution for hold fees
Hold fees can now be configured at multiple levels with automatic
fallback handling following standard circulation rule patterns.
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>
This commit adds the database migration to move hold fee configuration
from patron categories to circulation rules, providing more granular
control over hold fees based on patron category, item type, and branch.
The migration:
- Creates circulation rules for existing categories.reservefee values
- Removes the deprecated reservefee column from categories table
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>
Also fix QA failures:
[FAIL] installer/data/mysql/atomicupdate/bug_41624.pl
FAIL file_permissions
File must have the exec flag
FAIL forbidden_patterns
forbidden pattern: trailing space char (line 13)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Bug 41624: Revert "Bug 35211: (QA follow-up) Fix counting"
This reverts commit 97a022054d.
Bug 41624: Revert "Bug 35211: (QA follow-up) fix display of other results tab and move template toolkit logic outside of script array"
This reverts commit c4c2a4b235.
Bug 41624: Revert "Bug 35211: Removed local items from group tabs and removed group items from other holdings tab"
This reverts commit 124c6c8670.
Bug 41624: Revert "Bug 35211: Fixed group tab item building logic"
This reverts commit 23eb2dbd39.
Bug 41624: Revert "Bug 35211: Fix tab building"
This reverts commit f438535b5c.
Bug 41624: Revert "Bug 35211: Set SeparateHoldingsByGroup to separate based on SeparateHoldingsBranch syspref"
This reverts commit d2a0527d86.
Bug 41624: Revert "Bug 35211: Fixed count on logged in holdings"
This reverts commit 96c7eb3459.
Bug 41624: Revert "Bug 35211: Reordered tabs in Holdings Table and fixed group tabnames"
This reverts commit 5c2b0fd496.
Bug 41624: Revert "Bug 35211: Separate group holdings into individual tabs"
This reverts commit 08045c8009.
Bug 41624: DB update to remove the pref
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
xt/js_tidy.t does not check all files every run (it's using the incremental runs).
Something happened (node version changed?) and this is triggered because bug 41512 modified those files
Test plan:
0. Do not apply this patch
1. KOHA_CI_INCREMENTAL_RUNS=1 prove xt/js_tidy.t
=> fail
2. Apply this patch
3. Run prove again
=> pass
4. Run prove without the env var set
=> pass (i.e. there are no other JS files not tidy)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
1 - add 856$u " www.cheese.com" to a record
2 - confirm link does not work from results or details in either staff interface or OPAC
3 - add 856$u "www.cheese.com " to a record
4 - confirm link does not work from results or details in either staff interface or OPAC
7 - apply patch, restart_all
8 - reload pages, confirm both links work in results and details in staff interface and OPAC
Optional final step: have some cheese as a treat
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch updates the definition of the OAI-PMH:DeletedRecord
preference in sysprefs.sql, correcting the switched options and
explanation columns.
To test, apply the patch and run updatedatabase. Check that the database
has been updated correctly:
SELECT `options`, `explanation` FROM `systempreferences` WHERE
`variable` = 'OAI-PMH:DeletedRecord';
Sponsored-by: Athens County Public Libraries
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This highlights the issue mentioned in the last patch, reporting that
nothign will be done and exiting the script
To test:
1 - perl misc/cronjobs/longoverdue.pl -l 5=1 --library CPL
2 - Confirm script exits with warning that nothing has been done
Sponsored-by: MAIN Library Alliance
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The test plan here is largely confirming that nothing has changed.
Using CircControl here has an existing problem - when set to 'PickupLibrary'
the check in this cron looks for items from the userenv branch - the cron doesn't
have a userenv branch, it is undef, so for any library limiting by branch and
with CircControl set to 'PickupLibrary' / 'the library you are logged in at' the cron will do nothing
Sponsored-by: MAIN Library Alliance
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The get_plugins_intranet_js method now passes the current script name
(controller) to plugins via a 'page' parameter, allowing plugins to
determine which page/controller is being displayed.
This will allow plugin authors to only return JS that's relevant to the
current page when they wish to do so.
Test plan:
1. Create a plugin with an intranet_js method that logs the received parameters
2. Navigate to different pages in the staff interface
3. Verify that the plugin receives the page parameter containing the script name
4. Confirm that the script name reflects the actual controller (e.g.,
'/cgi-bin/koha/admin/admin-home.pl' when on the admin home page)
5. Run the test: prove t/db_dependent/Koha/Template/Plugin/KohaPlugins.t
6. Verify that the new test passes, confirming plugins receive the page parameter
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Tomás Cohen Arazi
Slava Shishkin
Pedro Amorim
Pedro Amorim
Paul Derscheid
Martin Renvoize
Lucas Gass
Jonathan Druart
Andrew Fuerste Henry
Owen Leonard
Nick Clemens
