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.
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
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
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
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"
(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.
"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.
> 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?
(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
Thanks. So here we have bug 31578
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>
Changed assignee to The Minh Luong - please update if not correct.
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.
(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
(In reply to Caroline Cyr La Rose from comment #13) Thanks Caroline!
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.
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>
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for upcoming 22.05.08
applied to 21.11 for 21.11.15
Not backported to 21.05.x