Bug 28967 - Patrons with no checkouts report shows patrons from other libraries with IndependentBranches
Summary: Patrons with no checkouts report shows patrons from other libraries with Inde...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Shi Yao Wang
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-09-07 20:54 UTC by Caroline Cyr La Rose
Modified: 2023-12-28 20:44 UTC (History)
10 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:
22.11.00, 22.05.08, 21.11.15


Attachments
Bug 28967: Patrons with no checkouts report shows patrons from other libraries with IndependentBranches (2.23 KB, patch)
2022-01-26 21:27 UTC, The Minh Luong
Details | Diff | Splinter Review
Bug 28967: Patrons with no checkouts report shows patrons from other libraries with IndependentBranches (2.08 KB, patch)
2022-01-27 15:09 UTC, The Minh Luong
Details | Diff | Splinter Review
Bug 28967: Patrons with no checkouts report shows patrons from other libraries with IndependentBranches (2.03 KB, patch)
2022-01-27 15:13 UTC, The Minh Luong
Details | Diff | Splinter Review
Bug 28967: Patrons with no checkouts report shows patrons from other libraries with IndependentBranches (2.07 KB, patch)
2022-09-25 18:39 UTC, David Nind
Details | Diff | Splinter Review
Bug 28967: Patrons with no checkouts report shows patrons from other libraries with IndependentBranches (2.13 KB, patch)
2022-10-16 14:58 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Caroline Cyr La Rose 2021-09-07 20:54:58 UTC
When IndependentBranches is activated and a non-superlibrarian user looks at the "Patrons with no checkouts" report, it shows patrons from other libraries.

To recreate:
1) Activate IndependentBranches, IndependentBranchesPatronModifications and IndependentBranchesTransfers
2) Create a staff user with limited permissions (NOT a superlibrarian), including reports permissions. Here are mine as an example
  - circulate (all)
  - catalogue
  - borrowers
    - delete_borrowers
    - edit_borrowers
  - reserveforothers (all)
  - reports (all)
3) Create (or make sure you already have) a patron in another branch (if using an existing patron, make sure they have no checkouts or checkout history)
4) Log in as your limited staff patron
5) Go to Reports > Inactive > Patrons who haven't checked out
6) Don't put in any limitations and click Submit
-- The patron from the other library appears in the list
7) Try to search for that other patron in the patron module
-- The patron from the other library is inaccessible

The current behaviour should stay the same for superlibrarians, but I don't think non-superlibrarians should see other libraries' patrons.
Comment 1 The Minh Luong 2022-01-26 21:27:08 UTC
Created attachment 129837 [details] [review]
Bug 28967: Patrons with no checkouts report shows patrons from other libraries with IndependentBranches

When IndependentBranches is activated and a non-superlibrarian user looks at the "Patrons with no checkouts" report, it shows patrons from other libraries.

To recreate:
1) Activate IndependentBranches, IndependentBranchesPatronModifications and IndependentBranchesTransfers
2) Create a staff user with limited permissions (NOT a superlibrarian), including reports permissions. Here are mine as an example
  - circulate (all)
  - catalogue
  - borrowers
    - delete_borrowers
    - edit_borrowers
  - reserveforothers (all)
  - reports (all)
3) Create (or make sure you already have) a patron in another branch (if using an existing patron, make sure they have no checkouts or checkout history)
4) Log in as your limited staff patron
5) Go to Reports > Inactive > Patrons who haven't checked out
6) Don't put in any limitations and click Submit
-- The patron from the other library appears in the list
7) Try to search for that other patron in the patron module
-- The patron from the other library is inaccessible
8) Apply the patch
9) Notice that the limited patron can only see patrons from his own
library
Comment 2 The Minh Luong 2022-01-27 15:09:00 UTC
Created attachment 129894 [details] [review]
Bug 28967: Patrons with no checkouts report shows patrons from other libraries with IndependentBranches

When IndependentBranches is activated and a non-superlibrarian user looks at the "Patrons with no checkouts" report, it shows patrons from other libraries.

To recreate:
1) Activate IndependentBranches, IndependentBranchesPatronModifications and IndependentBranchesTransfers
2) Create a staff user with limited permissions (NOT a superlibrarian), including reports permissions. Here are mine as an example
  - circulate (all)
  - catalogue
  - borrowers
    - delete_borrowers
    - edit_borrowers
  - reserveforothers (all)
  - reports (all)
3) Create (or make sure you already have) a patron in another branch (if using an existing patron, make sure they have no checkouts or checkout history)
4) Log in as your limited staff patron
5) Go to Reports > Inactive > Patrons who haven't checked out
6) Don't put in any limitations and click Submit
-- The patron from the other library appears in the list
7) Try to search for that other patron in the patron module
-- The patron from the other library is inaccessible
8) Apply the patch
9) Notice that the limited patron can only see patrons from his own
library
Comment 3 The Minh Luong 2022-01-27 15:13:21 UTC
Created attachment 129895 [details] [review]
Bug 28967: Patrons with no checkouts report shows patrons from other libraries with IndependentBranches

When IndependentBranches is activated and a non-superlibrarian user looks at the "Patrons with no checkouts" report, it shows patrons from other libraries.

To recreate:
1) Activate IndependentBranches, IndependentBranchesPatronModifications and IndependentBranchesTransfers
2) Create a staff user with limited permissions (NOT a superlibrarian), including reports permissions. Here are mine as an example
  - circulate (all)
  - catalogue
  - borrowers
    - delete_borrowers
    - edit_borrowers
  - reserveforothers (all)
  - reports (all)
3) Create (or make sure you already have) a patron in another branch (if using an existing patron, make sure they have no checkouts or checkout history)
4) Log in as your limited staff patron
5) Go to Reports > Inactive > Patrons who haven't checked out
6) Don't put in any limitations and click Submit
-- The patron from the other library appears in the list
7) Try to search for that other patron in the patron module
-- The patron from the other library is inaccessible
8) Apply the patch
9) Notice that the limited patron can only see patrons from his own
library
Comment 4 Axelle Aix-Marseille Université 2022-09-08 13:37:55 UTC
I tried this patch today on ByWater sandbox. I created a staff user with limited permissions and I logged in as this patron.

It failed at step 5. When I tried to go to Reports > Inactive > Patrons who haven't checked out, I had this message :

"CGI::Compile::ROOT::kohadevbox_koha_reports_borrowers_out_2epl::calculate(): DBI Exception: DBD::mysql::st execute failed: 'koha_axelle.borrowers.surname' isn't in GROUP BY at /kohadevbox/koha/reports/borrowers_out.pl line 60
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77"
Comment 5 Shi Yao Wang 2022-09-09 13:49:17 UTC
(In reply to Axelle Aix-Marseille Université from comment #4)
> I tried this patch today on ByWater sandbox. I created a staff user with
> limited permissions and I logged in as this patron.
> 
> It failed at step 5. When I tried to go to Reports > Inactive > Patrons who
> haven't checked out, I had this message :
> 
> "CGI::Compile::ROOT::kohadevbox_koha_reports_borrowers_out_2epl::calculate():
> DBI Exception: DBD::mysql::st execute failed:
> 'koha_axelle.borrowers.surname' isn't in GROUP BY at
> /kohadevbox/koha/reports/borrowers_out.pl line 60
>  at /usr/share/perl5/DBIx/Class/Exception.pm line 77"

I tried going to Reports > Inactive > Patrons who haven't checked out as the default user (un:koha/pw:koha) on the sandbox without applying the patch and ran into the same errors.
It seems to be an issue with the sandbox because it works well on my own installation.

Putting back to Needs Signoff for others to test.
Comment 6 Katrin Fischer 2022-09-11 13:04:36 UTC
"CGI::Compile::ROOT::kohadevbox_koha_reports_borrowers_out_2epl::calculate():
> > DBI Exception: DBD::mysql::st execute failed:
> > 'koha_axelle.borrowers.surname' isn't in GROUP BY at
> > /kohadevbox/koha/reports/borrowers_out.pl line 60
> >  at /usr/share/perl5/DBIx/Class/Exception.pm line 77"

The isn't in GROUP BY usually is an issue if you are using Koha with the strict SQL settings. Development installations and therefore sandboxes default to strict, to help catch the issues from being introduced in the first place and to make us fix them. 

Someone testing locally here might need to change their koha-conf.xml:

<strict_sql_modes>1</strict_sql_modes>

See: Bug 17258 - [OMNIBUS] MySQL 5.7

The compatibility problem with the report should best be reported separately.
Comment 7 Victor Grousset/tuxayo 2022-09-18 01:43:58 UTC
> It seems to be an issue with the sandbox because it works well on my own installation.


> Development installations and therefore sandboxes default to strict, to help catch the issues from being introduced in the first place and to make us fix them. 


So we can open a ticket about strict mode and the same test plan until step 5?
Is that what the issue is?
Comment 8 Katrin Fischer 2022-09-18 15:57:29 UTC
(In reply to Victor Grousset/tuxayo from comment #7)
> > It seems to be an issue with the sandbox because it works well on my own installation.
> 
> 
> > Development installations and therefore sandboxes default to strict, to help catch the issues from being introduced in the first place and to make us fix them. 
> 
> 
> So we can open a ticket about strict mode and the same test plan until step
> 5?
> Is that what the issue is?

I'd say so
Comment 9 Victor Grousset/tuxayo 2022-09-19 00:55:49 UTC
Thanks. So here we have bug 31578
Comment 10 David Nind 2022-09-25 18:39:03 UTC
Created attachment 140973 [details] [review]
Bug 28967: Patrons with no checkouts report shows patrons from other libraries with IndependentBranches

When IndependentBranches is activated and a non-superlibrarian user looks at the "Patrons with no checkouts" report, it shows patrons from other libraries.

To recreate:
1) Activate IndependentBranches, IndependentBranchesPatronModifications and IndependentBranchesTransfers
2) Create a staff user with limited permissions (NOT a superlibrarian), including reports permissions. Here are mine as an example
  - circulate (all)
  - catalogue
  - borrowers
    - delete_borrowers
    - edit_borrowers
  - reserveforothers (all)
  - reports (all)
3) Create (or make sure you already have) a patron in another branch (if using an existing patron, make sure they have no checkouts or checkout history)
4) Log in as your limited staff patron
5) Go to Reports > Inactive > Patrons who haven't checked out
6) Don't put in any limitations and click Submit
-- The patron from the other library appears in the list
7) Try to search for that other patron in the patron module
-- The patron from the other library is inaccessible
8) Apply the patch
9) Notice that the limited patron can only see patrons from his own
library

Signed-off-by: David Nind <david@davidnind.com>
Comment 11 David Nind 2022-09-25 18:40:35 UTC
Changed assignee to The Minh Luong - please update if not correct.
Comment 12 David Nind 2022-09-25 18:48:52 UTC
Testing notes (using koha-testing-docker):

- Turned off SQL strict mode so the 'Patrons with no checkouts' report will work (bug 31578 was previously logged to fix this) (edit /etc/koha/sites/kohadev/koha-conf.xml and changed <strict_sql_modes>0</strict_sql_modes>)

- Used patron Henry Acevedo (Midway) to test (changed permissions as per step 2 of the test plan, changed password so can log in).

- Using the sample data, Midway has 18 patrons - before applying the patch all patrons in the system are shown on the report, after applying the patch 18 patrons are shown and cannot see details for patrons not from Midway.
Comment 13 Caroline Cyr La Rose 2022-09-27 21:10:35 UTC
(In reply to David Nind from comment #11)
> Changed assignee to The Minh Luong - please update if not correct.

The Minh was an intern we had a couple of semesters ago... I changed it to Shi Yao, our part-time developer who currently works on bz's
Comment 14 David Nind 2022-09-27 21:21:52 UTC
(In reply to Caroline Cyr La Rose from comment #13)

Thanks Caroline!
Comment 15 Katrin Fischer 2022-10-16 14:57:00 UTC
This does exactly what the bug description says, but it leaves us with some other cases that are not taken into account. 

We do have a more complex system for visibility of patron data now, using permissions and library groups. I am not sure how that could be moved into the report, maybe by creating a list of allowed branchcodes to limit on? Filtering the result list before display? Filed: bug 31838.

Also: if you have access to reports... can you be stopped from seeing data you shouldn't? Guided and SQL reports cannot be limited to my knowledge.

I am inclined to pass QA on this one anyway as it's also a matter of convenience. If you have independent branches, you might only want to take care of your own users.
Comment 16 Katrin Fischer 2022-10-16 14:58:10 UTC
Created attachment 141945 [details] [review]
Bug 28967: Patrons with no checkouts report shows patrons from other libraries with IndependentBranches

When IndependentBranches is activated and a non-superlibrarian user looks at the "Patrons with no checkouts" report, it shows patrons from other libraries.

To recreate:
1) Activate IndependentBranches, IndependentBranchesPatronModifications and IndependentBranchesTransfers
2) Create a staff user with limited permissions (NOT a superlibrarian), including reports permissions. Here are mine as an example
  - circulate (all)
  - catalogue
  - borrowers
    - delete_borrowers
    - edit_borrowers
  - reserveforothers (all)
  - reports (all)
3) Create (or make sure you already have) a patron in another branch (if using an existing patron, make sure they have no checkouts or checkout history)
4) Log in as your limited staff patron
5) Go to Reports > Inactive > Patrons who haven't checked out
6) Don't put in any limitations and click Submit
-- The patron from the other library appears in the list
7) Try to search for that other patron in the patron module
-- The patron from the other library is inaccessible
8) Apply the patch
9) Notice that the limited patron can only see patrons from his own
library

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Tomás Cohen Arazi 2022-10-17 11:28:49 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 18 Lucas Gass 2022-12-05 20:24:46 UTC
Backported to 22.05.x for upcoming 22.05.08
Comment 19 Arthur Suzuki 2022-12-16 13:27:44 UTC
applied to 21.11 for 21.11.15
Comment 20 wainuiwitikapark 2023-03-15 00:56:29 UTC
Not backported to 21.05.x