Although SIP2 specifies the required AO field to be institution_id and it doesn't neccessarily map to a branch in Koha, this is probably the case for most. Allowing for AO to set the C4::Context->userenv->{branch} transaction-time would open up for a more flexible transaction scenario: - SIP2 connections could fetch from a pool of connections, so e.g. ten SIP2 workers could easily serve hundreds of clients - SIPConfig.xml would not need reconfiguration each time a new client is added Please advise if this is in collision with other SIP2 logic in Koha
Created attachment 35672 [details] [review] Bug 13666 - Allow SIP2 checkin/checkout to get branch from institution_id field AO This patch adds a policy flag in SIPConfig.xml that allows overriding branch on SIP2 checkout with the AO (institution id) field. Since SIP2 also uses C4::Context this patch has to modify C4::Context->userenv->{'branch'} Test plan: 1) Either use telnet or apply patch from bug #13159 to use sip command line client 2) Checkout book with SIP2 using a modified institution id (field AO) than the one defined in SIPConfig.xml for the account 3) Notice returned AO is still same as in SIPConfig account 4) Also notice checkout history in admin gives same 'checked out from' 5) Apply this patch and add 'allow_branch_override="true" to the policy of your ILS 6) Restart SIP server 7) Repeat 2) 8) Notice that 3) and 4) now reports the modified institution id
Hi! I would need more information about the use-cases. > - SIPConfig.xml would not need reconfiguration each time a new client is added I don't understand how this patch has to do with SIPConfig? There are no such followups/dependencies for that either. > - SIP2 connections could fetch from a pool of connections, so e.g. ten SIP2 workers could easily serve hundreds of clients SIP2 connections are already reused by the Net::Server?
Hi Olli! To recap: the SIP-server infers the transaction branch from the C4::Context->userenv->{'branch'}, but for some reason only on checkouts. This has to do with how SIP2 uses AddIssue and ModReserve to handle reserves and circulation. AddIssue uses patron context, while AddReturn takes branch as param. This is probably good enough for intranet use, but not for a service. Use-case that illustrate this enhancement: - library has need for a centralised handling of SIP connections, for handling e.g. 10 or, say, 50 concurrent connections. SIP clients range from SelfCheck automats to RFID clients and automated circulation systems. - this is best handled by a centralised service which maintains a pool of connections (users sippool1, sippool2, ...) - centralised service handles connections and could easily open and close connections at need, so we don't have to open connection, login, and close for each transaction, (poor performance) or worse, spawn 100+ workers with keep-alive connections - more dynamic solution than maintaining two sets of "SIP users", one in SIPConfig.xml and the other in Koha - centralised service could handle retry logic, logging, message rewriting, etc, and SIP server can focus on transactions. I believe any library with a hope to use SIP2 effectively would benefit from this patch.
Thanks for the input Benjamin! Shouldn't this patch be extended to set the AO-field for all SIP2 transactions where it is used? Then propagate the value to the transaction handlers? Now this special case is dealed with only for checkouts, which is not logical.
Actually, AO is respected in checkins already, as AddReturn takes $branch as param, whereas AddIssue does not. I suppose the only reason AO was not respected in checkout initially was due to the fact that AddIssue doesn't take $branch as param, and this probably was left to be solved in AddIssue at a later stage. The web interface requires user to specify branch before checkout, so C4::Context->userenv->{'branch'} is set which is the same as this patch does. For Renew, location has no effect, and hold takes its own param, pickup_location (BR), so that's the reason only checkout is handled in this patch, which, in my opinion, should have been handled by AO in the first place, as SIP is a transport protocol and should not have other dependencies than authentication. Also, the protocol specifies that AO is a required field in all transactions and maps to the library’s institution ID, so SIP clients should adher to this. Anyway, if AO is left empty, the transaction will fallback to user context, so the patch will not cause anyone trouble
Created attachment 40767 [details] [review] Bug 13666 - Allow SIP2 checkin/checkout to get branch from institution_id field AO
Created attachment 40768 [details] [review] Bug 13666 - Allow SIP2 checkin/checkout to get branch from institution_id field AO last patch failed tests due to fuzzy patching
Created attachment 40769 [details] [review] Bug 13666 - Allow SIP2 checkin/checkout to get branch from institution_id field AO giving up on patching /etc/SIPconfig.xml ... for now