Bug 40206 - Curbside pickups - Layout wrong when 'CircSidebar' enabled
Summary: Curbside pickups - Layout wrong when 'CircSidebar' enabled
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Testopia
URL:
Keywords:
Depends on: 30650
Blocks: 34772 40167
  Show dependency treegraph
 
Reported: 2025-06-20 13:16 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2025-06-24 09:59 UTC (History)
5 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function: Curbside pickups


Attachments
Bug 40206: Fix wrong layout in curbside pickups when 'CircSidebar' enabled (24.86 KB, patch)
2025-06-20 13:20 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 40206: Fix wrong layout in curbside pickups when 'CircSidebar' enabled (24.93 KB, patch)
2025-06-20 18:10 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2025-06-20 13:16:04 UTC
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.
Comment 1 Tomás Cohen Arazi (tcohen) 2025-06-20 13:20:59 UTC
Created attachment 183394 [details] [review]
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
Comment 2 Owen Leonard 2025-06-20 18:10:30 UTC
Created attachment 183399 [details] [review]
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>
Comment 3 Jonathan Druart 2025-06-24 09:59:58 UTC
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&amp;searchfield=ExtendedPatronAttributes">ExtendedPatronAttributes</a> system preference if you wish to enable this feature.</div