Bug 12539 - PROG/CCSR deprecation: Remove hardcoded theme from C4/Templates.pm
Summary: PROG/CCSR deprecation: Remove hardcoded theme from C4/Templates.pm
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Bernardo Gonzalez Kriegel
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 11346
  Show dependency treegraph
 
Reported: 2014-07-07 20:16 UTC by Bernardo Gonzalez Kriegel
Modified: 2015-06-04 23:23 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12539: PROG/CCSR deprecation: Remove hardcoded theme from C4/Templates.pm (3.41 KB, patch)
2014-07-07 20:21 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 12539: PROG/CCSR deprecation: Remove hardcoded theme from C4/Templates.pm (6.37 KB, patch)
2014-07-31 22:13 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 12539: PROG/CCSR deprecation: Remove hardcoded theme from C4/Templates.pm (6.42 KB, patch)
2014-08-02 08:08 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 12539: Follow up to fix fallback theme (1.53 KB, patch)
2014-08-03 12:52 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 12539: PROG/CCSR deprecation: Remove hardcoded theme from C4/Templates.pm (6.39 KB, patch)
2014-08-28 06:42 UTC, Petter Goksøyr Åsen
Details | Diff | Splinter Review
Bug 12539: PROG/CCSR deprecation: Remove hardcoded theme from C4/Templates.pm (6.46 KB, patch)
2014-08-28 06:45 UTC, Petter Goksøyr Åsen
Details | Diff | Splinter Review
Bug 12539: Follow up to fix fallback theme (1.60 KB, patch)
2014-08-28 06:45 UTC, Petter Goksøyr Åsen
Details | Diff | Splinter Review
Bug 12539: PROG/CCSR deprecation: Remove hardcoded theme from C4/Templates.pm (6.52 KB, patch)
2014-08-29 15:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12539: Follow up to fix fallback theme (1.66 KB, patch)
2014-08-29 15:12 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 Bernardo Gonzalez Kriegel 2014-07-07 20:16:00 UTC
C4/Templates.pm has hardcoded theme
Comment 1 Bernardo Gonzalez Kriegel 2014-07-07 20:21:35 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2014-07-31 21:36:28 UTC
Yeah, this breaks themes, which im not sure we need to do. We can remove the ccsr and prog themes from the distribution, and change the syspref without breaking it for everyone who has their own custom theme going.

I don't think we need to remove the ability to have more than one theme just to deprecate prog and ccsr

I would not like to see this patch pushed as it stands

My 2 cents
Comment 3 Bernardo Gonzalez Kriegel 2014-07-31 22:13:45 UTC Comment hidden (obsolete)
Comment 4 Bernardo Gonzalez Kriegel 2014-07-31 22:15:13 UTC
(In reply to Chris Cormack from comment #2)
> Yeah, this breaks themes, which im not sure we need to do. We can remove the
> ccsr and prog themes from the distribution, and change the syspref without
> breaking it for everyone who has their own custom theme going.
> 
> I don't think we need to remove the ability to have more than one theme just
> to deprecate prog and ccsr
> 
Agreed. New version preserves the feature.
Comment 5 Chris Cormack 2014-08-02 08:08:05 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2014-08-03 10:42:15 UTC
I am not sure why, but I am running into problems here:

1) removed opac-shelves.tt from the German bootstrap translation
2) switched opac to German bootstrap theme
3) tried accessing opac-shelves.pl

Template process failed: file error - cssinclude: not found at /home/katrin/kohaclone/C4/Templates.pm line 131.

Also and just as a note: It looks to me like the CCSR theme in master is partially broken without the patch already, Firebug shows lots of errors about missing Javascript libraries.
Comment 7 Bernardo Gonzalez Kriegel 2014-08-03 12:52:16 UTC Comment hidden (obsolete)
Comment 8 Bernardo Gonzalez Kriegel 2014-08-03 12:59:20 UTC
(In reply to Katrin Fischer from comment #6)
> I am not sure why, but I am running into problems here:
> 
> 1) removed opac-shelves.tt from the German bootstrap translation
> 2) switched opac to German bootstrap theme
> 3) tried accessing opac-shelves.pl
> 
> Template process failed: file error - cssinclude: not found at
> /home/katrin/kohaclone/C4/Templates.pm line 131.
> 

Katrin, the followup fix the problem, no sure about the correct status.
Switched to Need Signoff
Comment 9 Petter Goksøyr Åsen 2014-08-28 06:42:42 UTC Comment hidden (obsolete)
Comment 10 Petter Goksøyr Åsen 2014-08-28 06:45:16 UTC
sorry, missed my SO message
Comment 11 Petter Goksøyr Åsen 2014-08-28 06:45:19 UTC Comment hidden (obsolete)
Comment 12 Petter Goksøyr Åsen 2014-08-28 06:45:41 UTC Comment hidden (obsolete)
Comment 13 Petter Goksøyr Åsen 2014-08-28 06:47:51 UTC
Rebased updatedatabase.pl.

Followed test plan. Removed Norwegian template files on OPAC and intranet - fallback to English works, in all themes, with no issues.
Comment 14 Jonathan Druart 2014-08-29 13:55:23 UTC
When I remove
  koha-tmpl/opac-tmpl/bootstrap/fr-FR/modules/opac-memberentry.tt
I got

Template process failed: file error - cssinclude: not found at /home/koha/src/C4/Templates.pm line 131.

What I missed?
Comment 15 Bernardo Gonzalez Kriegel 2014-08-29 15:00:16 UTC
(In reply to Jonathan Druart from comment #14)
> When I remove
>   koha-tmpl/opac-tmpl/bootstrap/fr-FR/modules/opac-memberentry.tt
> I got
> 
> Template process failed: file error - cssinclude: not found at
> /home/koha/src/C4/Templates.pm line 131.
> 
> What I missed?

Have you changed OPACFallback to 'bootstrap'?
Default value is 'prog', but in this case you need to change it to 'bootstrap'
With that it works for me, shows english page.
Comment 16 Jonathan Druart 2014-08-29 15:10:29 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #15)
> Have you changed OPACFallback to 'bootstrap'?
> Default value is 'prog', but in this case you need to change it to
> 'bootstrap'
> With that it works for me, shows english page.

The pref is set to prog. I was expecting to get the prog page, but it would have been quite weird indeed.
Comment 17 Jonathan Druart 2014-08-29 15:12:37 UTC
Created attachment 31295 [details] [review]
Bug 12539: PROG/CCSR deprecation: Remove hardcoded theme from C4/Templates.pm

This patch removes hardcoded 'prog' from themelanguage on C4/Templates.pm
It adds a new syspref OPACFallback to hold the prefferred fallback
theme for new themes with minimal changes.

This does not brake CCSR if new syspref value is 'prog',
using 'bootstrap' works somehow, but with strange results.

To test:
1) Apply the patch
2) Run updatedatbase.pl
3) CCSR, prog & bootstrap must work as usual
4) Need to test theme/language fallback
 a) translate for your preferred language (xx-YY)
 b) enable language on opac
 c) go to opac, select that language
 d) login on opac, go to personal details
 e) remove koha-tmpl/opac-tmpl/bootstrap/xx-YY/modules/opac-memberentry.tt
 d) reload page, must load in English
 f) click other member options, must load on translated language
(can use other pages, ccsr, prog or bootstrap)
5) Do a similar test but for a staff page, enable lang, switch to it,
rm koha-tmpl/intranet-tmpl/prog/xx-YY/modules/admin/admin-home.tt,
any page must be translated except for admin home.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Petter Goksoyr Asen <boutrosboutrosboutros@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 18 Jonathan Druart 2014-08-29 15:12:45 UTC
Created attachment 31296 [details] [review]
Bug 12539: Follow up to fix fallback theme

The is a problem in the logic, fallback must be:
1. Requested (theme, lang, tmpl)
a. return    (theme, lang, tmpl), if not
b. return    (fallback, lang, tmpl), if not
c. return    (theme, 'en', tmpl), if not as last resort
d. return    (fallback, 'en', tmpl)

Previous patch missed option 'c', worked for CCSR
but not bootstrap, sorry.

Signed-off-by: Petter Goksoyr Asen <boutrosboutrosboutros@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 19 Jonathan Druart 2014-08-29 15:13:30 UTC
The changes in C4::Templates should have provided unit tests. But I did not manage to write them.
Comment 20 Chris Cormack 2014-09-02 20:15:41 UTC
Pushed to MM_OPAC/theme_dep
Comment 21 Tomás Cohen Arazi 2014-10-30 00:42:42 UTC
Patches merged into master.

Thanks Bernardo!