Bug 430010: Re-work the template hooks system so that template hooks always live in template/<lang>/hook/, both for extensions and for the base Bugzilla template/ directory.

Patch by Max Kanat-Alexander <mkanat@bugzilla.org> (module owner) a=mkanat

4 files changed, 153 insertions, 90 deletions

diff --git a/Bugzilla/Install/Util.pm b/Bugzilla/Install/Util.pm
index effb39ee8..d3fb4e5f8 100644
--- a/Bugzilla/Install/Util.pm
+++ b/Bugzilla/Install/Util.pm
@@ -41,6 +41,8 @@ our @EXPORT_OK = qw(
     install_string
     include_languages
     template_include_path
+    template_base_directories
+    template_lang_directories
     vers_cmp
     get_console_locale
     init_console
@@ -206,28 +208,59 @@ sub include_languages {
 
     return @usedlanguages;
 }
+
+# Used by template_include_path and Bugzilla::Template::Plugin::Hook.
+sub template_lang_directories {
+    my ($languages, $templatedir, $subdir_name) = @_;
     
-sub template_include_path {
-    my @usedlanguages = include_languages(@_);
-    # Now, we add template directories in the order they will be searched:
-    
+    my @add;
+    my $project = bz_locations->{'project'};
+    if ($subdir_name) {
+        @add = ("$subdir_name.custom", $subdir_name);
+        unshift(@add, "$subdir_name.$project") if $project;
+    }
+    else {
+        @add = ("custom", "default");
+        unshift(@add, $project) if $project;
+    }
+    my @result;
+    foreach my $lang (@$languages) {
+        foreach my $dir (@add) {
+            my $full_dir = "$templatedir/$lang/$dir";
+            if (-d $full_dir) {
+                trick_taint($full_dir);
+                push(@result, $full_dir);
+            }
+        }
+    }
+    return @result;
+}
+
+# Used by template_include_path and Bugzilla::Template::Plugin::Hook.
+sub template_base_directories {
     # First, we add extension template directories, because extension templates
     # override standard templates. Extensions may be localized in the same way
     # that Bugzilla templates are localized.
-    my @include_path;
+    my @template_dirs;
     my @extensions = glob(bz_locations()->{'extensionsdir'} . "/*");
     foreach my $extension (@extensions) {
-        next if -e "$extension/disabled";
-        foreach my $lang (@usedlanguages) {
-            _add_language_set(\@include_path, $lang, "$extension/template");
-        }
+        next if (-e "$extension/disabled" or !-d "$extension/template");
+        push(@template_dirs, "$extension/template");
     }
-    
-    # Then, we add normal template directories, sorted by language.
-    foreach my $lang (@usedlanguages) {
-        _add_language_set(\@include_path, $lang);
+    push(@template_dirs, bz_locations()->{'templatedir'});
+    return \@template_dirs;
+}
+
+sub template_include_path {
+    my @used_languages = include_languages(@_);
+    # Now, we add template directories in the order they will be searched:
+    my $template_dirs = template_base_directories(); 
+
+    my @include_path;
+    foreach my $template_dir (@$template_dirs) {
+        push(@include_path,
+             template_lang_directories(\@used_languages, $template_dir));
     }
-    
     return \@include_path;
 }
 
@@ -289,24 +322,6 @@ sub _get_string_from_file {
     return $strings{$string_id};
 }
 
-# Used by template_include_path.
-sub _add_language_set {
-    my ($array, $lang, $templatedir) = @_;
-    
-    $templatedir ||= bz_locations()->{'templatedir'};
-    my @add = ("$templatedir/$lang/custom", "$templatedir/$lang/default");
-    
-    my $project = bz_locations->{'project'};
-    unshift(@add, "$templatedir/$lang/$project") if $project;
-    
-    foreach my $dir (@add) {
-        if (-d $dir) {
-            trick_taint($dir);
-            push(@$array, $dir);
-        }
-    }
-}
-
 # Make an ordered list out of a HTTP Accept-Language header (see RFC 2616, 14.4)
 # We ignore '*' and <language-range>;q=0
 # For languages with the same priority q the order remains unchanged.
diff --git a/Bugzilla/Template/Plugin/Hook.pm b/Bugzilla/Template/Plugin/Hook.pm
index 99ece0880..e74363461 100644
--- a/Bugzilla/Template/Plugin/Hook.pm
+++ b/Bugzilla/Template/Plugin/Hook.pm
@@ -20,93 +20,98 @@
 # Contributor(s): Myk Melez <myk@mozilla.org>
 #                 Zach Lipton <zach@zachlipton.com>
 #                 Elliotte Martin <everythingsolved.com>
-#
+#                 Max Kanat-Alexander <mkanat@bugzilla.org>
 
 package Bugzilla::Template::Plugin::Hook;
-
 use strict;
+use base qw(Template::Plugin);
 
 use Bugzilla::Constants;
-use Bugzilla::Install::Util qw(include_languages);
-use Bugzilla::Template;
+use Bugzilla::Install::Util qw(include_languages template_base_directories
+                               template_lang_directories);
 use Bugzilla::Util;
 use Bugzilla::Error;
 use File::Spec;
 
-use base qw(Template::Plugin);
-
-sub load {
-    my ($class, $context) = @_;
-    return $class;
-}
 
 sub new {
     my ($class, $context) = @_;
     return bless { _CONTEXT => $context }, $class;
 }
 
+sub _context { return $_[0]->{_CONTEXT} }
+
 sub process {
     my ($self, $hook_name, $template) = @_;
-    $template ||= $self->{_CONTEXT}->stash->{component}->{name};
-
-    my @hooks;
+    my $context = $self->_context();
+    $template ||= $context->stash->{component}->{name};
 
     # sanity check:
     if (!$template =~ /[\w\.\/\-_\\]+/) {
-        ThrowCodeError('template_invalid', { name => $template});
+        ThrowCodeError('template_invalid', { name => $template });
     }
 
-    # also get extension hook files that live in extensions/:
-    # parse out the parts of the template name
-    my ($vol, $subpath, $filename) = File::Spec->splitpath($template);
-    $subpath = $subpath || '';
-    $filename =~ m/(.*)\.(.*)\.tmpl/;
-    my $templatename = $1;
+    my (undef, $path, $filename) = File::Spec->splitpath($template);
+    $path ||= '';
+    $filename =~ m/(.+)\.(.+)\.tmpl$/;
+    my $template_name = $1;
     my $type = $2;
-    # munge the filename to create the extension hook filename:
-    my $extensiontemplate = $subpath.'/'.$templatename.'-'.$hook_name.'.'.$type.'.tmpl';
-    my @extensions = glob(bz_locations()->{'extensionsdir'} . "/*");
-    my @usedlanguages = include_languages({use_languages => Bugzilla->languages});
-    foreach my $extension (@extensions) {
-        next if -e "$extension/disabled";
-        foreach my $language (@usedlanguages) {
-            my $file = $extension.'/template/'.$language.'/'.$extensiontemplate;
+
+    # Munge the filename to create the extension hook filename
+    my $extension_template = "$path/$template_name-$hook_name.$type.tmpl";
+
+    my $template_sets = _template_hook_include_path();
+
+    my @hooks;
+    foreach my $dir_set (@$template_sets) {
+        foreach my $lang_dir (@$dir_set) {
+            my $file = File::Spec->catdir($lang_dir, $extension_template);
             if (-e $file) {
-                # tt is stubborn and won't take a template file not in its
-                # include path, so we open a filehandle and give it to process()
-                # so the hook gets invoked:
-                open (my $fh, $file);
+                # TT won't take a template file not in its include path,
+                # so we open a filehandle and give it to process()
+                # instead of the file name.
+                open (my $fh, '<', $file) or die "$file: $!";
                 push(@hooks, $fh);
+                # Don't run the hook for more than one language.
+                last;
             }
         }
     }
 
-    my $paths = $self->{_CONTEXT}->{LOAD_TEMPLATES}->[0]->paths;
-    
-    # we keep this too since you can still put hook templates in 
-    # template/en/custom/hook
-    foreach my $path (@$paths) {
-        my @files = glob("$path/hook/$template/$hook_name/*.tmpl");
-
-        # Have to remove the templates path (INCLUDE_PATH) from the
-        # file path since the template processor auto-adds it back.
-        @files = map($_ =~ /^$path\/(.*)$/ ? $1 : {}, @files);
-
-        # Add found files to the list of hooks, but removing duplicates,
-        # which can happen when there are identical hooks or duplicate
-        # directories in the INCLUDE_PATH (the latter probably being a TT bug).
-        foreach my $file (@files) {
-            push(@hooks, $file) unless grep($file eq $_, @hooks);
-        }
-    }
-
     my $output;
-    foreach my $hook (@hooks) {
-        $output .= $self->{_CONTEXT}->process($hook);
+    foreach my $hook_fh (@hooks) {
+        $output .= $context->process($hook_fh);
+        close($hook_fh);
     }
     return $output;
 }
 
+sub _template_hook_include_path {
+    my $cache = Bugzilla->request_cache;
+    my $language = $cache->{language} || '';
+    my $cache_key = "template_plugin_hook_include_path_$language";
+    return $cache->{$cache_key} if defined $cache->{$cache_key};
+    
+    my @used_languages = include_languages({
+        use_languages => Bugzilla->languages,
+        only_language => $language });
+    my $template_dirs = template_base_directories();
+
+    # We create an array of arrayrefs, with each arrayref being a single
+    # extension's "language" directories. In addition to the extensions/
+    # directory, this also includes a set for the base template/ directory. 
+    my @template_sets;
+    foreach my $template_dir (@$template_dirs) {
+        my @language_dirs = template_lang_directories(\@used_languages, 
+                                                      $template_dir, 'hook');
+        if (scalar @language_dirs) {
+            push(@template_sets, \@language_dirs);
+        }
+    }
+    $cache->{$cache_key} = \@template_sets;
+    return $cache->{$cache_key};
+}
+
 1;
 
 __END__
@@ -165,8 +170,4 @@ Output from processing template extension.
 
 L<Template::Plugin>
 
-L<http://www.bugzilla.org/docs/tip/html/customization.html>
-
-L<http://bugzilla.mozilla.org/show_bug.cgi?id=229658>
-
-L<http://bugzilla.mozilla.org/show_bug.cgi?id=298341>
+L<http://wiki.mozilla.org/Bugzilla:Writing_Extensions>
diff --git a/extensions/example/template/en/hook/admin/sanitycheck/messages-statuses.html.tmpl b/extensions/example/template/en/hook/admin/sanitycheck/messages-statuses.html.tmpl
new file mode 100644
index 000000000..8a825e57c
--- /dev/null
+++ b/extensions/example/template/en/hook/admin/sanitycheck/messages-statuses.html.tmpl
@@ -0,0 +1,35 @@
+[%# -*- Mode: perl; indent-tabs-mode: nil -*-
+  #
+  # The contents of this file are subject to the Mozilla Public
+  # License Version 1.1 (the "License"); you may not use this file
+  # except in compliance with the License. You may obtain a copy of
+  # the License at http://www.mozilla.org/MPL/
+  #
+  # Software distributed under the License is distributed on an "AS
+  # IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+  # implied. See the License for the specific language governing
+  # rights and limitations under the License.
+  #
+  # The Original Code is the Bugzilla Example Plugin.
+  #
+  # The Initial Developer of the Original Code is ITA Software
+  # Portions created by the Initial Developer are Copyright (C) 2009
+  # the Initial Developer. All Rights Reserved.
+  #
+  # Contributor(s): Bradley Baetz <bbaetz@everythingsolved.com>
+  #%]
+
+[% IF    san_tag == "example_check_au_user" %]
+  <em>EXAMPLE PLUGIN</em> - Checking for non-Australian users.
+[% ELSIF san_tag == "example_check_au_user_alert" %]
+  User &lt;[% login FILTER html %]&gt; isn't Australian.
+  [% IF user.in_group('editusers') %]
+    <a href="editusers.cgi?id=[% userid FILTER none %]">Edit this user</a>.
+  [% END %]
+[% ELSIF san_tag == "example_check_au_user_prompt" %]
+  <a href="sanitycheck.cgi?example_repair_au_user=1">Fix these users</a>.
+[% ELSIF san_tag == "example_repair_au_user_start" %]
+  <em>EXAMPLE PLUGIN</em> - OK, would now make users Australian.
+[% ELSIF san_tag == "example_repair_au_user_end" %]
+  <em>EXAMPLE PLUGIN</em> - Users would now be Australian.
+[% END %]
diff --git a/extensions/example/template/en/hook/global/user-error-errors.html.tmpl b/extensions/example/template/en/hook/global/user-error-errors.html.tmpl
new file mode 100644
index 000000000..df5a203dd
--- /dev/null
+++ b/extensions/example/template/en/hook/global/user-error-errors.html.tmpl
@@ -0,0 +1,12 @@
+[%# Note that error messages should generally be indented four spaces, like
+  # below, because when Bugzilla translates an error message into plain
+  # text, it takes four spaces off the beginning of the lines. 
+  #
+  # Note also that I prefixed my error name with "example", the name of my
+  # extension, so that I wouldn't conflict with other error names in
+  # Bugzilla or other extensions.
+  #%]
+[% IF error == "example_my_error" %]
+   [% title = "Example Error Title" %]
+   This is the error message! It contains <em>some html</em>.
+[% END %]