From f36ae87ca41b49a735ae16cb77594a1029d20155 Mon Sep 17 00:00:00 2001 From: Paul Derscheid Date: Wed, 16 Oct 2024 14:13:42 +0200 Subject: [PATCH] Bug 38175: (follow-up) Refactor staff interface additions to vanilla JS; adjust modules, controllers for coding guidelines and consistency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the spirit of the current movement for more modern JS, the refactor tries to limit jQuery usage. Test plan: 1) Create a booking on a bookable item 2) Cancel it and see that it's simply deleted 3) Apply the patch and run “restart_all”. 4) Repeat the same cancel operation and see that it's still there, albeit with a different appearance. 5) Try the filters in the table --- Koha/Booking.pm | 35 +++-- Koha/Bookings.pm | 6 +- Koha/REST/V1/Bookings.pm | 9 +- .../prog/en/modules/bookings/list.tt | 123 +++++++++++------- .../prog/js/cancel_booking_modal.js | 112 +++++++++++----- .../prog/js/modals/place_booking.js | 6 +- t/db_dependent/Koha/Booking.t | 6 +- 7 files changed, 190 insertions(+), 107 deletions(-) diff --git a/Koha/Booking.pm b/Koha/Booking.pm index 5e03754ff3..c15775325e 100644 --- a/Koha/Booking.pm +++ b/Koha/Booking.pm @@ -27,6 +27,8 @@ use Koha::Libraries; use C4::Letters; +use List::Util qw(any); + use base qw(Koha::Object); =head1 NAME @@ -259,22 +261,26 @@ sub delete { =head3 edit -This method adds possibility to edit a booking +This method allows patching a booking =cut sub edit { my ( $self, $params ) = @_; - if ( $params->{status} ) { - if ( $params->{status} eq 'cancelled' ) { - $self->cancel( { send_letter => 1 } ); - } - $self->_set_status( $params->{status} ); + my $new_status = $params->{'status'}; + unless ($new_status) { + return $self->store; } - $self->store(); - return $self; + $self->_set_status($new_status); + + my $status = $self->status; + if ( $status eq 'cancelled' ) { + $self->cancel( { send_letter => 1 } ); + } + + return $self->store; } =head3 cancel @@ -290,7 +296,7 @@ sub cancel { my $patron = $self->patron; my $pickup_library = $self->pickup_library; - if ( $params->{send_letter} ) { + if ( $params->{'send_letter'} ) { my $letter = C4::Letters::GetPreparedLetter( module => 'bookings', letter_code => 'BOOKING_CANCELLATION', @@ -313,7 +319,9 @@ sub cancel { } -=head3 set_status +=head2 Internal methods + +=head3 _set_status This method changes the status of a booking @@ -322,16 +330,15 @@ This method changes the status of a booking sub _set_status { my ( $self, $new_status ) = @_; - my @valid_statuses = qw(pending completed cancelled); - unless ( grep { $_ eq $new_status } @valid_statuses ) { + my @valid_statuses = qw(new completed cancelled); + my $is_valid = any { $new_status eq $_ } @valid_statuses; + unless ($is_valid) { die "Invalid status: $new_status"; } $self->status($new_status); } -=head2 Internal methods - =head3 _type =cut diff --git a/Koha/Bookings.pm b/Koha/Bookings.pm index 77008e69b4..d63e063647 100644 --- a/Koha/Bookings.pm +++ b/Koha/Bookings.pm @@ -36,7 +36,7 @@ Koha::Bookings - Koha Booking object set class $bookings->filter_by_active; -Will return the bookings that have not ended and without "CANCELLED" status. +Will return the bookings that have not ended, were cancelled or are completed. =cut @@ -44,8 +44,8 @@ sub filter_by_active { my ($self) = @_; return $self->search( { - end_date => { '>=' => \'NOW()' }, - status => { '!=' => 'cancelled' } + end_date => { '>=' => \'NOW()' }, + status => { q{!=} => [ -and => qw(cancelled completed) ] } } ); } diff --git a/Koha/REST/V1/Bookings.pm b/Koha/REST/V1/Bookings.pm index 69badb404a..87e0161973 100644 --- a/Koha/REST/V1/Bookings.pm +++ b/Koha/REST/V1/Bookings.pm @@ -150,20 +150,19 @@ sub delete { =head3 edit -Controller function that editing an existing booking +Controller function that handles editing an existing booking =cut sub edit { - my $c = shift->openapi->valid_input or return; - my $body = $c->req->json; + my $c = shift->openapi->valid_input or return; - my $booking = Koha::Bookings->find( $c->param('booking_id') ); + my $booking = $c->objects->find_rs( Koha::Bookings->new, $c->param('booking_id') ); return $c->render_resource_not_found("Booking") unless $booking; return try { - $booking->edit($body); + $booking->edit( $c->req->json ); return $c->render( status => 200, openapi => $c->objects->to_api($booking), 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 e7a1526788..6619b20e00 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt @@ -50,9 +50,9 @@

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

-
- Show expired - Show cancelled +
+ Show expired + Show cancelled
@@ -118,6 +118,11 @@ var bookingsSet = new vis.DataSet(); for (booking of bookings[0]){ + const isActive = ["new", "pending", "active"].includes(booking.status); + const patronContent = $patron_to_html(booking.patron, { + display_cardnumber: true, + url: false + }); bookingsSet.add({ id: booking.booking_id, booking: booking.booking_id, @@ -125,18 +130,14 @@ pickup_library: booking.pickup_library_id, start: dayjs(booking.start_date).toDate(), end: dayjs(booking.end_date).toDate(), - content: $patron_to_html(booking.patron, { - display_cardnumber: true, - url: false - }), + content: !isActive ? `${patronContent}` : patronContent, [% IF CAN_user_circulate_manage_bookings %] - editable: booking.status != 'cancelled' ? { remove: true, updateTime: true } : false, + editable: booking.status !== "cancelled" ? { remove: true, updateTime: true } : false, [% ELSE %] editable: false, [% END %] type: 'range', - group: booking.item_id ? booking.item_id : 0, - className: booking.status, + group: booking.item_id ?? 0, }); } @@ -225,24 +226,23 @@ } ); - let filter_expired = true; - let filter_cancelled = false; - let additional_filters = { - end_date: function(){ - if ( filter_expired ) { + const filterStates = { expired: true, cancelled: false }; + const additional_filters = { + end_date: () => { + if (filterStates.expired) { let today = new Date(); - return { ">=": today.toISOString() } - } else { - return; + return { ">=": today.toISOString() }; } }, - status: function(){ - if ( filter_cancelled ) { - return { "=": 'cancelled' } - } else { - return; + status: () => { + const defaults = ["new", "pending", "active"]; + if (filterStates.cancelled) { + const filtered = [...defaults, "cancelled"]; + return { "-in": filtered }; } - } + + return { "-in": defaults }; + }, }; var bookings_table_url = "/api/v1/biblios/[% biblionumber | uri %]/bookings"; @@ -268,15 +268,23 @@ orderable: false, visible: false, render: function (data, type, row, meta) { - let is_expired = dayjs(row.end_date).isBefore(new Date()); - let is_cancelled = row.status == 'cancelled' ? 1 : 0; + const is_expired = dayjs(row.end_date).isBefore(new Date()); if (is_expired) { - return '' + _("Expired") + ''; + return ` + ${_("Expired")} + `; } + + const is_cancelled = row.status === "cancelled"; if (is_cancelled) { - return '' + _("Cancelled") + ''; + return ` + ${_("Cancelled")} + `; } - return '' + _("Active") + ''; + + return ` + ${_("New")} + `; } }, { @@ -340,9 +348,9 @@ "orderable": false, "render": function(data, type, row, meta) { let result = ""; - let is_cancelled = row.status == 'cancelled' ? 1 : 0; + let is_cancelled = row.status === "cancelled"; [% IF CAN_user_circulate_manage_bookings %] - if( !is_cancelled ){ + if (!is_cancelled) { result += ''; result += ''; } @@ -352,25 +360,46 @@ }] }, [], 0, additional_filters); - function setupFilter(filterId, activeText, inactiveText, filterVariable) { - $(filterId).on("click", function() { - if ($(this).hasClass('filtered')) { - filterVariable = false; - $(this).html(' ' + inactiveText); - } else { - filterVariable = true; - $(this).html(' ' + activeText); - } + document + .getElementById("expired_filter") + .addEventListener("click", (e) => + handleFilter( + e, + { active: _("Hide expired"), inactive: _("Show expired") }, + filterStates, + ), + ); - bookings_table.DataTable().ajax.reload(() => { - bookings_table.DataTable().column("status:name").visible(!filterVariable, false); - }); - $(this).toggleClass('filtered'); + document + .getElementById("cancelled_filter") + .addEventListener("click", (e) => + handleFilter( + e, + { active: _("Hide cancelled"), inactive: _("Show cancelled") }, + filterStates, + ), + ); + + function handleFilter(e, text, filterStates) { + const target = e.target; + const { filter } = target.dataset; + if (target.classList.contains("filtered")) { + filterStates[filter] = false; + target.innerHTML = ` ${text.inactive}`; + } else { + filterStates[filter] = true; + target.innerHTML = ` ${text.active}`; + } + + bookings_table.DataTable().ajax.reload(() => { + bookings_table + .DataTable() + .column("status:name") + .visible(filterStates[filter], false); }); - } - setupFilter("#expired_filter", _("Show expired"), _("Hide expired"), filter_expired); - setupFilter("#cancelled_filter", _("Show cancelled"), _("Hide cancelled"), filter_cancelled); + target.classList.toggle("filtered"); + } }); diff --git a/koha-tmpl/intranet-tmpl/prog/js/cancel_booking_modal.js b/koha-tmpl/intranet-tmpl/prog/js/cancel_booking_modal.js index bee9abf8d3..0c77696fbc 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/cancel_booking_modal.js +++ b/koha-tmpl/intranet-tmpl/prog/js/cancel_booking_modal.js @@ -1,36 +1,80 @@ -$('#cancelBookingModal').on('show.bs.modal', function(e) { - var button = $(e.relatedTarget); - var booking = button.data('booking'); - $('#cancel_booking_id').val(booking); -}); - -$("#cancelBookingForm").on('submit', function(e) { - e.preventDefault(); - - var booking_id = $('#cancel_booking_id').val(); - var url = '/api/v1/bookings/'+booking_id; - - var cancelling = $.ajax({ - 'method': "PATCH", - 'url': url, - 'data': JSON.stringify({"status": "cancelled"}), - 'contentType': "application/json" - }); - - - cancelling.done(function(data) { - cancel_success = 1; - if (bookings_table) { - bookings_table.api().ajax.reload(); +(() => { + document + .getElementById("cancelBookingModal") + ?.addEventListener("show.bs.modal", handleShowBsModal); + document + .getElementById("cancelBookingForm") + ?.addEventListener("submit", handleSubmit); + + async function handleSubmit(e) { + e.preventDefault(); + + const bookingIdInput = document.getElementById("cancel_booking_id"); + if (!bookingIdInput) { + return; + } + + const bookingId = bookingIdInput.value; + if (!bookingId) { + return; + } + + let [error, response] = await catchError( + fetch(`/api/v1/bookings/${bookingId}`, { + method: "PATCH", + body: JSON.stringify({ status: "cancelled" }), + headers: { + "Content-Type": "application/json", + }, + }) + ); + if (error || !response.ok) { + const alertContainer = document.getElementById( + "cancel_booking_result" + ); + alertContainer.outerHTML = ` +
+ ${__("Failure")} +
+ `; + + return; } - if (typeof timeline !== 'undefined') { - timeline.itemsData.remove(Number(booking_id)); + + cancel_success = true; + bookings_table?.api().ajax.reload(); + timeline?.itemsData.remove(Number(booking_id)); + + $("#cancelBookingModal").modal("hide"); + + const bookingsCount = document.querySelector(".bookings_count"); + if (!bookingsCount) { + return; } - $('.bookings_count').html(parseInt($('.bookings_count').html(), 10)-1); - $('#cancelBookingModal').modal('hide'); - }); - - cancelling.fail(function(data) { - $('#cancel_booking_result').replaceWith('
'+__("Failure")+'
'); - }); -}); + + bookingsCount.innerHTML = parseInt(bookingsCount.innerHTML, 10) - 1; + } + + function handleShowBsModal(e) { + const button = e.relatedTarget; + if (!button) { + return; + } + + const booking = button.dataset.booking; + if (!booking) { + return; + } + + const bookingIdInput = document.getElementById("cancel_booking_id"); + if (!bookingIdInput) { + return; + } + + bookingIdInput.value = booking; + } + + function catchError(promise) { + return promise.then(data => [undefined, data]).catch(error => [error]); + } +})(); diff --git a/koha-tmpl/intranet-tmpl/prog/js/modals/place_booking.js b/koha-tmpl/intranet-tmpl/prog/js/modals/place_booking.js index 4a168d107b..90b35e06a8 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/modals/place_booking.js +++ b/koha-tmpl/intranet-tmpl/prog/js/modals/place_booking.js @@ -301,7 +301,11 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { // Fetch list of existing bookings let bookingsFetch = $.ajax({ - url: "/api/v1/bookings?biblio_id=" + biblionumber + "&_per_page=-1", + url: + "/api/v1/bookings?biblio_id=" + + biblionumber + + "&_per_page=-1" + + '&q={"status":{"-in":["new","pending","active"]}}', dataType: "json", type: "GET", }); diff --git a/t/db_dependent/Koha/Booking.t b/t/db_dependent/Koha/Booking.t index 5727e6d6e8..b9669c9b68 100755 --- a/t/db_dependent/Koha/Booking.t +++ b/t/db_dependent/Koha/Booking.t @@ -481,16 +481,16 @@ subtest 'set_status() tests' => sub { )->store; my $booking_with_old_status = Koha::Bookings->find( $booking->booking_id ); - $booking_with_old_status->set_status('completed'); + $booking_with_old_status->_set_status('completed'); is( $booking_with_old_status->unblessed->{status}, 'completed', 'Booking status is now "completed"' ); - $booking_with_old_status->set_status('cancelled'); + $booking_with_old_status->_set_status('cancelled'); is( $booking_with_old_status->unblessed->{status}, 'cancelled', 'Booking status is now "cancelled"' ); subtest 'unauthorized status' => sub { plan tests => 2; - eval { $booking_with_old_status->set_status('blah'); }; + eval { $booking_with_old_status->_set_status('blah'); }; if ($@) { like( -- 2.47.0