Bug 17005 - Extend /checkouts route to list circulation history
Summary: Extend /checkouts route to list circulation history
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Josef Moravec
QA Contact:
URL: https://wiki.koha-community.org/wiki/...
Keywords:
Depends on: 13895 17003
Blocks: 16652 18407 18795 22613
  Show dependency treegraph
 
Reported: 2016-07-31 11:06 UTC by Jiri Kozlovsky
Modified: 2019-10-08 13:31 UTC (History)
10 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.11.00


Attachments
Bug 17005: REST API: add routes to list checkouts history (5.38 KB, patch)
2016-08-01 15:24 UTC, Jiri Kozlovsky
Details | Diff | Splinter Review
Bug 17005: REST API: add routes to list checkouts history (10.83 KB, patch)
2016-08-01 16:13 UTC, Jiri Kozlovsky
Details | Diff | Splinter Review
Bug 17005: REST API: add routes to list checkouts history (11.71 KB, patch)
2016-08-01 16:41 UTC, Jiri Kozlovsky
Details | Diff | Splinter Review
Bug 17005: REST API: add routes to list checkouts history (11.54 KB, patch)
2016-08-01 18:03 UTC, Jiri Kozlovsky
Details | Diff | Splinter Review
Bug 17005: Reword borrowers to patrons in Swagger tags for checkouts history (1.29 KB, patch)
2016-08-09 11:53 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17005: REST API: add routes to list checkouts history (25.76 KB, patch)
2016-10-20 14:37 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17005: REST API: add routes to list checkouts history (26.62 KB, patch)
2016-10-20 14:43 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17005 - REST API: add routes to list checkouts history - Followup1. Fix date_due timeformat in tests. (1.23 KB, patch)
2017-03-17 07:33 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 17005: Add checked_in checkouts to REST API response (4.15 KB, patch)
2019-04-01 08:25 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Test for checked_in parameter (2.87 KB, patch)
2019-04-01 08:25 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Add checked_in checkouts to REST API response (4.02 KB, patch)
2019-04-04 07:44 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Test for checked_in parameter (2.87 KB, patch)
2019-04-04 07:44 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Add plugin parameters to /checkouts endpoint (1.03 KB, patch)
2019-04-04 07:44 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Add checked_in checkouts to REST API response (4.00 KB, patch)
2019-04-04 10:49 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Test for checked_in parameter (2.87 KB, patch)
2019-04-04 10:49 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Add plugin parameters to /checkouts endpoint (1.01 KB, patch)
2019-04-04 10:49 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Add checked_in checkouts to REST API response (4.00 KB, patch)
2019-04-05 09:51 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Test for checked_in parameter (2.87 KB, patch)
2019-04-05 09:51 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Add plugin parameters to /checkouts endpoint (1.01 KB, patch)
2019-04-05 09:51 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Add checked_in checkouts to REST API response (4.09 KB, patch)
2019-04-05 11:57 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17005: Test for checked_in parameter (2.96 KB, patch)
2019-04-05 11:57 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17005: Add plugin parameters to /checkouts endpoint (1.11 KB, patch)
2019-04-05 11:57 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17005: Add checked_in checkouts to REST API response (4.09 KB, patch)
2019-04-18 07:31 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Test for checked_in parameter (2.96 KB, patch)
2019-04-18 07:31 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Add plugin parameters to /checkouts endpoint (1.11 KB, patch)
2019-04-18 07:31 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Add checked_in checkouts to REST API response (4.14 KB, patch)
2019-04-18 19:25 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17005: Test for checked_in parameter (3.01 KB, patch)
2019-04-18 19:25 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17005: Add plugin parameters to /checkouts endpoint (1.15 KB, patch)
2019-04-18 19:25 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17005: Add checked_in checkouts to REST API response (4.13 KB, patch)
2019-07-01 20:47 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Test for checked_in parameter (3.01 KB, patch)
2019-07-01 20:47 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Add plugin parameters to /checkouts endpoint (981 bytes, patch)
2019-07-01 20:47 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Add checked_in checkouts to REST API response (4.13 KB, patch)
2019-07-03 20:02 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Test for checked_in parameter (3.01 KB, patch)
2019-07-03 20:02 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Add plugin parameters to /checkouts endpoint (981 bytes, patch)
2019-07-03 20:02 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Fix bad rebase (924 bytes, patch)
2019-07-03 20:02 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17005: Add checked_in checkouts to REST API response (4.18 KB, patch)
2019-10-04 19:21 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17005: Test for checked_in parameter (3.06 KB, patch)
2019-10-04 19:21 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17005: Add plugin parameters to /checkouts endpoint (1.01 KB, patch)
2019-10-04 19:21 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17005: Fix bad rebase (979 bytes, patch)
2019-10-04 19:21 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jiri Kozlovsky 2016-07-31 11:06:01 UTC
Add API to list checkouts history for a user.

This is very useful for use with VuFind.
Comment 1 Jiri Kozlovsky 2016-08-01 15:24:10 UTC Comment hidden (obsolete)
Comment 2 Jiri Kozlovsky 2016-08-01 15:25:36 UTC
Since this bug depends on bug 13895, you should do "git bz apply 13895" first & then "git bz apply 17005"
Comment 3 Jiri Kozlovsky 2016-08-01 16:13:38 UTC Comment hidden (obsolete)
Comment 4 Jiri Kozlovsky 2016-08-01 16:41:45 UTC Comment hidden (obsolete)
Comment 5 Jiri Kozlovsky 2016-08-01 18:03:53 UTC
Created attachment 53858 [details] [review]
Bug 17005: REST API: add routes to list checkouts history

GET /checkouts/history                                 (get all old_issues' issue_id)
GET /checkouts/history?borrowernumber={borrowernumber} (get all borrower's old_issues' issue_id)
GET /checkouts/history/{checkout_id}                   (get old_issue)

+ unit tests in t/db_dependent/api/v1/checkoutshistory.t

Test plan:
1. Open a browser tab on Koha staff and log in (to create CGISESSID
   cookie). You should have permission circulate_remaining_permissions.
2. Go to http://yourlibrary/api/v1/checkouts/history?borrowernumber=XXX
   (replace XXX with a borrowernumber that has some checkouts history)
   and check you receive all the issues from patron's history
3. Go to http://yourlibrary/api/v1/checkouts/history/YYY (replace YYY
   with an existing checkout id) and check you receive correct data
4. Run unit tests in t/db_dependent/api/v1/checkoutshistory.t

This patch on top of previous changes adds the condition that the old
issue included in any response must have itemnumber. Those which does
not have itemnumber does not count as such old issue is completely
useless.
Comment 6 Frédéric Demians 2016-08-02 04:30:10 UTC
I've just tried this patch in order to test bug 17005 depending patch. For me
4 tests are failing: 23, 24, 26, 27. It seems that each time, I've got this
error:

  Can't call method "borrowernumber" on unblessed reference at
  <base>Koha/REST/V1/Checkout.pm line 81

It may be due to my testing environment. Can someone confirm that tests are
still working on master?
Comment 7 Jiri Kozlovsky 2016-08-02 08:19:47 UTC
(In reply to Frédéric Demians from comment #6)
> I've just tried this patch in order to test bug 17005 depending patch. For me
> 4 tests are failing: 23, 24, 26, 27. It seems that each time, I've got this
> error:
> 
>   Can't call method "borrowernumber" on unblessed reference at
>   <base>Koha/REST/V1/Checkout.pm line 81
> 
> It may be due to my testing environment. Can someone confirm that tests are
> still working on master?

Oh, did you apply the bug 13895 first before testing? It is based on that bug, which is not in master yet. Dependency is set already
Comment 8 Frédéric Demians 2016-08-02 09:11:17 UTC
(In reply to Jiri Kozlovsky from comment #7)

> Oh, did you apply the bug 13895 first before testing? It is based on that
> bug, which is not in master yet. Dependency is set already

Indeed, my comment was for bug 13895! I've mixed up my browser tabs. I will recopy my comment on bug 13895 thread, and restore your patch status to 'Needs Signoff'. Thks.
Comment 9 Lari Taskula 2016-08-05 14:44:37 UTC
Good work with the patch, Jiří!

I will sign-off, but first I would like to present some thoughts:
- Should we also allow user to access his own checkout history even if he does not have circulate_remaining_permissions? I think this would need to consider syspref 'opacreadinghistory'. Similar to Bug 13895.

- If GET /checkouts/history returns only issue_ids, should we come up with a way to GET multiple issue_id's in one request? Say a client wants to display all old checkouts for a patron on with detailed information for each checkout; He makes a GET to /checkouts/history?borrowernumber=123. 1000 issue_ids are returned. The question is, is it better to individually GET /checkouts/history/1 ... /checkouts/history/1000 or do one request GET /checkouts/history?issue_id=1+2+3+...+999+1000 ? Same like availability has in Bug 16826.
Comment 10 Lari Taskula 2016-08-09 11:53:22 UTC
Created attachment 54198 [details] [review]
Bug 17005: Reword borrowers to patrons in Swagger tags for checkouts history

Routes for checkouts history have tags called "borrowers". We should use "patrons"
instead in order not to have both (endpoints for patrons already add "patrons").

This patch changes the tags from borrowers to patrons in:
GET  /checkouts/history
GET  /checkouts/history/{checkout_id}
Comment 11 Lari Taskula 2016-10-20 14:37:44 UTC
Created attachment 56698 [details] [review]
Bug 17005: REST API: add routes to list checkouts history

GET /checkouts/history                                 (get all old_issues' issue_id)
GET /checkouts/history?borrowernumber={borrowernumber} (get all borrower's old_issues' issue_id)
GET /checkouts/history/{checkout_id}                   (get old_issue)

+ unit tests in t/db_dependent/api/v1/checkoutshistory.t

Test plan:
1. Open a browser tab on Koha staff and log in (to create CGISESSID
   cookie). You should have permission circulate_remaining_permissions.
2. Go to http://yourlibrary/api/v1/checkouts/history?borrowernumber=XXX
   (replace XXX with a borrowernumber that has some checkouts history)
   and check you receive all the issues from patron's history
3. Go to http://yourlibrary/api/v1/checkouts/history/YYY (replace YYY
   with an existing checkout id) and check you receive correct data
4. Run unit tests in t/db_dependent/api/v1/checkoutshistory.t

This patch on top of previous changes adds the condition that the old
issue included in any response must have itemnumber. Those which does
not have itemnumber does not count as such old issue is completely
useless.
Comment 12 Lari Taskula 2016-10-20 14:43:20 UTC
Created attachment 56699 [details] [review]
Bug 17005: REST API: add routes to list checkouts history

GET /checkouts/history                                 (get all old_issues' issue_id)
GET /checkouts/history?borrowernumber={borrowernumber} (get all borrower's old_issues' issue_id)
GET /checkouts/history/{checkout_id}                   (get old_issue)

+ unit tests in t/db_dependent/api/v1/checkoutshistory.t

Test plan:
1. Open a browser tab on Koha staff and log in (to create CGISESSID
   cookie). You should have permission circulate_remaining_permissions.
2. Go to http://yourlibrary/api/v1/checkouts/history?borrowernumber=XXX
   (replace XXX with a borrowernumber that has some checkouts history)
   and check you receive all the issues from patron's history
3. Go to http://yourlibrary/api/v1/checkouts/history/YYY (replace YYY
   with an existing checkout id) and check you receive correct data
4. Run unit tests in t/db_dependent/api/v1/checkoutshistory.t

This patch on top of previous changes adds the condition that the old
issue included in any response must have itemnumber. Those which does
not have itemnumber does not count as such old issue is completely
useless.
Comment 13 Lari Taskula 2016-10-20 14:43:56 UTC
Rebased on top of master and moved permissions into Swagger, and added a HTTP 401 and 403 test.
Comment 14 Josef Moravec 2016-11-08 22:20:59 UTC
Endpoint does work as expected, but the test checkoutshistory.t is broken and also koha-qa says:

 FAIL	Koha/REST/V1/Checkout.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   FAIL	  spelling
		 perfomance  ==> performance
Comment 15 Jiri Kozlovsky 2017-03-12 20:37:22 UTC
Someone please help me, I'm stuck in writing unit tests in kohadevbox.

This is what I got returned from sample API partons.t:

kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/api/v1/patrons.t 
t/db_dependent/api/v1/patrons.t .. DBD::mysql::st execute failed: Unknown column 'overdrive_auth_token' in 'field list' [for Statement "INSERT INTO `borrowers` ( `B_address`, `B_address2`, `B_city`, `B_country`, `B_email`, `B_phone`, `B_state`, `B_streetnumber`, `B_streettype`, `B_zipcode`, `address`, `address2`, `altcontactaddress1`, `altcontactaddress2`, `altcontactaddress3`, `altcontactcountry`, `altcontactfirstname`, `altcontactphone`, `altcontactstate`, `altcontactsurname`, `altcontactzipcode`, `borrowernotes`, `branchcode`, `cardnumber`, `categorycode`, `checkprevcheckout`, `city`, `contactfirstname`, `contactname`, `contactnote`, `contacttitle`, `country`, `dateenrolled`, `dateexpiry`, `dateofbirth`, `debarred`, `debarredcomment`, `email`, `emailpro`, `fax`, `firstname`, `flags`, `gonenoaddress`, `guarantorid`, `initials`, `lastseen`, `lost`, `mobile`, `opacnote`, `othernames`, `overdrive_auth_token`, `password`, `phone`, `phonepro`, `privacy`, `privacy_guarantor_checkouts`, `relationship`, `sex`, `sms_provider_id`, `smsalertnumber`, `sort1`, `sort2`, `state`, `streetnumber`, `streettype`, `surname`, `title`, `updated_on`, `userid`, `zipcode`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0='VjklbDmEmoWudPhpCWvz7rJAfEEmrCSjbnCf6S4v8DtGZjkCI7jaHoADwHWOGYysHLuBeMzYorwm3992o_UDF3YKUpGYgoPJ', 1='GZgOWb', 2='tSVrqQ8', 3='wk', 4='bNj', 5='BI2aSjf', 6='pTkX6zIVE', 7='BmMDX', 8='Df9lTEgMW770zsj6qMmgo90AcPmCZ2f5yxR2tHtd2W1', 9='NiPDa3Is5', 10='aMIrS', 11='o2fw2mw_gx', 12='QJ96zi10BKHjf45i7UI5qhWEwYcYH4iXBfH8pDeqlMV5xWg2eSyeXiBd0OIboV3mNVJffAKQ0u75wDpIY05H7Iqe3TIbHMUPQ5CoevT1eg87PlkSp6CxoZLKyEzRTNbZS88WKtrzDV6T3NJnn1yDEBfoQdM8zl7n9BYWS6oSacsGTXXJ1fFwiVuNN0GiiulwCC7nhzw5vCmF3eNNxVDj2LF92S22PbUi5pTEiClTLuTqa4zABSs2', 13='a9RpXJAOZTH81_8QUy0ufD0qF_dMHStD3yzzQIr9fcV4KCVRYT_GRsLp4O6iyMI2vHk2LARICmB3crQiA9HjJA4wM4LHfrKDfcUpRurK4R4sH4GS_LxvX', 14='w6f9XgdIm4fUDi1LAlxHlnv0k6bAWvnX_NA7OhYNeInF24CbCzLyOWcX0iyUWwYo1rjUOK4X5DC8zuqiWZdxLveQVOQWCTb0TwFP5bYXCvX0Y20eePvWShnkw3BVm8dnoo8pSvENIIBPsEcbj49uUtuxt_tgye45oX2vYGS75ICIWtjZB6fP8RN6e33SCtoIE4c0wiB68iG6Njs', 15='N', 16='L6xkGcRxTqabsrPIv296A_YUr_TYIn4k4U_p0ESh3iNPfxR2iAh2hVGHQLc8snQXsvjeKQ7Pca0gcp_k9DVhTPcXi3JvdErJ9UBI6SkvmYXxDI7RRI2rm2R4mOuMuf8PbcfAQdWjTbWFnCag', 17='xIn07_7bog', 18='t', 19='d25sJFVrJ6sqNW6o4jw0E76EZkC6w6zmX95Ixsa1yNqZ58zSrN63q1fqCpzTO4HDx0bGCxGXOXVpOTL9cvbaepJ6d_JjBDKUkGsJtfNd7bMlpAx0ypzNE9a3zmIQKw8_ZBeB4M0uk6RONs2t0Pe_MLGTTMP7', 20='Gm1Mgmb4dr5tTD30kXY0y', 21='F3wl', 22='Y6PWhl_UhS', 23='wkINA3tkWfTGo', 24='cR4nYYRnj', 25='ii9I', 26='xlZTwq8', 27='ThXks0vXrW', 28='jImAGNe', 29='YFQrtDRDCCQCLeYC7c_dErxY2Ol5F7InDZk3oEn41zMl2sN_AQxoNYZjqtOugdmSJzUx9HXmdutL45Js8ZUWvfelGTPvCfLdPDIZziX_hNBbVnmwHyXUIoZboIWusA7CZllwzA0TvC2zJ4gKx09v_p5Zu2_42msD0KskMdg4iFzCRl09e6IufqZvS9CJ4uRAA1x4hyUbdy', 30='Z_Fqh', 31='Oba', 32='2017-03-12', 33='2017-03-12', 34='2017-03-12', 35='2017-03-12', 36='Mwf1IB5_qAvR4Fm1Fg2JdqZ2YWofYwVcC5l4dwcnIit4fba2Yopjh5rxZh__Dqhz3yNiWRVzG0M90V2kVFZ0pNRVGpt6DqVqVuJpZrKz0PArDdo5Aept70lMR9mbRHLo6ydPMHVMJcf8NYSajFO1DtQ4JYaT_bRjtdj5Q6ftsOM2yZEME', 37='lHLdEMU', 38='szanHw_MI3q', 39='iXZmy', 40='hlQ0BFbYzW5', 41=0, 42=54, 43=1365868268, 44='uW', 45='2017-03-12 20:34:07', 46=108, 47='gUpy', 48='cfPPJnisWoT', 49='KJSvZZdrF', 50='qm8Xqtvkp', 51='CuaSVbZHRgPLRSM8o5kj', 52='RJJf2lN_m7y', 53='XgyQAVDu', 54=1483959990, 55=1, 56='DkqBG8ZnY3UdRnqDJhvuuW6nnxkFa7Ynp6le', 57='w', 58='4', 59='BpAvEd8VoHTlNcxMHobD', 60='y_7ohFg3sH2tV7LPRQHXZ2x39fdcflRNl_y9UFROZFwAg2stPmeolaYYEY8', 61='eqNIYiKKN0zqJWW_HXk3WZmTAiNjrdRFfiTRsli8X4hkhUkXxH1hcVj5gLV3dSIkX9uAp4l_pS89W63', 62='DWqrDgqkQW', 63='mKq', 64='z9apII7dgaGVu9', 65='Sqj', 66='WXReDMh', 67='2017-03-12 20:34:07', 68='zrG1UCYaWyq7Aq3eSUT8R6MW12jWL0', 69='IcdAqkr_KCQl_wJIErkLSP'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832.
DBIx::Class::Storage::DBI::_execute(): DBD::mysql::st execute failed: Unknown column 'overdrive_auth_token' in 'field list' [for Statement "INSERT INTO `borrowers` ( `B_address`, `B_address2`, `B_city`, `B_country`, `B_email`, `B_phone`, `B_state`, `B_streetnumber`, `B_streettype`, `B_zipcode`, `address`, `address2`, `altcontactaddress1`, `altcontactaddress2`, `altcontactaddress3`, `altcontactcountry`, `altcontactfirstname`, `altcontactphone`, `altcontactstate`, `altcontactsurname`, `altcontactzipcode`, `borrowernotes`, `branchcode`, `cardnumber`, `categorycode`, `checkprevcheckout`, `city`, `contactfirstname`, `contactname`, `contactnote`, `contacttitle`, `country`, `dateenrolled`, `dateexpiry`, `dateofbirth`, `debarred`, `debarredcomment`, `email`, `emailpro`, `fax`, `firstname`, `flags`, `gonenoaddress`, `guarantorid`, `initials`, `lastseen`, `lost`, `mobile`, `opacnote`, `othernames`, `overdrive_auth_token`, `password`, `phone`, `phonepro`, `privacy`, `privacy_guarantor_checkouts`, `relationship`, `sex`, `sms_provider_id`, `smsalertnumber`, `sort1`, `sort2`, `state`, `streetnumber`, `streettype`, `surname`, `title`, `updated_on`, `userid`, `zipcode`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0='VjklbDmEmoWudPhpCWvz7rJAfEEmrCSjbnCf6S4v8DtGZjkCI7jaHoADwHWOGYysHLuBeMzYorwm3992o_UDF3YKUpGYgoPJ', 1='GZgOWb', 2='tSVrqQ8', 3='wk', 4='bNj', 5='BI2aSjf', 6='pTkX6zIVE', 7='BmMDX', 8='Df9lTEgMW770zsj6qMmgo90AcPmCZ2f5yxR2tHtd2W1', 9='NiPDa3Is5', 10='aMIrS', 11='o2fw2mw_gx', 12='QJ96zi10BKHjf45i7UI5qhWEwYcYH4iXBfH8pDeqlMV5xWg2eSyeXiBd0OIboV3mNVJffAKQ0u75wDpIY05H7Iqe3TIbHMUPQ5CoevT1eg87PlkSp6CxoZLKyEzRTNbZS88WKtrzDV6T3NJnn1yDEBfoQdM8zl7n9BYWS6oSacsGTXXJ1fFwiVuNN0GiiulwCC7nhzw5vCmF3eNNxVDj2LF92S22PbUi5pTEiClTLuTqa4zABSs2', 13='a9RpXJAOZTH81_8QUy0ufD0qF_dMHStD3yzzQIr9fcV4KCVRYT_GRsLp4O6iyMI2vHk2LARICmB3crQiA9HjJA4wM4LHfrKDfcUpRurK4R4sH4GS_LxvX', 14='w6f9XgdIm4fUDi1LAlxHlnv0k6bAWvnX_NA7OhYNeInF24CbCzLyOWcX0iyUWwYo1rjUOK4X5DC8zuqiWZdxLveQVOQWCTb0TwFP5bYXCvX0Y20eePvWShnkw3BVm8dnoo8pSvENIIBPsEcbj49uUtuxt_tgye45oX2vYGS75ICIWtjZB6fP8RN6e33SCtoIE4c0wiB68iG6Njs', 15='N', 16='L6xkGcRxTqabsrPIv296A_YUr_TYIn4k4U_p0ESh3iNPfxR2iAh2hVGHQLc8snQXsvjeKQ7Pca0gcp_k9DVhTPcXi3JvdErJ9UBI6SkvmYXxDI7RRI2rm2R4mOuMuf8PbcfAQdWjTbWFnCag', 17='xIn07_7bog', 18='t', 19='d25sJFVrJ6sqNW6o4jw0E76EZkC6w6zmX95Ixsa1yNqZ58zSrN63q1fqCpzTO4HDx0bGCxGXOXVpOTL9cvbaepJ6d_JjBDKUkGsJtfNd7bMlpAx0ypzNE9a3zmIQKw8_ZBeB4M0uk6RONs2t0Pe_MLGTTMP7', 20='Gm1Mgmb4dr5tTD30kXY0y', 21='F3wl', 22='Y6PWhl_UhS', 23='wkINA3tkWfTGo', 24='cR4nYYRnj', 25='ii9I', 26='xlZTwq8', 27='ThXks0vXrW', 28='jImAGNe', 29='YFQrtDRDCCQCLeYC7c_dErxY2Ol5F7InDZk3oEn41zMl2sN_AQxoNYZjqtOugdmSJzUx9HXmdutL45Js8ZUWvfelGTPvCfLdPDIZziX_hNBbVnmwHyXUIoZboIWusA7CZllwzA0TvC2zJ4gKx09v_p5Zu2_42msD0KskMdg4iFzCRl09e6IufqZvS9CJ4uRAA1x4hyUbdy', 30='Z_Fqh', 31='Oba', 32='2017-03-12', 33='2017-03-12', 34='2017-03-12', 35='2017-03-12', 36='Mwf1IB5_qAvR4Fm1Fg2JdqZ2YWofYwVcC5l4dwcnIit4fba2Yopjh5rxZh__Dqhz3yNiWRVzG0M90V2kVFZ0pNRVGpt6DqVqVuJpZrKz0PArDdo5Aept70lMR9mbRHLo6ydPMHVMJcf8NYSajFO1DtQ4JYaT_bRjtdj5Q6ftsOM2yZEME', 37='lHLdEMU', 38='szanHw_MI3q', 39='iXZmy', 40='hlQ0BFbYzW5', 41=0, 42=54, 43=1365868268, 44='uW', 45='2017-03-12 20:34:07', 46=108, 47='gUpy', 48='cfPPJnisWoT', 49='KJSvZZdrF', 50='qm8Xqtvkp', 51='CuaSVbZHRgPLRSM8o5kj', 52='RJJf2lN_m7y', 53='XgyQAVDu', 54=1483959990, 55=1, 56='DkqBG8ZnY3UdRnqDJhvuuW6nnxkFa7Ynp6le', 57='w', 58='4', 59='BpAvEd8VoHTlNcxMHobD', 60='y_7ohFg3sH2tV7LPRQHXZ2x39fdcflRNl_y9UFROZFwAg2stPmeolaYYEY8', 61='eqNIYiKKN0zqJWW_HXk3WZmTAiNjrdRFfiTRsli8X4hkhUkXxH1hcVj5gLV3dSIkX9uAp4l_pS89W63', 62='DWqrDgqkQW', 63='mKq', 64='z9apII7dgaGVu9', 65='Sqj', 66='WXReDMh', 67='2017-03-12 20:34:07', 68='zrG1UCYaWyq7Aq3eSUT8R6MW12jWL0', 69='IcdAqkr_KCQl_wJIErkLSP'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832. at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 249
# Looks like your test exited with 255 before it could output anything.
t/db_dependent/api/v1/patrons.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 21/21 subtests 

Test Summary Report
-------------------
t/db_dependent/api/v1/patrons.t (Wstat: 65280 Tests: 0 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 21 tests but ran 0.
Files=1, Tests=0,  2 wallclock secs ( 0.02 usr  0.01 sys +  1.44 cusr  0.16 csys =  1.63 CPU)
Result: FAIL


I think it's obvious there is some problem with DB, but I have no idea what it could be about :(

It's also obvious I cannot write any unit tests for this API because they would eventually fail from similar reason so I wouldn't see if they pass or no.
Comment 16 Josef Moravec 2017-03-13 08:30:25 UTC
(In reply to Jiri Kozlovsky from comment #15)
> Someone please help me, I'm stuck in writing unit tests in kohadevbox.
>
> t/db_dependent/api/v1/patrons.t .. DBD::mysql::st execute failed: Unknown
> column 'overdrive_auth_token' in 'field list'
> 
> I think it's obvious there is some problem with DB, but I have no idea what
> it could be about :(
> 


You are right, looks like you do need to update your database schema
Comment 17 Jiri Kozlovsky 2017-03-13 09:39:40 UTC
(In reply to Josef Moravec from comment #16)
> (In reply to Jiri Kozlovsky from comment #15)
> > Someone please help me, I'm stuck in writing unit tests in kohadevbox.
> >
> > t/db_dependent/api/v1/patrons.t .. DBD::mysql::st execute failed: Unknown
> > column 'overdrive_auth_token' in 'field list'
> > 
> > I think it's obvious there is some problem with DB, but I have no idea what
> > it could be about :(
> > 
> 
> You are right, looks like you do need to update your database schema

I thought kohadevbox takes care of that?

Btw I'm running it with this command:

LOCAL_ANSIBLE=1 SYNC_REPO="/etc/koha/kohaclone" SKIP_WEBINSTALLER=1 CREATE_ADMIN_USER=1 vagrant up --provision
Comment 18 Josef Moravec 2017-03-13 09:47:32 UTC
(In reply to Jiri Kozlovsky from comment #17)
> (In reply to Josef Moravec from comment #16)
> > (In reply to Jiri Kozlovsky from comment #15)
> > > Someone please help me, I'm stuck in writing unit tests in kohadevbox.
> > >
> > > t/db_dependent/api/v1/patrons.t .. DBD::mysql::st execute failed: Unknown
> > > column 'overdrive_auth_token' in 'field list'
> > > 
> > > I think it's obvious there is some problem with DB, but I have no idea what
> > > it could be about :(
> > > 
> > 
> > You are right, looks like you do need to update your database schema
> 
> I thought kohadevbox takes care of that?
> 
> Btw I'm running it with this command:
> 
> LOCAL_ANSIBLE=1 SYNC_REPO="/etc/koha/kohaclone" SKIP_WEBINSTALLER=1
> CREATE_ADMIN_USER=1 vagrant up --provision

I don't think you'd like to run provision just for updating the database, there is quicker way:

cd kohaclone
sudo koha-shell kohadev
installer/data/mysql/updatedatabase.pl
Comment 19 Olli-Antti Kivilahti 2017-03-17 07:33:50 UTC
Created attachment 61197 [details] [review]
Bug 17005 - REST API: add routes to list checkouts history - Followup1. Fix date_due timeformat in tests.
Comment 20 Tomás Cohen Arazi 2017-11-17 14:06:33 UTC
This bug reproduces a design issue with our DB structure.

Checkouts and OldCheckouts are the same thing, they've just been splitted for some reason (which leads to several problems). That said, I think this functionality should be implemented in /checkouts. And there should be a checked_in boolean for 'OldCheckouts'.

Once this is actually implemented on the DB, it will be simpler. In the meantime, we can do it on the controller.
Comment 21 Josef Moravec 2019-04-01 08:25:34 UTC
Created attachment 87254 [details] [review]
Bug 17005: Add checked_in checkouts to REST API response

This patch implements parameter 'checked_in' on checkouts endpoint to
enable getting circulation history.

Test plan:
1) Apply the patch and restart plack
2) Use your favorite REST API tester and play with /checkouts endpoint:
    - use it without checked_in parameter
    - use checked_in=1 for getting all (not only current) checkouts
Comment 22 Josef Moravec 2019-04-01 08:25:38 UTC
Created attachment 87255 [details] [review]
Bug 17005: Test for checked_in parameter

Test plan:
prove t/db_dependent/api/v1/checkouts.t
Comment 23 Josef Moravec 2019-04-04 07:44:39 UTC
Created attachment 87383 [details] [review]
Bug 17005: Add checked_in checkouts to REST API response

This patch implements parameter 'checked_in' on checkouts endpoint to
enable getting circulation history.

Test plan:
1) Apply the patch and restart plack
2) Use your favorite REST API tester and play with /checkouts endpoint:
    - use it without checked_in parameter
    - use checked_in=1 for getting returned checkouts
Comment 24 Josef Moravec 2019-04-04 07:44:43 UTC
Created attachment 87384 [details] [review]
Bug 17005: Test for checked_in parameter

Test plan:
prove t/db_dependent/api/v1/checkouts.t
Comment 25 Josef Moravec 2019-04-04 07:44:46 UTC
Created attachment 87385 [details] [review]
Bug 17005: Add plugin parameters to /checkouts endpoint
Comment 26 Ere Maijala 2019-04-04 09:13:31 UTC
If you have paging parameters, the response should also include the total number of records. Otherwise it's difficult to build a pager.
Comment 27 Josef Moravec 2019-04-04 09:19:43 UTC
(In reply to Ere Maijala from comment #26)
> If you have paging parameters, the response should also include the total
> number of records. Otherwise it's difficult to build a pager.

According to Pagination plugin, there should be header X-Total-Count in response and Link headers with rel=prev|next|first|last

see bug 19196, I am not aware of any documentation for this, but it looks like we need some ;)
Comment 28 Ere Maijala 2019-04-04 09:25:25 UTC
Oh, right, sorry, I totally missed the headers!
Comment 29 Katrin Fischer 2019-04-04 09:46:31 UTC
> see bug 19196, I am not aware of any documentation for this, but it looks
> like we need some ;)

Did I hear 'documentation'? :) Should this be added to the wiki for devs?
Comment 30 Josef Moravec 2019-04-04 09:49:31 UTC
(In reply to Katrin Fischer from comment #29)
> > see bug 19196, I am not aware of any documentation for this, but it looks
> > like we need some ;)
> 
> Did I hear 'documentation'? :) Should this be added to the wiki for devs?

You hear right ;) Yes, I think it should be added to wiki. We need both, the guide for developers how to use this plugin (even it is relatively easy) and for api consumers what they could expect from this features.
Comment 31 Josef Moravec 2019-04-04 10:49:32 UTC
Created attachment 87405 [details] [review]
Bug 17005: Add checked_in checkouts to REST API response

This patch implements parameter 'checked_in' on checkouts endpoint to
enable getting circulation history.

Test plan:
1) Apply the patch and restart plack
2) Use your favorite REST API tester and play with /checkouts endpoint:
    - use it without checked_in parameter
    - use checked_in=1 for getting returned checkouts
Comment 32 Josef Moravec 2019-04-04 10:49:36 UTC
Created attachment 87406 [details] [review]
Bug 17005: Test for checked_in parameter

Test plan:
prove t/db_dependent/api/v1/checkouts.t
Comment 33 Josef Moravec 2019-04-04 10:49:40 UTC
Created attachment 87407 [details] [review]
Bug 17005: Add plugin parameters to /checkouts endpoint
Comment 34 Ere Maijala 2019-04-05 08:30:09 UTC
I bz-applied this including dependencies but there seems to be a conflict:

Applying: Bug 17005: Add checked_in checkouts to REST API response
Using index info to reconstruct a base tree...
M	Koha/REST/V1/Checkouts.pm
M	api/v1/swagger/paths/checkouts.json
Falling back to patching base and 3-way merge...
Auto-merging api/v1/swagger/paths/checkouts.json
Auto-merging Koha/REST/V1/Checkouts.pm
CONFLICT (content): Merge conflict in Koha/REST/V1/Checkouts.pm
Comment 35 Josef Moravec 2019-04-05 09:51:23 UTC
Created attachment 87442 [details] [review]
Bug 17005: Add checked_in checkouts to REST API response

This patch implements parameter 'checked_in' on checkouts endpoint to
enable getting circulation history.

Test plan:
1) Apply the patch and restart plack
2) Use your favorite REST API tester and play with /checkouts endpoint:
    - use it without checked_in parameter
    - use checked_in=1 for getting returned checkouts
Comment 36 Josef Moravec 2019-04-05 09:51:27 UTC
Created attachment 87443 [details] [review]
Bug 17005: Test for checked_in parameter

Test plan:
prove t/db_dependent/api/v1/checkouts.t
Comment 37 Josef Moravec 2019-04-05 09:51:30 UTC
Created attachment 87444 [details] [review]
Bug 17005: Add plugin parameters to /checkouts endpoint
Comment 38 Josef Moravec 2019-04-05 09:52:13 UTC
(In reply to Ere Maijala from comment #34)
> I bz-applied this including dependencies but there seems to be a conflict:
> 
> Applying: Bug 17005: Add checked_in checkouts to REST API response
> Using index info to reconstruct a base tree...
> M	Koha/REST/V1/Checkouts.pm
> M	api/v1/swagger/paths/checkouts.json
> Falling back to patching base and 3-way merge...
> Auto-merging api/v1/swagger/paths/checkouts.json
> Auto-merging Koha/REST/V1/Checkouts.pm
> CONFLICT (content): Merge conflict in Koha/REST/V1/Checkouts.pm

I rebased the patches on to of bug 17003, should apply cleanly now
Comment 39 Michal Denar 2019-04-05 11:57:17 UTC
Created attachment 87447 [details] [review]
Bug 17005: Add checked_in checkouts to REST API response

This patch implements parameter 'checked_in' on checkouts endpoint to
enable getting circulation history.

Test plan:
1) Apply the patch and restart plack
2) Use your favorite REST API tester and play with /checkouts endpoint:
    - use it without checked_in parameter
    - use checked_in=1 for getting returned checkouts

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 40 Michal Denar 2019-04-05 11:57:21 UTC
Created attachment 87448 [details] [review]
Bug 17005: Test for checked_in parameter

Test plan:
prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 41 Michal Denar 2019-04-05 11:57:25 UTC
Created attachment 87449 [details] [review]
Bug 17005: Add plugin parameters to /checkouts endpoint

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 42 Josef Moravec 2019-04-18 07:31:40 UTC
Created attachment 88263 [details] [review]
Bug 17005: Add checked_in checkouts to REST API response

This patch implements parameter 'checked_in' on checkouts endpoint to
enable getting circulation history.

Test plan:
1) Apply the patch and restart plack
2) Use your favorite REST API tester and play with /checkouts endpoint:
    - use it without checked_in parameter
    - use checked_in=1 for getting returned checkouts

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 43 Josef Moravec 2019-04-18 07:31:45 UTC
Created attachment 88264 [details] [review]
Bug 17005: Test for checked_in parameter

Test plan:
prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 44 Josef Moravec 2019-04-18 07:31:50 UTC
Created attachment 88265 [details] [review]
Bug 17005: Add plugin parameters to /checkouts endpoint

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 45 Josef Moravec 2019-04-18 07:32:32 UTC
Just rebased
Comment 46 Michal Denar 2019-04-18 19:25:37 UTC
Created attachment 88329 [details] [review]
Bug 17005: Add checked_in checkouts to REST API response

This patch implements parameter 'checked_in' on checkouts endpoint to
enable getting circulation history.

Test plan:
1) Apply the patch and restart plack
2) Use your favorite REST API tester and play with /checkouts endpoint:
    - use it without checked_in parameter
    - use checked_in=1 for getting returned checkouts

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 47 Michal Denar 2019-04-18 19:25:41 UTC
Created attachment 88330 [details] [review]
Bug 17005: Test for checked_in parameter

Test plan:
prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 48 Michal Denar 2019-04-18 19:25:45 UTC
Created attachment 88331 [details] [review]
Bug 17005: Add plugin parameters to /checkouts endpoint

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 49 Michal Denar 2019-06-28 19:51:11 UTC
Please rebase on master.

Thank you
Comment 50 Josef Moravec 2019-07-01 20:47:23 UTC
Created attachment 91166 [details] [review]
Bug 17005: Add checked_in checkouts to REST API response

This patch implements parameter 'checked_in' on checkouts endpoint to
enable getting circulation history.

Test plan:
1) Apply the patch and restart plack
2) Use your favorite REST API tester and play with /checkouts endpoint:
    - use it without checked_in parameter
    - use checked_in=1 for getting returned checkouts

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 51 Josef Moravec 2019-07-01 20:47:28 UTC
Created attachment 91167 [details] [review]
Bug 17005: Test for checked_in parameter

Test plan:
prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 52 Josef Moravec 2019-07-01 20:47:34 UTC
Created attachment 91168 [details] [review]
Bug 17005: Add plugin parameters to /checkouts endpoint

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 53 Josef Moravec 2019-07-01 20:48:11 UTC
Rebased on master
Comment 54 Josef Moravec 2019-07-03 20:02:00 UTC
Created attachment 91235 [details] [review]
Bug 17005: Add checked_in checkouts to REST API response

This patch implements parameter 'checked_in' on checkouts endpoint to
enable getting circulation history.

Test plan:
1) Apply the patch and restart plack
2) Use your favorite REST API tester and play with /checkouts endpoint:
    - use it without checked_in parameter
    - use checked_in=1 for getting returned checkouts

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 55 Josef Moravec 2019-07-03 20:02:07 UTC
Created attachment 91236 [details] [review]
Bug 17005: Test for checked_in parameter

Test plan:
prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 56 Josef Moravec 2019-07-03 20:02:13 UTC
Created attachment 91237 [details] [review]
Bug 17005: Add plugin parameters to /checkouts endpoint

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 57 Josef Moravec 2019-07-03 20:02:20 UTC
Created attachment 91238 [details] [review]
Bug 17005: Fix bad rebase

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 58 Katrin Fischer 2019-10-03 14:32:30 UTC
Patch still applies - pinging Tomas about this one.
Comment 59 Tomás Cohen Arazi 2019-10-04 19:21:28 UTC
Created attachment 93757 [details] [review]
Bug 17005: Add checked_in checkouts to REST API response

This patch implements parameter 'checked_in' on checkouts endpoint to
enable getting circulation history.

Test plan:
1) Apply the patch and restart plack
2) Use your favorite REST API tester and play with /checkouts endpoint:
    - use it without checked_in parameter
    - use checked_in=1 for getting returned checkouts

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 60 Tomás Cohen Arazi 2019-10-04 19:21:33 UTC
Created attachment 93758 [details] [review]
Bug 17005: Test for checked_in parameter

Test plan:
prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 61 Tomás Cohen Arazi 2019-10-04 19:21:38 UTC
Created attachment 93759 [details] [review]
Bug 17005: Add plugin parameters to /checkouts endpoint

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 62 Tomás Cohen Arazi 2019-10-04 19:21:42 UTC
Created attachment 93760 [details] [review]
Bug 17005: Fix bad rebase

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 63 Martin Renvoize 2019-10-08 13:31:18 UTC
Nice work!

Pushed to master for 19.11.00