Add API to list checkouts history for a user. This is very useful for use with VuFind.
Created attachment 53843 [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) 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 correct data 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
Since this bug depends on bug 13895, you should do "git bz apply 13895" first & then "git bz apply 17005"
Created attachment 53851 [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) 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 correct data 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 Added promised unit tests
Created attachment 53852 [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 Now the endpoint with borrowernumber does not return all the details from within the old issues, but only their issue_id. This is made in order to enhance API performance. E.g. VuFind can retrieve all the issue_ids & use AJAX to query details about those, so user sees how the details come up one by one & does not have to wait for all of them at once (could be very long with really old users) In order to get issue details, use the endpoint with the checkout_id.
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.
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?
(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
(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.
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.
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}
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.
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.
Rebased on top of master and moved permissions into Swagger, and added a HTTP 401 and 403 test.
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
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.
(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
(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
(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
Created attachment 61197 [details] [review] Bug 17005 - REST API: add routes to list checkouts history - Followup1. Fix date_due timeformat in tests.
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.
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
Created attachment 87255 [details] [review] Bug 17005: Test for checked_in parameter Test plan: prove t/db_dependent/api/v1/checkouts.t
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
Created attachment 87384 [details] [review] Bug 17005: Test for checked_in parameter Test plan: prove t/db_dependent/api/v1/checkouts.t
Created attachment 87385 [details] [review] Bug 17005: Add plugin parameters to /checkouts endpoint
If you have paging parameters, the response should also include the total number of records. Otherwise it's difficult to build a pager.
(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 ;)
Oh, right, sorry, I totally missed the headers!
> 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?
(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.
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
Created attachment 87406 [details] [review] Bug 17005: Test for checked_in parameter Test plan: prove t/db_dependent/api/v1/checkouts.t
Created attachment 87407 [details] [review] Bug 17005: Add plugin parameters to /checkouts endpoint
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
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
Created attachment 87443 [details] [review] Bug 17005: Test for checked_in parameter Test plan: prove t/db_dependent/api/v1/checkouts.t
Created attachment 87444 [details] [review] Bug 17005: Add plugin parameters to /checkouts endpoint
(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
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>
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>
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>
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>
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>
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>
Just rebased
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>
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>
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>
Please rebase on master. Thank you
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>
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>
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>
Rebased on master
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>
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>
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>
Created attachment 91238 [details] [review] Bug 17005: Fix bad rebase Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Patch still applies - pinging Tomas about this one.
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>
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>
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>
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>
Nice work! Pushed to master for 19.11.00