Since Bug 17578 removed C4::Member::GetMemberDetails, the information "is_expired" disappeared from ILS-DI service GetPatronInfo. I propose to restaure in by adding the code that exisited in C4::Member::GetMemberDetails directly in C4::ILSDI::Services.
Created attachment 82323 [details] [review] Bug 21832: Restore is_expired in ILS-DI GetPatronInfo service Since Bug 17578 removed C4::Member::GetMemberDetails, the information "is_expired" disappeared from ILS-DI service GetPatronInfo. I propose to restore in by adding the code that exisited in C4::Member::GetMemberDetails directly in C4::ILSDI::Services. Test plan : 1) Enable ILS-DI webservice 2) Choose a patron not expired 3) Call web-service (replace X with patron's borrowernumber) : /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X 4) You see : <is_expired>0</is_expired> 5) Choose a patron expired 6) Call web-service (replace X with patron's borrowernumber) : /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X 7) You see : <is_expired>1</is_expired>
call Koha::Patron->is_expired instead.
Created attachment 82363 [details] [review] Bug 21832: Restore is_expired in ILS-DI GetPatronInfo service Since Bug 17578 removed C4::Member::GetMemberDetails, the information "is_expired" disappeared from ILS-DI service GetPatronInfo. I propose to restore in by adding the code that exisited in C4::Member::GetMemberDetails directly in C4::ILSDI::Services. Test plan : 1) Enable ILS-DI webservice 2) Choose a patron not expired 3) Call web-service (replace X with patron's borrowernumber) : /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X 4) You see : <is_expired>0</is_expired> 5) Choose a patron expired 6) Call web-service (replace X with patron's borrowernumber) : /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X 7) You see : <is_expired>1</is_expired>
(In reply to Jonathan Druart from comment #2) > call Koha::Patron->is_expired instead. Indeed, its much better
Created attachment 84894 [details] [review] Bug 21832: Restore is_expired in ILS-DI GetPatronInfo service Since Bug 17578 removed C4::Member::GetMemberDetails, the information "is_expired" disappeared from ILS-DI service GetPatronInfo. I propose to restore in by adding the code that exisited in C4::Member::GetMemberDetails directly in C4::ILSDI::Services. Test plan : 1) Enable ILS-DI webservice 2) Choose a patron not expired 3) Call web-service (replace X with patron's borrowernumber) : /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X 4) You see : <is_expired>0</is_expired> 5) Choose a patron expired 6) Call web-service (replace X with patron's borrowernumber) : /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X 7) You see : <is_expired>1</is_expired> Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Created attachment 84909 [details] [review] Bug 21832: Restore is_expired in ILS-DI GetPatronInfo service Since Bug 17578 removed C4::Member::GetMemberDetails, the information "is_expired" disappeared from ILS-DI service GetPatronInfo. I propose to restore in by adding the code that exisited in C4::Member::GetMemberDetails directly in C4::ILSDI::Services. Test plan : 1) Enable ILS-DI webservice 2) Choose a patron not expired 3) Call web-service (replace X with patron's borrowernumber) : /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X 4) You see : <is_expired>0</is_expired> 5) Choose a patron expired 6) Call web-service (replace X with patron's borrowernumber) : /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X 7) You see : <is_expired>1</is_expired> Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
This works nicely and is an easy fix which is unlikely to have any side effects. It would probably still be nice to have a regression test to avoid future breakage. Leaving final decision to Nick.
Agreed, should be a simple enough test, please add a followup patch
Created attachment 85022 [details] [review] Bug 21832: add unit test Adds a check that is_expired exists in service GetPatronInfo. Test plan : Run : prove t/db_dependent/ILSDI_Services.t
(In reply to Nick Clemens from comment #8) > Agreed, should be a simple enough test, please add a followup patch Here it is. Viva unit tests \o/
Created attachment 85800 [details] [review] Bug 21832: Restore is_expired in ILS-DI GetPatronInfo service Since Bug 17578 removed C4::Member::GetMemberDetails, the information "is_expired" disappeared from ILS-DI service GetPatronInfo. I propose to restore in by adding the code that exisited in C4::Member::GetMemberDetails directly in C4::ILSDI::Services. Test plan : 1) Enable ILS-DI webservice 2) Choose a patron not expired 3) Call web-service (replace X with patron's borrowernumber) : /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X 4) You see : <is_expired>0</is_expired> 5) Choose a patron expired 6) Call web-service (replace X with patron's borrowernumber) : /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X 7) You see : <is_expired>1</is_expired> Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 85801 [details] [review] Bug 21832: add unit test Adds a check that is_expired exists in service GetPatronInfo. Test plan : Run : prove t/db_dependent/ILSDI_Services.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 86063 [details] [review] Bug 21832: Restore is_expired in ILS-DI GetPatronInfo service Since Bug 17578 removed C4::Member::GetMemberDetails, the information "is_expired" disappeared from ILS-DI service GetPatronInfo. I propose to restore in by adding the code that exisited in C4::Member::GetMemberDetails directly in C4::ILSDI::Services. Test plan : 1) Enable ILS-DI webservice 2) Choose a patron not expired 3) Call web-service (replace X with patron's borrowernumber) : /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X 4) You see : <is_expired>0</is_expired> 5) Choose a patron expired 6) Call web-service (replace X with patron's borrowernumber) : /cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=X 7) You see : <is_expired>1</is_expired> Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 86064 [details] [review] Bug 21832: add unit test Adds a check that is_expired exists in service GetPatronInfo. Test plan : Run : prove t/db_dependent/ILSDI_Services.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Amended patch: remove ref to bug number, we have git for that
one sec. I had (and still have) issues when I refactored C4 code called from the ILSDI module. The problem is: what is really expected and what was there by 'accident'? IMO is_expired was in the response by accident, proof of that: it is not in the "Example response" on http://catalogue.kohadev.org/cgi-bin/koha/ilsdi.pl?service=Describe&verb=GetPatronInfo Moreover we have dateexpiry, so client can guess if the patron's account has expired or not :) Swithing to In Discussion.
But several portal use it already with this tag. If it does no harm why not keeping it ?
(In reply to Fridolin SOMERS from comment #16) > But several portal use it already with this tag. > If it does no harm why not keeping it ? Maybe adding it to the doc? :)
(In reply to Jonathan Druart from comment #17) > (In reply to Fridolin SOMERS from comment #16) > > But several portal use it already with this tag. > > If it does no harm why not keeping it ? > > Maybe adding it to the doc? :) Very good idea, I'm on it.
Created attachment 86243 [details] [review] Bug 21832: add is_expired to ILS-DI example Test plan : 1) Apply patch 2) Enable ILS-DI 3) Go to <opac URL>/cgi-bin/koha/ilsdi.pl?service=Describe&verb=GetPatronInfo 4) Check you see is_expired in example response
Created attachment 86701 [details] [review] Bug 21832: add is_expired to ILS-DI example Test plan : 1) Apply patch 2) Enable ILS-DI 3) Go to <opac URL>/cgi-bin/koha/ilsdi.pl?service=Describe&verb=GetPatronInfo 4) Check you see is_expired in example response Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.04
backported to 18.05.x for 18.05.12
Pushed to 17.11.x for 17.11.18