Bug 5199

Summary: Renewals do not add branchcode to statistics table
Product: Koha Reporter: Ian Walls <koha.sekjal>
Component: CirculationAssignee: Chris Cormack <chris>
Status: CLOSED FIXED QA Contact: Bugs List <koha-bugs>
Severity: enhancement    
Priority: PATCH-Sent (DO NOT USE) CC: chris, gmcharlt, jwagner, katrin.fischer, nengard
Version: 3.6   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Proposed Patch
Follow up patch

Description Ian Walls 2010-09-03 16:09:42 UTC
The AddRenewal() function of Circulation.pm is defined to take $branch as the third parameter, but most calls to this function, including OPAC renewal and Self Checkout, only take the first two, $borrowernumber and $itemnumber.  This leaves the statistics table full of NULL values in the $branch field for renewal statistics.

The $branch value passed should be the branch of the currently logged in user.  This would be the current branch selected in the staff client, the patron's home branch in the OPAC, and the homebranch of the Self Checkout user for SCO.
Comment 1 Owen Leonard 2010-09-07 12:53:32 UTC
This problem has plagued us (and our statistics preparation) for a long time. I don't know how SCO determines the user's branch--does it credit the patron's home branch with the circulation rather than the location at which the checkout is being made?  My preference for crediting the branch for online renewals would be to give the circ statistic to the branch which originated the checkout rather than to the patron's home branch.  In my multi-library system the patron's home branch doesn't necessarily have much to do with the libraries they actually use.
Comment 2 Nicole C. Engard 2010-09-07 12:57:20 UTC
I haven't tested this - but does your SCO staff user have a branch associated with them? I would think this is the branch that should be credited - just like when the staff log in their branch is credited with the check outs.  This of course means you need a SCO user for each branch (which is also a good idea).
Comment 3 Ian Walls 2010-09-10 18:50:46 UTC
For SCO, the homebranch of the SCO staff account should be used, since that can be customized by the library on a per-SCO-terminal basis.

For opac renewals, there seem to be a few options:

-Credit the library that originated the circ
-Credit the patron's home branch
-Credit the library that owns the item
-Use CircControlBranch to determine which of the above three to use
-Create a new system preference just like CircControlBranch, but for OPAC renewal

Other options include:
-Use a code like "OPAC" (would break if there were an 'OPAC' branchcode)
-Define a new statistic type "Self Renewal"
-Leave it as null

Thoughts?
Comment 4 Owen Leonard 2010-09-10 18:57:04 UTC
> -Use a code like "OPAC" (would break if there were an 'OPAC' branchcode)

This creates difficulties if you're designing reports around your branch codes, i.e. creating a table of circ stats with one branch in each column.

> -Define a new statistic type "Self Renewal"

I don't think this helps the situation. We're still left with wanting to be able to aggregate circulation stats for each branch. At best this makes explicit what is now only implied by a null value.

My vote is definitely to credit the library which originated the checkout.
Comment 5 Nicole C. Engard 2010-09-10 21:36:21 UTC
I'm with crediting the library that initiated the checkout - since that's how it's done at the circ desk.  What I'm wondering is is there any other way to know that it was done at the self checkout terminal?  Can that be done by querying against the self checkout user that is logged into the terminal? If not then there should be a way to get numbers of how many people used the self checkout versus how many items were checked out from the circ desk.
Comment 6 Nicole C. Engard 2011-04-25 14:07:22 UTC
Just want to give this bug a little bump, since it's effecting statistics reports for librarians.
Comment 7 Chris Cormack 2011-05-05 21:12:39 UTC
This was discussed today on irc and on the mailing list.

I think for opac renewals the best option is a sys pref where the behaviour can be chosen.

Will work on a patch
Comment 8 Chris Cormack 2011-05-05 23:16:43 UTC
I think I have it working now, changing the system preference means it records the appropriate branch

select * from statistics;
+---------------------+-----------+----------+--------+-------+-------+----------+------------+----------+----------------+--------------------+
| datetime            | branch    | proccode | value  | type  | other | usercode | itemnumber | itemtype | borrowernumber | associatedborrower |
+---------------------+-----------+----------+--------+-------+-------+----------+------------+----------+----------------+--------------------+
| 2011-05-06 11:12:59 | CPL       | NULL     | 5.0000 | issue |       | NULL     |          1 | BK       |             51 |               NULL |
| 2011-05-06 11:14:47 | CPL       | NULL     | 5.0000 | renew |       | NULL     |          1 | BK       |             51 |               NULL |
| 2011-05-06 11:15:48 | OPACRenew | NULL     | 5.0000 | renew |       | NULL     |          1 | BK       |             51 |               NULL |
+---------------------+-----------+----------+--------+-------+-------+----------+------------+----------+----------------+--------------------+

Patch to follow
Comment 9 Chris Cormack 2011-05-05 23:19:04 UTC
The patch does not change the self checkout part, only OPAC renewals.
Comment 10 Chris Cormack 2011-05-05 23:19:30 UTC
Created attachment 4045 [details] [review]
Proposed Patch
Comment 11 Nicole C. Engard 2011-05-06 15:09:42 UTC
Chris, this works beautifully unless you choose NULL after having it set to another value.  I had it set to OPACRenew and then I set it to items branch and they both worked then I set it to NULL and it put OPACRenew in the table.  We might need someone else to test to confirm my results.
Comment 12 Chris Cormack 2011-05-06 19:21:46 UTC
I see the problem, follow up patch sent
Comment 13 Chris Cormack 2011-05-06 19:23:11 UTC
Created attachment 4052 [details] [review]
Follow up patch
Comment 14 Ian Walls 2011-06-25 21:02:08 UTC
Verified that "OpacRenew" option should work, even if it's not a valid branchcode in the system.

Patch covers all languages syspref files, updatedatabase, preference editor, and code itself.  Followup corrects comparison error.  Marking as Passed QA
Comment 15 Chris Cormack 2011-07-02 20:53:42 UTC
Pushed to master, please test
Comment 16 Ian Walls 2011-08-12 20:32:04 UTC
While this patch fixes the issue going forward, it does not repair lost statistics for systems that ran with the bug in effect.  A separate cleanup script should be written so folks can restore their OPAC renewal stats, based on the current system preference's setting.
Comment 17 Jared Camins-Esakov 2012-05-23 12:11:37 UTC
This appears to be working. Closing.