Bug 18882 - Add location code to statistics table for checkouts and renewals
Summary: Add location code to statistics table for checkouts and renewals
Status: Pushed to Master
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: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-29 14:56 UTC by Kyle M Hall
Modified: 2017-11-08 16:50 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 18882 - Add location code to statistics table for checkouts and renewals (8.72 KB, patch)
2017-06-29 15:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18882 [QA Followup] - Ensure stats with no or undef location are set to NULL in db (3.99 KB, patch)
2017-06-30 14:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18882 - Add location code to statistics table for checkouts and renewals (8.77 KB, patch)
2017-06-30 15:20 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 18882 [QA Followup] - Ensure stats with no or undef location are set to NULL in db (4.20 KB, patch)
2017-06-30 15:20 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 18882 - Add location code to statistics table for checkouts and renewals (8.87 KB, patch)
2017-11-03 10:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18882 [QA Followup] - Ensure stats with no or undef location are set to NULL in db (4.29 KB, patch)
2017-11-03 10:59 UTC, Marcel de Rooy
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 2017-06-29 14:56:20 UTC
Some libraries wish to track what the current location of items was at the time they were checked out. This will help libraries track which physical locations in the library patrons are more likely to check out a given book from.
Comment 1 Kyle M Hall 2017-06-29 15:19:01 UTC
Created attachment 64727 [details] [review]
Bug 18882 - Add location code to statistics table for checkouts and renewals

Some libraries wish to track what the current location of items was at the time they were checked out. This will help libraries track which physical locations in the library patrons are more likely to check out a given book from.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Check out an item that has a location set
4) Renew that item
5) View the checkout and renewal in the statistics table,
   verify each has the location column populated correctly
Comment 2 M. Tompsett 2017-06-29 17:08:39 UTC
The code puts '' if there is no location key. But if the location key is NULL, because I hadn't set my shelving location, there is NULL. Do we want NULL's? If so, I'll sign this off. If not, could you tweak that?
Comment 3 Kyle M Hall 2017-06-30 14:37:31 UTC
Created attachment 64756 [details] [review]
Bug 18882 [QA Followup] - Ensure stats with no or undef location are set to NULL in db
Comment 4 Kyle M Hall 2017-06-30 14:38:26 UTC
(In reply to M. Tompsett from comment #2)
> The code puts '' if there is no location key. But if the location key is
> NULL, because I hadn't set my shelving location, there is NULL. Do we want
> NULL's? If so, I'll sign this off. If not, could you tweak that?

I think we should always use NULL is the location is not passed or is set to undef. It seems like odd behavior to convert undef values to empty strings.
Comment 5 M. Tompsett 2017-06-30 15:20:52 UTC
Created attachment 64758 [details] [review]
Bug 18882 - Add location code to statistics table for checkouts and renewals

Some libraries wish to track what the current location of items was at the time they were checked out. This will help libraries track which physical locations in the library patrons are more likely to check out a given book from.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Check out an item that has a location set
4) Renew that item
5) View the checkout and renewal in the statistics table,
   verify each has the location column populated correctly

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 6 M. Tompsett 2017-06-30 15:20:59 UTC
Created attachment 64759 [details] [review]
Bug 18882 [QA Followup] - Ensure stats with no or undef location are set to NULL in db

Confirm that only NULL or a string are in the location field of the
statistics table. Test with two items, one with shelving location set,
and one without it set.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 7 Marcel de Rooy 2017-11-03 10:58:10 UTC
(In reply to Kyle M Hall from comment #4)
> (In reply to M. Tompsett from comment #2)
> > The code puts '' if there is no location key. But if the location key is
> > NULL, because I hadn't set my shelving location, there is NULL. Do we want
> > NULL's? If so, I'll sign this off. If not, could you tweak that?
> 
> I think we should always use NULL is the location is not passed or is set to
> undef. It seems like odd behavior to convert undef values to empty strings.

What we are doing now however, is storing undef only for location. Which is inconsistent. You change all lines in the second patch, only set undef for location.
No blocker, but could be improved.
I think that empty string would have been better now.
Comment 8 Marcel de Rooy 2017-11-03 10:59:04 UTC
Created attachment 68911 [details] [review]
Bug 18882 - Add location code to statistics table for checkouts and renewals

Some libraries wish to track what the current location of items was at the time they were checked out. This will help libraries track which physical locations in the library patrons are more likely to check out a given book from.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Check out an item that has a location set
4) Renew that item
5) View the checkout and renewal in the statistics table,
   verify each has the location column populated correctly

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2017-11-03 10:59:08 UTC
Created attachment 68912 [details] [review]
Bug 18882 [QA Followup] - Ensure stats with no or undef location are set to NULL in db

Confirm that only NULL or a string are in the location field of the
statistics table. Test with two items, one with shelving location set,
and one without it set.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Jonathan Druart 2017-11-08 16:50:30 UTC
Pushed to master for 17.11, thanks to everybody involved!