From c7673a9d6a947fcc921832ae2d2d1106aaad3a06 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 8 Jun 2026 15:06:06 +0100 Subject: [PATCH] Bug 41898: Introduce 'issued' status and update booking display/filtering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements a 4-state booking lifecycle: new → issued (on checkout) → completed (on return), plus cancelled at any point. Backend changes: - AddIssue sets booking status to 'issued' (was incorrectly 'completed') - AddReturn transitions 'issued' → 'completed' via booking_id on the issue - filter_by_active updated to status-based: status IN ('new', 'issued'); previously it filtered by date range — active/inactive now reflect booking lifecycle rather than the current date relative to start/end dates - _is_final_status_transition reverted to only skip clash detection for terminal statuses 'cancelled' and 'completed'; 'issued' is active and does not bypass clash detection - GET /api/v1/bookings no longer pre-filters to active bookings server-side; callers must supply an explicit status filter if they need active-only Display changes: - Patron bookings tab: replaces the date-based expired/active toggle with per-status filters (completed, cancelled) using the AdditionalFilters framework; active bookings (new/issued) are shown by default - Biblio bookings list: same status-based filters; removes the date-based expired filter that was still present - Status column is always visible on both views (was previously hidden on the patron tab unless expired bookings were shown) - Status badges: new=bg-success, issued=bg-info, completed/cancelled=bg-secondary - Timeline isActive check corrected to ['new','issued'] - patron-detail-tabs.inc: rename expired_bookings_count → closed_bookings_count to reflect status-based (not date-based) counting - Filter button colour fixed to use $link-color so both views are consistent Test plan: 1. Enable the EnableBookings system preference. 2. Create a biblio with at least one bookable item. 3. Place a booking for a patron via the biblio's Bookings tab. - The booking should appear with status badge "New" (green). 4. Check out the booked item to that patron (Circulation → Check out). - Reload the Bookings tab; the booking should now show "Issued" (blue). 5. Return the item (Circulation → Check in). - Reload the Bookings tab; the booking should now show "Completed" (grey). 6. Place a second booking and cancel it from the Bookings tab Actions menu. - The booking should show "Cancelled" (grey), with the reason if provided. 7. On the biblio Bookings tab: - Completed and cancelled bookings are hidden by default. - Clicking "Include completed" shows completed bookings. - Clicking "Include cancelled" shows cancelled bookings. - Both can be toggled independently. - The Status column is always visible. 8. Navigate to the patron's detail page → Bookings tab. - Active bookings (new/issued) are shown by default. - Completed and cancelled can be toggled independently using the same filter buttons. - The Status column is always visible. 9. Confirm via the REST API that GET /api/v1/bookings (no filters) returns bookings of all statuses, not just active ones. Sponsored-by: OpenFifth Sponsored-by: Westminster City Council Signed-off-by: Hannah Dunne-Howrie Signed-off-by: Paul Derscheid Signed-off-by: Pedro Amorim --- C4/Circulation.pm | 13 ++- Koha/Booking.pm | 13 +-- Koha/Bookings.pm | 10 +- Koha/REST/V1/Bookings.pm | 2 +- admin/columns_settings.yml | 1 - .../prog/css/src/staff-global.scss | 1 + .../prog/en/includes/patron-detail-tabs.inc | 13 +-- .../prog/en/modules/bookings/list.tt | 48 ++-------- .../prog/en/modules/members/moremember.tt | 1 + .../intranet-tmpl/prog/js/tables/bookings.js | 92 ++++++------------- 10 files changed, 65 insertions(+), 129 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 0129ba055c..67c84266c0 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -105,6 +105,7 @@ use Koha::Calendar; use Koha::Checkouts; use Koha::ILL::Requests; use Koha::ILL::ISO18626::Requests; +use Koha::Bookings; use Koha::Items; use Koha::Patrons; use Koha::Patron::Debarments qw( DelUniqueDebarment AddUniqueDebarment ); @@ -1854,8 +1855,8 @@ sub AddIssue { { if ( $booking->patron_id == $patron->borrowernumber ) { - # Patron's own booking - mark as completed and link checkout to booking - $booking->status('completed')->store; + # Patron's own booking - mark as issued and link checkout to booking + $booking->status('issued')->store; $issue_attributes->{'booking_id'} = $booking->booking_id; } else { @@ -2491,6 +2492,7 @@ sub AddReturn { $patron or warn "AddReturn without current borrower"; if ($patron) { + my $booking_id = $issue->booking_id; eval { MarkIssueReturned( $borrowernumber, $item->itemnumber, $return_date, $patron->privacy, @@ -2513,6 +2515,13 @@ sub AddReturn { } ); } + + if ($booking_id) { + my $booking = Koha::Bookings->find($booking_id); + if ( $booking && $booking->status eq 'issued' ) { + $booking->status('completed')->store; + } + } } else { my $error = "$@"; carp diff --git a/Koha/Booking.pm b/Koha/Booking.pm index 2bd609b4e6..2d0e9dd7f3 100644 --- a/Koha/Booking.pm +++ b/Koha/Booking.pm @@ -173,11 +173,9 @@ sub store { value => $self->biblio_id, ) unless ( $self->biblio ); - # Skip clash detection when transitioning to a final - # status. During checkout C4::Circulation sets status - # to 'completed' and calls ->store; the clash checks - # are irrelevant at that point and can produce false - # positives. + # Skip clash detection when transitioning to a final/terminal + # status (cancelled or completed); the checks are irrelevant + # at those points and can produce false positives. unless ( $self->_is_final_status_transition ) { # Throw exception for item level booking clash @@ -348,9 +346,8 @@ sub to_api_mapping { my $bool = $self->_is_final_status_transition; -Returns true when the booking is being transitioned to a final status -(cancelled or completed). Used to skip clash detection that is only -meaningful for active bookings. +Returns true when the booking is being transitioned to a terminal status where +clash detection is no longer meaningful: C or C. =cut diff --git a/Koha/Bookings.pm b/Koha/Bookings.pm index 39fb118e01..1944a94e77 100644 --- a/Koha/Bookings.pm +++ b/Koha/Bookings.pm @@ -36,18 +36,14 @@ Koha::Bookings - Koha Booking object set class $bookings->filter_by_active; -Will return the bookings that have not ended, were cancelled or are completed. +Returns bookings that are still active, i.e. awaiting collection (C) or +currently on loan (C). Cancelled and completed bookings are excluded. =cut sub filter_by_active { my ($self) = @_; - return $self->search( - { - end_date => { '>=' => \'NOW()' }, - status => { '-not_in' => [ 'cancelled', 'completed' ] } - } - ); + return $self->search( { status => { '-in' => [ 'new', 'issued' ] } } ); } =head2 Internal Methods diff --git a/Koha/REST/V1/Bookings.pm b/Koha/REST/V1/Bookings.pm index 6807ebb65d..fafa7767b7 100644 --- a/Koha/REST/V1/Bookings.pm +++ b/Koha/REST/V1/Bookings.pm @@ -37,7 +37,7 @@ sub list { my $c = shift->openapi->valid_input or return; return try { - my $bookings = $c->objects->search( Koha::Bookings->filter_by_active ); + my $bookings = $c->objects->search( Koha::Bookings->new ); return $c->render( status => 200, openapi => $bookings ); } catch { $c->unhandled_exception($_); diff --git a/admin/columns_settings.yml b/admin/columns_settings.yml index 6bd3e83119..28c524a68c 100644 --- a/admin/columns_settings.yml +++ b/admin/columns_settings.yml @@ -1427,7 +1427,6 @@ modules: is_hidden: 1 - columnname: status - is_hidden: 1 - columnname: title - diff --git a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss index 47399eff34..b10bfda49e 100644 --- a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss +++ b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss @@ -5135,6 +5135,7 @@ div .suggestion_note { } a { + color: $link-color; cursor: pointer; } } diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-detail-tabs.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-detail-tabs.inc index 5c3937c8b4..9320839dfa 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-detail-tabs.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-detail-tabs.inc @@ -40,7 +40,7 @@ [% IF Koha.Preference('EnableBooking') %] [% WRAPPER tab_item tabname="bookings" %] [% SET bookings_count = patron.bookings.filter_by_active.count %] - [% SET expired_bookings_count = patron.bookings.count - bookings_count %] + [% SET closed_bookings_count = patron.bookings.count - bookings_count %] Bookings [% IF ( bookings_count ) %] [% bookings_count || 0 | html %] @@ -229,15 +229,8 @@ [% IF Koha.Preference('EnableBooking') %] [% WRAPPER tab_panel tabname="bookings" %] - [% IF ( bookings_count ) %] -
- Show expired -
-
- [% ELSIF ( expired_bookings_count ) %] -
- Hide expired -
+ [% IF ( bookings_count || closed_bookings_count ) %] + [% INCLUDE 'additional-filters.inc' filters = [ { id = 'filter-completed', label_show = t('Include completed'), label_hide = t('Exclude completed') }, { id = 'filter-cancelled', label_show = t('Include cancelled'), label_hide = t('Exclude cancelled') } ] filter_class = 'bookings' %]
[% ELSE %]

Patron has nothing booked.

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt index f9c093ad2c..7837c05d0d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt @@ -16,7 +16,6 @@ - [% WRAPPER 'header.inc' %] [% INCLUDE 'cat-search.inc' %] [% END %] @@ -41,7 +40,7 @@

Bookings for [% INCLUDE 'biblio-title-head.inc' %]

- [% INCLUDE 'additional-filters.inc' filters = [ { id = 'filter-expired', label_show = t('Include expired'), label_hide = t('Exclude expired') }, { id = 'filter-cancelled', label_show = t('Include cancelled'), label_hide = t('Exclude cancelled') }, { id = 'filter-completed', label_show = t('Include completed'), label_hide = t('Exclude completed') },] filter_class = 'bookings' %] + [% INCLUDE 'additional-filters.inc' filters = [ { id = 'filter-cancelled', label_show = t('Include cancelled'), label_hide = t('Exclude cancelled') }, { id = 'filter-completed', label_show = t('Include completed'), label_hide = t('Exclude completed') },] filter_class = 'bookings' %]
@@ -107,7 +106,7 @@ var bookingsSet = new vis.DataSet(); for (booking of bookings[0]) { - const isActive = ["new", "pending", "active"].includes(booking.status); + const isActive = ["new", "issued"].includes(booking.status); const patronContent = $patron_to_html(booking.patron, { display_cardnumber: true, url: false, @@ -124,7 +123,7 @@ end: $toDisplayDate(endServerTz), content: !isActive ? `${patronContent}` : patronContent, className: booking.status === "cancelled" ? "cancelled" : "", - ...(Koha.permissions.CAN_user_circulate_manage_bookings ? { editable: !["cancelled", "completed"].includes(booking.status) ? { remove: true, updateTime: true } : false } : { editable: false }), + ...(Koha.permissions.CAN_user_circulate_manage_bookings ? { editable: !["cancelled", "issued", "completed"].includes(booking.status) ? { remove: true, updateTime: true } : false } : { editable: false }), type: "range", group: booking.item_id ?? 0, }); @@ -225,19 +224,13 @@ } ); - const additional_filters = AdditionalFilters.init(["filter-expired", "filter-cancelled", "filter-completed"]) + const additional_filters = AdditionalFilters.init(["filter-cancelled", "filter-completed"]) .onChange((filters, { anyFiltersNotApplied }) => { bookings_table.DataTable().ajax.reload(); }) .build({ - end_date: ({ filters, isNotApplied }) => { - if (isNotApplied(filters["filter-expired"])) { - let today = new Date(); - return { ">=": today.toISOString() }; - } - }, status: ({ filters, isNotApplied }) => { - const defaults = ["new", "pending", "active"]; + const defaults = ["new", "issued"]; const filtered = [...defaults]; if (isNotApplied(filters["filter-cancelled"])) { filtered.push("cancelled"); @@ -270,40 +263,19 @@ orderable: false, visible: true, render: function (data, type, row, meta) { - const isExpired = date => dayjs(date).isBefore(new Date()); - const isActive = (startDate, endDate) => { - const now = dayjs(); - return now.isAfter(dayjs(startDate)) && now.isBefore(dayjs(endDate).add(1, "day")); - }; - const statusMap = { - new: () => { - if (isExpired(row.end_date)) { - return _("Expired"); - } - - if (isActive(row.start_date, row.end_date)) { - return _("Active"); - } - - if (dayjs(row.start_date).isAfter(new Date())) { - return _("Pending"); - } - - return _("New"); - }, + new: () => _("New"), cancelled: () => [_("Cancelled"), row.cancellation_reason].filter(Boolean).join(": "), + issued: () => _("Issued"), completed: () => _("Completed"), }; const statusText = statusMap[row.status] ? statusMap[row.status]() : _("Unknown"); const classMap = [ - { status: _("Expired"), class: "bg-secondary" }, { status: _("Cancelled"), class: "bg-secondary" }, - { status: _("Pending"), class: "bg-warning" }, - { status: _("Active"), class: "bg-primary" }, - { status: _("Completed"), class: "bg-info" }, + { status: _("Completed"), class: "bg-secondary" }, + { status: _("Issued"), class: "bg-info" }, { status: _("New"), class: "bg-success" }, ]; @@ -373,7 +345,7 @@ orderable: false, render: function (data, type, row, meta) { let result = ""; - const is_readonly = ["cancelled", "completed"].includes(row.status); + const is_readonly = ["cancelled", "issued", "completed"].includes(row.status); if (Koha.permissions.CAN_user_circulate_manage_bookings && !is_readonly) { result += `