Summary: | SIP does not return checked out (charged) items on patron_information request | ||
---|---|---|---|
Product: | Koha | Reporter: | Josef Moravec <josef.moravec> |
Component: | SIP2 | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Josef Moravec <josef.moravec> |
Severity: | critical | ||
Priority: | P5 - low | CC: | alexbuckley, aniltiwari86, black23, colin.campbell, fridolin.somers, jonathan.druart, josef.moravec, magnus, martin.renvoize, nick |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Bug Depends on: | 19935 | ||
Bug Blocks: | |||
Attachments: |
Bug 21486: Re-added get_pending_issues function to C4/SIP/ILS/Patron.pm
Bug 21486: Fix 'AU' for SIP Signed off patch Bug 21486: Fix 'AU' for SIP Bug 21486: Only fetch the items' barcodes patch restricting to barcodes Bug 21486: Fix 'AU' for SIP Bug 21486: Only fetch the items' barcodes Bug 21486: Fix 'AU' for SIP Bug 21486: Only fetch the items' barcodes |
Description
Josef Moravec
2018-10-04 11:12:49 UTC
Created attachment 80251 [details] [review] Bug 21486: Re-added get_pending_issues function to C4/SIP/ILS/Patron.pm This allows the list of a borrower's checked out items to be returned by SIP. Test plan: 1. Make sure SIP is working with the commands: sudo koha-enable-sip <instancename> sudo koha-start-sip <instancename> 2. Check SIP is working with the command: telnet localhost 6001 3. Choose a patron with items checked out to them and set their username and password in the SIPconfig.xml file 4. From inside the Koha-shell from the Koha home directory run the command: misc/sip_cli_emulator.pl -a localhost -p 6001 -su <sip_user_username> -sp <sip_user_password> -l <sip_user_librarybranch> --patron <sip_user_cardnumber> -s " Y " -m patron_information This command is PATRON_INFORMATION, it should return information about the patron. The Y in the 3rd position of the quotes means return the list of the borrowers charged items (items issued to the borrower). 5. Notice the "AU" field is empty: e.g. READ: 64 00120181008 161428000100000001000000000002AOCPL|AA8765432|AEMika SMith|BLY|CC5|AU|PCB|PIY|AFGreetings from Koha. | i.e. There should be a barcode after the AU field as the borrower has a item issued to them 6. Apply patch 7. Restart memcached and plack with the commands: sudo service memcached restart sudo koha-plack --restart <instancename> 8. Stop SIP: sudo koha-stop-sip <instancename> 9. Then enable and start SIP again: sudo koha-enable-sip <instancename> sudo koha-start-sip <instancename> 10. Repeat step 4 and notice the AU field does have a item barcode value: e.g. READ: 64 00120181008 161859000100000001000000000002AOCPL|AA8765432|AEMika SMith|BLY|CC5|AUTEST7474747474|PCB|PIY|AFGreetings from Koha. | In this case the item barcode value for the AU field is: TEST7474747474 Sponsored-By: Brimbank Library, Australia Actually I want to make a few more changes to this solution for master. Surely you dont need to fetch the info from borrowers - its being called from the ILS::Patron object we should have that info already Created attachment 80522 [details] [review] Bug 21486: Fix 'AU' for SIP (In reply to Jonathan Druart from comment #4) > Created attachment 80522 [details] [review] [review] > Bug 21486: Fix 'AU' for SIP We should not re-add code we removed :) One possible fix would be to call unblessed_all_relateds, but we should be more specific. The problem here is that I have no idea which info is needed in $ilspatron{items}. Created attachment 80655 [details] [review] Signed off patch Signed off Jonathan's patch as it works for me in testing, Alex can you confirm? (In reply to Jonathan Druart from comment #5) > The problem here is that I have no idea which info is needed > in $ilspatron{items}. Colin, do you know the answer? (In reply to Jonathan Druart from comment #7) > (In reply to Jonathan Druart from comment #5) > > The problem here is that I have no idea which info is needed > > in $ilspatron{items}. > > Colin, do you know the answer? Yes (infact we collided) items is an array of barcodes of items issued to this patron (In reply to Colin Campbell from comment #8) > (In reply to Jonathan Druart from comment #7) > > (In reply to Jonathan Druart from comment #5) > > > The problem here is that I have no idea which info is needed > > > in $ilspatron{items}. > > > > Colin, do you know the answer? > > Yes (infact we collided) items is an array of barcodes of items issued to > this patron So we only need the barcodes, for other (if exist) calls as well? If so I can provide a better patch. (In reply to Jonathan Druart from comment #9) > (In reply to Colin Campbell from comment #8) > > (In reply to Jonathan Druart from comment #7) > > > (In reply to Jonathan Druart from comment #5) > > > > The problem here is that I have no idea which info is needed > > > > in $ilspatron{items}. > > > > > > Colin, do you know the answer? > > > > Yes (infact we collided) items is an array of barcodes of items issued to > > this patron > > So we only need the barcodes, for other (if exist) calls as well? > If so I can provide a better patch. Yes the logic in the client machines is to get a list of relevant barcodes for the queries and it can then issue item information queries of those. The exception (theres always one) is fine items as item_id does not give you relevant info (thats not specific to Koha, it a fairly useless option in most LMSs) Koha has some specific code there Created attachment 80729 [details] [review] Bug 21486: Fix 'AU' for SIP Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com> Created attachment 80730 [details] [review] Bug 21486: Only fetch the items' barcodes and avoid unecessary processing Created attachment 80976 [details] [review] patch restricting to barcodes Tested patch and works as advertised - signed off I've signed off Jonathan's two patches they restore the previous behaviour. Alex can you confirm this does all you envisioned and can go with it in place of your original patch? We should endeavour to get this into 18.11 or their is potential for working setups to be broken (In response to Colin Campbell in comment 14): Yes I can confirm Jonathan's patches do what I had envisaged and can replace my patch. Created attachment 81001 [details] [review] Bug 21486: Fix 'AU' for SIP Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com> Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Created attachment 81002 [details] [review] Bug 21486: Only fetch the items' barcodes and avoid unecessary processing Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com> Tested and found correct AU data is returned, and passes QA test tool Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Bonus - fixes a failure to process renew requests I've just spotted Upgrading severity as it impacts the last stable version. Created attachment 81050 [details] [review] Bug 21486: Fix 'AU' for SIP Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com> Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 81051 [details] [review] Bug 21486: Only fetch the items' barcodes and avoid unecessary processing Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com> Tested and found correct AU data is returned, and passes QA test tool Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> (In reply to Jonathan Druart from comment #19) > Upgrading severity as it impacts the last stable version. Yes, this is definitely needed in 18.05.x. Awesome work all! Pushed to master for 18.11 Pushed to 18.05.x for 18.05.06 Depends on Bug 19935 not in 17.11.x *** Bug 20493 has been marked as a duplicate of this bug. *** |