Bug 21326

Summary: Add branch param to C4::SIP->checkout
Product: Koha Reporter: Benjamin Rokseth <benjamin.rokseth>
Component: SIP2Assignee: Benjamin Rokseth <benjamin.rokseth>
Status: Failed QA --- QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: aleisha, black23, colin.campbell, josef.moravec, martin.renvoize
Version: MainKeywords: Academy
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 21326: C4::SIP::ILS add missing branch param
Bug 21326: C4::SIP::ILS add missing branch param

Description Benjamin Rokseth 2018-09-10 13:28:48 UTC
For some reason branch is not included in the SIP::ILS::Transaction object, but rather fetched inside C4::Circulation using context. This bug adds branch to object so it can be passed in as param to ILS::Transaction->checkout

(this is already present in ILS::Transaction->checkin)
Comment 1 Benjamin Rokseth 2018-09-10 13:35:49 UTC
Created attachment 78541 [details] [review]
Bug 21326: C4::SIP::ILS add missing branch param

ILS::Transaction was missing branch param for unknown reason.
Since every transactions needs a branch, it makes sense to include it in
the object and allow as param.

This patch only adds the param to the object and ILS->checkout so it
can be used in submodules.
Comment 2 Aleisha Amohia 2018-12-19 22:51:30 UTC
Hi Benjamin,

Can you please provide a test plan for this? Maybe ensuring it doesn't break anything?

Thanks
Aleisha
Comment 3 Benjamin Rokseth 2018-12-20 00:01:53 UTC
Well there isn't much to test really, it's just an extra param that isnt used anywhere yet. Afraid I cannot help more, since we dont know if anyone else use it
Comment 4 Katrin Fischer 2018-12-20 21:33:51 UTC
Hi Benjamin, please don't forget to assign the bug to yourself when providing a patch!
Comment 5 Ethan 2019-01-15 21:01:12 UTC
Created attachment 84014 [details] [review]
Bug 21326: C4::SIP::ILS add missing branch param

ILS::Transaction was missing branch param for unknown reason.
Since every transactions needs a branch, it makes sense to include it in
the object and allow as param.

This patch only adds the param to the object and ILS->checkout so it
can be used in submodules.

Signed-off-by: Ethan Amohia <ethan.amohia@gmail.com>
Comment 6 Martin Renvoize (ashimema) 2019-02-08 15:47:39 UTC
I can't see that this achieves anything, but please correct me if I'm wrong.

For checkouts we use the AddIssue function and that doesn't expect a branch to be passed (and nor are we passing one to any other routines called from within C4::SIP::ILS::Transaction::Checkout->checkout_ok)

I can't see that this addition actually goes anywhere.. failing QA for now until I get some feedback/perspective.
Comment 7 Martin Renvoize (ashimema) 2019-02-08 15:50:23 UTC
Please clarify 'used in submodules'.. that might be a use case I'm entirely overlooking/not understanding.