Bug 21832 - Restore is_expired in ILS-DI GetPatronInfo service
Summary: Restore is_expired in ILS-DI GetPatronInfo service
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Web services (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on: 17578
Blocks:
  Show dependency treegraph
 
Reported: 2018-11-14 09:57 UTC by Fridolin Somers
Modified: 2020-01-06 20:14 UTC (History)
5 users (show)

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


Attachments
Bug 21832: Restore is_expired in ILS-DI GetPatronInfo service (1.76 KB, patch)
2018-11-14 10:53 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 21832: Restore is_expired in ILS-DI GetPatronInfo service (1.33 KB, patch)
2018-11-15 14:46 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 21832: Restore is_expired in ILS-DI GetPatronInfo service (1.38 KB, patch)
2019-02-08 19:41 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 21832: Restore is_expired in ILS-DI GetPatronInfo service (1.50 KB, patch)
2019-02-09 20:00 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21832: add unit test (1.24 KB, patch)
2019-02-12 14:54 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 21832: Restore is_expired in ILS-DI GetPatronInfo service (1.55 KB, patch)
2019-02-27 20:49 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21832: add unit test (1.29 KB, patch)
2019-02-27 20:49 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21832: Restore is_expired in ILS-DI GetPatronInfo service (1.63 KB, patch)
2019-03-05 13:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21832: add unit test (1.39 KB, patch)
2019-03-05 13:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21832: add is_expired to ILS-DI example (1.10 KB, patch)
2019-03-07 13:48 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 21832: add is_expired to ILS-DI example (1.17 KB, patch)
2019-03-16 00:32 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2018-11-14 09:57:21 UTC
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.
Comment 1 Fridolin Somers 2018-11-14 10:53:57 UTC
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>
Comment 2 Jonathan Druart 2018-11-14 14:43:50 UTC
call Koha::Patron->is_expired instead.
Comment 3 Fridolin Somers 2018-11-15 14:46:48 UTC
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>
Comment 4 Fridolin Somers 2018-11-15 14:49:46 UTC
(In reply to Jonathan Druart from comment #2)
> call Koha::Patron->is_expired instead.

Indeed, its much better
Comment 5 Maryse Simard 2019-02-08 19:41:54 UTC
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>
Comment 6 Katrin Fischer 2019-02-09 20:00:23 UTC
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>
Comment 7 Katrin Fischer 2019-02-09 20:01:47 UTC
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.
Comment 8 Nick Clemens 2019-02-11 13:28:04 UTC
Agreed, should be a simple enough test, please add a followup patch
Comment 9 Fridolin Somers 2019-02-12 14:54:17 UTC
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
Comment 10 Fridolin Somers 2019-02-12 14:55:06 UTC
(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/
Comment 11 Josef Moravec 2019-02-27 20:49:05 UTC
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>
Comment 12 Josef Moravec 2019-02-27 20:49:10 UTC
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>
Comment 13 Jonathan Druart 2019-03-05 13:10:07 UTC
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>
Comment 14 Jonathan Druart 2019-03-05 13:10:15 UTC
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
Comment 15 Jonathan Druart 2019-03-05 13:14:31 UTC
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.
Comment 16 Fridolin Somers 2019-03-06 15:20:25 UTC
But several portal use it already with this tag.
If it does no harm why not keeping it ?
Comment 17 Jonathan Druart 2019-03-06 15:45:55 UTC
(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? :)
Comment 18 Fridolin Somers 2019-03-07 08:58:08 UTC
(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.
Comment 19 Fridolin Somers 2019-03-07 13:48:14 UTC
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
Comment 20 Jonathan Druart 2019-03-16 00:32:50 UTC
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>
Comment 21 Nick Clemens 2019-03-16 13:54:16 UTC
Awesome work all!

Pushed to master for 19.05
Comment 22 Martin Renvoize 2019-03-19 11:05:14 UTC
Pushed to 18.11.x for 18.11.04
Comment 23 Lucas Gass 2019-03-26 23:55:49 UTC
backported to 18.05.x for 18.05.12
Comment 24 Fridolin Somers 2019-04-24 07:51:53 UTC
Pushed to 17.11.x for 17.11.18