Bug 21326 - Add branch param to C4::SIP->checkout
Summary: Add branch param to C4::SIP->checkout
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Benjamin Rokseth
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2018-09-10 13:28 UTC by Benjamin Rokseth
Modified: 2019-02-08 15:50 UTC (History)
6 users (show)

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


Attachments
Bug 21326: C4::SIP::ILS add missing branch param (1.87 KB, patch)
2018-09-10 13:35 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 21326: C4::SIP::ILS add missing branch param (1.93 KB, patch)
2019-01-15 21:01 UTC, Ethan
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 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 2019-02-08 15:50:23 UTC
Please clarify 'used in submodules'.. that might be a use case I'm entirely overlooking/not understanding.