Bugzilla – Attachment 190800 Details for
Bug 40134
Fix and optimise 'Any item' functionality of bookings
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40134: Implement optimal item selection for 'any item' bookings
5dfa8a2.patch (text/plain), 11.30 KB, created by
Martin Renvoize (ashimema)
on 2025-12-31 17:12:39 UTC
(
hide
)
Description:
Bug 40134: Implement optimal item selection for 'any item' bookings
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-12-31 17:12:39 UTC
Size:
11.30 KB
patch
obsolete
>From 5dfa8a21139264e4940b80e0901842480bd90156 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Wed, 31 Dec 2025 16:52:50 +0000 >Subject: [PATCH] Bug 40134: Implement optimal item selection for 'any item' > bookings > >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 >--- > Koha/Booking.pm | 130 ++++++++++++++++-- > Koha/REST/V1/Bookings.pm | 44 +++++- > api/v1/swagger/definitions/booking.yaml | 8 +- > api/v1/swagger/definitions/booking_patch.yaml | 7 +- > 4 files changed, 174 insertions(+), 15 deletions(-) > >diff --git a/Koha/Booking.pm b/Koha/Booking.pm >index 3f83fe25c44..4e62da2a73e 100644 >--- a/Koha/Booking.pm >+++ b/Koha/Booking.pm >@@ -39,6 +39,22 @@ Koha::Booking - Koha Booking object class > > =head2 Class methods > >+=head3 set_itemtype_filter >+ >+ $booking->set_itemtype_filter($itemtype_id); >+ >+Sets a transient itemtype filter for item selection. This is used when >+creating "any item" bookings to filter items by type before optimal selection. >+This value is not persisted to the database. >+ >+=cut >+ >+sub set_itemtype_filter { >+ my ( $self, $itemtype_id ) = @_; >+ $self->{_itemtype_filter} = $itemtype_id; >+ return $self; >+} >+ > =head3 biblio > > Returns the related Koha::Biblio object for this booking >@@ -261,17 +277,28 @@ sub _assign_item_for_booking { > my $checkouts = > $biblio->current_checkouts->search( { date_due => { '>=' => $dtf->format_datetime($start_date) } } ); > >- my $bookable_items = $biblio->bookable_items->search( >- { >- itemnumber => [ >- '-and' => { '-not_in' => $existing_bookings->_resultset->get_column('item_id')->as_query }, >- { '-not_in' => $checkouts->_resultset->get_column('itemnumber')->as_query } >- ] >- }, >- { order_by => \'RAND()', rows => 1 } >- ); >+ # Build search conditions for bookable items >+ my $item_search_conditions = { >+ itemnumber => [ >+ '-and' => { '-not_in' => $existing_bookings->_resultset->get_column('item_id')->as_query }, >+ { '-not_in' => $checkouts->_resultset->get_column('itemnumber')->as_query } >+ ] >+ }; >+ >+ # Filter by itemtype if specified (for "any item" bookings) >+ if ( $self->{_itemtype_filter} ) { >+ $item_search_conditions->{itype} = $self->{_itemtype_filter}; >+ } > >- my $itemnumber = $bookable_items->single->itemnumber; >+ my $bookable_items = $biblio->bookable_items->search($item_search_conditions); >+ >+ # Use optimal selection instead of random selection >+ my $optimal_item = $self->_select_optimal_item($bookable_items); >+ >+ Koha::Exceptions::Booking::Clash->throw("No available items found for booking") >+ unless $optimal_item; >+ >+ my $itemnumber = $optimal_item->itemnumber; > return $self->item_id($itemnumber); > } > >@@ -309,6 +336,89 @@ sub to_api_mapping { > > =head2 Internal methods > >+=head3 _select_optimal_item >+ >+ my $item_id = $self->_select_optimal_item($available_items); >+ >+Selects the optimal item from a set of available items by choosing the item >+with the longest future availability after the booking ends. >+ >+This maximizes future booking opportunities by preserving items with shorter >+future availability for bookings that specifically need them. >+ >+=cut >+ >+sub _select_optimal_item { >+ my ( $self, $available_items ) = @_; >+ >+ return unless $available_items && $available_items->count > 0; >+ >+ # If only one item available, return it immediately >+ return $available_items->next if $available_items->count == 1; >+ >+ my $check_date = dt_from_string( $self->end_date )->add( days => 1 ); >+ my $dtf = Koha::Database->new->schema->storage->datetime_parser; >+ >+ # Get item IDs from the available items >+ my @item_ids = map { $_->itemnumber } $available_items->as_list; >+ >+ # Find the item with the latest (furthest in future) next booking start date >+ # Items with no future bookings will have NULL and sort first (most desirable) >+ my $search_conditions = { >+ item_id => { '-in' => \@item_ids }, >+ start_date => { '>=' => $dtf->format_datetime($check_date) }, >+ status => { '-not_in' => [ 'cancelled', 'completed' ] } >+ }; >+ >+ # Exclude current booking if we're editing >+ if ( $self->in_storage ) { >+ $search_conditions->{booking_id} = { '!=' => $self->booking_id }; >+ } >+ >+ # Query to find the earliest future booking for each item >+ my $rs = Koha::Bookings->search( >+ $search_conditions, >+ { >+ select => [ 'item_id', { min => 'start_date', -as => 'next_booking_start' } ], >+ as => [ 'item_id', 'next_booking_start' ], >+ group_by => ['item_id'], >+ } >+ ); >+ >+ # Get items that have future bookings, sorted by next booking date (desc = later is better) >+ my %next_booking_by_item; >+ while ( my $row = $rs->next ) { >+ $next_booking_by_item{ $row->get_column('item_id') } = $row->get_column('next_booking_start'); >+ } >+ >+ # Find the best item: items without future bookings first, then by latest next booking >+ my $best_item_id; >+ my $latest_next_booking; >+ >+ foreach my $item_id (@item_ids) { >+ my $next_booking = $next_booking_by_item{$item_id}; >+ >+ # Items with no future bookings are best (infinite availability) >+ if ( !defined $next_booking ) { >+ $best_item_id = $item_id; >+ last; >+ } >+ >+ # Otherwise, prefer items with later (further in future) next bookings >+ if ( !defined $latest_next_booking || $next_booking gt $latest_next_booking ) { >+ $latest_next_booking = $next_booking; >+ $best_item_id = $item_id; >+ } >+ } >+ >+ # Return the optimal item >+ return Koha::Items->find($best_item_id) if $best_item_id; >+ >+ # Fallback: shouldn't reach here, but return first available item >+ $available_items->reset; >+ return $available_items->next; >+} >+ > =head3 _send_notice > > $self->_send_notice(); >diff --git a/Koha/REST/V1/Bookings.pm b/Koha/REST/V1/Bookings.pm >index 52b8e724750..6807ebb65dd 100644 >--- a/Koha/REST/V1/Bookings.pm >+++ b/Koha/REST/V1/Bookings.pm >@@ -75,7 +75,36 @@ sub add { > my $c = shift->openapi->valid_input or return; > > return try { >- my $booking = Koha::Booking->new_from_api( $c->req->json ); >+ my $body = $c->req->json; >+ >+ # Validate that exactly one of item_id or itemtype_id is provided >+ my $has_item_id = defined $body->{item_id}; >+ my $has_itemtype_id = defined $body->{itemtype_id}; >+ >+ if ( !$has_item_id && !$has_itemtype_id ) { >+ return $c->render( >+ status => 400, >+ openapi => { error => "Either item_id or itemtype_id must be provided" } >+ ); >+ } >+ >+ if ( $has_item_id && $has_itemtype_id ) { >+ return $c->render( >+ status => 400, >+ openapi => { error => "Cannot specify both item_id and itemtype_id" } >+ ); >+ } >+ >+ # Extract and remove itemtype_id from body (it's not a database column) >+ my $itemtype_id = delete $body->{itemtype_id}; >+ >+ my $booking = Koha::Booking->new_from_api($body); >+ >+ # Set transient itemtype filter if provided (for server-side optimal selection) >+ if ($itemtype_id) { >+ $booking->set_itemtype_filter($itemtype_id); >+ } >+ > $booking->store; > $booking->discard_changes; > $c->res->headers->location( $c->req->url->to_string . '/' . $booking->booking_id ); >@@ -117,7 +146,18 @@ sub update { > unless $booking; > > return try { >- $booking->set_from_api( $c->req->json ); >+ my $body = $c->req->json; >+ >+ # Extract and remove itemtype_id from body (it's not a database column) >+ my $itemtype_id = delete $body->{itemtype_id}; >+ >+ $booking->set_from_api($body); >+ >+ # Set transient itemtype filter if provided (for server-side optimal selection) >+ if ($itemtype_id) { >+ $booking->set_itemtype_filter($itemtype_id); >+ } >+ > $booking->store(); > $booking->discard_changes; > return $c->render( status => 200, openapi => $c->objects->to_api($booking) ); >diff --git a/api/v1/swagger/definitions/booking.yaml b/api/v1/swagger/definitions/booking.yaml >index 0893dfdc24b..42f3b0ff8c3 100644 >--- a/api/v1/swagger/definitions/booking.yaml >+++ b/api/v1/swagger/definitions/booking.yaml >@@ -25,10 +25,15 @@ properties: > format: date-time > type: string > item_id: >- description: Internal item identifier >+ description: Internal item identifier for specific item bookings > type: > - integer > - 'null' >+ itemtype_id: >+ description: Item type identifier for "any item" bookings. Either item_id or itemtype_id must be provided. This is used for server-side optimal item selection and is not persisted. >+ type: >+ - string >+ - 'null' > item: > description: Embedable item representation > type: >@@ -66,7 +71,6 @@ properties: > - 'null' > required: > - biblio_id >- - item_id > - patron_id > - pickup_library_id > - start_date >diff --git a/api/v1/swagger/definitions/booking_patch.yaml b/api/v1/swagger/definitions/booking_patch.yaml >index dee20853e8a..c1d0fbe00c3 100644 >--- a/api/v1/swagger/definitions/booking_patch.yaml >+++ b/api/v1/swagger/definitions/booking_patch.yaml >@@ -23,10 +23,15 @@ properties: > format: date-time > type: string > item_id: >- description: Internal item identifier >+ description: Internal item identifier for specific item bookings > type: > - integer > - 'null' >+ itemtype_id: >+ description: Item type identifier for "any item" bookings. Either item_id or itemtype_id must be provided. This is used for server-side optimal item selection and is not persisted. >+ type: >+ - string >+ - 'null' > modification_date: > description: Modification date and time of this booking > readOnly: true >-- >2.52.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40134
:
183211
|
186371
| 190800 |
190801
|
190802
|
190803