Attachment 172770 Details for Bug 38175 – Bug 38175: Improve Bookings feature with status

[patch] Bug 38175: Improve Bookings feature with status

Bug-38175-Improve-Bookings-feature-with-status.patch (text/plain), 17.09 KB, created by Thibaud Guillot (thibaud_g) on 2024-10-15 12:46:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Thibaud Guillot (thibaud_g)

Created: 2024-10-15 12:46:48 UTC

Size: 17.09 KB

patch

obsolete

>From ec1bdef6ed3aba6e5bcf8a5df7dfde4948a5f12e Mon Sep 17 00:00:00 2001
>From: Thibaud Guillot <thibaud.guillot@biblibre.com>
>Date: Tue, 15 Oct 2024 12:29:22 +0000
>Subject: [PATCH] Bug 38175: Improve Bookings feature with status
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>With the integration of the new status column for bookings, it would be preferable to keep the booking in database and simply change its status to 'cancel' when you cancel it via the action button or the timeline.
>
>So I've added partial updating via the API with a new PATCH method to partially edit a booking. It is currently active for status changes.
>Graphically, this translates into the disappearance of the action buttons if the booking has already been canceled, although it remains visible in the table and timeline (the style is a proposal for the moment, intended simply to graphically differentiate a canceled booking from others).
>
>I've also added a filter to the filter_by_active method to exclude cancelled bookings.
>
>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
>
>Sponsored by: BibLibre
>---
> Koha/Booking.pm                               | 87 +++++++++++++++----
> Koha/Bookings.pm                              |  9 +-
> Koha/REST/V1/Bookings.pm                      | 27 +++++-
> api/v1/swagger/paths/bookings.yaml            | 55 ++++++++++++
> .../prog/en/modules/bookings/list.tt          | 80 ++++++++++++-----
> .../prog/js/cancel_booking_modal.js           | 13 +--
> 6 files changed, 222 insertions(+), 49 deletions(-)
>
>diff --git a/Koha/Booking.pm b/Koha/Booking.pm
>index e5297d8d17..5e03754ff3 100644
>--- a/Koha/Booking.pm
>+++ b/Koha/Booking.pm
>@@ -253,30 +253,81 @@ sub to_api_mapping {
> sub delete {
>     my ($self) = @_;
> 
>+    my $deleted = $self->SUPER::delete($self);
>+    return $deleted;
>+}
>+
>+=head3 edit
>+
>+This method adds possibility to edit 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} );
>+    }
>+    $self->store();
>+
>+    return $self;
>+}
>+
>+=head3 cancel
>+
>+This method adds possibility of cancelling a booking (kept in table but flagged with 'cancelled' status)
>+Also adds param to send a letter to the borrower affected by the cancellation
>+
>+=cut
>+
>+sub cancel {
>+    my ( $self, $params ) = @_;
>+
>     my $patron         = $self->patron;
>     my $pickup_library = $self->pickup_library;
> 
>-    my $letter = C4::Letters::GetPreparedLetter(
>-        module                 => 'bookings',
>-        letter_code            => 'BOOKING_CANCELLATION',
>-        message_transport_type => 'email',
>-        branchcode             => $pickup_library->branchcode,
>-        lang                   => $patron->lang,
>-        objects                => { booking => $self }
>-    );
>-
>-    if ($letter) {
>-        C4::Letters::EnqueueLetter(
>-            {
>-                letter                 => $letter,
>-                borrowernumber         => $patron->borrowernumber,
>-                message_transport_type => 'email',
>-            }
>+    if ( $params->{send_letter} ) {
>+        my $letter = C4::Letters::GetPreparedLetter(
>+            module                 => 'bookings',
>+            letter_code            => 'BOOKING_CANCELLATION',
>+            message_transport_type => 'email',
>+            branchcode             => $pickup_library->branchcode,
>+            lang                   => $patron->lang,
>+            objects                => { booking => $self }
>         );
>+
>+        if ($letter) {
>+            C4::Letters::EnqueueLetter(
>+                {
>+                    letter                 => $letter,
>+                    borrowernumber         => $patron->borrowernumber,
>+                    message_transport_type => 'email',
>+                }
>+            );
>+        }
>     }
>+}
> 
>-    my $deleted = $self->SUPER::delete($self);
>-    return $deleted;
>+
>+=head3 set_status
>+
>+This method changes the status of a booking
>+
>+=cut
>+
>+sub _set_status {
>+    my ( $self, $new_status ) = @_;
>+
>+    my @valid_statuses = qw(pending completed cancelled);
>+    unless ( grep { $_ eq $new_status } @valid_statuses ) {
>+        die "Invalid status: $new_status";
>+    }
>+
>+    $self->status($new_status);
> }
> 
> =head2 Internal methods
>diff --git a/Koha/Bookings.pm b/Koha/Bookings.pm
>index 86eeb3a5c8..77008e69b4 100644
>--- a/Koha/Bookings.pm
>+++ b/Koha/Bookings.pm
>@@ -36,13 +36,18 @@ Koha::Bookings - Koha Booking object set class
> 
>     $bookings->filter_by_active;
> 
>-Will return the bookings that have not ended.
>+Will return the bookings that have not ended and without "CANCELLED" status.
> 
> =cut
> 
> sub filter_by_active {
>     my ($self) = @_;
>-    return $self->search( { end_date => { '>=' => \'NOW()' } } );
>+    return $self->search(
>+        {
>+            end_date => { '>=' => \'NOW()' },
>+            status   => { '!=' => 'cancelled' }
>+        }
>+    );
> }
> 
> =head2 Internal Methods
>diff --git a/Koha/REST/V1/Bookings.pm b/Koha/REST/V1/Bookings.pm
>index 945614cd90..69badb404a 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->new );
>+        my $bookings = $c->objects->search( Koha::Bookings->filter_by_active );
>         return $c->render( status => 200, openapi => $bookings );
>     } catch {
>         $c->unhandled_exception($_);
>@@ -148,4 +148,29 @@ sub delete {
>     };
> }
> 
>+=head3 edit
>+
>+Controller function that editing an existing booking
>+
>+=cut
>+
>+sub edit {
>+    my $c    = shift->openapi->valid_input or return;
>+    my $body = $c->req->json;
>+
>+    my $booking = Koha::Bookings->find( $c->param('booking_id') );
>+    return $c->render_resource_not_found("Booking")
>+        unless $booking;
>+
>+    return try {
>+        $booking->edit($body);
>+        return $c->render(
>+            status  => 200,
>+            openapi => $c->objects->to_api($booking),
>+        );
>+    } catch {
>+        $c->unhandled_exception($_);
>+    };
>+}
>+
> 1;
>diff --git a/api/v1/swagger/paths/bookings.yaml b/api/v1/swagger/paths/bookings.yaml
>index 9cb8a8546c..047329ab5f 100644
>--- a/api/v1/swagger/paths/bookings.yaml
>+++ b/api/v1/swagger/paths/bookings.yaml
>@@ -252,3 +252,58 @@
>       permissions:
>         circulate: manage_bookings
>     x-mojo-to: Bookings#update
>+  patch:
>+    x-mojo-to: Bookings#edit
>+    operationId: editBooking
>+    tags:
>+      - bookings
>+    summary: Edit booking
>+    parameters:
>+      - $ref: "../swagger.yaml#/parameters/booking_id_pp"
>+      - name: body
>+        in: body
>+        description: A JSON object containing fields to modify
>+        required: true
>+        schema:
>+          type: object
>+          properties:
>+            status:
>+              description: Set booking status
>+              type: string
>+          additionalProperties: false
>+    consumes:
>+      - application/json
>+    produces:
>+      - application/json
>+    responses:
>+      200:
>+        description: Updated booking
>+        schema:
>+          $ref: ../swagger.yaml#/definitions/booking
>+      400:
>+        description: Bad request
>+        schema:
>+          $ref: ../swagger.yaml#/definitions/error
>+      401:
>+        description: Authentication required
>+        schema:
>+          $ref: ../swagger.yaml#/definitions/error
>+      403:
>+        description: Access forbidden
>+        schema:
>+          $ref: ../swagger.yaml#/definitions/error
>+      404:
>+        description: Booking not found
>+        schema:
>+          $ref: ../swagger.yaml#/definitions/error
>+      500:
>+        description: Internal error
>+        schema:
>+          $ref: ../swagger.yaml#/definitions/error
>+      503:
>+        description: Under maintenance
>+        schema:
>+          $ref: ../swagger.yaml#/definitions/error
>+    x-koha-authorization:
>+      permissions:
>+        circulate: manage_bookings
>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 ce2aaeb006..e7a1526788 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt
>@@ -11,6 +11,13 @@
>     [% t("Koha") | html %]
> [% END %]</title>
> [% INCLUDE 'doc-head-close.inc' %]
>+    <style>
>+        #bookings-timeline .vis-item.vis-range {
>+            &.cancelled {
>+               background: rgba(128, 128, 128, 0.3);
>+            }
>+        }
>+    </style>
> </head>
> 
> <body id="circ_request" class="catalog">
>@@ -43,8 +50,9 @@
>                     <h1>Bookings for [% INCLUDE 'biblio-title-head.inc' %]</h1>
>                     <div class="page-section" id="bookings-timeline"></div>
>                     <div class="page-section">
>-                        <fieldset class="action filters" style="cursor:pointer;">
>+                        <fieldset class="action filters" style="cursor:pointer; display: flex; flex-direction: column;">
>                             <a id="expired_filter" class="filtered"><i class="fa fa-bars"></i> Show expired</a>
>+                            <a id="cancelled_filter" class="filtered"><i class="fa fa-bars"></i> Show cancelled</a>
>                         </fieldset>
> 
>                         <table id="bookings_table"></table>
>@@ -122,15 +130,27 @@
>                             url: false
>                         }),
>                         [% IF CAN_user_circulate_manage_bookings %]
>-                        editable: { remove: true, updateTime: true },
>+                        editable: booking.status != 'cancelled' ? { remove: true, updateTime: true } : false,
>                         [% ELSE %]
>                         editable: false,
>                         [% END %]
>                         type: 'range',
>-                        group: booking.item_id ? booking.item_id : 0
>+                        group: booking.item_id ? booking.item_id : 0,
>+                        className: booking.status,
>                     });
>                 }
> 
>+                const cancelledItems = document.querySelectorAll('#bookings-timeline .vis-item.vis-range.cancelled');
>+                cancelledItems.forEach(item => {
>+                    item.style.background = 'repeating-linear-gradient(' +
>+                        '135deg,' +
>+                        'rgba(211, 211, 211, 0.5) 0,' +
>+                        'rgba(211, 211, 211, 0.5) 10px,' +
>+                        'transparent 10px,' +
>+                        'transparent 20px' +
>+                    ');';
>+                });
>+
>                 var container = document.getElementById('bookings-timeline');
>                 var options = {
>                     stack: true,
>@@ -206,6 +226,7 @@
>         );
> 
>         let filter_expired = true;
>+        let filter_cancelled = false;
>         let additional_filters = {
>             end_date: function(){
>                 if ( filter_expired ) {
>@@ -214,6 +235,13 @@
>                 } else {
>                     return;
>                 }
>+            },
>+            status: function(){
>+                if ( filter_cancelled ) {
>+                    return { "=": 'cancelled' }
>+                } else {
>+                    return;
>+                }
>             }
>         };
> 
>@@ -241,10 +269,13 @@
>                 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;
>                     if (is_expired) {
>                         return '<span class="badge rounded-pill bg-secondary">' + _("Expired") + '</span>';
>                     }
>-
>+                    if (is_cancelled) {
>+                        return '<span class="badge rounded-pill bg-secondary">' + _("Cancelled") + '</span>';
>+                    }
>                     return '<span class="badge rounded-pill bg-success">' + _("Active") + '</span>';
>                 }
>             },
>@@ -309,34 +340,37 @@
>                 "orderable": false,
>                 "render": function(data, type, row, meta) {
>                     let result = "";
>+                    let is_cancelled = row.status == 'cancelled' ? 1 : 0;
>                     [% IF CAN_user_circulate_manage_bookings %]
>-                    result += '<button type="button" class="btn btn-default btn-xs edit-action" data-bs-toggle="modal" data-bs-target="#placeBookingModal" data-booking="'+row.booking_id+'" data-biblionumber="[% biblionumber | uri %]" data-itemnumber="'+row.item_id+'" data-patron="'+row.patron_id+'" data-pickup_library="'+row.pickup_library_id+'" data-start_date="'+row.start_date+'" data-end_date="'+row.end_date+'"><i class="fa fa-pencil" aria-hidden="true"></i> '+_("Edit")+'</button>';
>-                    result += '<button type="button" class="btn btn-default btn-xs cancel-action" data-bs-toggle="modal" data-bs-target="#cancelBookingModal" data-booking="'+row.booking_id+'"><i class="fa fa-trash" aria-hidden="true"></i> '+_("Cancel")+'</button>';
>+                    if( !is_cancelled ){
>+                        result += '<button type="button" class="btn btn-default btn-xs edit-action" data-bs-toggle="modal" data-bs-target="#placeBookingModal" data-booking="'+row.booking_id+'" data-biblionumber="[% biblionumber | uri %]" data-itemnumber="'+row.item_id+'" data-patron="'+row.patron_id+'" data-pickup_library="'+row.pickup_library_id+'" data-start_date="'+row.start_date+'" data-end_date="'+row.end_date+'"><i class="fa fa-pencil" aria-hidden="true"></i> '+_("Edit")+'</button>';
>+                        result += '<button type="button" class="btn btn-default btn-xs cancel-action" data-bs-toggle="modal" data-bs-target="#cancelBookingModal" data-booking="'+row.booking_id+'"><i class="fa fa-trash" aria-hidden="true"></i> '+_("Cancel")+'</button>';
>+                    }
>                     [% END %]
>                     return result;
>                 }
>             }]
>         }, [], 0, additional_filters);
> 
>-        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);
>-            }
>+        function setupFilter(filterId, activeText, inactiveText, filterVariable) {
>+            $(filterId).on("click", function() {
>+                if ($(this).hasClass('filtered')) {
>+                    filterVariable = false;
>+                    $(this).html('<i class="fa fa-filter"></i> ' + inactiveText);
>+                } else {
>+                    filterVariable = true;
>+                    $(this).html('<i class="fa fa-bars"></i> ' + activeText);
>+                }
> 
>-            bookings_table.DataTable().ajax.reload(() => {
>-                bookings_table
>-                    .DataTable()
>-                    .column("status:name")
>-                    .visible(!filter_expired, false);
>+                bookings_table.DataTable().ajax.reload(() => {
>+                    bookings_table.DataTable().column("status:name").visible(!filterVariable, false);
>+                });
>+                $(this).toggleClass('filtered');
>             });
>-            $(this).toggleClass('filtered');
>-        });
>+        }
>+
>+        setupFilter("#expired_filter", _("Show expired"), _("Hide expired"), filter_expired);
>+        setupFilter("#cancelled_filter", _("Show cancelled"), _("Hide cancelled"), filter_cancelled);
> 
>     });
> </script>
>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 03c9e0b83b..bee9abf8d3 100644
>--- a/koha-tmpl/intranet-tmpl/prog/js/cancel_booking_modal.js
>+++ b/koha-tmpl/intranet-tmpl/prog/js/cancel_booking_modal.js
>@@ -10,12 +10,15 @@ $("#cancelBookingForm").on('submit', function(e) {
>     var booking_id = $('#cancel_booking_id').val();
>     var url = '/api/v1/bookings/'+booking_id;
> 
>-    var deleting = $.ajax({
>-        'method': "DELETE",
>-        'url': url
>+    var cancelling = $.ajax({
>+        'method': "PATCH",
>+        'url': url,
>+        'data': JSON.stringify({"status": "cancelled"}),
>+        'contentType': "application/json"
>     });
> 
>-    deleting.done(function(data) {
>+
>+    cancelling.done(function(data) {
>         cancel_success = 1;
>         if (bookings_table) {
>             bookings_table.api().ajax.reload();
>@@ -27,7 +30,7 @@ $("#cancelBookingForm").on('submit', function(e) {
>         $('#cancelBookingModal').modal('hide');
>     });
> 
>-    deleting.fail(function(data) {
>+    cancelling.fail(function(data) {
>         $('#cancel_booking_result').replaceWith('<div id="booking_result" class="alert alert-danger">'+__("Failure")+'</div>');
>     });
> });
>-- 
>2.25.1

Actions: View | Diff | Splinter Review

Attachments on bug 38175: 172770 | 172771 | 172772 | 172773 | 172814 | 172815 | 172816 | 172817 | 172880 | 172881 | 172882 | 172883 | 172884 | 173172 | 173173 | 173174 | 173175 | 173176 | 173177 | 173178 | 173201 | 173202 | 173203 | 173346 | 173915