Bug 24377 - Record branch in statistics for auto-renewal
Summary: Record branch in statistics for auto-renewal
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: 19.05
Hardware: All All
: P5 - low major with 20 votes (vote)
Assignee: Nick Clemens
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 22600
Blocks:
  Show dependency treegraph
 
Reported: 2020-01-08 16:20 UTC by Andrew Fuerste-Henry
Modified: 2020-02-06 20:02 UTC (History)
6 users (show)

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


Attachments
Bug 24377: Unit test (1.91 KB, patch)
2020-01-16 13:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24377: Record passed branch if renewing via cron (1.32 KB, patch)
2020-01-16 13:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24377: Unit test (1.97 KB, patch)
2020-01-16 13:46 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 24377: Record passed branch if renewing via cron (1.39 KB, patch)
2020-01-16 13:46 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 24377: Record passed branch if renewing via cron (1.43 KB, patch)
2020-01-16 15:53 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 24377: Unit test (2.04 KB, patch)
2020-01-30 15:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24377: Record passed branch if renewing via cron (1.50 KB, patch)
2020-01-30 15:05 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2020-01-08 16:20:02 UTC
As of 19.05, statistics.branch is null when recording a renewal performed by the auto-renew cron. Prior to 19.05, a branch was recorded (though I'm not clear if that was the branch whose rules were used or the branch from the issues table). It looks like this was changed by bug 22600.
Comment 1 Ed Veal 2020-01-13 21:00:20 UTC
This causes significant statistical differences in circulation numbers post upgrade. I think this is a regression and should be addressed.
Comment 2 Nick Clemens 2020-01-16 13:13:58 UTC
Created attachment 97438 [details] [review]
Bug 24377: Unit test
Comment 3 Nick Clemens 2020-01-16 13:14:01 UTC
Created attachment 97439 [details] [review]
Bug 24377: Record passed branch if renewing via cron

To test:
1 - Checkout an item with auto renewal set and eligible for auto renew
2 - Set no renewal before in the circ rules to 99 (something greater than the checkout period)
3 - perl misc/cronjobs/automatic_renewals.pl
4 - Check the statistics table, note the branch for renewal is null
    SELECT * FROM statistics WHERE itemnumber={itemnumber} AND type='RENEWAL'
5 - Apply patch
6 - Repeat 1-4
7 - Note the branch is set to the issuing branch
Comment 4 ByWater Sandboxes 2020-01-16 13:46:54 UTC
Created attachment 97441 [details] [review]
Bug 24377: Unit test

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 5 ByWater Sandboxes 2020-01-16 13:46:56 UTC
Created attachment 97442 [details] [review]
Bug 24377: Record passed branch if renewing via cron

To test:
1 - Checkout an item with auto renewal set and eligible for auto renew
2 - Set no renewal before in the circ rules to 99 (something greater than the checkout period)
3 - perl misc/cronjobs/automatic_renewals.pl
4 - Check the statistics table, note the branch for renewal is null
    SELECT * FROM statistics WHERE itemnumber={itemnumber} AND type='RENEWAL'
5 - Apply patch
6 - Repeat 1-4
7 - Note the branch is set to the issuing branch

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 6 ByWater Sandboxes 2020-01-16 15:53:23 UTC
Created attachment 97453 [details] [review]
Bug 24377: Record passed branch if renewing via cron

To test:
1 - Checkout an item with auto renewal set and eligible for auto renew
2 - Set no renewal before in the circ rules to 99 (something greater than the checkout period)
3 - perl misc/cronjobs/automatic_renewals.pl
4 - Check the statistics table, note the branch for renewal is null
    SELECT * FROM statistics WHERE itemnumber={itemnumber} AND type='RENEWAL'
5 - Apply patch
6 - Repeat 1-4
7 - Note the branch is set to the issuing branch

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
Comment 7 Nick Clemens 2020-01-29 19:53:59 UTC
Bumping to major, this causes data loss for statistics
Comment 8 Martin Renvoize 2020-01-30 15:05:09 UTC
Created attachment 98167 [details] [review]
Bug 24377: Unit test

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2020-01-30 15:05:12 UTC
Created attachment 98168 [details] [review]
Bug 24377: Record passed branch if renewing via cron

To test:
1 - Checkout an item with auto renewal set and eligible for auto renew
2 - Set no renewal before in the circ rules to 99 (something greater than the checkout period)
3 - perl misc/cronjobs/automatic_renewals.pl
4 - Check the statistics table, note the branch for renewal is null
    SELECT * FROM statistics WHERE itemnumber={itemnumber} AND type='RENEWAL'
5 - Apply patch
6 - Repeat 1-4
7 - Note the branch is set to the issuing branch

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2020-01-30 15:05:47 UTC
Yikes, that is not a side effect I had imagined.. sorry guys. Thanks for the quick fix Nick.. passing QA
Comment 11 Martin Renvoize 2020-01-30 15:46:00 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 12 Joy Nelson 2020-01-31 00:48:42 UTC
Pushed to 19.11.x branch for 19.11.03
Comment 13 Lucas Gass 2020-02-06 20:02:21 UTC
backported to 19.05.x for 19.05.08