From 5dfa8a21139264e4940b80e0901842480bd90156 Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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