Bug 6335 - Branch not set consistently in all SIP transactions
Summary: Branch not set consistently in all SIP transactions
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: 3.10
Hardware: All All
: P3 normal (vote)
Assignee: Colin Campbell
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-12 14:03 UTC by Colin Campbell
Modified: 2013-12-05 20:01 UTC (History)
5 users (show)

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


Attachments
Proposed Patch (2.10 KB, patch)
2011-06-02 11:32 UTC, Colin Campbell
Details | Diff | Splinter Review
Bug 6335 Set branch in sip session (2.12 KB, patch)
2012-04-07 10:40 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Campbell 2011-05-12 14:03:52 UTC
The SIP config file allows an institution id to be set which is used in some transactions to give the branch id. However this is not always the case because it is not (unlike in an iteractive staff session) passed to the session, therefore some activities (e.g. checkout ) occur. They default to branch of the sip user. This can skew statistics and the location of returned items ( and transits). To behave consistently the session branch should be set in the session from the value in config.

Have a patch running in production, will submit after testing in a couple of other config environments
Comment 1 Colin Campbell 2011-06-02 11:32:22 UTC Comment hidden (obsolete)
Comment 2 Colin Campbell 2011-06-02 11:35:22 UTC
Patch sets branch in sip servers session. Otherwise the branch being applied in e.g. checkin was not the same as was being reported to the unit in the status return. In practice this led to incorrect stats and incorrect transit and hold capture behaviour
Comment 3 Liz Rea 2011-06-10 15:26:49 UTC
Colin, 

What is the best way to test this please?

LR
Comment 4 Colin Campbell 2011-06-10 17:26:26 UTC
For testing: if the checkin does not send a location (this is the case with 3M units) the rules applied will be those of the sip user  not of the machine location in its configuration.
 If the syspreference AutomaticItemReturn is set to 'Do' the behaviour is driven by a comparison of the item's home location with that of the checkin transaction. Without the fix items from branch A were not being transferred back from a unit in branch B. (Sip user home location being A) Implementing the fix meant the specified behaviour did occur
Comment 5 Liz Rea 2011-06-11 02:58:38 UTC
Colin, 

many thanks for the test plan. I'll check it when I get in on Monday.

LR
Comment 6 Chris Cormack 2011-08-28 23:28:37 UTC
Liz, did you have a chance to test this?
Comment 7 Paul Poulain 2011-10-24 11:38:23 UTC
Updating Version : This ENH will be for Koha 3.8
Comment 8 Paul Poulain 2011-10-25 15:06:01 UTC
Bug versionned for master. entries will be made against rel_3_8 once the patch has been applied (see thread about that on koha-devel yesterday)
Comment 9 Mickey Coalwell 2012-02-17 22:35:43 UTC
Ok thanks.I would have thought we had hundreds of gig available for our test server. Please excuse my ignorance on this matter. How much do you allocate to our test instance?
Comment 10 Liz Rea 2012-02-17 22:54:29 UTC
(In reply to comment #9)
> Ok thanks.I would have thought we had hundreds of gig available for our test
> server. Please excuse my ignorance on this matter. How much do you allocate to
> our test instance?

I think you meant to put this in the ticketing system for your support provider.
Comment 11 Chris Cormack 2012-04-07 10:40:03 UTC
Created attachment 9002 [details] [review]
Bug 6335 Set branch in sip session

The server returns its branch as the institution in the config file
However in transactions it will default to the homebranch of the
sip user which may not be consistent with that. Needs to set branch
in the session if there is a value in config otherwise statistics and
behaviour can be wrong (esp in discharge)

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 12 Marcel de Rooy 2012-05-10 07:27:14 UTC
QA Comment:
Did not test this patch myself, but base my comments on tests of Chris who signed off, Liz and Colin, as comments above show. 
Main thing this patch changes, is passing branchcode to Auth.pm via CGI object.
Looks good to me.
Passed QA
Comment 13 Paul Poulain 2012-05-11 09:17:37 UTC
QA comment : this patch also (silently) remove indentation and commented code in api_auth, but it to be more coding guideline-compliant, so I won't complain for that.

I also don't think it's an ENH, but a bugfix (please change the severity again if i'm wrong)

patch pushed
Comment 14 Chris Cormack 2012-05-11 20:55:13 UTC
This will be in 3.8.1