Bugzilla – Attachment 185468 Details for
Bug 40656
bookings/list.tt needs to be refactored
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40656: Refactor bookings/list.tt to use better patterns and improve code organization
Bug-40656-Refactor-bookingslisttt-to-use-better-pa.patch (text/plain), 17.40 KB, created by
Paul Derscheid
on 2025-08-15 15:24:02 UTC
(
hide
)
Description:
Bug 40656: Refactor bookings/list.tt to use better patterns and improve code organization
Filename:
MIME Type:
Creator:
Paul Derscheid
Created:
2025-08-15 15:24:02 UTC
Size:
17.40 KB
patch
obsolete
>From 0bb53089f35942070880eb49308bcbc14e899413 Mon Sep 17 00:00:00 2001 >From: Paul Derscheid <paul.derscheid@lmscloud.de> >Date: Fri, 15 Aug 2025 11:00:03 +0000 >Subject: [PATCH] Bug 40656: Refactor bookings/list.tt to use better patterns > and improve code organization > >This patch aims to improve bookings/list.tt: >- Converting filter attributes to boolean data attributes (presence-based) >- Creating a reusable table-filters.inc for filter controls >- Moving inline styles to proper SCSS files for better caching and separation of concerns >- Fixing a runtime error when cancelling bookings from the timeline >- Improving JavaScript readability with helper functions >- Addressing the QA complaints from Bug 36789: > - Cleaning up the long lines for the button elements by moving them into template literals > - Using the format method instead of direct interpolation > - Using escape_str for all values received from the API. > >Test plan: >1. Apply the patch and compile css >2. Navigate to a biblio record with bookings (Catalog > Search > View record > Bookings) >3. Verify the filter buttons (Exclude expired/cancelled) appear correctly >4. Click each filter button and verify: > - The button text toggles between "Include" and "Exclude" > - The table updates to show/hide the appropriate bookings > - The status column appears when any items are filtered >5. In the timeline view: > - Verify cancelled bookings show with a strikethrough pattern > - Try to cancel a booking by clicking the X on a timeline item > - Confirm the cancel modal opens without JavaScript errors >6. Check that the table actions (Edit, Cancel) work correctly >7. Verify no inline styles remain in the page source >8. Confirm the filter styles are properly loaded from compiled CSS >--- > .../intranet-tmpl/prog/css/src/bookings.scss | 13 ++ > .../prog/css/src/staff-global.scss | 14 ++ > .../prog/en/includes/table-filters.inc | 30 ++++ > .../prog/en/modules/bookings/list.tt | 162 ++++++++++-------- > 4 files changed, 145 insertions(+), 74 deletions(-) > create mode 100644 koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss > create mode 100644 koha-tmpl/intranet-tmpl/prog/en/includes/table-filters.inc > >diff --git a/koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss b/koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss >new file mode 100644 >index 00000000000..3f72fbf488f >--- /dev/null >+++ b/koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss >@@ -0,0 +1,13 @@ >+#bookings-timeline { >+ .vis-item.vis-range { >+ &.cancelled { >+ background: repeating-linear-gradient( >+ 135deg, >+ rgba(211, 211, 211, 0.5) 0, >+ rgba(211, 211, 211, 0.5) 10px, >+ transparent 10px, >+ transparent 20px >+ ); >+ } >+ } >+} >\ No newline at end of file >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 5777e8211e1..b8c51371cc2 100644 >--- a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss >+++ b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss >@@ -4895,3 +4895,17 @@ div .suggestion_note { > } > } > } >+ >+[class*="filters-"] { >+ [data-filtered] span:nth-child(2) { >+ display: none; >+ } >+ >+ a:not([data-filtered]) span:nth-child(1) { >+ display: none; >+ } >+ >+ a { >+ cursor: pointer; >+ } >+} >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/table-filters.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/table-filters.inc >new file mode 100644 >index 00000000000..714ce3c1d78 >--- /dev/null >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/table-filters.inc >@@ -0,0 +1,30 @@ >+[%# TABLE FILTER CONTROLS USAGE %] >+[%# [ INCLUDE 'table-filters.inc' %] >+[%# filters = [ %] >+[%# { id = 'expired', label_show = t('Show expired'), label_hide = t('Hide expired') }, %] >+[%# { id = 'cancelled', label_show = t('Show cancelled'), label_hide = t('Hide cancelled') }, %] >+[%# { id = 'completed', label_show = t('Show completed'), label_hide = t('Hide completed') } %] >+[%# ] %] >+[%# filter_class = 'bookings' # Optional, defaults to 'filters' %] >+[%# ] %] >+[%# %] >+[%# Parameters: %] >+[%# - filters: Array of filter objects with: %] >+[%# - id: Unique identifier for the filter %] >+[%# - label_show: Text when filter shows items %] >+[%# - label_hide: Text when filter hides items %] >+[%# - active: Boolean - filter starts active (optional, defaults to true) %] >+[%# - filter_class: CSS class prefix (optional, defaults to 'filters') %] >+ >+[% IF filters && filters.size > 0 %] >+ [% filter_class = filter_class || 'filters' %] >+ >+ <fieldset class="action filters filters-[% filter_class | html %] d-flex gap-2"> >+ [% FOREACH filter IN filters %] >+ <a id="filter-[% filter.id | html %]" data-filter="[% filter.id | html %]" [% IF !filter.defined('active') || filter.active %]data-filtered[% END %]> >+ <span><i class="fa fa-bars"></i> [% filter.label_show | html %]</span> >+ <span><i class="fa fa-filter"></i> [% filter.label_hide | html %]</span> >+ </a> >+ [% END %] >+ </fieldset> >+[% END %] >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 f9e4c193785..f3341d5b714 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt >@@ -12,13 +12,7 @@ > [% END %]</title > > > [% INCLUDE 'doc-head-close.inc' %] >-<style> >- #bookings-timeline .vis-item.vis-range { >- &.cancelled { >- background: rgba(128, 128, 128, 0.3); >- } >- } >-</style> >+[% Asset.css("css/bookings.css") | $raw %] > </head> > > <body id="circ_request" class="catalog"> >@@ -47,10 +41,14 @@ > <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 d-flex gap-2" style="cursor: pointer;"> >- <a id="expired_filter" data-filter="expired"><i class="fa fa-bars"></i> Show expired</a> >- <a id="cancelled_filter" data-filter="cancelled"><i class="fa fa-bars"></i> Show cancelled</a> >- </fieldset> >+ [% >+ INCLUDE 'table-filters.inc' >+ filters = [ >+ { id = 'expired', label_show = t('Include expired'), label_hide = t('Exclude expired') }, >+ { id = 'cancelled', label_show = t('Include cancelled'), label_hide = t('Exclude cancelled') }, >+ ] >+ filter_class = 'bookings' >+ %] > > <table id="bookings_table"></table> > </div> >@@ -124,6 +122,7 @@ > start: dayjs(booking.start_date).toDate(), > end: dayjs(booking.end_date).toDate(), > content: !isActive ? `<s>${patronContent}</s>` : patronContent, >+ className: booking.status === "cancelled" ? "cancelled" : "", > [% IF CAN_user_circulate_manage_bookings %] > editable: booking.status !== "cancelled" ? { remove: true, updateTime: true } : false, > [% ELSE %] >@@ -134,17 +133,6 @@ > }); > } > >- 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, >@@ -201,7 +189,9 @@ > }); > }, > onRemove: function (item, callback) { >- $('#cancelBookingModal').modal('show', $('<button data-booking="'+item.id+'">')); >+ const button = document.createElement('button'); >+ button.dataset.booking = item.booking || item.id; >+ $('#cancelBookingModal').modal('show', button); > $('#cancelBookingModal').on('hide.bs.modal', function(e) { > if (cancel_success) { > cancel_success = 0; >@@ -219,25 +209,53 @@ > } > ); > >- const filterStates = { expired: true, cancelled: false }; >+ const filters = ["expired", "cancelled"].reduce((acc, filter) => { >+ acc[filter] = document.getElementById(`filter-${filter}`); >+ return acc; >+ }, {}); >+ >+ Object.values(filters).forEach(filter => { >+ filter.addEventListener("click", handleFilter); >+ }); >+ >+ const isFilterActive = (filter) => "filtered" in filter.dataset; >+ const isShowingItems = (filter) => !isFilterActive(filter); > const additional_filters = { > end_date: () => { >- if (filterStates.expired) { >+ if (isShowingItems(filters.expired)) { > let today = new Date(); > return { ">=": today.toISOString() }; > } > }, > status: () => { > const defaults = ["new", "pending", "active"]; >- if (filterStates.cancelled) { >- const filtered = [...defaults, "cancelled"]; >- return { "-in": filtered }; >+ const filtered = [...defaults]; >+ if (isShowingItems(filters.cancelled)) { >+ filtered.push("cancelled"); > } >- >- return { "-in": defaults }; >+ return { "-in": filtered }; > }, > }; > >+ function handleFilter(e) { >+ const target = e.target; >+ const anchor = target.closest("a"); >+ >+ if (isFilterActive(anchor)) { >+ delete anchor.dataset.filtered; >+ } else { >+ anchor.dataset.filtered = ""; >+ } >+ >+ bookings_table.DataTable().ajax.reload(() => { >+ const anyFiltersActive = Object.values(filters).some(isShowingItems); >+ bookings_table >+ .DataTable() >+ .column("status:name") >+ .visible(anyFiltersActive, false); >+ }); >+ } >+ > var bookings_table_url = "/api/v1/biblios/%s/bookings".format(biblionumber); > bookings_table = $('#bookings_table').kohaTable({ > "ajax": { >@@ -374,11 +392,47 @@ > "orderable": false, > "render": function(data, type, row, meta) { > let result = ""; >- let is_cancelled = row.status === "cancelled"; >+ const is_readonly = row.status === "cancelled"; > [% IF CAN_user_circulate_manage_bookings %] >- 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="%s" data-biblionumber="%s" data-itemnumber="%s" data-patron="%s" data-pickup_library="%s" data-start_date="%s" data-end_date="%s" data-item_type_id="%s"><i class="fa fa-pencil" aria-hidden="true"></i> %s</button>'.format(row.booking_id, biblionumber, row.item_id, row.patron_id, row.pickup_library_id, row.start_date, row.end_date, row.item.item_type_id, _("Edit")); >- result += '<button type="button" class="btn btn-default btn-xs cancel-action" data-bs-toggle="modal" data-bs-target="#cancelBookingModal" data-booking="%s"><i class="fa fa-trash" aria-hidden="true"></i> %s</button>'.format(row.booking_id, _("Cancel")); >+ if (!is_readonly) { >+ result += ` >+ <button type="button" class="btn btn-default btn-xs edit-action" >+ data-bs-toggle="modal" >+ data-bs-target="#placeBookingModal" >+ data-booking="%s" >+ data-biblionumber="%s" >+ data-itemnumber="%s" >+ data-patron="%s" >+ data-pickup_library="%s" >+ data-start_date="%s" >+ data-end_date="%s" >+ data-item_type_id="%s" >+ > >+ <i class="fa fa-pencil" aria-hidden="true"></i> %s >+ </button> >+ `.format( >+ escape_str(row.booking_id), >+ escape_str(biblionumber), >+ escape_str(row.item_id), >+ escape_str(row.patron_id), >+ escape_str(row.pickup_library_id), >+ escape_str(row.start_date), >+ escape_str(row.end_date), >+ escape_str(row.item.item_type_id), >+ _("Edit") >+ ); >+ result += ` >+ <button type="button" class="btn btn-default btn-xs cancel-action" >+ data-bs-toggle="modal" >+ data-bs-target="#cancelBookingModal" >+ data-booking="%s" >+ > >+ <i class="fa fa-trash" aria-hidden="true"></i> %s >+ </button> >+ `.format( >+ escape_str(row.booking_id), >+ _("Cancel") >+ ); > } > [% END %] > return result; >@@ -386,51 +440,11 @@ > }] > }, undefined, 0, additional_filters); > >- document >- .getElementById("expired_filter") >- .addEventListener("click", (e) => >- handleFilter( >- e, >- { active: _("Hide expired"), inactive: _("Show expired") }, >- filterStates, >- ), >- ); >- >- document >- .getElementById("cancelled_filter") >- .addEventListener("click", (e) => >- handleFilter( >- e, >- { active: _("Hide cancelled"), inactive: _("Show cancelled") }, >- filterStates, >- ), >- ); >- > $("#cancellation-reason").comboBox({ > displayProperty: 'name', > placeholder: _("Select or type a reason"), > }); > >- function handleFilter(e, text, filterStates) { >- const target = e.target; >- const { filter } = target.dataset; >- if (target.classList.contains("filtered")) { >- filterStates[filter] = false; >- target.innerHTML = `<i class="fa fa-bars"></i> ${text.inactive}`; >- } else { >- filterStates[filter] = true; >- target.innerHTML = `<i class="fa fa-filter"></i> ${text.active}`; >- } >- >- bookings_table.DataTable().ajax.reload(() => { >- bookings_table >- .DataTable() >- .column("status:name") >- .visible(filterStates[filter], false); >- }); >- >- target.classList.toggle("filtered"); >- } > }); > </script> > [% END %] >-- >2.39.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40656
: 185468 |
185469
|
185470