Bug 22602 - OverDrive circulation integration is broken when user is referred to Koha from another site
Summary: OverDrive circulation integration is broken when user is referred to Koha fro...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 21078
Blocks:
  Show dependency treegraph
 
Reported: 2019-03-28 11:48 UTC by Nick Clemens
Modified: 2020-06-04 20:34 UTC (History)
5 users (show)

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


Attachments
Bug 22602: Fix OverDrive circulation when coming from another site (2.09 KB, patch)
2019-04-03 12:24 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22602: Fix OverDrive circulation when coming from another site (2.15 KB, patch)
2019-08-29 18:50 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22602: Fix OverDrive circulation when coming from another site (2.16 KB, patch)
2019-09-06 11:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23272: (RM followup) Fix failing tests (1.60 KB, patch)
2019-09-13 20:08 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 22602: Fix OverDrive circulation when coming from another site (2.30 KB, patch)
2019-09-27 06:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2019-03-28 11:48:06 UTC
To recreate:
1 - Have a working OverDrive account and setup
2 - Put a link to your koha site on another site and follow the link to open Koha in a new tab.window
3 - Do an overdrive search
4 - Note that copies listed as available are correct, however, all items only have a place hold button

The issue is in the code below - if we came from an outside site we cannot access p and we 'return' which never instantiates checkout_popup which is needed   for creating the checkout buttons later

 87     var checkout_popup = null;
 88     $( document ).ready(function() {
 89         var p = window.opener;
 90         if (p) {
 91             try { cb = p.refresh_overdrive_account_details;}
 92             catch(err){ return; } //Catch error if opener is not accessible
 93             if (cb) {
 94                 cb();
 95             } else {
 96                 p.location.reload();
 97             }
 98             window.close();
 99         }
100         checkout_popup = $("#overdrive-checkout");
101         $("#overdrive-login-form").submit(function(e){
102             e.preventDefault();
103             $("#overdrive-login").modal('hide');
104             var ODpassword = $("input[name='ODpassword']").val();
105             login( ODpassword );
106         });
107     });
Comment 1 Nick Clemens 2019-04-03 12:24:20 UTC
Created attachment 87352 [details] [review]
Bug 22602: Fix OverDrive circulation when coming from another site

On bug 21078 we caught an error and returned, this makes us skip some variable setting that we need.

This patch moves that code into its own function.

To test:
0 - Have OverDrive circulation setup and working
1 - Place a link to your kohadev installation on another site
2 - Follow that link to the opac
3 - Note all items show as 'Place hold' even if available copies
4 - Apply patch
5 - Repeat 1&2
6 - Note the items availability shows correctly
Comment 2 Liz Rea 2019-05-11 02:56:28 UTC
I was having trouble replicating the original problem, does it still happen for you? 

Liz
Comment 3 Nick Clemens 2019-08-29 18:50:13 UTC
Created attachment 92501 [details] [review]
Bug 22602: Fix OverDrive circulation when coming from another site

On bug 21078 we caught an error and returned, this makes us skip some variable setting that we need.

This patch moves that code into its own function.

To test:
0 - Have OverDrive circulation setup and working
1 - Place a link to your kohadev installation on another site
2 - Follow that link to the opac
3 - Note all items show as 'Place hold' even if available copies
4 - Apply patch
5 - Repeat 1&2
6 - Note the items availability shows correctly

Signed-off-by: Elizabeth Quinn <EQuinn@chplnj.org>
Comment 4 Nick Clemens 2019-08-29 18:50:51 UTC
(In reply to Liz Rea from comment #2)
> I was having trouble replicating the original problem, does it still happen
> for you? 
> 
> Liz

Yes, we still have libraries exhibiting this issue
Comment 5 Katrin Fischer 2019-09-01 21:45:51 UTC
Is there any way to test this without an Overdrive account? Or way to get one for testing purposes?
Comment 6 Marcel de Rooy 2019-09-06 06:00:14 UTC
From: root <root@f1ebe1bec408>

You might wanna fix that too ;)
Comment 7 Nick Clemens 2019-09-06 11:14:08 UTC
Created attachment 92633 [details] [review]
Bug 22602: Fix OverDrive circulation when coming from another site

On bug 21078 we caught an error and returned, this makes us skip some variable setting that we need.

This patch moves that code into its own function.

To test:
0 - Have OverDrive circulation setup and working
1 - Place a link to your kohadev installation on another site
2 - Follow that link to the opac
3 - Note all items show as 'Place hold' even if available copies
4 - Apply patch
5 - Repeat 1&2
6 - Note the items availability shows correctly

Signed-off-by: Elizabeth Quinn <EQuinn@chplnj.org>
Comment 8 Bouzid Fergani 2019-09-13 20:08:56 UTC
Created attachment 92821 [details] [review]
Bug 23272: (RM followup) Fix failing tests

This bug introduced a regression whereby if any authorized value has no
branch limitations then the authorized value administration page would
fail to display at all.

A simple ternary corrects the issue, but I'm now wondering about the
logic of returning 'undef' from 'get_branch_limits'.. perhaps an empty
(or full) resultset might be a better option to prevent us from
requireing ternaries before calling chained methods on such objects.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 9 Marcel de Rooy 2019-09-27 06:32:44 UTC
(In reply to Bouzid Fergani from comment #8)
> Created attachment 92821 [details] [review] [review]
> Bug 23272: (RM followup) Fix failing tests
> 
> This bug introduced a regression whereby if any authorized value has no
> branch limitations then the authorized value administration page would
> fail to display at all.
> 
> A simple ternary corrects the issue, but I'm now wondering about the
> logic of returning 'undef' from 'get_branch_limits'.. perhaps an empty
> (or full) resultset might be a better option to prevent us from
> requireing ternaries before calling chained methods on such objects.
> 
> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
> Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>

Bouzid: buy a new keyboard ;)
Comment 10 Marcel de Rooy 2019-09-27 06:39:30 UTC
Created attachment 93181 [details] [review]
Bug 22602: Fix OverDrive circulation when coming from another site

On bug 21078 we caught an error and returned, this makes us skip some variable setting that we need.

This patch moves that code into its own function.

To test:
0 - Have OverDrive circulation setup and working
1 - Place a link to your kohadev installation on another site
2 - Follow that link to the opac
3 - Note all items show as 'Place hold' even if available copies
4 - Apply patch
5 - Repeat 1&2
6 - Note the items availability shows correctly

Signed-off-by: Elizabeth Quinn <EQuinn@chplnj.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Looks good to me, but cannot test Overdrive.
Comment 11 Martin Renvoize 2019-09-27 12:59:27 UTC
Nice work!

Pushed to master for 19.11.00
Comment 12 Fridolin Somers 2019-10-08 07:01:45 UTC
Pushed to 19.05.x for 19.05.05
Comment 13 Lucas Gass 2019-10-17 20:03:59 UTC
backported to 18.11.x for 18.11.11