Bugzilla – Attachment 183399 Details for
Bug 40206
Curbside pickups - Layout wrong when 'CircSidebar' enabled
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40206: Fix wrong layout in curbside pickups when 'CircSidebar' enabled
Bug-40206-Fix-wrong-layout-in-curbside-pickups-whe.patch (text/plain), 24.93 KB, created by
Owen Leonard
on 2025-06-20 18:10:30 UTC
(
hide
)
Description:
Bug 40206: Fix wrong layout in curbside pickups when 'CircSidebar' enabled
Filename:
MIME Type:
Creator:
Owen Leonard
Created:
2025-06-20 18:10:30 UTC
Size:
24.93 KB
patch
obsolete
>From f0e9036bf5715e219630d85f41d79364bf1b2812 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Thu, 19 Jun 2025 12:16:06 -0300 >Subject: [PATCH] Bug 40206: Fix wrong layout in curbside pickups when > 'CircSidebar' enabled > >This patch solves the following issues: > >* Template flow issue: the template called STOP which prevented the > WRAPPER to complete and affected how things were rendered. >* Redirect to 404 if feature disabled. This is common practice in other > modules. The fact things can be executed even with the feature > disabled is not ok (syspref). >* Shortcircuit earlier. Similar to the above, if the feature is > disabled for the current branch, no DB queries and stuff should take > place. Notice the lack of checks actually makes the controller perform > the actions. It is just that it is not displayed afterwards! > >To test: >1. Enable the `CurbsidePickup` syspref. >2. 'Activate' the `CircSidebar` syspref. >3. Be on a branch that doesn't have them enabled >4. Go to Circulation > Check in > => SUCCESS: A nice left-hand sidebar is displayed with access to > Circulation actions >5. Click on 'Curbside pickups' > => FAIL: The sidebar doesn't display. boo! >6. Apply this patch >7. Restart plack > $ ktd --shell > $ koha-plack --restart kohadev >8. Repeat 4 > => SUCCESS: It renders much better! >9. Sign off :-D > >Signed-off-by: Owen Leonard <oleonard@myacpl.org> >--- > circ/curbside_pickups.pl | 20 +- > .../prog/en/modules/circ/curbside_pickups.tt | 395 +++++++++--------- > 2 files changed, 218 insertions(+), 197 deletions(-) > >diff --git a/circ/curbside_pickups.pl b/circ/curbside_pickups.pl >index 354e927a2e1..0a9c63ff50a 100755 >--- a/circ/curbside_pickups.pl >+++ b/circ/curbside_pickups.pl >@@ -28,7 +28,13 @@ use Koha::CurbsidePickupPolicies; > use Koha::Libraries; > use Koha::Patrons; > >-my $input = CGI->new; >+my $input = CGI->new; >+ >+unless ( C4::Context->preference('CurbsidePickup') ) { >+ print $input->redirect("/cgi-bin/koha/errors/404.pl"); >+ exit; >+} >+ > my $op = $input->param('op') || 'list'; > my $tab = $input->param('tab'); > my $auto_refresh = $input->param('auto_refresh'); >@@ -45,7 +51,15 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( > ); > > my $branchcode = C4::Context->userenv()->{'branch'}; >-my $libraries = Koha::Libraries->search( {}, { order_by => ['branchname'] } ); >+ >+my $policy = Koha::CurbsidePickupPolicies->find( { branchcode => $branchcode } ); >+unless ( $policy && $policy->enabled ) { >+ $template->param( disabled_for_branch => 1 ); >+ output_html_with_http_headers $input, $cookie, $template->output; >+ exit; >+} >+ >+my $libraries = Koha::Libraries->search( {}, { order_by => ['branchname'] } ); > if ( $op eq 'find-patron' ) { > my $borrowernumber = $input->param('borrowernumber'); > >@@ -150,7 +164,7 @@ $template->param( > tab => $tab, > auto_refresh => $auto_refresh, > refresh_delay => $refresh_delay, >- policy => Koha::CurbsidePickupPolicies->find( { branchcode => $branchcode } ), >+ policy => $policy, > curbside_pickups => Koha::CurbsidePickups->search( > { > branchcode => $branchcode, >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/curbside_pickups.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/curbside_pickups.tt >index e6503096cc4..529147b1708 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/curbside_pickups.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/curbside_pickups.tt >@@ -84,16 +84,9 @@ > [% END %] > [% END %] > >-[% SET aside = Koha.Preference('CircSidebar') ? 'circ-nav' : '' %] >-[% WRAPPER 'main-container.inc' aside=aside %] >+[% BLOCK curbside_pickups_enabled %] > <h1>Curbside pickups</h1> > >- [% UNLESS policy.enabled %] >- <div class="alert alert-warning"> Curbside pickups are not enabled for your library. </div> >- [% INCLUDE 'intranet-bottom.inc' %] >- [% STOP %] >- [% END %] >- > [% FOR m IN messages %] > <div class="alert alert-[% m.type | html %]"> > [% SWITCH m.code %] >@@ -457,239 +450,253 @@ > [% END # /tab_panel %] > [% END # /WRAPPER tab_panels %] > [% END # /WRAPPER tabs %] >-[% END %] > >-[% MACRO jsinclude BLOCK %] >- [% Asset.js("lib/dayjs/plugin/isSameOrAfter.js") | $raw %] >- <script> >- dayjs.extend(window.dayjs_plugin_isSameOrAfter); >- </script> >- <script> >- dayjs.extend(window.dayjs_plugin_customParseFormat); >- </script> >- [% INCLUDE 'calendar.inc' %] >- [% INCLUDE 'js-patron-format.inc' %] >- [% INCLUDE 'datatables.inc' %] >- <script> >- let pickups = [% To.json(curbside_pickups.unblessed) | $raw %]; >- let policy = [% To.json(policy.unblessed) | $raw %]; >- >- let existingPickupMoments = []; >- pickups.forEach(function(pickup){ >- let scheduled_pickup_datetime = pickup.scheduled_pickup_datetime; >- let pickupMoment = dayjs(scheduled_pickup_datetime); >- >- existingPickupMoments.push(pickupMoment); >- }); >- >- let opening_slots = [% To.json(policy.opening_slots.unblessed) | $raw %]; >- let slots_per_day = {}; >- opening_slots.forEach(function(slot){ >- let day = slot.day; >- if(!slots_per_day[day]) slots_per_day[day] = []; >- slots_per_day[day].push(slot); >- }); >- >- $(document).ready(function() { >- >- $('#schedule-pickup-tab').on('click', function() { >- $('#find-patron').focus(); >+ [% MACRO jsinclude BLOCK %] >+ [% Asset.js("lib/dayjs/plugin/isSameOrAfter.js") | $raw %] >+ <script> >+ dayjs.extend(window.dayjs_plugin_isSameOrAfter); >+ </script> >+ <script> >+ dayjs.extend(window.dayjs_plugin_customParseFormat); >+ </script> >+ [% INCLUDE 'calendar.inc' %] >+ [% INCLUDE 'js-patron-format.inc' %] >+ [% INCLUDE 'datatables.inc' %] >+ <script> >+ let pickups = [% To.json(curbside_pickups.unblessed) | $raw %]; >+ let policy = [% To.json(policy.unblessed) | $raw %]; >+ >+ let existingPickupMoments = []; >+ pickups.forEach(function(pickup){ >+ let scheduled_pickup_datetime = pickup.scheduled_pickup_datetime; >+ let pickupMoment = dayjs(scheduled_pickup_datetime); >+ >+ existingPickupMoments.push(pickupMoment); > }); > >- [% IF tab == 'schedule-pickup_panel' %] >- $('#find-patron').focus(); >- [% END %] >+ let opening_slots = [% To.json(policy.opening_slots.unblessed) | $raw %]; >+ let slots_per_day = {}; >+ opening_slots.forEach(function(slot){ >+ let day = slot.day; >+ if(!slots_per_day[day]) slots_per_day[day] = []; >+ slots_per_day[day].push(slot); >+ }); > >- const pickup_date = document.querySelector("#pickup_date"); >- if ( pickup_date ) { >- const fp = pickup_date._flatpickr; >- fp.set('disable', [function(date) { >- return !slots_per_day.hasOwnProperty(date.getDay()); >- }]); >- } >+ $(document).ready(function() { > >- $("#pickup_date").on('change', function() { >+ $('#schedule-pickup-tab').on('click', function() { >+ $('#find-patron').focus(); >+ }); > >- $('#pickup-times').empty(); >- $('#schedule-pickup-button').prop( 'disabled', 1 ); >+ [% IF tab == 'schedule-pickup_panel' %] >+ $('#find-patron').focus(); >+ [% END %] > >- var currentDate = $(this).val(); >+ const pickup_date = document.querySelector("#pickup_date"); >+ if ( pickup_date ) { >+ const fp = pickup_date._flatpickr; >+ fp.set('disable', [function(date) { >+ return !slots_per_day.hasOwnProperty(date.getDay()); >+ }]); >+ } > >- let selectedDate = dayjs(currentDate); >+ $("#pickup_date").on('change', function() { > >- let pickupSlots = []; >- let available_count = 0; >- let dow = selectedDate.day(); // Sunday is 0 (at least for now) >- if (!slots_per_day[dow]){ >- $('#pickup-times').html("<div>"+_("No pickup time defined for this day.")+"</div>"); >- return; >- } >+ $('#pickup-times').empty(); >+ $('#schedule-pickup-button').prop( 'disabled', 1 ); > >- slots_per_day[dow].sort((a, b) => a.start_hour - b.start_hour).forEach(function(slot){ >- let pickup_interval = policy.pickup_interval; >- if (!pickup_interval) { >+ var currentDate = $(this).val(); >+ >+ let selectedDate = dayjs(currentDate); >+ >+ let pickupSlots = []; >+ let available_count = 0; >+ let dow = selectedDate.day(); // Sunday is 0 (at least for now) >+ if (!slots_per_day[dow]){ > $('#pickup-times').html("<div>"+_("No pickup time defined for this day.")+"</div>"); > return; > } > >- let listStartMoment = selectedDate.hour(slot.start_hour).minute(slot.start_minute); >- let listEndMoment = selectedDate.hour(slot.end_hour).minute(slot.end_minute); >- >- let keep_going = true; >- let now = dayjs(); >+ slots_per_day[dow].sort((a, b) => a.start_hour - b.start_hour).forEach(function(slot){ >+ let pickup_interval = policy.pickup_interval; >+ if (!pickup_interval) { >+ $('#pickup-times').html("<div>"+_("No pickup time defined for this day.")+"</div>"); >+ return; >+ } > >- // Initialize pickup slots starting at opening time >- let pickupIntervalStartMoment = listStartMoment; >- let pickupIntervalEndMoment = listStartMoment.add(pickup_interval, 'minutes'); >- while (keep_going) { >- let available = true; >- let display_slot = true >+ let listStartMoment = selectedDate.hour(slot.start_hour).minute(slot.start_minute); >+ let listEndMoment = selectedDate.hour(slot.end_hour).minute(slot.end_minute); > >- if (pickupIntervalStartMoment.isBefore(now)) { >- // Slots in the past are unavailable >- available = false; >- display_slot = false; >- } >+ let keep_going = true; >+ let now = dayjs(); > >- if (pickupIntervalEndMoment.isAfter(listEndMoment)) { >- // Slots after the end of pickup times for the day are unavailable >- available = false; >- } >+ // Initialize pickup slots starting at opening time >+ let pickupIntervalStartMoment = listStartMoment; >+ let pickupIntervalEndMoment = listStartMoment.add(pickup_interval, 'minutes'); >+ while (keep_going) { >+ let available = true; >+ let display_slot = true > >- let pickups_scheduled = 0; >- existingPickupMoments.forEach(function(pickupMoment){ >- // An existing pickup time >- if (pickupMoment.isSameOrAfter(pickupIntervalStartMoment) && pickupMoment.isBefore(pickupIntervalEndMoment)) { >- // This calculated pickup is in use by another scheduled pickup >- pickups_scheduled++; >+ if (pickupIntervalStartMoment.isBefore(now)) { >+ // Slots in the past are unavailable >+ available = false; >+ display_slot = false; > } >- }); > >- if (pickups_scheduled >= policy.patrons_per_interval) { >- available = false; >- } >+ if (pickupIntervalEndMoment.isAfter(listEndMoment)) { >+ // Slots after the end of pickup times for the day are unavailable >+ available = false; >+ } > >- if ( display_slot ) { >- pickupSlots.push( >- { >- "available": available, >- "moment": pickupIntervalStartMoment, >- "pickups_scheduled": pickups_scheduled >+ let pickups_scheduled = 0; >+ existingPickupMoments.forEach(function(pickupMoment){ >+ // An existing pickup time >+ if (pickupMoment.isSameOrAfter(pickupIntervalStartMoment) && pickupMoment.isBefore(pickupIntervalEndMoment)) { >+ // This calculated pickup is in use by another scheduled pickup >+ pickups_scheduled++; > } >- ); >- } >+ }); > >- if ( available ) { >- available_count++; >- } >+ if (pickups_scheduled >= policy.patrons_per_interval) { >+ available = false; >+ } > >- pickupIntervalStartMoment = pickupIntervalEndMoment; >- pickupIntervalEndMoment = pickupIntervalStartMoment.add(pickup_interval, 'minutes'); >- if (pickupIntervalEndMoment.isAfter(listEndMoment)) { >- // This latest slot is after the end of pickup times for the day, so we can stop >- keep_going = false; >+ if ( display_slot ) { >+ pickupSlots.push( >+ { >+ "available": available, >+ "moment": pickupIntervalStartMoment, >+ "pickups_scheduled": pickups_scheduled >+ } >+ ); >+ } >+ >+ if ( available ) { >+ available_count++; >+ } >+ >+ pickupIntervalStartMoment = pickupIntervalEndMoment; >+ pickupIntervalEndMoment = pickupIntervalStartMoment.add(pickup_interval, 'minutes'); >+ if (pickupIntervalEndMoment.isAfter(listEndMoment)) { >+ // This latest slot is after the end of pickup times for the day, so we can stop >+ keep_going = false; >+ } > } >+ }); >+ >+ for (let i = 0; i < pickupSlots.length; i++) { >+ let pickupSlot = pickupSlots[i]; >+ let optText = pickupSlot.moment.format("HH:mm"); >+ let optValue = pickupSlot.moment.format("YYYY-MM-DD HH:mm:ss"); >+ let pickups_scheduled = pickupSlot.pickups_scheduled; >+ let disabled = pickupSlot.available ? "" : "disabled"; >+ $("#pickup-times").append(`<span class="pickup_time"><input type="radio" id="slot_${i}" name="pickup_time" value="${optValue}" ${disabled} /> <label for="slot_${i}">${optText} (${pickups_scheduled})</label></span>`); > } >- }); > >- for (let i = 0; i < pickupSlots.length; i++) { >- let pickupSlot = pickupSlots[i]; >- let optText = pickupSlot.moment.format("HH:mm"); >- let optValue = pickupSlot.moment.format("YYYY-MM-DD HH:mm:ss"); >- let pickups_scheduled = pickupSlot.pickups_scheduled; >- let disabled = pickupSlot.available ? "" : "disabled"; >- $("#pickup-times").append(`<span class="pickup_time"><input type="radio" id="slot_${i}" name="pickup_time" value="${optValue}" ${disabled} /> <label for="slot_${i}">${optText} (${pickups_scheduled})</label></span>`); >- } >+ $('#pickup-times').show(); >+ $('#schedule-pickup-button').prop( 'disabled', available_count <= 0 ); >+ }); > >- $('#pickup-times').show(); >- $('#schedule-pickup-button').prop( 'disabled', available_count <= 0 ); >- }); >+ $("#create-pickup").on('submit', function(){ >+ if ( ! $("input[type='radio']:checked").length ) { >+ alert(_("Please select a date and a pickup time")) >+ return false; >+ } >+ return true; >+ }); > >- $("#create-pickup").on('submit', function(){ >- if ( ! $("input[type='radio']:checked").length ) { >- alert(_("Please select a date and a pickup time")) >- return false; >+ if ( $("#find-patron").length ) { >+ patron_autocomplete($("#find-patron"), { 'on-select-callback': function( event, ui ) { >+ window.location.href = "/cgi-bin/koha/circ/curbside_pickups.pl?op=find-patron&borrowernumber=" + ui.item.patron_id; >+ return false; >+ } >+ }); > } >- return true; >- }); > >- if ( $("#find-patron").length ) { >- patron_autocomplete($("#find-patron"), { 'on-select-callback': function( event, ui ) { >- window.location.href = "/cgi-bin/koha/circ/curbside_pickups.pl?op=find-patron&borrowernumber=" + ui.item.patron_id; >- return false; >- } >+ $("#pickup-tabs a[data-bs-toggle='tab']").on("shown.bs.tab", function (e) { >+ $("#current-tab").val($(this).attr('href').substring(1)); // Remove # >+ }); >+ $("#auto_refresh,#refresh_delay").on("change", function(){ >+ set_interval_if_needed(); > }); >- } > >- $("#pickup-tabs a[data-bs-toggle='tab']").on("shown.bs.tab", function (e) { >- $("#current-tab").val($(this).attr('href').substring(1)); // Remove # >- }); >- $("#auto_refresh,#refresh_delay").on("change", function(){ > set_interval_if_needed(); >- }); > >- set_interval_if_needed(); > >+ let dt_settings = []; >+ dt_settings.push( [% TablesSettings.GetTableSettings('circ', 'curbside_pickups', 'to_be_stagedt', 'json') | $raw %] ); >+ dt_settings.push( [% TablesSettings.GetTableSettings('circ', 'curbside_pickups', 'staged_and_readyt', 'json') | $raw %] ); >+ dt_settings.push( [% TablesSettings.GetTableSettings('circ', 'curbside_pickups', 'patron_is_outsidet', 'json') | $raw %] ); >+ dt_settings.push( [% TablesSettings.GetTableSettings('circ', 'curbside_pickups', 'delivered_todayt', 'json') | $raw %] ); >+ >+ let dt_tables = new Array("to_be_stagedt", "staged_and_readyt", "patron_is_outsidet", "delivered_todayt"); >+ dt_tables.forEach(function( id, index ){ >+ $("#" + id).kohaTable({ >+ "autoWidth": false >+ }, dt_settings[ index ] ); >+ }); > >- let dt_settings = []; >- dt_settings.push( [% TablesSettings.GetTableSettings('circ', 'curbside_pickups', 'to_be_stagedt', 'json') | $raw %] ); >- dt_settings.push( [% TablesSettings.GetTableSettings('circ', 'curbside_pickups', 'staged_and_readyt', 'json') | $raw %] ); >- dt_settings.push( [% TablesSettings.GetTableSettings('circ', 'curbside_pickups', 'patron_is_outsidet', 'json') | $raw %] ); >- dt_settings.push( [% TablesSettings.GetTableSettings('circ', 'curbside_pickups', 'delivered_todayt', 'json') | $raw %] ); > >- let dt_tables = new Array("to_be_stagedt", "staged_and_readyt", "patron_is_outsidet", "delivered_todayt"); >- dt_tables.forEach(function( id, index ){ >- $("#" + id).kohaTable({ >- "autoWidth": false >- }, dt_settings[ index ] ); > }); > >+ let refresh_interval_id = 0; >+ let countdown_interval_id = 0; >+ function set_interval(refresh_delay_ms){ >+ clear_intervals(); >+ let next_refresh = new Date(); >+ next_refresh.setSeconds(next_refresh.getSeconds() + refresh_delay_ms / 1000); >+ >+ countdown_interval_id = setInterval(function() { >+ const now = new Date().getTime(); >+ const seconds = Math.floor(( next_refresh - now + 1 ) / 1000); >+ if ( seconds > 0 ) { >+ $("#refresh_info").text(_("Refresh in %s seconds").format(seconds)); >+ } else { >+ $("#refresh_info").text(""); // In case something is going wrong >+ } >+ }, 1000); > >- }); >+ setInterval(function() { >+ $(".refresh-form:visible").submit(); >+ }, refresh_delay_ms); >+ } >+ function clear_intervals(){ >+ if (refresh_interval_id) { >+ clearInterval(refresh_interval_id); >+ refresh_interval_id = 0; >+ } >+ if (countdown_interval_id) { >+ clearInterval(countdown_interval_id); >+ countdown_interval_id = 0; >+ } >+ } > >- let refresh_interval_id = 0; >- let countdown_interval_id = 0; >- function set_interval(refresh_delay_ms){ >- clear_intervals(); >- let next_refresh = new Date(); >- next_refresh.setSeconds(next_refresh.getSeconds() + refresh_delay_ms / 1000); >+ function set_interval_if_needed(){ >+ const refresh_delay = $("#refresh_delay").val(); >+ const auto_refresh = $("#auto_refresh").is(":checked"); > >- countdown_interval_id = setInterval(function() { >- const now = new Date().getTime(); >- const seconds = Math.floor(( next_refresh - now + 1 ) / 1000); >- if ( seconds > 0 ) { >- $("#refresh_info").text(_("Refresh in %s seconds").format(seconds)); >+ if (auto_refresh && refresh_delay){ >+ set_interval(refresh_delay * 1000); > } else { >- $("#refresh_info").text(""); // In case something is going wrong >+ clear_intervals(); > } >- }, 1000); >- >- setInterval(function() { >- $(".refresh-form:visible").submit(); >- }, refresh_delay_ms); >- } >- function clear_intervals(){ >- if (refresh_interval_id) { >- clearInterval(refresh_interval_id); >- refresh_interval_id = 0; > } >- if (countdown_interval_id) { >- clearInterval(countdown_interval_id); >- countdown_interval_id = 0; >- } >- } >+ </script> >+ [% END %] >+[% END # /BLOCK enabled %] > >- function set_interval_if_needed(){ >- const refresh_delay = $("#refresh_delay").val(); >- const auto_refresh = $("#auto_refresh").is(":checked"); >+[% BLOCK curbside_pickups_disabled %] >+ <h1>Curbside pickups</h1> >+ <div class="alert alert-warning">Curbside pickups are not enabled for your library.</div> >+[% END # /BLOCK disabled %] > >- if (auto_refresh && refresh_delay){ >- set_interval(refresh_delay * 1000); >- } else { >- clear_intervals(); >- } >- } >- </script> >-[% END %] >+[% SET aside = Koha.Preference('CircSidebar') ? 'circ-nav' : '' %] >+[% WRAPPER 'main-container.inc' aside=aside %] >+ [% IF !disabled_for_branch %] >+ [% PROCESS curbside_pickups_enabled %] >+ [% ELSE %] >+ [% PROCESS curbside_pickups_disabled %] >+ [% END %] >+[% END # /WRAPPER %] > > [% INCLUDE 'intranet-bottom.inc' %] >-- >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 40206
:
183394
| 183399