Bug 17602 - Integrate support for RecordedBooks (formerly OneClickDigital) API
Summary: Integrate support for RecordedBooks (formerly OneClickDigital) API
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Chris Cormack
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 16034
Blocks: 22010 21536 21567
  Show dependency treegraph
 
Reported: 2016-11-09 19:34 UTC by Chris Cormack
Modified: 2018-12-17 13:35 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
This feature integrates RecordedBooks functionality into the catalog, following the model of OverDrive. Searches on the OPAC will return a link to results in the RecordedBooks catalog if they are found. From that results page a user that is signed in and has a valid email matching a RecordedBooks account will be able to checkout the books directly from the results, and download them via their account page on the OPAC. Users will also be able to place holds on unavailable items.
Version(s) released in:


Attachments
Bug 17602 : Add OneClickDigital Integration to Koha (6.29 KB, patch)
2016-11-09 20:58 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 17602 OneClickDigital* sysprefs (3.30 KB, patch)
2016-11-09 21:12 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 17602 Add Oneclickdigital integration to Koha (30.18 KB, patch)
2016-11-09 21:15 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 17602 : Add OneClickDigital Integration to Koha (6.68 KB, patch)
2017-03-07 04:09 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 OneClickDigital* sysprefs (3.58 KB, patch)
2017-03-07 04:09 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Add Oneclickdigital integration to Koha (31.07 KB, patch)
2017-03-07 04:12 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Add Oneclickdigital integration to Koha (112.20 KB, patch)
2017-04-05 05:13 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Add Oneclickdigital integration to Koha (112.25 KB, patch)
2017-04-06 00:42 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Koha::ExternalContent->koha_patron() will return undef rather than die if user not logged in (2.17 KB, patch)
2017-04-18 02:44 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Add OneClickDigital Integration to Koha (7.02 KB, patch)
2017-04-18 02:45 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 OneClickDigital* sysprefs (3.58 KB, patch)
2017-04-18 02:45 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Add Oneclickdigital integration to Koha (112.32 KB, patch)
2017-04-18 02:46 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Add Oneclickdigital integration to Koha (112.37 KB, patch)
2017-05-12 01:57 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Add Oneclickdigital integration to Koha (115.47 KB, patch)
2017-05-29 01:31 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Add OneClickDigital Integration to Koha (10.18 KB, patch)
2017-05-29 03:11 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Add Oneclickdigital integration to Koha (115.67 KB, patch)
2017-05-29 03:12 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Add Oneclickdigital integration to Koha (116.02 KB, patch)
2017-05-31 00:38 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Add OneClickDigital Integration to Koha (10.18 KB, patch)
2017-06-07 01:37 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Add Oneclickdigital integration to Koha (116.11 KB, patch)
2017-06-07 01:37 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Add OneClickDigital Integration to Koha (10.39 KB, patch)
2017-06-08 00:41 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Add Oneclickdigital integration to Koha (116.94 KB, patch)
2017-06-08 00:41 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in (2.23 KB, patch)
2017-06-08 10:30 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602 Add OneClickDigital Integration to Koha (10.45 KB, patch)
2017-06-08 10:30 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602 Add Oneclickdigital integration to Koha (117.00 KB, patch)
2017-06-08 10:30 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602 RecordedBooks Integration to Koha - sysprefs and Koha::ExternalContent::RecordedBooks (10.47 KB, patch)
2017-08-15 00:33 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 RecordedBooks integration to Koha - OPAC (118.07 KB, patch)
2017-08-15 00:34 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602 Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in (2.28 KB, patch)
2017-08-22 13:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602 RecordedBooks Integration to Koha - sysprefs and Koha::ExternalContent::RecordedBooks (10.52 KB, patch)
2017-08-22 13:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602 RecordedBooks integration to Koha - OPAC (118.12 KB, patch)
2017-08-22 13:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in (2.28 KB, patch)
2017-09-20 15:30 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17602: Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in (2.28 KB, patch)
2017-09-20 15:30 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17602: RecordedBooks Integration to Koha (10.61 KB, patch)
2017-09-20 15:30 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17602: OPAC integration of RecordedBooks (118.17 KB, patch)
2017-09-20 15:31 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17602 follow-up: t/Koha_ExternalContent_RecordedBooks.t (2.06 KB, patch)
2017-10-10 01:29 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
t/Koha_ExternalContent_OverDrive.t correction (1.16 KB, patch)
2017-10-10 01:29 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602: Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in (2.29 KB, patch)
2017-11-08 13:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: RecordedBooks Integration to Koha (10.64 KB, patch)
2017-11-08 13:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: OPAC integration of RecordedBooks (34.60 KB, patch)
2017-11-08 13:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602 follow-up: t/Koha_ExternalContent_RecordedBooks.t (2.06 KB, patch)
2017-11-08 13:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Update opac.css (83.56 KB, patch)
2017-11-08 13:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Ease translation (2.29 KB, patch)
2017-11-08 13:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Handle '60+' if there are more than 60 results (1.39 KB, patch)
2017-11-08 13:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Adapt test plan to make the tests pass (771 bytes, patch)
2017-11-08 13:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Fix few minor QA issues (1.84 KB, patch)
2017-11-08 13:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Translations rework (10.57 KB, patch)
2017-11-13 01:49 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602: Translations rework (15.75 KB, patch)
2017-11-27 04:41 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602: fix RecordedBooks results pagination (3.32 KB, patch)
2017-11-27 04:41 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602: Removed unused param to RecordedBooks API (905 bytes, patch)
2017-11-27 04:41 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602: Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in (2.28 KB, patch)
2018-04-05 18:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: RecordedBooks Integration to Koha (10.13 KB, patch)
2018-04-05 18:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: OPAC integration of RecordedBooks (34.24 KB, patch)
2018-04-05 18:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602 follow-up: t/Koha_ExternalContent_RecordedBooks.t (2.06 KB, patch)
2018-04-05 18:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Ease translation (2.28 KB, patch)
2018-04-05 18:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Handle '60+' if there are more than 60 results (1.38 KB, patch)
2018-04-05 18:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Adapt test plan to make the tests pass (764 bytes, patch)
2018-04-05 18:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Fix few minor QA issues (1.83 KB, patch)
2018-04-05 18:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Translations rework (15.75 KB, patch)
2018-04-05 18:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: fix RecordedBooks results pagination (3.32 KB, patch)
2018-04-05 18:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Removed unused param to RecordedBooks API (905 bytes, patch)
2018-04-05 18:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: (follow-up) Adjust js paths to use version add spinner (4.90 KB, patch)
2018-04-05 18:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: (follow-up) Adjust js paths to use version add spinner (89.55 KB, patch)
2018-04-06 00:56 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 17602: Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in (2.29 KB, patch)
2018-06-19 18:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: RecordedBooks Integration to Koha (10.16 KB, patch)
2018-06-19 18:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: OPAC integration of RecordedBooks (33.67 KB, patch)
2018-06-19 18:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602 follow-up: t/Koha_ExternalContent_RecordedBooks.t (2.06 KB, patch)
2018-06-19 18:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Ease translation (2.29 KB, patch)
2018-06-19 18:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Handle '60+' if there are more than 60 results (1.39 KB, patch)
2018-06-19 18:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Adapt test plan to make the tests pass (771 bytes, patch)
2018-06-19 18:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Fix few minor QA issues (1.84 KB, patch)
2018-06-19 18:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Translations rework (15.78 KB, patch)
2018-06-19 18:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: fix RecordedBooks results pagination (3.33 KB, patch)
2018-06-19 18:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Removed unused param to RecordedBooks API (912 bytes, patch)
2018-06-19 18:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: (follow-up) Adjust js paths to use version add spinner (93.68 KB, patch)
2018-06-19 18:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: update compiled opac.css (87.06 KB, patch)
2018-06-19 18:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Use Asset TT plugin and fix indentation (3.05 KB, patch)
2018-06-19 18:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in (2.28 KB, patch)
2018-09-12 22:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: RecordedBooks Integration to Koha (10.13 KB, patch)
2018-09-12 22:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: OPAC integration of RecordedBooks (33.65 KB, patch)
2018-09-12 22:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602 follow-up: t/Koha_ExternalContent_RecordedBooks.t (2.06 KB, patch)
2018-09-12 22:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Ease translation (2.28 KB, patch)
2018-09-12 22:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Handle '60+' if there are more than 60 results (1.38 KB, patch)
2018-09-12 22:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Adapt test plan to make the tests pass (764 bytes, patch)
2018-09-12 22:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Fix few minor QA issues (1.83 KB, patch)
2018-09-12 22:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Translations rework (5.38 KB, patch)
2018-09-12 22:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: fix RecordedBooks results pagination (10.69 KB, patch)
2018-09-12 22:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Removed unused param to RecordedBooks API (905 bytes, patch)
2018-09-12 22:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: (follow-up) Adjust js paths to use version add spinner (4.93 KB, patch)
2018-09-12 22:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Use Asset TT plugin and fix indentation (3.04 KB, patch)
2018-09-12 22:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: (QA follow-up) Fix tests (1.20 KB, patch)
2018-09-12 22:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Move spinner location to include file (1.81 KB, patch)
2018-09-21 19:01 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17602: Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in (2.37 KB, patch)
2018-09-30 23:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: RecordedBooks Integration to Koha (10.23 KB, patch)
2018-09-30 23:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: OPAC integration of RecordedBooks (33.78 KB, patch)
2018-09-30 23:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602 follow-up: t/Koha_ExternalContent_RecordedBooks.t (2.13 KB, patch)
2018-09-30 23:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Ease translation (2.36 KB, patch)
2018-09-30 23:26 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Handle '60+' if there are more than 60 results (1.46 KB, patch)
2018-09-30 23:26 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Adapt test plan to make the tests pass (844 bytes, patch)
2018-09-30 23:26 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Fix few minor QA issues (1.91 KB, patch)
2018-09-30 23:26 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Translations rework (5.46 KB, patch)
2018-09-30 23:26 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: fix RecordedBooks results pagination (10.78 KB, patch)
2018-09-30 23:26 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Removed unused param to RecordedBooks API (985 bytes, patch)
2018-09-30 23:26 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: (follow-up) Adjust js paths to use version add spinner (5.02 KB, patch)
2018-09-30 23:26 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Use Asset TT plugin and fix indentation (3.13 KB, patch)
2018-09-30 23:26 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: (QA follow-up) Fix tests (1.28 KB, patch)
2018-09-30 23:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Move spinner location to include file (1.89 KB, patch)
2018-09-30 23:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17602: Add missing html filters (5.16 KB, patch)
2018-10-01 17:19 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 Chris Cormack 2016-11-09 19:34:28 UTC
Recorded Books / Oneclickdigital are a provider of electronic audio books.

They have an API that lets users checkout/checkin/search/place holds etc on their items.

It would be great if users could do this within Koha

http://developer.oneclickdigital.us/
Comment 1 Chris Cormack 2016-11-09 20:58:12 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2016-11-09 21:12:51 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2016-11-09 21:15:25 UTC Comment hidden (obsolete)
Comment 4 Nick Clemens 2017-01-10 10:19:50 UTC
Testing this with the OneClick digital credentials shared between ByWater/Catalyst I am not able to get any search results, I either get an error or a constant spin - does this work in your testing currently?
Comment 5 Srdjan Jankovic 2017-03-07 04:09:26 UTC Comment hidden (obsolete)
Comment 6 Srdjan Jankovic 2017-03-07 04:09:44 UTC Comment hidden (obsolete)
Comment 7 Srdjan Jankovic 2017-03-07 04:12:14 UTC Comment hidden (obsolete)
Comment 8 Srdjan Jankovic 2017-04-05 05:13:57 UTC Comment hidden (obsolete)
Comment 9 Srdjan Jankovic 2017-04-06 00:42:54 UTC Comment hidden (obsolete)
Comment 10 Nick Clemens 2017-04-10 21:46:50 UTC
Basic functionality seems to be working right now, a couple issues:
1 - We should be able to get results without being signed in - this would mirror functionality for other sources
2 - The account page should offer a 'logout' option
3 - When no items are checked out or on hold the account tab should not display the headings in the account tab
4 - Image links in OneClick results lead no where, ideally these would point out to the oneclick site for the library
5 - It would be nice to have a few more details on the results page if possible
Comment 11 Srdjan Jankovic 2017-04-18 02:44:31 UTC Comment hidden (obsolete)
Comment 12 Srdjan Jankovic 2017-04-18 02:45:11 UTC Comment hidden (obsolete)
Comment 13 Srdjan Jankovic 2017-04-18 02:45:28 UTC Comment hidden (obsolete)
Comment 14 Srdjan Jankovic 2017-04-18 02:46:46 UTC Comment hidden (obsolete)
Comment 15 Nick Clemens 2017-04-27 14:32:30 UTC
Searchign when nit signed in works, but I am required to sign in once I click the results link

While my acocunt works on the oneclick site I am getting this error on results page:
Invalid user_id nick@bywatersolutions.com Invalid Basic Token or permission denied at /usr/local/share/perl/5.20.2/WebService/ILS.pm line 748. at /home/vagrant/kohaclone/Koha/ExternalContent/OneClickDigital.pm line 61. 

When I click the image on the results page it points to:
http://localhost:8080/cgi-bin/koha/XXX

User tab either has error above or:
Bad Request
Comment 16 Srdjan Jankovic 2017-05-12 01:57:57 UTC Comment hidden (obsolete)
Comment 17 Srdjan Jankovic 2017-05-29 01:31:06 UTC Comment hidden (obsolete)
Comment 18 Srdjan Jankovic 2017-05-29 03:11:48 UTC Comment hidden (obsolete)
Comment 19 Srdjan Jankovic 2017-05-29 03:12:43 UTC Comment hidden (obsolete)
Comment 20 Srdjan Jankovic 2017-05-31 00:38:26 UTC Comment hidden (obsolete)
Comment 21 Srdjan Jankovic 2017-06-07 01:37:36 UTC Comment hidden (obsolete)
Comment 22 Srdjan Jankovic 2017-06-07 01:37:47 UTC Comment hidden (obsolete)
Comment 23 Srdjan Jankovic 2017-06-08 00:41:15 UTC Comment hidden (obsolete)
Comment 24 Srdjan Jankovic 2017-06-08 00:41:32 UTC Comment hidden (obsolete)
Comment 25 Nick Clemens 2017-06-08 10:30:26 UTC
Created attachment 64098 [details] [review]
Bug 17602 Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 26 Nick Clemens 2017-06-08 10:30:28 UTC Comment hidden (obsolete)
Comment 27 Nick Clemens 2017-06-08 10:30:32 UTC Comment hidden (obsolete)
Comment 28 Tomás Cohen Arazi 2017-07-03 19:04:43 UTC
Hi everyone here. I tested this with some credentials Nick gave me. I have a couple observations about this one.

Important
=========

- I first tested opac-oneclick-search.pl using the q= query param. That way I found stuff in Oneclick. The searched on the main opac search for a term I knew should give results (q=1776), and I got (the first time) "Error searching OneClickDigital collection: Not Found". I then reloaded the results page, and got "Found 1 results in OneClickDigital collection". This one seemed ok
- When clicking in the '1 results' link, I got a red message "Error searching OneClickDigital collection: Not Found", on reloading, I got the search results for 1776.

=> FAIL: It seems there's a problem on the first query, or a throttling stuff getting in the middle.

Moderate
========

=> FAIL: The search results on opac-oneclick-search.pl have a different styling than Koha's search results. They look nicer, but I think the same XSLT's shold be used.

I understood that this is a transition until this search results are displayed inline along with the regular (local) search results. I wouldn't say the second item is a blocker, but it might be a reason for others to vote against its inclusion.

Minor
=====

- Another (minor) thing I would consider changing is the wording for the sysprefs description. I think they should clearly match what Oneclick provides the user to configure Koha (LibraryID vs. library #)

- Warnings are logged:
Use of uninitialized value in concatenation (.) or string at /usr/local/share/perl/5.20.2/WebService/ILS/OneClickDigital/PartnerPatron.pm line 58.
Use of uninitialized value in concatenation (.) or string at /home/vagrant/kohaclone/Koha/ExternalContent/OneClickDigital.pm line 73.

QA scripts
==========
The QA scripts complain about oneclick-checkout.inc
=>
 FAIL	koha-tmpl/opac-tmpl/bootstrap/en/includes/oneclick-checkout.inc
   FAIL	  forbidden patterns
		forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 1)
Comment 29 Srdjan Jankovic 2017-07-11 02:04:57 UTC
(In reply to Tomás Cohen Arazi from comment #28)

> QA scripts
> ==========
> The QA scripts complain about oneclick-checkout.inc
> =>
>  FAIL	koha-tmpl/opac-tmpl/bootstrap/en/includes/oneclick-checkout.inc
>    FAIL	  forbidden patterns
> 		forbidden pattern: Contains old bootstrap style for buttons (see bug
> 16239) (line 1)

That is due to <div class="modal hide"> for pop-ups. There's a number of templates using that. What should we use under the circumstances?
Comment 30 Srdjan Jankovic 2017-08-15 00:33:09 UTC Comment hidden (obsolete)
Comment 31 Srdjan Jankovic 2017-08-15 00:34:20 UTC Comment hidden (obsolete)
Comment 32 Nick Clemens 2017-08-22 13:58:51 UTC Comment hidden (obsolete)
Comment 33 Nick Clemens 2017-08-22 13:58:55 UTC Comment hidden (obsolete)
Comment 34 Nick Clemens 2017-08-22 13:59:00 UTC Comment hidden (obsolete)
Comment 35 Tomás Cohen Arazi 2017-09-20 15:30:08 UTC Comment hidden (obsolete)
Comment 36 Tomás Cohen Arazi 2017-09-20 15:30:34 UTC
Created attachment 67237 [details] [review]
Bug 17602: Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 37 Tomás Cohen Arazi 2017-09-20 15:30:54 UTC Comment hidden (obsolete)
Comment 38 Tomás Cohen Arazi 2017-09-20 15:31:04 UTC Comment hidden (obsolete)
Comment 39 Tomás Cohen Arazi 2017-09-20 15:43:56 UTC
Ok, I tested this patchset and have some comments.

I used the credentials ByWater provided, with a logged-in user with Nick's email address set as primary (which is recognized as valid for the RecordedBooks credentials).

From the functional perspective, everything works as expected:

  * Searches
  * Checkout
  * Checkin
  * Holds

I would only try to make the UI tell the users it is working on something and prevent them from (re)clicking other RecordedBooks buttons (spinning symbol or similar). Not worth failing the patchset because of it, but definitely a needed followup.

Regarding the whole patchset, I'd say tests coverage is not good enough. And having the tests require valid credentials for the proprietary external system seems too much. There are two options IMHO:

- If the provider does have a sandbox/testbed infrastructure, use it
- If it doesn't, keep the tests (which skip if credentials are missing) but add new db-independent tests with mocked behaviour for WebService::ILS. Auth_with_ldap.t can be a good source of examples for completely mocking the tests.

I'm looking forward to help having this in 17.11, so don't hesitate to contact me for quick feedback.
Comment 40 Tomás Cohen Arazi 2017-10-09 14:03:25 UTC
I'm not sure how to deal with this kind of patchsets. I'm passing it to the RM. The functionality is the expected one. The main problem is the fact that the vendor is not providing good testing tools.
I think it deserves to go in. The users sponsoring it will for sure make us improve it if needed.
Comment 41 Jonathan Druart 2017-10-09 21:05:20 UTC
(In reply to Tomás Cohen Arazi from comment #39)
> Regarding the whole patchset, I'd say tests coverage is not good enough. And
> having the tests require valid credentials for the proprietary external
> system seems too much. There are two options IMHO:
> 
> - If the provider does have a sandbox/testbed infrastructure, use it
> - If it doesn't, keep the tests (which skip if credentials are missing) but
> add new db-independent tests with mocked behaviour for WebService::ILS.
> Auth_with_ldap.t can be a good source of examples for completely mocking the
> tests.

Chris, Srdjan, any ideas how we could improve that?
Can someone share the test credentials with me?
Comment 42 Srdjan Jankovic 2017-10-10 01:29:29 UTC Comment hidden (obsolete)
Comment 43 Srdjan Jankovic 2017-10-10 01:29:54 UTC Comment hidden (obsolete)
Comment 44 Chris Cormack 2017-10-10 01:38:27 UTC
(In reply to Jonathan Druart from comment #41)
> (In reply to Tomás Cohen Arazi from comment #39)
> > Regarding the whole patchset, I'd say tests coverage is not good enough. And
> > having the tests require valid credentials for the proprietary external
> > system seems too much. There are two options IMHO:
> > 
> > - If the provider does have a sandbox/testbed infrastructure, use it
> > - If it doesn't, keep the tests (which skip if credentials are missing) but
> > add new db-independent tests with mocked behaviour for WebService::ILS.
> > Auth_with_ldap.t can be a good source of examples for completely mocking the
> > tests.
> 
> Chris, Srdjan, any ideas how we could improve that?
> Can someone share the test credentials with me?

We have no contractual relationship, or any clients who use it. It is probably best to talk to Bywater or someone who have clients that might have a test account.
Comment 45 Srdjan Jankovic 2017-10-10 01:43:02 UTC
I've added a patch with some limited value. And another seemingly unrelated, which you may choose to apply or drop depending on the policies/feelings.

My position is that as is any proper automated testing would be difficult and expensive in terms of effort. Considering it is a niche area, I doubt it is worth it. If it is any comfort you can take my word that automated tests for WebService::ILS are passing.
It is unfortunate that I started this work at the time when api was not established. The right thing would be to move svc code to api/v1, at which point it would be much easier and make sense to do Mojo tests. But again, would it be worth the effort.
Comment 46 Jonathan Druart 2017-11-06 19:35:14 UTC
Quick note: use String.format() when needed.
Comment 47 Srdjan Jankovic 2017-11-07 02:06:33 UTC
(In reply to Jonathan Druart from comment #46)
> Quick note: use String.format() when needed.

Money or somewhere else?
Comment 48 Jonathan Druart 2017-11-07 14:29:27 UTC
(In reply to Srdjan Jankovic from comment #47)
> (In reply to Jonathan Druart from comment #46)
> > Quick note: use String.format() when needed.
> 
> Money or somewhere else?

Hum? Money?
I am talking about that kind of generated strings:
$( '#recordedbooks-status' ).html( '<strong>' + _("Found") + ' ' + data.total + ' ' + _("results in the library's RecordedBooks collection.") + '</strong>' );

should be

_("Found %s results in the library's RecordedBooks collection.").format(data.total)

But it was a not for myself.
Comment 49 Jonathan Druart 2017-11-07 14:30:23 UTC
(In reply to Srdjan Jankovic from comment #43)
> Created attachment 67860 [details] [review] [review]
> t/Koha_ExternalContent_OverDrive.t correction

Why this change?
See commit c1185533c61e8266f9d89136e8e0debddf71d497
    Bug 18851: Make OverDrive.T pass if dbms is not running
Comment 50 Srdjan Jankovic 2017-11-07 23:50:36 UTC
Comment on attachment 67860 [details] [review]
t/Koha_ExternalContent_OverDrive.t correction

Obsoleted by 18851
Comment 51 Srdjan Jankovic 2017-11-07 23:53:16 UTC
(In reply to Jonathan Druart from comment #48)

> _("Found %s results in the library's RecordedBooks
> collection.").format(data.total)
> 
> But it was a not for myself.

Did not know how we store translatables. Want me to change it?
Comment 52 Jonathan Druart 2017-11-08 13:22:15 UTC Comment hidden (obsolete)
Comment 53 Jonathan Druart 2017-11-08 13:22:21 UTC Comment hidden (obsolete)
Comment 54 Jonathan Druart 2017-11-08 13:22:26 UTC Comment hidden (obsolete)
Comment 55 Jonathan Druart 2017-11-08 13:22:29 UTC Comment hidden (obsolete)
Comment 56 Jonathan Druart 2017-11-08 13:22:35 UTC Comment hidden (obsolete)
Comment 57 Jonathan Druart 2017-11-08 13:22:39 UTC Comment hidden (obsolete)
Comment 58 Jonathan Druart 2017-11-08 13:22:42 UTC Comment hidden (obsolete)
Comment 59 Jonathan Druart 2017-11-08 13:22:46 UTC Comment hidden (obsolete)
Comment 60 Jonathan Druart 2017-11-08 13:22:50 UTC Comment hidden (obsolete)
Comment 61 Jonathan Druart 2017-11-08 13:47:11 UTC
Some additional remarks:
1. At some point I got the following error:
{"error":"Checkout item already exists"}Status: 200 OK
Pragma: no-cache
X-frame-options: SAMEORIGIN
Cache-control: no-cache
Content-Type: application/json; charset=UTF-8

{"action":null,"is_identified":true,"error":"ARRAY(0xbb2fe50)"}

When I clicked several time on the "checkout" button.
I think we should hide them or add a spinner to tell the user something is going on.

2. Strings from .js files are not translatable, `git grep MSG_` to know how to make them translatable.

3. I see code related to pagination, but I do not see the previous/next buttons. I only see the OPACnumSearchResults first results.

And a question:
Koha/ExternalContent/RecordedBooks.pm
 76                 user_agent_params => { agent => $class->agent_string }
Where is this used?
Comment 62 Srdjan Jankovic 2017-11-13 01:49:43 UTC Comment hidden (obsolete)
Comment 63 Srdjan Jankovic 2017-11-13 01:59:57 UTC
(In reply to Jonathan Druart from comment #61)
> Some additional remarks:
> 1. At some point I got the following error:
> {"error":"Checkout item already exists"}Status: 200 OK

I have to let Nick decide on that one

> 2. Strings from .js files are not translatable, `git grep MSG_` to know how
> to make them translatable.

Done. Please let me know if it's not what you had in mind.

> 3. I see code related to pagination, but I do not see the previous/next
> buttons. I only see the OPACnumSearchResults first results.

Yes, at some stage the idea was to integrate all external searches into the main results page, so I stopped short of implementing paging - will be thrown away. Is it acceptable to leave as is?

> 
> And a question:
> Koha/ExternalContent/RecordedBooks.pm
>  76                 user_agent_params => { agent => $class->agent_string }
> Where is this used?

It is a param for WebService::ILS. Cannot remember any more, possibly was a requirement for a provider that even did not get implemented.
Comment 64 Jonathan Druart 2017-11-26 18:14:34 UTC
(In reply to Srdjan Jankovic from comment #63)
> (In reply to Jonathan Druart from comment #61)
> > Some additional remarks:
> > 1. At some point I got the following error:
> > {"error":"Checkout item already exists"}Status: 200 OK
> 
> I have to let Nick decide on that one

Nick?

> > 2. Strings from .js files are not translatable, `git grep MSG_` to know how
> > to make them translatable.
> 
> Done. Please let me know if it's not what you had in mind.

I am not sure to understand what you did, I already fixed the some translations (patch "Bug 17602: Ease translation") from .tt, you needed to fix the strings in .js files (.inc and .tt were ok)

> > 3. I see code related to pagination, but I do not see the previous/next
> > buttons. I only see the OPACnumSearchResults first results.
> 
> Yes, at some stage the idea was to integrate all external searches into the
> main results page, so I stopped short of implementing paging - will be
> thrown away. Is it acceptable to leave as is?

I do not think we should integrate code that is not used.

> > And a question:
> > Koha/ExternalContent/RecordedBooks.pm
> >  76                 user_agent_params => { agent => $class->agent_string }
> > Where is this used?
> 
> It is a param for WebService::ILS. Cannot remember any more, possibly was a
> requirement for a provider that even did not get implemented.

So we remove?

Moving out the NSO queue.
Comment 65 Srdjan Jankovic 2017-11-27 00:24:29 UTC
(In reply to Jonathan Druart from comment #64)

> I am not sure to understand what you did, I already fixed the some
> translations (patch "Bug 17602: Ease translation") from .tt, you needed to
> fix the strings in .js files (.inc and .tt were ok)

Right. Ok, before I do that. how do we handle messages that are the same in both OPAC and intranet?
Comment 66 Tomás Cohen Arazi 2017-11-27 01:33:59 UTC
(In reply to Srdjan Jankovic from comment #65)
> (In reply to Jonathan Druart from comment #64)
> 
> > I am not sure to understand what you did, I already fixed the some
> > translations (patch "Bug 17602: Ease translation") from .tt, you needed to
> > fix the strings in .js files (.inc and .tt were ok)
> 
> Right. Ok, before I do that. how do we handle messages that are the same in
> both OPAC and intranet?

I suggest you just duplicate them.
Comment 67 Srdjan Jankovic 2017-11-27 04:41:17 UTC Comment hidden (obsolete)
Comment 68 Srdjan Jankovic 2017-11-27 04:41:29 UTC Comment hidden (obsolete)
Comment 69 Srdjan Jankovic 2017-11-27 04:41:41 UTC Comment hidden (obsolete)
Comment 70 Srdjan Jankovic 2017-11-27 04:43:32 UTC
(In reply to Jonathan Druart from comment #64)

> > > 2. Strings from .js files are not translatable, `git grep MSG_` to know how
> > > to make them translatable.
> > 
> > Done. Please let me know if it's not what you had in mind.
> 
> I am not sure to understand what you did, I already fixed the some
> translations (patch "Bug 17602: Ease translation") from .tt, you needed to
> fix the strings in .js files (.inc and .tt were ok)

This should be fine now.

> 
> > > 3. I see code related to pagination, but I do not see the previous/next
> > > buttons. I only see the OPACnumSearchResults first results.
> > 
> > Yes, at some stage the idea was to integrate all external searches into the
> > main results page, so I stopped short of implementing paging - will be
> > thrown away. Is it acceptable to leave as is?
> 
> I do not think we should integrate code that is not used.

Actually that was a bug. Fixed.

> 
> > > And a question:
> > > Koha/ExternalContent/RecordedBooks.pm
> > >  76                 user_agent_params => { agent => $class->agent_string }
> > > Where is this used?
> > 
> > It is a param for WebService::ILS. Cannot remember any more, possibly was a
> > requirement for a provider that even did not get implemented.
> 
> So we remove?

Removed.
Comment 71 Nick Clemens 2017-12-05 12:19:48 UTC
(In reply to Jonathan Druart from comment #64)
> (In reply to Srdjan Jankovic from comment #63)
> > (In reply to Jonathan Druart from comment #61)
> > > Some additional remarks:
> > > 1. At some point I got the following error:
> > > {"error":"Checkout item already exists"}Status: 200 OK
> > 
> > I have to let Nick decide on that one
> 
> Nick?
> 
I was not able to recreate the error, a spinner would be nice here to indicate action is being performed
Comment 72 Nick Clemens 2018-04-05 18:14:48 UTC
Created attachment 73708 [details] [review]
Bug 17602: Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 73 Nick Clemens 2018-04-05 18:14:52 UTC
Created attachment 73709 [details] [review]
Bug 17602: RecordedBooks Integration to Koha

This patch introduces the required sysprefs and Koha::ExternalContent::RecordedBooks

Koha::ExternalContent::RecordedBooks - a wrapper around
WebService::ILS::RecordedBooks::PartnerPatron

RecordedBooks* sysprefs

Nothing functional to test with this patch yet.
But you can run the tests that come with it

 t/db_dependent/Koha_ExternalContent_RecordedBooks.t

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 74 Nick Clemens 2018-04-05 18:14:55 UTC
Created attachment 73710 [details] [review]
Bug 17602: OPAC integration of RecordedBooks

Add RecordedBooks to the users page in the public interface

To Test:
1/ Apply all 3 patches
2/ Set the sysprefs to valid values (you will need a test account with RecordedBooks)
3/ Try a search
4/ Login to the OPAC, try to place a hold, or check an item out
5/ Check the opac-user page, see if your items are showing on the oneclickdigital tab

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 75 Nick Clemens 2018-04-05 18:14:58 UTC
Created attachment 73711 [details] [review]
Bug 17602 follow-up: t/Koha_ExternalContent_RecordedBooks.t
Comment 76 Nick Clemens 2018-04-05 18:15:02 UTC
Created attachment 73712 [details] [review]
Bug 17602: Ease translation

Use String.format() to make translation easier
Comment 77 Nick Clemens 2018-04-05 18:15:07 UTC
Created attachment 73713 [details] [review]
Bug 17602: Handle '60+' if there are more than 60 results

The service returns "60+" if there are more than 60 results.
Without this patch the "Found %s results in RecordedBooks collection"
line is removed.
Comment 78 Nick Clemens 2018-04-05 18:15:11 UTC
Created attachment 73714 [details] [review]
Bug 17602: Adapt test plan to make the tests pass
Comment 79 Nick Clemens 2018-04-05 18:15:15 UTC
Created attachment 73715 [details] [review]
Bug 17602: Fix few minor QA issues
Comment 80 Nick Clemens 2018-04-05 18:15:19 UTC
Created attachment 73716 [details] [review]
Bug 17602: Translations rework
Comment 81 Nick Clemens 2018-04-05 18:15:22 UTC
Created attachment 73717 [details] [review]
Bug 17602: fix RecordedBooks results pagination
Comment 82 Nick Clemens 2018-04-05 18:15:26 UTC
Created attachment 73718 [details] [review]
Bug 17602: Removed unused param to RecordedBooks API
Comment 83 Nick Clemens 2018-04-05 18:15:30 UTC Comment hidden (obsolete)
Comment 84 Srdjan Jankovic 2018-04-06 00:56:02 UTC
Created attachment 73725 [details] [review]
Bug 17602: (follow-up) Adjust js paths to use version add spinner

Also included recompiled version of opac.css that we forgot.

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Comment 85 Jonathan Druart 2018-06-19 18:54:35 UTC
1. Maybe add a ref to OneClickdigital in the pref?
 "No system preferences matched your search for: OneClickdigital"
(not blocking)

2. Found 241+ results in the library's RecordedBooks collection
"+" ?

3. /opac-tmpl/lib/jquery/plugins/themes/classic/throbber.gif
must be using [% theme %]/[% interface %]

4. In the checkout list there is a link .epub (below "downloads"), but it downloads a .acsm file (??) Is it expected?

5. expires.toLocaleString() does not work what you are expecting, the date is always displayed in the same format (do not depend on dateformat)

6. https://screenshots.firefox.com/C5OmithULkvMQtTe/catalogue.kohadev.org
"Check out" and "Place hold" button are too close and not well styled (not blocking)

7. Place hold, hit cgi-bin/koha/opac-user.pl, cancel the hold. The hold is still listed (not blocking)

8. /cgi-bin/koha/opac-recordedbooks-search.pl?q=d
click on an image, https://carnegiestoutia.oneclickdigital.com/#titles/9781470360603 => "Your connection is not secure", awesome...
Comment 86 Jonathan Druart 2018-06-19 18:55:23 UTC
Created attachment 76180 [details] [review]
Bug 17602: Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 87 Jonathan Druart 2018-06-19 18:55:33 UTC
Created attachment 76181 [details] [review]
Bug 17602: RecordedBooks Integration to Koha

This patch introduces the required sysprefs and Koha::ExternalContent::RecordedBooks

Koha::ExternalContent::RecordedBooks - a wrapper around
WebService::ILS::RecordedBooks::PartnerPatron

RecordedBooks* sysprefs

Nothing functional to test with this patch yet.
But you can run the tests that come with it

 t/db_dependent/Koha_ExternalContent_RecordedBooks.t

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 88 Jonathan Druart 2018-06-19 18:55:41 UTC
Created attachment 76182 [details] [review]
Bug 17602: OPAC integration of RecordedBooks

Add RecordedBooks to the users page in the public interface

To Test:
1/ Apply all 3 patches
2/ Set the sysprefs to valid values (you will need a test account with RecordedBooks)
3/ Try a search
4/ Login to the OPAC, try to place a hold, or check an item out
5/ Check the opac-user page, see if your items are showing on the oneclickdigital tab

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 89 Jonathan Druart 2018-06-19 18:55:48 UTC
Created attachment 76183 [details] [review]
Bug 17602 follow-up: t/Koha_ExternalContent_RecordedBooks.t
Comment 90 Jonathan Druart 2018-06-19 18:55:56 UTC
Created attachment 76184 [details] [review]
Bug 17602: Ease translation

Use String.format() to make translation easier
Comment 91 Jonathan Druart 2018-06-19 18:56:01 UTC
Created attachment 76185 [details] [review]
Bug 17602: Handle '60+' if there are more than 60 results

The service returns "60+" if there are more than 60 results.
Without this patch the "Found %s results in RecordedBooks collection"
line is removed.
Comment 92 Jonathan Druart 2018-06-19 18:56:07 UTC
Created attachment 76186 [details] [review]
Bug 17602: Adapt test plan to make the tests pass
Comment 93 Jonathan Druart 2018-06-19 18:56:13 UTC
Created attachment 76187 [details] [review]
Bug 17602: Fix few minor QA issues
Comment 94 Jonathan Druart 2018-06-19 18:56:21 UTC
Created attachment 76188 [details] [review]
Bug 17602: Translations rework
Comment 95 Jonathan Druart 2018-06-19 18:56:29 UTC
Created attachment 76189 [details] [review]
Bug 17602: fix RecordedBooks results pagination
Comment 96 Jonathan Druart 2018-06-19 18:56:36 UTC
Created attachment 76190 [details] [review]
Bug 17602: Removed unused param to RecordedBooks API
Comment 97 Jonathan Druart 2018-06-19 18:56:45 UTC
Created attachment 76191 [details] [review]
Bug 17602: (follow-up) Adjust js paths to use version add spinner

Also included recompiled version of opac.css that we forgot.

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Comment 98 Jonathan Druart 2018-06-19 18:56:54 UTC
Created attachment 76192 [details] [review]
Bug 17602: update compiled opac.css
Comment 99 Jonathan Druart 2018-06-19 18:57:01 UTC
Created attachment 76193 [details] [review]
Bug 17602: Use Asset TT plugin and fix indentation
Comment 100 Jonathan Druart 2018-06-19 19:04:20 UTC
Please keep changes to opac.css in a separated patch to ease rebases.
Comment 101 Nick Clemens 2018-09-12 22:45:03 UTC
Created attachment 78602 [details] [review]
Bug 17602: Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 102 Nick Clemens 2018-09-12 22:45:08 UTC
Created attachment 78603 [details] [review]
Bug 17602: RecordedBooks Integration to Koha

This patch introduces the required sysprefs and Koha::ExternalContent::RecordedBooks

Koha::ExternalContent::RecordedBooks - a wrapper around
WebService::ILS::RecordedBooks::PartnerPatron

RecordedBooks* sysprefs

Nothing functional to test with this patch yet.
But you can run the tests that come with it

 t/db_dependent/Koha_ExternalContent_RecordedBooks.t

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 103 Nick Clemens 2018-09-12 22:45:13 UTC
Created attachment 78604 [details] [review]
Bug 17602: OPAC integration of RecordedBooks

Add RecordedBooks to the users page in the public interface

To Test:
1/ Apply all 3 patches
2/ Set the sysprefs to valid values (you will need a test account with RecordedBooks)
3/ Try a search
4/ Login to the OPAC, try to place a hold, or check an item out
5/ Check the opac-user page, see if your items are showing on the oneclickdigital tab

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 104 Nick Clemens 2018-09-12 22:45:18 UTC
Created attachment 78605 [details] [review]
Bug 17602 follow-up: t/Koha_ExternalContent_RecordedBooks.t
Comment 105 Nick Clemens 2018-09-12 22:45:22 UTC
Created attachment 78606 [details] [review]
Bug 17602: Ease translation

Use String.format() to make translation easier
Comment 106 Nick Clemens 2018-09-12 22:45:27 UTC
Created attachment 78607 [details] [review]
Bug 17602: Handle '60+' if there are more than 60 results

The service returns "60+" if there are more than 60 results.
Without this patch the "Found %s results in RecordedBooks collection"
line is removed.
Comment 107 Nick Clemens 2018-09-12 22:45:31 UTC
Created attachment 78608 [details] [review]
Bug 17602: Adapt test plan to make the tests pass
Comment 108 Nick Clemens 2018-09-12 22:45:36 UTC
Created attachment 78609 [details] [review]
Bug 17602: Fix few minor QA issues
Comment 109 Nick Clemens 2018-09-12 22:45:40 UTC
Created attachment 78610 [details] [review]
Bug 17602: Translations rework
Comment 110 Nick Clemens 2018-09-12 22:45:45 UTC
Created attachment 78611 [details] [review]
Bug 17602: fix RecordedBooks results pagination
Comment 111 Nick Clemens 2018-09-12 22:45:49 UTC
Created attachment 78612 [details] [review]
Bug 17602: Removed unused param to RecordedBooks API
Comment 112 Nick Clemens 2018-09-12 22:45:54 UTC
Created attachment 78613 [details] [review]
Bug 17602: (follow-up) Adjust js paths to use version add spinner

Also included recompiled version of opac.css that we forgot.

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Comment 113 Nick Clemens 2018-09-12 22:45:58 UTC
Created attachment 78614 [details] [review]
Bug 17602: Use Asset TT plugin and fix indentation
Comment 114 Nick Clemens 2018-09-12 22:46:04 UTC
Created attachment 78615 [details] [review]
Bug 17602: (QA follow-up) Fix tests
Comment 115 Nick Clemens 2018-09-12 22:50:09 UTC
(In reply to Jonathan Druart from comment #85)
> 1. Maybe add a ref to OneClickdigital in the pref?
>  "No system preferences matched your search for: OneClickdigital"
> (not blocking)

They are changing the name over to RecordedBooks, I think we should stick with this
> 
> 2. Found 241+ results in the library's RecordedBooks collection
> "+" ?

This is a function of the values returned from the API, unfortunate but not in our control

> 
> 3. /opac-tmpl/lib/jquery/plugins/themes/classic/throbber.gif
> must be using [% theme %]/[% interface %]

This stil needs to be fixed, these lines are in the js file so don't have template variables
> 
> 4. In the checkout list there is a link .epub (below "downloads"), but it
> downloads a .acsm file (??) Is it expected?

Yes, due to DRM you must get your epub via the adobe app which  uses acsm
> 
> 5. expires.toLocaleString() does not work what you are expecting, the date
> is always displayed in the same format (do not depend on dateformat)

This is what exists in the code for overdrive currently I believe - fix on a future bug?


> 
> 6. https://screenshots.firefox.com/C5OmithULkvMQtTe/catalogue.kohadev.org
> "Check out" and "Place hold" button are too close and not well styled (not
> blocking)

I will ask Owen for an eye

> 
> 7. Place hold, hit cgi-bin/koha/opac-user.pl, cancel the hold. The hold is
> still listed (not blocking)

Let's leave as improvement
> 
> 8. /cgi-bin/koha/opac-recordedbooks-search.pl?q=d
> click on an image,
> https://carnegiestoutia.oneclickdigital.com/#titles/9781470360603 => "Your
> connection is not secure", awesome...

This is a configuratio issue - testing link used *.oneclickdigital.com - should now use *.rbdigital.com
Comment 116 Nick Clemens 2018-09-21 19:01:43 UTC
Created attachment 79247 [details] [review]
Bug 17602: Move spinner location to include file
Comment 117 Jonathan Druart 2018-09-30 23:25:27 UTC
Created attachment 79668 [details] [review]
Bug 17602: Koha::ExternalContent->koha_patron() will retutn undef rather than die if user not logged in

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 118 Jonathan Druart 2018-09-30 23:25:35 UTC
Created attachment 79669 [details] [review]
Bug 17602: RecordedBooks Integration to Koha

This patch introduces the required sysprefs and Koha::ExternalContent::RecordedBooks

Koha::ExternalContent::RecordedBooks - a wrapper around
WebService::ILS::RecordedBooks::PartnerPatron

RecordedBooks* sysprefs

Nothing functional to test with this patch yet.
But you can run the tests that come with it

 t/db_dependent/Koha_ExternalContent_RecordedBooks.t

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 119 Jonathan Druart 2018-09-30 23:25:43 UTC
Created attachment 79670 [details] [review]
Bug 17602: OPAC integration of RecordedBooks

Add RecordedBooks to the users page in the public interface

To Test:
1/ Apply all 3 patches
2/ Set the sysprefs to valid values (you will need a test account with RecordedBooks)
3/ Try a search
4/ Login to the OPAC, try to place a hold, or check an item out
5/ Check the opac-user page, see if your items are showing on the oneclickdigital tab

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 120 Jonathan Druart 2018-09-30 23:25:51 UTC
Created attachment 79671 [details] [review]
Bug 17602 follow-up: t/Koha_ExternalContent_RecordedBooks.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 121 Jonathan Druart 2018-09-30 23:26:00 UTC
Created attachment 79672 [details] [review]
Bug 17602: Ease translation

Use String.format() to make translation easier

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 122 Jonathan Druart 2018-09-30 23:26:10 UTC
Created attachment 79673 [details] [review]
Bug 17602: Handle '60+' if there are more than 60 results

The service returns "60+" if there are more than 60 results.
Without this patch the "Found %s results in RecordedBooks collection"
line is removed.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 123 Jonathan Druart 2018-09-30 23:26:18 UTC
Created attachment 79674 [details] [review]
Bug 17602: Adapt test plan to make the tests pass

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 124 Jonathan Druart 2018-09-30 23:26:25 UTC
Created attachment 79675 [details] [review]
Bug 17602: Fix few minor QA issues

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 125 Jonathan Druart 2018-09-30 23:26:30 UTC
Created attachment 79676 [details] [review]
Bug 17602: Translations rework

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 126 Jonathan Druart 2018-09-30 23:26:36 UTC
Created attachment 79677 [details] [review]
Bug 17602: fix RecordedBooks results pagination

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 127 Jonathan Druart 2018-09-30 23:26:44 UTC
Created attachment 79678 [details] [review]
Bug 17602: Removed unused param to RecordedBooks API

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 128 Jonathan Druart 2018-09-30 23:26:52 UTC
Created attachment 79679 [details] [review]
Bug 17602: (follow-up) Adjust js paths to use version add spinner

Also included recompiled version of opac.css that we forgot.

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 129 Jonathan Druart 2018-09-30 23:26:59 UTC
Created attachment 79680 [details] [review]
Bug 17602: Use Asset TT plugin and fix indentation

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 130 Jonathan Druart 2018-09-30 23:27:06 UTC
Created attachment 79681 [details] [review]
Bug 17602: (QA follow-up) Fix tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 131 Jonathan Druart 2018-09-30 23:27:13 UTC
Created attachment 79682 [details] [review]
Bug 17602: Move spinner location to include file

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 132 Nick Clemens 2018-10-01 16:42:41 UTC
Awesome work all!

Pushed to master for 18.11
Comment 133 Jonathan Druart 2018-10-01 17:19:34 UTC
Created attachment 79752 [details] [review]
Bug 17602: Add missing html filters
Comment 134 Nick Clemens 2018-10-02 00:12:39 UTC
(In reply to Jonathan Druart from comment #133)
> Created attachment 79752 [details] [review] [review]
> Bug 17602: Add missing html filters

Followup pushed to master, thanks Jonathan!
Comment 135 Martin Renvoize 2018-10-08 08:02:42 UTC
New Feature, will not be backported to 18.05.x series.
Comment 136 Jonathan Druart 2018-10-18 12:37:08 UTC
From the tests:

t/Koha_ExternalContent_RecordedBooks.t:        skip "no RecordedBooks partner credentials", 1 unless $ENV{RECORDEDBOOKS_TEST_LIBRARY_ID};
t/Koha_ExternalContent_RecordedBooks.t:        t::lib::Mocks::mock_preference('RecordedBooksLibraryID', $ENV{RECORDEDBOOKS_TEST_LIBRARY_ID});

We never set this ENV var, are there data we could use to make it pass?
See bug 21567.
Comment 137 Srdjan Jankovic 2018-10-18 23:15:34 UTC
It is a huge pain, you have to have their test account. Which includes a password, and I'm not sure how is that dealt with in automated tests scenarios. The test is more or less a repeat of what is done in WebService::ILS itself, just through Koha. The main reason why I made WebService::ILS outside Koha is to cheat a bit - I could tests the whole structure locally and relieve that pain to some extent.