Bug 18160

Summary: Error when OverDriveCirculation not enabled
Product: Koha Reporter: Nick Clemens <nick>
Component: OPACAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: jonathan.druart, katrin.fischer, kyle, mtompset
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 16034    
Bug Blocks:    
Attachments: Bug 18160 - Error when OverDriveCirculation not enabled
[SIGNED-OFF] Bug 18160 - Error when OverDriveCirculation not enabled
[SIGNED-OFF] Bug 18160 - Error when OverDriveCirculation not enabled
Bug 18160: Follow up tweak if condition
Bug 18160 - Error when OverDriveCirculation not enabled
Bug 18160: Follow up tweak if condition

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 Mark 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 Mark 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 Mark 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 Mark 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 Mark 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.