Bug 18160 - Error when OverDriveCirculation not enabled
Summary: Error when OverDriveCirculation not enabled
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 16034
Blocks:
  Show dependency treegraph
 
Reported: 2017-02-23 00:33 UTC by Nick Clemens
Modified: 2017-03-12 22:27 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 18160 - Error when OverDriveCirculation not enabled (1.77 KB, patch)
2017-02-23 00:37 UTC, Nick Clemens
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18160 - Error when OverDriveCirculation not enabled (1.83 KB, patch)
2017-02-23 01:33 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18160 - Error when OverDriveCirculation not enabled (1.87 KB, patch)
2017-02-23 01:35 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 18160: Follow up tweak if condition (1.12 KB, patch)
2017-02-23 16:49 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 18160 - Error when OverDriveCirculation not enabled (1.94 KB, patch)
2017-02-27 15:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18160: Follow up tweak if condition (1.20 KB, patch)
2017-02-27 15:23 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 Nick Clemens 2017-02-23 00:33:17 UTC
There is an "Internal server error" inserted into the summary tab of a patron's account when OverDriveCirculation is not enabled.
Comment 1 Nick Clemens 2017-02-23 00:37:12 UTC
Created attachment 60588 [details] [review]
Bug 18160 - Error when OverDriveCirculation not enabled

This patch only executes OverDrive circulation JS if
OverDriveCirculation pref is enabled

To test:
1 - Disable OverDriveCirculation
2 - Log in to opac and view summary tab
3 - Note 'Internal server error'
4 - Apply patch
5 - Repeat and note error is not present
Comment 2 Srdjan Jankovic 2017-02-23 01:33:13 UTC
Created attachment 60589 [details] [review]
[SIGNED-OFF] Bug 18160 - Error when OverDriveCirculation not enabled

This patch only executes OverDrive circulation JS if
OverDriveCirculation pref is enabled

To test:
1 - Disable OverDriveCirculation
2 - Log in to opac and view summary tab
3 - Note 'Internal server error'
4 - Apply patch
5 - Repeat and note error is not present

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Comment 3 M. Tompsett 2017-02-23 01:35:29 UTC
Created attachment 60590 [details] [review]
[SIGNED-OFF] Bug 18160 - Error when OverDriveCirculation not enabled

This patch only executes OverDrive circulation JS if
OverDriveCirculation pref is enabled

To test:
1 - Disable OverDriveCirculation
2 - Log in to opac and view summary tab
3 - Note 'Internal server error'
4 - Apply patch
5 - Repeat and note error is not present

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

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 4 Srdjan Jankovic 2017-02-23 01:36:02 UTC
I did not get an error, but still, fix stands, Overdrive login link should not be shown if Overdrive is not enabled.
Comment 5 M. Tompsett 2017-02-23 01:47:22 UTC
(In reply to Srdjan Jankovic from comment #4)
> I did not get an error, but still, fix stands, Overdrive login link should
> not be shown if Overdrive is not enabled.

I did get an error, and this corrects it for me.
Comment 6 M. Tompsett 2017-02-23 01:53:05 UTC
It should be noted that I did not have any overdrive system preferences even filled in. I suspect that enabling and disabling would make no difference if you had valid values set up.
Comment 7 Jonathan Druart 2017-02-23 16:24:35 UTC
I still get

  Could not compile /home/vagrant/kohaclone/opac/svc/overdrive: Can't locate WebService/ILS/OverDrive/Patron.pm

in the logs, why does it not appear in the dependency list?
Comment 8 Jonathan Druart 2017-02-23 16:28:32 UTC
Why is OverDriveCirculation pass to the template when we should use the TT plugin?
Comment 9 Jonathan Druart 2017-02-23 16:34:02 UTC
Failing QA because it seems that overdrive.js should not be included if pref is off (ie. shift the IF one line above).
Comment 10 M. Tompsett 2017-02-23 16:40:17 UTC
(In reply to Jonathan Druart from comment #7)
> I still get
> 
>   Could not compile /home/vagrant/kohaclone/opac/svc/overdrive: Can't locate
> WebService/ILS/OverDrive/Patron.pm
> 
> in the logs, why does it not appear in the dependency list?

Perhaps a follow up is needed to bug 16034?
Comment 11 M. Tompsett 2017-02-23 16:49:45 UTC
Created attachment 60622 [details] [review]
Bug 18160: Follow up tweak if condition

Based on comment #9, moved if condition.
Based on comment #8, tweaked condition to grab preference.
Comment 12 Jonathan Druart 2017-02-27 15:23:48 UTC
Created attachment 60718 [details] [review]
Bug 18160 - Error when OverDriveCirculation not enabled

This patch only executes OverDrive circulation JS if
OverDriveCirculation pref is enabled

To test:
1 - Disable OverDriveCirculation
2 - Log in to opac and view summary tab
3 - Note 'Internal server error'
4 - Apply patch
5 - Repeat and note error is not present

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

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2017-02-27 15:23:54 UTC
Created attachment 60719 [details] [review]
Bug 18160: Follow up tweak if condition

Based on comment #9, moved if condition.
Based on comment #8, tweaked condition to grab preference.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Kyle M Hall 2017-03-03 16:54:43 UTC
Pushed to master for 17.05, thanks Nick, Mark!
Comment 15 Katrin Fischer 2017-03-12 22:27:19 UTC
Depends on bug 16034 which is not in 16.11.x.