Bug 40817

Summary: Holds charges should be accessible from Holds
Product: Koha Reporter: Martin Renvoize (ashimema) <martin.renvoize>
Component: Hold requestsAssignee: Martin Renvoize (ashimema) <martin.renvoize>
Status: Needs Signoff --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, lisette, testopia, tomascohen
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 3492    
Bug Blocks:    
Attachments: Bug 40817: Update database for hold-account linking
Bug 40817: Schema updates
Bug 40817: Add Koha object methods for hold-account line linking
Bug 40817: Add REST API support for hold-account line embedding
Bug 40817: Add API tests for hold-debits embedding
Bug 40817: Add comprehensive unit tests for hold-account line linking

Description Martin Renvoize (ashimema) 2025-09-16 09:17:31 UTC
It should be easy to get from a hold to it's associated charges.. i.e. Hold fee, Cancellation fee, any processing fees.

This would be useful for API responses and display purposes.
Comment 1 Martin Renvoize (ashimema) 2025-09-16 12:57:36 UTC
Created attachment 186476 [details] [review]
Bug 40817: Update database for hold-account linking

This patch adds database schema changes to support direct linking
between holds and their associated account lines (fees/charges):

* Add reserve_id and old_reserve_id columns to accountlines table
* Add foreign key constraints with CASCADE/SET NULL behavior
* Add indexes for performance on the new foreign key columns
* Add atomic update script for safe database migration

These schema changes enable:
- Direct linking between current holds and their fees
- Automatic migration of account line links when holds move to old_reserves
- Proper referential integrity with cascading behavior
Comment 2 Martin Renvoize (ashimema) 2025-09-16 12:57:38 UTC
Created attachment 186477 [details] [review]
Bug 40817: Schema updates
Comment 3 Martin Renvoize (ashimema) 2025-09-16 12:57:40 UTC
Created attachment 186478 [details] [review]
Bug 40817: Add Koha object methods for hold-account line linking

This patch implements the core object methods to support direct linking
between holds and their associated account lines:

Koha::Account.pm:
- Enhance add_debit() to accept hold_id parameter for linking fees to holds
- Add automatic detection of current vs old holds for proper field assignment
- Add graceful handling of non-existent holds

Koha::Account::Line.pm:
- Add hold() method to retrieve associated hold (current or old)
- Support bidirectional navigation between account lines and holds

Koha::Hold.pm:
- Add debits() method to get all charges associated with the hold
- Update charge_hold_fee() to link created fees to the hold
- Enhance _move_to_old() to migrate account line relationships
- Add hold_id parameter to fee creation calls

Koha::Old::Hold.pm:
- Add debits() method for accessing charges linked to old holds
Comment 4 Martin Renvoize (ashimema) 2025-09-16 12:57:42 UTC
Created attachment 186479 [details] [review]
Bug 40817: Add REST API support for hold-account line embedding

This patch extends the REST API to support embedding debits in hold
responses and linking holds in debit responses:

API Schema Changes:
- Add debits field to hold.yaml definition for embedding account lines
- Add hold field to debit.yaml definition for embedding associated holds
- Update holds.yaml and patrons_holds.yaml endpoints to support debits embedding

Koha::Account::Debit.pm:
- Add hold() method for API embedding support
- Enable bidirectional API relationships between debits and holds

These changes allow API clients to:
- Retrieve holds with their associated charges in a single request
- Access hold information from debit/charge records
- Reduce API round trips by embedding related data
- Support efficient financial reporting and hold management workflows
Comment 5 Martin Renvoize (ashimema) 2025-09-16 12:57:44 UTC
Created attachment 186480 [details] [review]
Bug 40817: Add API tests for hold-debits embedding

This patch adds comprehensive API tests to verify the new hold-debits
embedding functionality:

holds.t:
- Test GET /holds with debits embedding returns correct structure
- Test POST /holds response includes embedded debits when requested
- Verify embedded debits contain expected account line data
- Test embedding works with both successful and failed hold creation

patrons_holds.t:
- Test GET /patrons/{id}/holds with debits embedding
- Verify patron-specific hold debits are correctly returned
- Test embedding parameter validation and error handling
- Ensure proper authorization and access control
Comment 6 Martin Renvoize (ashimema) 2025-09-16 12:57:46 UTC
Created attachment 186481 [details] [review]
Bug 40817: Add comprehensive unit tests for hold-account line linking

This patch adds comprehensive unit test coverage for the hold-account
line linking functionality:

t/db_dependent/Koha/Account/Line.t:
- Test hold() method returns correct Koha::Hold/Koha::Old::Hold objects
- Test handling of reserve_id and old_reserve_id fields
- Test graceful handling of NULL and invalid references

t/db_dependent/Koha/Hold.t:
- Test debits() method returns correct Koha::Account::Debits collection
- Test _move_to_old() account line migration functionality
- Test proper linking of fees to holds during creation and lifecycle

t/db_dependent/Koha/Old/Hold.t:
- Test debits() method for historical holds
- Test account line relationships for completed/cancelled holds

t/db_dependent/Koha/Account.t:
- Test add_debit() with hold_id parameter for current and old holds
- Test automatic hold detection and field assignment
- Test error handling for non-existent holds