Bug 33244 - Do not show lists in OPAC if OpacPublic is disabled
Summary: Do not show lists in OPAC if OpacPublic is disabled
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
: 30264 (view as bug list)
Depends on:
Blocks: 36337
  Show dependency treegraph
 
Reported: 2023-03-16 14:19 UTC by jpl
Modified: 2024-03-17 17:00 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.01


Attachments
Bug 33244: Do not show lists in OPAC if OpacPublic is disabled (16.49 KB, patch)
2023-11-21 16:33 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 33244: Do not show lists in OPAC if OpacPublic is disabled (16.54 KB, patch)
2023-11-22 00:12 UTC, David Cook
Details | Diff | Splinter Review
Bug 33244: Do not show lists in OPAC if OpacPublic is disabled (16.62 KB, patch)
2023-11-24 07:10 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description jpl 2023-03-16 14:19:50 UTC
Koha will still show public lists next to the logo in OPAC even when OpacPublic is disabled, i.e. patrons are required to login to access anything.

This can be a privacy issue, since list names may contain things that are not meant to be public and are intended to been seen only by registered patrons.

Also it is not useful anyway, as clicking on a list list name does not actually bring up that list and just shows the login form.

The whole list tab should be hidden when OpacPublic is disabled and a user is not logged in.
Comment 1 jpl 2023-10-25 21:20:06 UTC
Quick workaround I came up with. Works for me with Koha 22.11.10.000 Rosalie. This will only shows the lists when a user is logged in if opacpublic is set to false.

--- /koha/src/masthead.inc	2023-10-25 23:02:01.538423660 +0200
+++ masthead.inc		2023-10-25 23:03:45.558420570 +0200
@@ -37,6 +37,7 @@
                 [% END %]
                 <li class="divider-vertical"></li>
                 [% IF ( Koha.Preference( 'virtualshelves' ) == 1 ) %]
+	                [% IF (( Koha.Preference( 'opacpublic' ) == 0 ) && ( logged_in_user )) || ( Koha.Preference( 'opacpublic' )) %]
                     <li class="nav-item dropdown">
                         <a href="/cgi-bin/koha/opac-shelves.pl" title="Show lists" class="nav-link dropdown-toggle" id="listsmenu" data-toggle="dropdown" role="button" aria-label="Show dropdown with detailed list information" aria-haspopup="true" aria-expanded="false"
                             ><i class="fa fa-list fa-icon-black" aria-hidden="true"></i> <span class="listslabel">Lists</span>
@@ -81,6 +82,7 @@
                         </div> <!-- / .dropdown-menu -->
                     </li> <!-- / .nav-item.dropdown -->
                 [% END # / IF virtualshelves %]
+                [% END %]
             </ul> <!-- / .navbar-nav -->
 
             [% IF Koha.Preference( 'opacuserlogin' ) == 1 ||  Koha.Preference( 'EnableOpacSearchHistory') || Koha.Preference( 'opaclanguagesdisplay' ) %]
Comment 2 Owen Leonard 2023-11-21 16:33:04 UTC
Created attachment 159170 [details] [review]
Bug 33244: Do not show lists in OPAC if OpacPublic is disabled

This patch updates markup and CSS in the OPAC header so that the Cart
and Lists controls don't appear when OpacPublic is disabled.

The patch also moves the "Your cookies" link into the group of links
with "Log in to your account" and "Search history." This makes the
template logic a little simpler and also seems like a more logical
grouping.

To test, apply the patch and enable the CookieContent system preference.

- View the OPAC with the OpacPublic system preference enabled.
- If necessary, click the "Accept all cookies" button.
- The "Your cookies" link should now appear in the header between the
  log-in link and the search history link.
- Test the page at various browser widths to confirm that the header
  still responds well to changes.
- Go back to system preferences in the staff client and change the
  OpacPublic system preference to "disabled."
- Return to the OPAC and refresh the page. You should be redirected to
  the login page, and the Cart and Lists buttons should no longer appear
  in the header.
- The links in the header for login and cookies should look correct at
  various browser widths.
Comment 3 David Cook 2023-11-21 22:10:55 UTC
*** Bug 30264 has been marked as a duplicate of this bug. ***
Comment 4 David Cook 2023-11-21 23:25:32 UTC
I'm not sure why these 2 unrelated changes are on the same bug report.

I don't understand why only anonymous users see the "Your cookies" link, and why logged in users have to go into their account and look at "Consents". But I think that's an issue in master... so I'm not going to worry about it.
Comment 5 David Cook 2023-11-21 23:27:51 UTC
(In reply to David Cook from comment #4)
> I'm not sure why these 2 unrelated changes are on the same bug report.

Oh wait... I understand now. It is related because you're hiding the Cart and Lists. Yep yep yep. Gotcha. Makes sense. All good.
Comment 6 David Cook 2023-11-22 00:12:49 UTC
Created attachment 159188 [details] [review]
Bug 33244: Do not show lists in OPAC if OpacPublic is disabled

This patch updates markup and CSS in the OPAC header so that the Cart
and Lists controls don't appear when OpacPublic is disabled.

The patch also moves the "Your cookies" link into the group of links
with "Log in to your account" and "Search history." This makes the
template logic a little simpler and also seems like a more logical
grouping.

To test, apply the patch and enable the CookieContent system preference.

- View the OPAC with the OpacPublic system preference enabled.
- If necessary, click the "Accept all cookies" button.
- The "Your cookies" link should now appear in the header between the
  log-in link and the search history link.
- Test the page at various browser widths to confirm that the header
  still responds well to changes.
- Go back to system preferences in the staff client and change the
  OpacPublic system preference to "disabled."
- Return to the OPAC and refresh the page. You should be redirected to
  the login page, and the Cart and Lists buttons should no longer appear
  in the header.
- The links in the header for login and cookies should look correct at
  various browser widths.

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 7 Jonathan Druart 2023-11-23 12:40:36 UTC
"Log in to your account" is now on the left, is that expected?
Comment 8 David Cook 2023-11-23 23:25:21 UTC
(In reply to Jonathan Druart from comment #7)
> "Log in to your account" is now on the left, is that expected?

The test plan should've said that you need to run "perl build-resources.PL" to re-generate the CSS. 

If you don't, it will appear on the left. But if you do run it, then "Log in to your account" is still on the right as expected.
Comment 9 David Cook 2023-11-23 23:25:59 UTC
(In reply to David Cook from comment #8)
> (In reply to Jonathan Druart from comment #7)
> > "Log in to your account" is now on the left, is that expected?
> 
> The test plan should've said that you need to run "perl build-resources.PL"
> to re-generate the CSS. 

I should've noted it when testing due to its absence. My bad.
Comment 10 Jonathan Druart 2023-11-24 07:09:25 UTC
I could have guessed...
Comment 11 Jonathan Druart 2023-11-24 07:10:15 UTC
Created attachment 159225 [details] [review]
Bug 33244: Do not show lists in OPAC if OpacPublic is disabled

This patch updates markup and CSS in the OPAC header so that the Cart
and Lists controls don't appear when OpacPublic is disabled.

The patch also moves the "Your cookies" link into the group of links
with "Log in to your account" and "Search history." This makes the
template logic a little simpler and also seems like a more logical
grouping.

To test, apply the patch and enable the CookieContent system preference.

- View the OPAC with the OpacPublic system preference enabled.
- If necessary, click the "Accept all cookies" button.
- The "Your cookies" link should now appear in the header between the
  log-in link and the search history link.
- Test the page at various browser widths to confirm that the header
  still responds well to changes.
- Go back to system preferences in the staff client and change the
  OpacPublic system preference to "disabled."
- Return to the OPAC and refresh the page. You should be redirected to
  the login page, and the Cart and Lists buttons should no longer appear
  in the header.
- The links in the header for login and cookies should look correct at
  various browser widths.

Signed-off-by: David Cook <dcook@prosentient.com.au>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Jonathan Druart 2023-11-24 07:11:10 UTC
Ideally we should redesign C4::Auth::get_template_and_user to not pass the lists in this case. But... well... you know... :D
Comment 13 Katrin Fischer 2023-12-11 18:46:39 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 14 Fridolin Somers 2023-12-12 07:31:58 UTC
Pushed to 23.11.x for 23.11.01
Comment 15 Lucas Gass 2023-12-14 21:07:36 UTC
Merge conflicts with 23.05.x, no backport. If needed please rebase.
Comment 16 Michał 2024-03-06 07:40:39 UTC
Hello, this patch seems to have a side effect of the "Language" dropdown now being awkwardly placed on center-right (regardless of whether user is logged in our out). This is caused by the below CSS rules whose blame traces back to this patch, and removing them fixes the issue.

#members {
+    flex-grow: 1;
+     justify-content: flex-end;

Is this intentional change? I can only assume it's not, since I don't see any mention of this in the discussion.

Screenshots of the problem: https://imgur.com/a/hDZvn7B

It looks like this patch conditionally hides cart+lists. The CSS above then makes only language shown on the left, and the login on the right. But when things aren't hidden, it changes the original position.

Should I create a new bug about this?
Comment 17 David Cook 2024-03-06 22:31:52 UTC
(In reply to M from comment #16)
> Should I create a new bug about this?

Yep. Create a new bug and have it depend on this one.
Comment 18 Katrin Fischer 2024-03-17 17:00:05 UTC
(In reply to David Cook from comment #17)
> (In reply to M from comment #16)
> > Should I create a new bug about this?
> 
> Yep. Create a new bug and have it depend on this one.

Filed as: Bug 36337 - Hiding lists with OpacPublic breaks styling for language list