Bug 14945 - Add the ability to store the last patron to return an item
Summary: Add the ability to store the last patron to return an item
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
: 9011 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-10-02 17:26 UTC by Kyle M Hall
Modified: 2017-12-07 22:15 UTC (History)
21 users (show)

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


Attachments
Bug 14945 - Add the ability to store the last patron to return an item (20.58 KB, patch)
2015-10-02 17:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14945 - Add the ability to store the last patron to return an item (20.64 KB, patch)
2015-10-02 17:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14945 - Add the ability to store the last patron to return an item (18.69 KB, patch)
2015-10-02 17:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED OFF] Bug 14945 - Add the ability to store the last patron to return an item (18.75 KB, patch)
2015-10-02 18:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 14954 - Remove C4::Dates from holiday related files in folder tools (11.42 KB, patch)
2015-10-27 14:48 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 14945 - Add the ability to store the last patron to return an item (18.78 KB, patch)
2015-11-25 15:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14945 - Add the ability to store the last patron to return an item (18.84 KB, patch)
2015-12-07 17:10 UTC, Nicole C. Engard
Details | Diff | Splinter Review
Bug 14945 - Add the ability to store the last patron to return an item (18.86 KB, patch)
2015-12-08 20:29 UTC, Tom Misilo
Details | Diff | Splinter Review
Bug 14945 - Add the ability to store the last patron to return an item (18.86 KB, patch)
2015-12-08 20:30 UTC, Tom Misilo
Details | Diff | Splinter Review
Bug 14945 - Add the ability to store the last patron to return an item (18.93 KB, patch)
2015-12-09 10:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14945: Add 2 more tests for StoreLastBorrower (2.22 KB, patch)
2015-12-09 10:31 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 Kyle M Hall 2015-10-02 17:26:58 UTC
Currently if the AnonymousPatron system preference is in use, all patron data is
anonymized. Some libraries would like to be able to see the last patron who
returned out an item ( in case of damage ) but still keep all other patrons anonymized.
Comment 1 Kyle M Hall 2015-10-02 17:27:22 UTC
*** Bug 9011 has been marked as a duplicate of this bug. ***
Comment 2 Kyle M Hall 2015-10-02 17:52:35 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2015-10-02 17:56:02 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2015-10-02 17:58:06 UTC Comment hidden (obsolete)
Comment 5 Nick Clemens 2015-10-02 18:46:44 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2015-10-06 11:36:47 UTC
As bug 9011 raised some discussion about this feature, I am ccing the people involved in previous bug and would like to get another signoff on this one, please.
Comment 7 Kyle M Hall 2015-10-06 11:42:23 UTC
(In reply to Jonathan Druart from comment #6)
> As bug 9011 raised some discussion about this feature, I am ccing the people
> involved in previous bug and would like to get another signoff on this one,
> please.

I agree everyone involved on bug 9011 should be aware of the new patches ( which is why I marked the bug moved ). However, you have provided no justification for requiring a second signoff on this patch.

This version is the end product of all the discussion on bug 9011. This version uses a join table to track the last returner for an item. This solves the privacy issues of the first version, and the speed issues of the second version.

Anyone and everyone is free to fail qa on this patch due to concerns they would like addressed, but there is nothing to be gained from requiring a second signoff.
Comment 8 Katrin Fischer 2015-10-06 12:01:19 UTC
It gains us more eyes and should not be too hard to achieve given that there is a lot of active interest in thsi feature.
Comment 9 Mirko Tietgen 2015-10-27 14:48:15 UTC Comment hidden (obsolete)
Comment 10 Mirko Tietgen 2015-10-27 15:11:17 UTC
Comment on attachment 44075 [details] [review]
Bug 14954 - Remove C4::Dates from holiday related files in folder tools

Sorry, wrong bug (typo)
Comment 11 Nick Clemens 2015-11-18 23:16:13 UTC
Small conflict in the tests
Comment 12 Kyle M Hall 2015-11-25 15:49:16 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2015-12-02 10:15:56 UTC
I personally don't plan to QA this patch as long as we don't get another signoff from someone involved on the discussion of bug 9011.
Comment 14 HB-NEKLS 2015-12-02 21:19:39 UTC
I don't have the ability to test this right now. But based on the description, I think this may be a good solution.

I would be interested in having a date returned timestamp as well in the table, unless it's easily queried from another table. Items can get checked out over and over again by the same patron -- it may get difficult to distinguish which time was what to provide more info to the patron, plus the less tables you have to query, the easier the reports are to write and return.

What happens when the item is returned again? Does the previous entry for the same item get deleted?
Comment 15 Nicole C. Engard 2015-12-07 17:10:29 UTC Comment hidden (obsolete)
Comment 16 Tom Misilo 2015-12-08 20:29:56 UTC Comment hidden (obsolete)
Comment 17 Tom Misilo 2015-12-08 20:30:08 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2015-12-09 10:24:43 UTC
Created attachment 45522 [details] [review]
Bug 14945 - Add the ability to store the last patron to return an item

Currently if the AnonymousPatron system preference is in use, all patron
data is anonymized. Some libraries would like to be able to see the last
patron who returned out an item ( in case of damage ) but still keep all
other patrons anonymized.

* Add the table items_last_borrower ( id, itemnumber, borrowernumber )
* Add new system preference StoreLastBorrower
* If StoreLastBorrower is enabled, upon checkin have Koha insert into
  this table the patron who last returned this item. Replace existing
  row based on itemnumber if exists.
* If table has a row for a given item, link to the patron from the item
  details page.

Test plan:
1) Apply patch
2) Run updatedatabase.pl
3) Enable StoreLastBorrower
4) Issue an item to a patron and return said item
5) Issue the same item to a second patron, do not return it.
6) View moredetail.pl for the given bib, find the given item. There
   should be a new field in the history list 'Last returned by' with a link
   to the last patron to return the item.

Optionally, you can also verify this works even if patron issuing
history has been set to anonymize issues upon return.

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>

Signed-off-by: Jen DeMuth <JDeMuth@roseville.ca.us>
Signed-off-by: Tom Misilo <misilot@fit.edu>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 19 Jonathan Druart 2015-12-09 10:27:42 UTC
All works fine, thanks Kyle for your tenacity :)
Thanks for the additional signoffs too!

I have just noticed 1 weird behavior on testing:
If the last patron to return has required privacy, on the items detail page, the "Last borrower" and "Last returned by" will differ. I know that it's all the point of this patch, but the info are not obvious in this case.
Comment 20 Jonathan Druart 2015-12-09 10:31:02 UTC
Created attachment 45523 [details] [review]
Bug 14945: Add 2 more tests for StoreLastBorrower

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 21 Tom Misilo 2015-12-11 12:01:36 UTC
I was thinking about your comment Jonathan, about "the "Last borrower" and "Last returned by" will differ."

Would it make sense sense to only display Last borrower, but show the new value that is stored and display with "Last returned by"? It removes the confusion that might happen after the data is anonymized?
Comment 22 Brendan Gallagher 2015-12-31 19:53:05 UTC
pushed to master - thanks