Attachment 172880 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.24 KB, created by Martin Renvoize (ashimema) on 2024-10-17 14:09:59 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2024-10-17 14:09:59 UTC

Size: 17.24 KB

patch

obsolete

>From 600cc4830417b063b0e142074f70ba5b1f4012a4 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
>Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> Koha/Booking.pm                               | 86 +++++++++++++++----
> 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, 221 insertions(+), 49 deletions(-)
>
>diff --git a/Koha/Booking.pm b/Koha/Booking.pm
>index 879ae3b6f6c..bc9dd5b66a1 100644
>--- a/Koha/Booking.pm
>+++ b/Koha/Booking.pm
>@@ -306,32 +306,82 @@ 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 $branch         = C4::Context->userenv->{'branch'};
>     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             => $branch,
>-        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             => $branch,
>+            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 86eeb3a5c84..77008e69b43 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 945614cd900..69badb404a1 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 9cb8a8546cf..047329ab5f2 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 ce2aaeb0062..e7a15267880 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 03c9e0b83bb..bee9abf8d30 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.47.0

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