Bug 18882

Summary: Add location code to statistics table for checkouts and renewals
Product: Koha Reporter: Kyle M Hall <kyle>
Component: CirculationAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, kyle.m.hall, m.de.rooy, mtompset
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 18882 - Add location code to statistics table for checkouts and renewals
Bug 18882 [QA Followup] - Ensure stats with no or undef location are set to NULL in db
Bug 18882 - Add location code to statistics table for checkouts and renewals
Bug 18882 [QA Followup] - Ensure stats with no or undef location are set to NULL in db
Bug 18882 - Add location code to statistics table for checkouts and renewals
Bug 18882 [QA Followup] - Ensure stats with no or undef location are set to NULL in db

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 Mark 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 Mark 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 Mark 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!