Bug 11009 - Do not allow reading records of anonymous patron
Summary: Do not allow reading records of anonymous patron
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-07 15:32 UTC by Fridolin Somers
Modified: 2016-08-16 09:07 UTC (History)
4 users (show)

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


Attachments
Bug 11009 - Do not allow reading records of anonymous patron (2.94 KB, patch)
2013-10-09 12:11 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 11009 - Add hardcoded limit to OPAC reading records (3.33 KB, patch)
2013-10-09 12:21 UTC, Fridolin Somers
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11009 - Do not allow reading records of anonymous patron (3.20 KB, patch)
2013-11-14 14:16 UTC, Owen Leonard
Details | Diff | Splinter Review
[PASSED-QA] Bug 11009 - Do not allow reading records of anonymous patron (3.23 KB, patch)
2013-11-16 20:16 UTC, Brendan Gallagher
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2013-10-07 15:32:07 UTC
When using an anonymous patron to anonymise issues history, this patron may have a huge number of old issues.
In this case, trying to display the reading history of this patron will perform a huge SQL query.
It is not usefull to have the reading history of this anonymous patron.
Comment 1 Fridolin Somers 2013-10-09 12:11:24 UTC Comment hidden (obsolete)
Comment 2 Fridolin Somers 2013-10-09 12:15:39 UTC
Each reading record will fetch from database all columns of biblio, biblioitems (including marcxml), items and issues or old_issues.
So if there are a great number of reading records, the page may create a heavy load on server.
Considering the page exists at OPAC, this may be used to overload the server.

So I created a follow-up that adds an hardcoded limit to reading records.
Comment 3 Fridolin Somers 2013-10-09 12:21:03 UTC Comment hidden (obsolete)
Comment 4 cedric.vita 2013-10-17 09:44:41 UTC
After applying the patch :

http://pro.test1.biblibre.com/cgi-bin/koha/members/member.pl

Software error:

Base class package "DBIx::Class::Schema" is empty.
    (Perhaps you need to 'use' the module which defines that package first,
    or make that module available in @INC (@INC contains: /home/koha/src /etc/perl /usr/local/lib/perl/5.10.1 /usr/local/share/perl/5.10.1 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.10 /usr/share/perl/5.10 /usr/local/lib/site_perl .).
 at /home/koha/src/Koha/Schema.pm line 10
BEGIN failed--compilation aborted at /home/koha/src/Koha/Schema.pm line 10.
Compilation failed in require at /home/koha/src/Koha/Database.pm line 38.
BEGIN failed--compilation aborted at /home/koha/src/Koha/Database.pm line 38.
Compilation failed in require at /home/koha/src/Koha/List/Patron.pm line 32.
BEGIN failed--compilation aborted at /home/koha/src/Koha/List/Patron.pm line 32.
Compilation failed in require at /home/koha/src/members/member.pl line 35.
BEGIN failed--compilation aborted at /home/koha/src/members/member.pl line 35.
Comment 5 Fridolin Somers 2013-10-17 13:01:19 UTC
(In reply to cedric.vita from comment #4)
> After applying the patch :
> Base class package "DBIx::Class::Schema" is empty.
This is because the DBIx::Class::Schema::Loarder, package libdbix-class-schema-loader-perl must be installed.
I have installed it on all sandboxes.
Please retest.
Comment 6 Brendan Gallagher 2013-10-21 18:59:31 UTC
I think it would be better to have a widget or choice in the staff side for the user to choose the limit of reading records to show for anonymous patron and then have the fall back to the hardcoded limit ? 500 : 50
Comment 7 Fridolin Somers 2013-10-22 07:40:12 UTC
Comment on attachment 21898 [details] [review]
Bug 11009 - Add hardcoded limit to OPAC reading records

Set has obsolete, should be in another bug
Comment 8 Fridolin Somers 2013-10-22 07:41:53 UTC
I removed follow-up (addind OPAC limit) so that this bug can focus on the anonymous patron problem.
This follow-up may be used for a new bug.
Comment 9 Fridolin Somers 2013-10-25 13:13:42 UTC
I've set this as major because it may really block the server.
Comment 10 Owen Leonard 2013-11-14 14:16:54 UTC
Created attachment 22931 [details] [review]
[SIGNED-OFF] Bug 11009 - Do not allow reading records of anonymous patron

When using an anonymous patron to anonymise issues history, this patron
may have a huge number of old issues. In this case, trying to display
the reading history of this patron will perform a huge SQL query.
It is not usefull to have the reading history of this anonymous patron.

This patch adds an alert instead of old issues when displaying reading
records of anonymous patron.

Test plan :
- Set syspref AnonymousPatron to 0.
- Select a borrower with old issues. For example 123.
- Look at its reading records page : members/readingrec.pl
=> Old issues are displayed in a datatable
- Set syspref AnonymousPatron with this borrower number. For example 123.
- Look at its reading records page
=> Old issues are not displayed and an alert is displayed
- Using SQL query, remove old issues of this borrower : DELETE FROM old_issues WHERE borrowernumber=123.
- Look at its reading records page
=> A message is displayed

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

This works as advertised and seems like a reasonable thing to do. I
suspect that someone will object... Perhaps that person will implement a
solution which uses an AJAX DataTable.
Comment 11 Brendan Gallagher 2013-11-16 20:16:42 UTC
Created attachment 22977 [details] [review]
[PASSED-QA] Bug 11009 - Do not allow reading records of anonymous patron

When using an anonymous patron to anonymise issues history, this patron
may have a huge number of old issues. In this case, trying to display
the reading history of this patron will perform a huge SQL query.
It is not usefull to have the reading history of this anonymous patron.

This patch adds an alert instead of old issues when displaying reading
records of anonymous patron.

Test plan :
- Set syspref AnonymousPatron to 0.
- Select a borrower with old issues. For example 123.
- Look at its reading records page : members/readingrec.pl
=> Old issues are displayed in a datatable
- Set syspref AnonymousPatron with this borrower number. For example 123.
- Look at its reading records page
=> Old issues are not displayed and an alert is displayed
- Using SQL query, remove old issues of this borrower : DELETE FROM old_issues WHERE borrowernumber=123.
- Look at its reading records page
=> A message is displayed

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

This works as advertised and seems like a reasonable thing to do. I
suspect that someone will object... Perhaps that person will implement a
solution which uses an AJAX DataTable.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 12 Galen Charlton 2013-12-10 20:23:50 UTC
This reminds me of the old joke:

"Doctor, it hurts when I do this!"
"Well, stop doing it then!"

Anyway, pushed to master -- though I would not be entirely surprised if somebody will end up wanting to be able to display a list of anonymized loans.  Thanks, Fridolin!
Comment 13 Fridolin Somers 2013-12-24 11:15:36 UTC
Patches pushed to 3.14.x, will be in 3.14.2.