[Bug 40206] New: Cubrside pickups - Layout wrong when 'CircSidebar' enabled
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Bug ID: 40206 Summary: Cubrside pickups - Layout wrong when 'CircSidebar' enabled Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Templates Assignee: oleonard@myacpl.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org The current template uses a `[% STOP %]` statement that seems to break the `[% WRAPPER %]` flow, yielding an incomplete page. To reproduce: 1. Enable curbside pickups. 2. 'Activate' the 'CircSidebar' system preference. 3. Go to 'Circulation' > 'Curbside pickups' => FAIL: The circulation sidebar is not displayed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|oleonard@myacpl.org |tomascohen@gmail.com CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 183394 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183394&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |30650 Blocks| |40167 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30650 [Bug 30650] Add a curbside pickup module https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40167 [Bug 40167] Curbside pickups - Show link if not configured -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |oleonard@myacpl.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Circulation| |Curbside pickups function| | Component|Templates |Circulation CC| |gmcharlt@gmail.com, | |kyle.m.hall@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Cubrside pickups - Layout |Curbside pickups - Layout |wrong when 'CircSidebar' |wrong when 'CircSidebar' |enabled |enabled -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34772 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34772 [Bug 34772] Curbside pickup - Assign multiple pickups to a single patron -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183394|0 |1 is obsolete| | --- Comment #2 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 183399 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183399&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@gmail.com --- Comment #3 from Jonathan Druart <jonathan.druart@gmail.com> --- I don't think redirect to 404 is a consistent behaviour. We do that at the OPAC, not for staff. I think we need to link to the admin page. Or the syspref: koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt: <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=ExtendedPatronAttributes">ExtendedPatronAttributes</a> system preference if you wish to enable this feature.</div -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183399|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 --- Comment #4 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201759 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201759&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 --- Comment #5 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #3)
I don't think redirect to 404 is a consistent behaviour. We do that at the OPAC, not for staff.
I agree it is not ideal. I based it on the closest module I know get's disabled by a syspref: ILLModule. It is fixed now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 --- Comment #6 from Jonathan Druart <jonathan.druart@gmail.com> --- I am planning to remove this syspref on bug 42344 that is Signed Off already. Why not helping there instead of reviving this bug now? :-( -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Sponsorship status|--- |Unsponsored QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201759|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 --- Comment #7 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201993 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201993&action=edit 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> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 --- Comment #8 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201994 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201994&action=edit Bug 40206: (QA follow-up) Fix JS error and misleading message on disabled curbside pickups page The early-exit added for the CurbsidePickup syspref check left policy/curbside_pickups unset, but intranet-bottom.inc's jsinclude still referenced them unconditionally, causing a client-side TypeError on every render of the disabled state. Guard the whole jsinclude block with disabled_for_branch. Also distinguish the two reasons the page can be disabled: the CurbsidePickup syspref being off globally vs. this branch simply not having an enabled curbside pickup policy configured. The message and link now point staff to the correct admin page for each case, instead of always suggesting the syspref (which may already be on). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 --- Comment #9 from Jonathan Druart <jonathan.druart@gmail.com> --- Martin, did you see my previous comment? I don't understand the need to push this to main with bug 42344 that is signed off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@openfifth.co.u | |k --- Comment #10 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Hi guys, my suggestion here: Make bug 42344 depend on bug 40206. Bug 40206 is a bug fix highly likely to be backported. Bug 42344 and bug 42330 are enhancements which will not be backported. The exception would be if this bug exists only on main. Is this fair? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 --- Comment #11 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Pedro Amorim (ammopt) from comment #10)
Hi guys, my suggestion here: Make bug 42344 depend on bug 40206. Bug 40206 is a bug fix highly likely to be backported. Bug 42344 and bug 42330 are enhancements which will not be backported.
The exception would be if this bug exists only on main. Is this fair?
Or we only apply the change to the stable branches. It's really weird to see patch waiting 1 year in FQA then back into the queue at the same time another one already signed off... Also links to syspref should now use the new TT plugin (see bug 41674). Anyway, whatever is decided, I will rebase. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 --- Comment #12 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- (In reply to Jonathan Druart from comment #11)
Or we only apply the change to the stable branches.
I'm on board with having this applied only to 26.05 and down as a bug fix for those releases, but we'd need confirmation that bug 42344 implicitly also achieves the same fix for main. Tomas, Martin, thoughts? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42344 --- Comment #13 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Pedro Amorim (ammopt) from comment #12)
Tomas, Martin, thoughts?
I will rebase. No need to spend more time ;) Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42344 [Bug 42344] Remove CircSidebar -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |26.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40206 --- Comment #14 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Thanks everyone! Pushed to main for 26.11! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org