Bug 13473 - Plugins fail when loading templates
Summary: Plugins fail when loading templates
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 13639
Blocks:
  Show dependency treegraph
 
Reported: 2014-12-16 16:22 UTC by Bob Ewart
Modified: 2015-12-03 22:02 UTC (History)
9 users (show)

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


Attachments
koha error log (1.40 KB, text/plain)
2014-12-16 16:22 UTC, Bob Ewart
Details
Bug 13473 - Plugins fail in 3.18 (1.06 KB, patch)
2015-02-17 11:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED OFF] Bug 13473 - Plugins fail (1.10 KB, patch)
2015-02-17 12:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 13473 - Plugins fail (1.14 KB, patch)
2015-02-23 14:02 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 Bob Ewart 2014-12-16 16:22:05 UTC
Created attachment 34465 [details]
koha error log

This was working in 3.16.  I was able to enable plugins and upload the Kitchen-Sink plugin (see bug 13472 for the correct way to enable plugins.)

When trying to execute or configure the plugin on 3.18 I got:

Template process failed: file error - doc-head-open.inc: not found at /usr/share/koha/lib/C4/Templates.pm line 129.

The koha error log is attached.

It fails in 3.18 whether installed via command line (openSUSE 13.1) or apt-get (Debian 7).
Comment 1 Pete Edwards 2015-01-15 11:13:11 UTC
I am getting the same issue - is there a workaround?
Comment 2 Pete Edwards 2015-01-15 15:10:19 UTC
I'm not familiar enough with Koha to offer a patch but here is a workaround that is OK for me.

I had to modify C4::Templates::themelanguage(). For normal pages this is fine but plugin templates have an absolute path (in $tmpl) so the code to check for a template in the correct language completely fails:
    for my $theme (@themes) {
        if ( -e "$htdocs/$theme/$lang/$where/$tmpl" ) {
            return ( $theme, $lang, uniq( \@themes ) );
        }
    }
Instead, we drop through both loops and come out of the function without any return. This seems pretty bad practice. At the end of the function I added:
    if ($tmpl =~ /^\//) {
        return ( $themes[0], $lang, uniq( \@themes ) );
    }
This probably doesn't fix every possible situation, nor do I know about themes but it might help whoever does try and fix the problem.

As an aside, it would be better to run uniq on the themes array before checking for files existing so it reduces the number of times it may try the wrong location.
Comment 3 Bob Ewart 2015-01-16 02:55:28 UTC
That worked for me on installing KitchenSink.  I have some other plugins that I will test.

Thanks
Comment 4 Zeno Tajoli 2015-01-29 13:33:15 UTC
About this topic see also bug 13639
Comment 5 Zeno Tajoli 2015-01-29 13:51:02 UTC
The bare minumum to create a patch
(reading http://wiki.koha-community.org/wiki/Version_Control_Using_Git)

1)Install and setup git
2)git clone git://git.koha-community.org/koha.git kohaclone
3)'kohaclone' will be a new dir
4)cd kohaclone
5)Do your change with vi
6)git commit -a 
7)git format-patch origin 
8)Uplad the created file into bugzilla

It is not the best workflow (git clone is quite long and dir kohaclone is around 1 Gb), but it is the easiest.

To use git in better way you need to study it.
Fro example from http://git-scm.com/documentation
Comment 6 Nick Clemens 2015-02-16 20:25:33 UTC
I am seeing this on master as well
Comment 7 Kyle M Hall 2015-02-17 11:25:48 UTC
Excellent work! I was able to create a patch based on your fix here!

(In reply to Pete Edwards from comment #2)
> I'm not familiar enough with Koha to offer a patch but here is a workaround
> that is OK for me.
> 
> I had to modify C4::Templates::themelanguage(). For normal pages this is
> fine but plugin templates have an absolute path (in $tmpl) so the code to
> check for a template in the correct language completely fails:
>     for my $theme (@themes) {
>         if ( -e "$htdocs/$theme/$lang/$where/$tmpl" ) {
>             return ( $theme, $lang, uniq( \@themes ) );
>         }
>     }
> Instead, we drop through both loops and come out of the function without any
> return. This seems pretty bad practice. At the end of the function I added:
>     if ($tmpl =~ /^\//) {
>         return ( $themes[0], $lang, uniq( \@themes ) );
>     }
> This probably doesn't fix every possible situation, nor do I know about
> themes but it might help whoever does try and fix the problem.
> 
> As an aside, it would be better to run uniq on the themes array before
> checking for files existing so it reduces the number of times it may try the
> wrong location.
Comment 8 Kyle M Hall 2015-02-17 11:26:12 UTC Comment hidden (obsolete)
Comment 9 Nick Clemens 2015-02-17 12:12:57 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2015-02-23 14:02:15 UTC
Created attachment 36120 [details] [review]
Bug 13473 - Plugins fail

When trying to execute or configure the plugin on 3.18 I got:

Template process failed: file error - doc-head-open.inc: not found at
/usr/share/koha/lib/C4/Templates.pm line 129.

Test Plan:
1) Install the Kitchen Sink plugin
2) Go to the configuration page of the plugin
3) Note the template processing error
4) Apply this patch
5) Refresh the page
6) Note the page now loads

Signed-off-by: Nick <nick@quecheelibrary.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 11 Tomás Cohen Arazi 2015-02-24 18:15:24 UTC
Patch pushed to master.

Thanks Kyle!
Comment 12 Chris Cormack 2015-03-12 06:59:41 UTC
Pushed to 3.18.x will be in 3.18.5
Comment 13 Héctor 2015-03-16 14:23:23 UTC
This works, but the system language is shown only in English, although it is set to Spanish
Comment 14 Tomás Cohen Arazi 2015-03-16 22:07:53 UTC
(In reply to Héctor from comment #13)
> This works, but the system language is shown only in English, although it is
> set to Spanish

Could you please fill a new bug for that? Put this one in the 'see also'.
Comment 15 Héctor 2015-03-18 11:39:02 UTC
(In reply to Tomás Cohen Arazi from comment #14)
> (In reply to Héctor from comment #13)
> > This works, but the system language is shown only in English, although it is
> > set to Spanish
> 
> Could you please fill a new bug for that? Put this one in the 'see also'.

But the error occurs when you apply this patch.

I "solved" this by replacing:

    #Otherwise return theme/'en', last resort fallback/'en'
    for my $theme (@themes) {
        if ( -e "$htdocs/$theme/en/$where/$tmpl" ) {
            return ( $theme, 'en', uniq( \@themes ) );
        }
    }

by

    # Otherwise return theme/'en', last resort fallback/'en'
    for my $theme (@themes) {
        if ( -e "$htdocs/$theme/es-ES/$where/$tmpl" ) {
            return ( $theme, 'es-ES', uniq( \@themes ) );
        }
    }
Comment 16 Tomás Cohen Arazi 2015-03-18 12:38:36 UTC
(In reply to Héctor from comment #15)
> (In reply to Tomás Cohen Arazi from comment #14)
> > (In reply to Héctor from comment #13)
> > > This works, but the system language is shown only in English, although it is
> > > set to Spanish
> > 
> > Could you please fill a new bug for that? Put this one in the 'see also'.
> 
> But the error occurs when you apply this patch.
> 
> I "solved" this by replacing:
> 
>     #Otherwise return theme/'en', last resort fallback/'en'
>     for my $theme (@themes) {
>         if ( -e "$htdocs/$theme/en/$where/$tmpl" ) {
>             return ( $theme, 'en', uniq( \@themes ) );
>         }
>     }
> 
> by
> 
>     # Otherwise return theme/'en', last resort fallback/'en'
>     for my $theme (@themes) {
>         if ( -e "$htdocs/$theme/es-ES/$where/$tmpl" ) {
>             return ( $theme, 'es-ES', uniq( \@themes ) );
>         }
>     }

That part of the code deals with the fallback language, in the situation the required file doesn't exist in the requested language. So it is ok for that to fallback to english.

Please fill a new bug if you think/see that the plugins don't respect the selected language. And please specify the steps to reproduce it. On a new bug report.