Bug 23916 - Issuer should be recorded and visible in patron circulation history
Summary: Issuer should be recorded and visible in patron circulation history
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Andrew Isherwood
QA Contact: Testopia
URL:
Keywords:
Depends on: 24321
Blocks:
  Show dependency treegraph
 
Reported: 2019-10-29 11:49 UTC by Andrew Isherwood
Modified: 2020-03-02 11:06 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 23916: Add issues.issuer column and FK (6.63 KB, patch)
2019-10-31 14:40 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Add "RecordIssuer" syspref (3.37 KB, patch)
2019-10-31 14:40 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Record and display item issuer (7.44 KB, patch)
2019-10-31 14:40 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Anonymise 'issuer' when required (1.04 KB, patch)
2019-10-31 14:40 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Add unit tests (5.49 KB, patch)
2019-10-31 14:40 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: (follow-up) Fix column name collision (1.93 KB, patch)
2019-11-18 12:33 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Add issues.issuer column and FK (6.68 KB, patch)
2019-11-18 13:48 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Add "RecordIssuer" syspref (3.42 KB, patch)
2019-11-18 13:48 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Record and display item issuer (7.49 KB, patch)
2019-11-18 13:48 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Anonymise 'issuer' when required (1.09 KB, patch)
2019-11-18 13:48 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Add unit tests (5.54 KB, patch)
2019-11-18 13:48 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: (follow-up) Fix column name collision (1.98 KB, patch)
2019-11-18 13:48 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Add issues.issuer column and FK (6.68 KB, patch)
2020-01-22 13:44 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Add "RecordIssuer" syspref (3.42 KB, patch)
2020-01-22 13:44 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Record and display item issuer (7.36 KB, patch)
2020-01-22 13:44 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Anonymise 'issuer' when required (1.09 KB, patch)
2020-01-22 13:44 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Add unit tests (5.39 KB, patch)
2020-01-22 13:44 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: (follow-up) Fix column name collision (1.98 KB, patch)
2020-01-22 13:44 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Add issues.issuer column and FK (6.73 KB, patch)
2020-02-07 20:17 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 23916: Add "RecordIssuer" syspref (3.46 KB, patch)
2020-02-07 20:17 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 23916: Record and display item issuer (7.38 KB, patch)
2020-02-07 20:17 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 23916: Anonymise 'issuer' when required (1.15 KB, patch)
2020-02-07 20:17 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 23916: Add unit tests (5.43 KB, patch)
2020-02-07 20:17 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 23916: (follow-up) Fix column name collision (2.03 KB, patch)
2020-02-07 20:17 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 24495: Reword change collection feature (3.14 KB, patch)
2020-02-07 21:13 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 23916: Add issues.issuer column and FK (6.74 KB, patch)
2020-03-02 11:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Add "RecordIssuer" syspref (3.47 KB, patch)
2020-03-02 11:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Record and display item issuer (6.74 KB, patch)
2020-03-02 11:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Anonymise 'issuer' when required (1.15 KB, patch)
2020-03-02 11:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: Add unit tests (5.45 KB, patch)
2020-03-02 11:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23916: (follow-up) Fix column name collision (2.04 KB, patch)
2020-03-02 11:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Isherwood 2019-10-29 11:49:22 UTC
Currently, when an item is issued, the issuer (member of staff, or self check or whatever) is not recorded directly with the issue. If the syspref is turned on then it is recorded in the associated action log. The issuer should be directly recorded with the issue (in a new "issues.issuer" column) and then displayed in the patron's circulation history.

As part of the DB upgrade, an attempt should also be made to retrospectively populate issuers using any available action logs.
Comment 1 Katrin Fischer 2019-10-29 21:25:31 UTC
With data privacy in mind... wouldn't recording the issue_id in statistics or action_logs maybe be enough? Or could this be made optional?
I think having basically the same data in multiple places might not be the right way, especially with no tools to clean/anonymize the data.
Comment 2 Andrew Isherwood 2019-10-30 10:50:57 UTC
Hi Katrin

To me it seems that the correct place to record this is with the issue, storing an issue ID in the action logs feels a bit like a hack. We have been known to purge action_logs at a customer's request during an upgrade. This would mean that relationship between the issue and they issuer would then be lost. Storing the issuer with the issue will be forever! :)

I'm not sure it raises any data privacy issues that do not equally apply to storing the borrower with the issue, which we already do, as well as in the action_logs.
Comment 3 Andrew Isherwood 2019-10-30 10:53:20 UTC
Development note: The data being stored here (issuer) should also be made visible in a biblio's "Checkout history" page
Comment 4 Katrin Fischer 2019-10-30 19:39:04 UTC
For the borrower-issue information we have a means of anonymizing via the GUI - so libraries can clean the data and it's not stored forever. The action_logs can also be cleaned by cronjob if the library chooses to. Cleaning/anonymizing statistics has been discussed in the GDPR meetings (https://wiki.koha-community.org/wiki/Gdpr)

I think we should make it a general habit not to add more person related information, without thinking about how to limit/anonymize/remove it if required, keeping the principle of data minimization in mind. (GDPR etc.)

Libraries here are rather critical about reading histories and the like already, so that's why I was asking for optional. :) I'd be really interested what the use case is. - Always good to learn about others needs/requirements.
Comment 5 Chris Cormack 2019-10-30 19:43:47 UTC
NZ is just going through privacy legislation rule, and will bring us much closer to the GDPR too, so I agree this should be optional

(The library will need to assume responsibility for a data breach, and report it and be liable for damages etc, so it's a choice they should conciously make)
Comment 6 Andrew Isherwood 2019-10-31 09:24:56 UTC
Hi both

Thanks for the input. I can put it behind a syspref. I can also ensure that Koha::Patrons::anonymise_issue_history anonymises the 'issuer' column, in addition to 'borrowernumber'. Would this address the concerns?

As far as a use case is concern, it's displaying the issuer, so staff can see an issue's issuer
Comment 7 Katrin Fischer 2019-10-31 12:39:20 UTC
> As far as a use case is concern, it's displaying the issuer, so staff can
> see an issue's issuer

Still wondering in which context the person who issued something is interesting :)
Comment 8 Andrew Isherwood 2019-10-31 14:40:03 UTC
Created attachment 94914 [details] [review]
Bug 23916: Add issues.issuer column and FK

This patch adds the issues.issuer column which is a foreign key to
borrowers.borrowernumber
Comment 9 Andrew Isherwood 2019-10-31 14:40:06 UTC
Created attachment 94915 [details] [review]
Bug 23916: Add "RecordIssuer" syspref

This patch adds the "RecordIssuer" syspref, which is disabled by default
Comment 10 Andrew Isherwood 2019-10-31 14:40:09 UTC
Created attachment 94916 [details] [review]
Bug 23916: Record and display item issuer

This patch adds the recording and display of the item issuer. This
behaviour is governed by the RecordIssuer syspref, if disabled (the
default), no recording or display of issuer will take place.
Comment 11 Andrew Isherwood 2019-10-31 14:40:12 UTC
Created attachment 94917 [details] [review]
Bug 23916: Anonymise 'issuer' when required

This patch adds the anonymisation of 'old_issues.issuer' alongside the
existing anonymisation of 'old_issues.borrowernumber'
Comment 12 Andrew Isherwood 2019-10-31 14:40:16 UTC
Created attachment 94918 [details] [review]
Bug 23916: Add unit tests

This patch adds unit tests for the new functions
Comment 13 Andrew Isherwood 2019-10-31 16:36:36 UTC
Test plan:
---------

1.  Ensure the new "RecordIssuer" is turned off (the default) setting.
2.  Issue an item to a patron
3.  Check the patron's circulation history
4.  TEST => Observe that the table does not contain a "Checked out by" column
5.  Check the item's checkout history
6.  TEST => Observe that the table does not contain a "Checked out by" column
7.  Turn the "RecordIssuer" syspref on
8.  Issue another item to the patron
9.  Check the patron's circulation history
10. TEST => Observe that the table now contains an "Checked out by" column
11. TEST => Observe that the column contains the details of the issuing user with a link to their patron record ONLY for the issue for which the syspref was turned on
11. Check the item's checkout history
12. TEST => Observe that the table now contains an "Checked out by" column
13. TEST => Observe that the column contains the details of the issuing user with a link to their patron record ONLY for the issue for which the syspref was turned on
Comment 14 Andrew Isherwood 2019-11-18 12:33:23 UTC
Created attachment 95503 [details] [review]
Bug 23916: (follow-up) Fix column name collision

Seems that joining 'borrowers' into the query run in
C4::Members::GetAllIssues caused a column name collision, which blatted
the retrieval of issues.branchcode. So we now are being a bit more
specific in what is coming from where in the query.
Comment 15 Andrew Isherwood 2019-11-18 13:48:06 UTC
Created attachment 95504 [details] [review]
Bug 23916: Add issues.issuer column and FK

This patch adds the issues.issuer column which is a foreign key to
borrowers.borrowernumber

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Comment 16 Andrew Isherwood 2019-11-18 13:48:09 UTC
Created attachment 95505 [details] [review]
Bug 23916: Add "RecordIssuer" syspref

This patch adds the "RecordIssuer" syspref, which is disabled by default

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Comment 17 Andrew Isherwood 2019-11-18 13:48:12 UTC
Created attachment 95506 [details] [review]
Bug 23916: Record and display item issuer

This patch adds the recording and display of the item issuer. This
behaviour is governed by the RecordIssuer syspref, if disabled (the
default), no recording or display of issuer will take place.

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Comment 18 Andrew Isherwood 2019-11-18 13:48:15 UTC
Created attachment 95507 [details] [review]
Bug 23916: Anonymise 'issuer' when required

This patch adds the anonymisation of 'old_issues.issuer' alongside the
existing anonymisation of 'old_issues.borrowernumber'

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Comment 19 Andrew Isherwood 2019-11-18 13:48:18 UTC
Created attachment 95508 [details] [review]
Bug 23916: Add unit tests

This patch adds unit tests for the new functions

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Comment 20 Andrew Isherwood 2019-11-18 13:48:22 UTC
Created attachment 95509 [details] [review]
Bug 23916: (follow-up) Fix column name collision

Seems that joining 'borrowers' into the query run in
C4::Members::GetAllIssues caused a column name collision, which blatted
the retrieval of issues.branchcode. So we now are being a bit more
specific in what is coming from where in the query.

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Comment 21 Andrew Isherwood 2020-01-22 13:44:07 UTC
Created attachment 97719 [details] [review]
Bug 23916: Add issues.issuer column and FK

This patch adds the issues.issuer column which is a foreign key to
borrowers.borrowernumber

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Comment 22 Andrew Isherwood 2020-01-22 13:44:10 UTC
Created attachment 97720 [details] [review]
Bug 23916: Add "RecordIssuer" syspref

This patch adds the "RecordIssuer" syspref, which is disabled by default

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Comment 23 Andrew Isherwood 2020-01-22 13:44:13 UTC
Created attachment 97721 [details] [review]
Bug 23916: Record and display item issuer

This patch adds the recording and display of the item issuer. This
behaviour is governed by the RecordIssuer syspref, if disabled (the
default), no recording or display of issuer will take place.

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Comment 24 Andrew Isherwood 2020-01-22 13:44:16 UTC
Created attachment 97722 [details] [review]
Bug 23916: Anonymise 'issuer' when required

This patch adds the anonymisation of 'old_issues.issuer' alongside the
existing anonymisation of 'old_issues.borrowernumber'

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Comment 25 Andrew Isherwood 2020-01-22 13:44:19 UTC
Created attachment 97723 [details] [review]
Bug 23916: Add unit tests

This patch adds unit tests for the new functions

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Comment 26 Andrew Isherwood 2020-01-22 13:44:22 UTC
Created attachment 97724 [details] [review]
Bug 23916: (follow-up) Fix column name collision

Seems that joining 'borrowers' into the query run in
C4::Members::GetAllIssues caused a column name collision, which blatted
the retrieval of issues.branchcode. So we now are being a bit more
specific in what is coming from where in the query.

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Comment 27 Andrew Isherwood 2020-01-22 13:44:37 UTC
Rebased
Comment 28 Bouzid Fergani 2020-02-07 20:17:18 UTC
Created attachment 98586 [details] [review]
Bug 23916: Add issues.issuer column and FK

This patch adds the issues.issuer column which is a foreign key to
borrowers.borrowernumber

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 29 Bouzid Fergani 2020-02-07 20:17:23 UTC
Created attachment 98587 [details] [review]
Bug 23916: Add "RecordIssuer" syspref

This patch adds the "RecordIssuer" syspref, which is disabled by default

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 30 Bouzid Fergani 2020-02-07 20:17:28 UTC
Created attachment 98588 [details] [review]
Bug 23916: Record and display item issuer

This patch adds the recording and display of the item issuer. This
behaviour is governed by the RecordIssuer syspref, if disabled (the
default), no recording or display of issuer will take place.

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 31 Bouzid Fergani 2020-02-07 20:17:32 UTC
Created attachment 98589 [details] [review]
Bug 23916: Anonymise 'issuer' when required

This patch adds the anonymisation of 'old_issues.issuer' alongside the
existing anonymisation of 'old_issues.borrowernumber'

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 32 Bouzid Fergani 2020-02-07 20:17:36 UTC
Created attachment 98590 [details] [review]
Bug 23916: Add unit tests

This patch adds unit tests for the new functions

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 33 Bouzid Fergani 2020-02-07 20:17:40 UTC
Created attachment 98591 [details] [review]
Bug 23916: (follow-up) Fix column name collision

Seems that joining 'borrowers' into the query run in
C4::Members::GetAllIssues caused a column name collision, which blatted
the retrieval of issues.branchcode. So we now are being a bit more
specific in what is coming from where in the query.

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 34 Bouzid Fergani 2020-02-07 21:13:58 UTC Comment hidden (obsolete)
Comment 35 Andrew Isherwood 2020-03-02 11:06:41 UTC
Created attachment 99886 [details] [review]
Bug 23916: Add issues.issuer column and FK

This patch adds the issues.issuer column which is a foreign key to
borrowers.borrowernumber

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 36 Andrew Isherwood 2020-03-02 11:06:44 UTC
Created attachment 99887 [details] [review]
Bug 23916: Add "RecordIssuer" syspref

This patch adds the "RecordIssuer" syspref, which is disabled by default

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 37 Andrew Isherwood 2020-03-02 11:06:47 UTC
Created attachment 99888 [details] [review]
Bug 23916: Record and display item issuer

This patch adds the recording and display of the item issuer. This
behaviour is governed by the RecordIssuer syspref, if disabled (the
default), no recording or display of issuer will take place.

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 38 Andrew Isherwood 2020-03-02 11:06:50 UTC
Created attachment 99889 [details] [review]
Bug 23916: Anonymise 'issuer' when required

This patch adds the anonymisation of 'old_issues.issuer' alongside the
existing anonymisation of 'old_issues.borrowernumber'

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 39 Andrew Isherwood 2020-03-02 11:06:53 UTC
Created attachment 99890 [details] [review]
Bug 23916: Add unit tests

This patch adds unit tests for the new functions

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 40 Andrew Isherwood 2020-03-02 11:06:56 UTC
Created attachment 99891 [details] [review]
Bug 23916: (follow-up) Fix column name collision

Seems that joining 'borrowers' into the query run in
C4::Members::GetAllIssues caused a column name collision, which blatted
the retrieval of issues.branchcode. So we now are being a bit more
specific in what is coming from where in the query.

Signed-off-by: Ben Veasey <B.T.Veasey@lboro.ac.uk>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>