bug 10292: improve fallback logic for picking XSLT

This patch fixes a bug whereby XSLT files from the
prog theme would be used (for English OPACs and staff
interfaces) even if the user had created and enabled a
custom theme that provided override XSLT files.

This patch provides a clearer implementation of the fallback
logic and adds test cases.

To reproduce the bug:

[1] Set OPACXSLTDetailsDisplay to 'default' and English as the OPAC
    language.
[2] Create a new OPAC theme, including copying the XSLT files.
[3] Set opactheme to the new theme.
[4] Make a change to koha-tmpl/opac-tmpl/NEWTHEME/en/xslt/MARC21slim2OPACDetail.xsl
[5] View a bib record in the OPAC. The change made in the previous step
    is not reflected.

To test after applying the patch:

[6] Reload the bib record in the OPAC. The change made in step 4 should
    now be reflected.
[7] (To be thorough) Go through the test plan for bug 8947
    and verify that there is no regression.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>

diff --git a/C4/XSLT.pm b/C4/XSLT.pm
index 6951674..53f07d3 100644 (file)
--- a/C4/XSLT.pm
+++ b/C4/XSLT.pm
@@ -136,6 +136,26 @@ sub getAuthorisedValues4MARCSubfields {
 
 my $stylesheet;
 
+sub _get_best_default_xslt_filename {
+    my ($htdocs, $theme, $lang, $base_xslfile) = @_;
+
+    my @candidates = (
+        "$htdocs/$theme/$lang/xslt/${base_xslfile}", # exact match
+        "$htdocs/$theme/en/xslt/${base_xslfile}",    # if not, preferred theme in English
+        "$htdocs/prog/$lang/xslt/${base_xslfile}",   # if not, 'prog' theme in preferred language
+        "$htdocs/prog/en/xslt/${base_xslfile}",      # otherwise, prog theme in English; should always
+                                                     # exist
+    );
+    my $xslfilename;
+    foreach my $filename (@candidates) {
+        $xslfilename = $filename;
+        if (-f $filename) {
+            last; # we have a winner!
+        }
+    }
+    return $xslfilename;
+}
+
 sub XSLTParse4Display {
     my ( $biblionumber, $orig_record, $xslsyspref, $fixamps, $hidden_items ) = @_;
     my $xslfilename = C4::Context->preference($xslsyspref);
@@ -165,10 +185,7 @@ sub XSLTParse4Display {
             $xslfile = C4::Context->preference('marcflavour') .
                        "slim2OPACResults.xsl";
         }
-        $xslfilename = "$htdocs/$theme/$lang/xslt/$xslfile";
-        $xslfilename = "$htdocs/$theme/en/xslt/$xslfile" unless ( $lang ne 'en' && -f $xslfilename );
-        $xslfilename = "$htdocs/prog/$lang/xslt/$xslfile" unless ( -f $xslfilename );
-        $xslfilename = "$htdocs/prog/en/xslt/$xslfile" unless ( $lang ne 'en' && -f $xslfilename );
+        $xslfilename = _get_best_default_xslt_filename($htdocs, $theme, $lang, $xslfile);
     }
 
     if ( $xslfilename =~ m/\{langcode\}/ ) {

diff --git a/t/XSLT.t b/t/XSLT.t
index 1f78eb9..64cd597 100755 (executable)
--- a/t/XSLT.t
+++ b/t/XSLT.t
@@ -6,9 +6,44 @@
 use strict;
 use warnings;
 
-use Test::More tests => 1;
+use Test::More tests => 8;
+use File::Temp;
+use File::Path qw/make_path/;
 
 BEGIN {
         use_ok('C4::XSLT');
 }
 
+my $dir = File::Temp->newdir();
+my @themes = ('prog', 'test');
+my @langs = ('en', 'es-ES');
+
+# create temporary files to be tested later
+foreach my $theme (@themes) {
+    foreach my $lang (@langs) {
+        make_path("$dir/$theme/$lang/xslt");
+        open my $fh, '>', "$dir/$theme/$lang/xslt/my_file.xslt";
+        print $fh "Theme $theme, language $lang";
+        close $fh;
+    }
+}
+
+sub find_and_slurp {
+    my ($dir, $theme, $lang) = @_;
+
+    my $filename = C4::XSLT::_get_best_default_xslt_filename($dir, $theme, $lang, 'my_file.xslt');
+    open my $fh, '<', $filename;
+    my $str = <$fh>;
+    close $fh;
+    return $str;
+}
+
+# These tests verify that we're finding the right XSLT file when present,
+# and falling back to the right XSLT file when an exact match is not present.
+is(find_and_slurp($dir, 'test', 'en'   ), 'Theme test, language en',    'Found test/en');
+is(find_and_slurp($dir, 'test', 'es-ES'), 'Theme test, language es-ES', 'Found test/es-ES');
+is(find_and_slurp($dir, 'prog', 'en',  ), 'Theme prog, language en',    'Found test/en');
+is(find_and_slurp($dir, 'prog', 'es-ES'), 'Theme prog, language es-ES', 'Found test/es-ES');
+is(find_and_slurp($dir, 'test', 'fr-FR'), 'Theme test, language en',    'Fell back to test/en for test/fr-FR');
+is(find_and_slurp($dir, 'nope', 'es-ES'), 'Theme prog, language es-ES', 'Fell back to prog/es-ES for nope/es-ES');
+is(find_and_slurp($dir, 'nope', 'fr-FR'), 'Theme prog, language en',    'Fell back to prog/en for nope/fr-FR');