Bugzilla – Attachment 183394 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.86 KB, created by
Tomás Cohen Arazi (tcohen)
on 2025-06-20 13:20:59 UTC
(
hide
)
Description:
Bug 40206: Fix wrong layout in curbside pickups when 'CircSidebar' enabled
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2025-06-20 13:20:59 UTC
Size:
24.86 KB
patch
obsolete
>From a179832aa7bfb888610ee6958fd05e961b62ca6e 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 > k$ koha-plack --restart kohadev >8. Repeat 4 >=> SUCCESS: It renders much better! >9. Sign off :-D >--- > 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.50.0
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