Bug 20934 - Biblio checkout history shows only current checkout
Summary: Biblio checkout history shows only current checkout
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low critical
Assignee: Jonathan Druart
QA Contact: Josef Moravec
URL:
Keywords:
Depends on: 18403 20899
Blocks:
  Show dependency treegraph
 
Reported: 2018-06-13 15:15 UTC by Josef Moravec
Modified: 2019-10-14 19:57 UTC (History)
7 users (show)

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


Attachments
Bug 20934: Fix display of old checkouts in the checkout history page (4.68 KB, patch)
2018-06-13 16:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20934: Fix display of old checkouts in the checkout history page (4.74 KB, patch)
2018-06-15 18:57 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 20934: Fix display of old checkouts in the checkout history page (4.79 KB, patch)
2018-06-18 15:53 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20934: (QA follow-up) Fix pod in Koha::Old::Checkout (968 bytes, patch)
2018-06-18 15:53 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20934: (QA follow-up) Fix showing number of records in checkout history (1.33 KB, patch)
2018-06-18 15:53 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Josef Moravec 2018-06-13 15:15:23 UTC
To reproduce:

1) Checkout an item to a patron

2) Go to the biblio record for this item and show checkouts history - you see the current checkout

3) Check in the item back

4) Repeat 2), you will see there is not any checkout in checkouts history for this biblio
Comment 1 Jonathan Druart 2018-06-13 16:34:59 UTC
Created attachment 76043 [details] [review]
Bug 20934: Fix display of old checkouts in the checkout history page

Again a regression caused by
  commit fa54100dffe092e606f79b15692eedaf78f42e45
  Bug 18403: Use patron-title.inc when hidepatronname is used [SPECIFIC for issuehistory]

GetBiblioIssues does a union all with issues and old_issues, so we
should old_issues as well.
To make the join on the items table we need to define the item and
patron methods. For consistency the relationships have been redefined
(item instead of itemnumber, borrower instead of borrowernumber) in the
DBIx::Class definition.

This is not perfect but I think the best way to provide an easy to
backport patch.

It highlights that we need improvements in this area.
Comment 2 Maryse Simard 2018-06-15 18:57:16 UTC
Created attachment 76104 [details] [review]
Bug 20934: Fix display of old checkouts in the checkout history page

Again a regression caused by
  commit fa54100dffe092e606f79b15692eedaf78f42e45
  Bug 18403: Use patron-title.inc when hidepatronname is used [SPECIFIC for issuehistory]

GetBiblioIssues does a union all with issues and old_issues, so we
should old_issues as well.
To make the join on the items table we need to define the item and
patron methods. For consistency the relationships have been redefined
(item instead of itemnumber, borrower instead of borrowernumber) in the
DBIx::Class definition.

This is not perfect but I think the best way to provide an easy to
backport patch.

It highlights that we need improvements in this area.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
The patch works as expected.
Comment 3 Josef Moravec 2018-06-18 15:53:17 UTC
Created attachment 76141 [details] [review]
Bug 20934: Fix display of old checkouts in the checkout history page

Again a regression caused by
  commit fa54100dffe092e606f79b15692eedaf78f42e45
  Bug 18403: Use patron-title.inc when hidepatronname is used [SPECIFIC for issuehistory]

GetBiblioIssues does a union all with issues and old_issues, so we
should old_issues as well.
To make the join on the items table we need to define the item and
patron methods. For consistency the relationships have been redefined
(item instead of itemnumber, borrower instead of borrowernumber) in the
DBIx::Class definition.

This is not perfect but I think the best way to provide an easy to
backport patch.

It highlights that we need improvements in this area.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
The patch works as expected.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 4 Josef Moravec 2018-06-18 15:53:21 UTC
Created attachment 76142 [details] [review]
Bug 20934: (QA follow-up) Fix pod in Koha::Old::Checkout

Test plan:
Run qa tools - it shouldn't complain about pod levels

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 5 Josef Moravec 2018-06-18 15:53:24 UTC
Created attachment 76143 [details] [review]
Bug 20934: (QA follow-up) Fix showing number of records in checkout history

Test plan:
0) Do not apply this patch
1) Go to biblio checkout history page (issuehistory.pl)
2) You'll see something like "Checked out ARRAY(0xf830378) times"
3) Apply this patch
4) Now you should see number of checkout history records

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 6 Josef Moravec 2018-06-18 15:55:45 UTC
(In reply to Jonathan Druart from comment #1)
> 
> This is not perfect but I think the best way to provide an easy to
> backport patch.
> 
> It highlights that we need improvements in this area.

Maybe not perfect, but fixing the bug ;) Hope some thing will be fixed during merging issues and oldissues in future...
Comment 7 Nick Clemens (kidclamp) 2018-06-22 13:49:24 UTC
Awesome work all!

Pushed to master for 18.11.x
Comment 8 Adam Goforth 2018-07-16 16:33:21 UTC
The checkout history is used in a couple of workflows we have at my library, so it would be really nice if we didn't have to wait until upgrading to 18.11 to get this fix. Are there currently any plans to backport the fix to 18.05? If not, would it be helpful for me to create a patch for it?
Comment 9 Martin Renvoize (ashimema) 2018-07-17 06:40:03 UTC
Hmm, this was just an oversight on my part... Not sure how I missed it ﷐[U+1F617]﷑. I'll grab it shortly and reporeport back
Comment 10 Martin Renvoize (ashimema) 2018-07-18 12:36:30 UTC
Pushed to 18.05.x for 18.05.02
Comment 11 Fridolin Somers 2018-07-26 07:37:35 UTC
Depends on Bug 20899 not in 17.11.x