Bug 41898: Introduce 'issued' status and update booking display/filtering

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 <https://openfifth.co.uk/>
Sponsored-by: Westminster City Council <https://www.westminster.gov.uk/>
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Martin Renvoize 2026-06-08 15:06:06 +01:00 committed by Pedro Amorim
parent fde2bc5680
commit c7673a9d6a
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
10 changed files with 65 additions and 129 deletions

View File

@ -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

View File

@ -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<cancelled> or C<completed>.
=cut

View File

@ -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<new>) or
currently on loan (C<issued>). 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

View File

@ -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($_);

View File

@ -1427,7 +1427,6 @@ modules:
is_hidden: 1
-
columnname: status
is_hidden: 1
-
columnname: title
-

View File

@ -5135,6 +5135,7 @@ div .suggestion_note {
}
a {
color: $link-color;
cursor: pointer;
}
}

View File

@ -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 %]
<span>Bookings</span>
[% IF ( bookings_count ) %]
<span class="badge text-bg-info">[% bookings_count || 0 | html %]</span>
@ -229,15 +229,8 @@
[% IF Koha.Preference('EnableBooking') %]
[% WRAPPER tab_panel tabname="bookings" %]
[% IF ( bookings_count ) %]
<fieldset class="action filters" style="cursor:pointer;">
<a id="expired_filter" class="filtered"><i class="fa fa-bars"></i> Show expired</a>
</fieldset>
<table id="bookings_table" style="width: 100% !Important;"></table>
[% ELSIF ( expired_bookings_count ) %]
<fieldset class="action filters" style="cursor:pointer;">
<a id="expired_filter"><i class="fa fa-filter"></i> Hide expired</a>
</fieldset>
[% 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' %]
<table id="bookings_table" style="width: 100% !Important;"></table>
[% ELSE %]
<p>Patron has nothing booked.</p>

View File

@ -16,7 +16,6 @@
</head>
<body id="circ_request" class="catalog">
[% WRAPPER 'header.inc' %]
[% INCLUDE 'cat-search.inc' %]
[% END %]
@ -41,7 +40,7 @@
<h1>Bookings for [% INCLUDE 'biblio-title-head.inc' %]</h1>
<div class="page-section" id="bookings-timeline"></div>
<div class="page-section">
[% 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' %]
<table id="bookings_table"></table>
</div>
</div>
@ -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 ? `<s>${patronContent}</s>` : 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 += `
<button type="button" class="btn btn-default btn-xs edit-action"

View File

@ -855,6 +855,7 @@
[% Asset.js("js/holds.js") | $raw %]
[% INCLUDE 'calendar.inc' %]
[% IF Koha.Preference('EnableBooking') %]
[% Asset.js("js/additional-filters.js") | $raw %]
[% Asset.js("js/cancel_booking_modal.js") | $raw %]
[% Asset.js("js/tables/bookings.js") | $raw %]
[% END %]

View File

@ -1,8 +1,32 @@
// Bookings
var bookings_table;
$(document).ready(function () {
// Determine whether we have a filtered list
let filter_expired = $("#expired_filter").hasClass("filtered");
const af = AdditionalFilters.init([
"filter-completed",
"filter-cancelled",
]).onChange(() => {
if (bookings_table) {
bookings_table.DataTable().ajax.reload();
}
});
const additional_filters = {
patron_id: patron_borrowernumber,
...af.build({
status: ({ filters, isNotApplied }) => {
const defaults = ["new", "issued"];
const filtered = [...defaults];
if (isNotApplied(filters["filter-cancelled"])) {
filtered.push("cancelled");
}
if (isNotApplied(filters["filter-completed"])) {
filtered.push("completed");
}
return { "-in": filtered };
},
}),
};
// Load bookings table on page load
if (window.location.hash === "#bookings_panel") {
loadBookingsTable();
@ -13,18 +37,6 @@ $(document).ready(function () {
});
function loadBookingsTable() {
let additional_filters = {
patron_id: patron_borrowernumber,
end_date: function () {
if (filter_expired) {
let today = new Date();
return { ">=": today.toISOString() };
} else {
return;
}
},
};
if (!bookings_table) {
var bookings_table_url = "/api/v1/bookings";
bookings_table = $("#bookings_table").kohaTable(
@ -123,35 +135,13 @@ $(document).ready(function () {
}
function renderStatus(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"),
};
@ -160,11 +150,9 @@ $(document).ready(function () {
: __("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" },
];
@ -174,24 +162,4 @@ $(document).ready(function () {
return `<span class="badge rounded-pill ${badgeClass}">${statusText}</span>`;
}
var txtActivefilter = __("Show expired");
var txtInactivefilter = __("Hide expired");
$("#expired_filter").on("click", function () {
if ($(this).hasClass("filtered")) {
filter_expired = false;
$(this).html('<i class="fa fa-filter"></i> ' + txtInactivefilter);
} else {
filter_expired = true;
$(this).html('<i class="fa fa-bars"></i> ' + txtActivefilter);
}
bookings_table.DataTable().ajax.reload(() => {
bookings_table
.DataTable()
.column("status:name")
.visible(!filter_expired, false);
});
$(this).toggleClass("filtered");
});
});