Since 'prog' theme doesn't exist anymore, OPACFallback syspref is without object anymore.
I think the idea was to avoid a similar problem in the future and also allow poeple to add their own theme and have it fall back to the main theme.
*** Bug 13444 has been marked as a duplicate of this bug. ***
Created attachment 40946 [details] [review] Bug 13291 - remove prog as fallback theme The 'prog' theme for OPAC was removed from Koha. The OPACFallback syspref was added in 3.18.0 with prog|bootstrap options. This patch proposes to remove the prog option, but still maintain the syspref for reasons outlined in Bug 13291 comment #1. Test plan : run updatedatabase.pl and check the value of OpacFallback under OPAC sysprefs.
Comment on attachment 40946 [details] [review] Bug 13291 - remove prog as fallback theme Review of attachment 40946 [details] [review]: ----------------------------------------------------------------- ::: installer/data/mysql/atomicupdate/bug_13291-remove-prog-as-listed-fallback-theme.sql @@ +1,1 @@ > +INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type) VALUES ('OPACFallback', 'bootstrap', 'Define the fallback theme for the OPAC interface.', 'bootstrap', 'Themes'); Works fine for systems lacking the system preference, but where's the UPDATE for those which already have it set?
Created attachment 40947 [details] [review] Bug 13291 - remove prog as fallback theme The 'prog' theme for OPAC was removed from Koha. The OPACFallback syspref was added in 3.18.0 with prog|bootstrap options. This patch proposes to remove the prog option, but still maintain the syspref for reasons outlined in Bug 13291 comment #1. Test plan : run updatedatabase.pl and check the value of OpacFallback under OPAC sysprefs. Current patch-complexity: Trivial patch
Created attachment 40948 [details] [review] Bug 13291 - remove prog as fallback theme The 'prog' theme for OPAC was removed from Koha. The OPACFallback syspref was added in 3.18.0 with prog|bootstrap options. This patch proposes to remove the prog option, but still maintain the syspref for reasons outlined in Bug 13291 comment #1. Test plan : run updatedatabase.pl and check the value of OpacFallback under OPAC sysprefs.
(In reply to M. Tompsett from comment #4) > Comment on attachment 40946 [details] [review] [review] > Bug 13291 - remove prog as fallback theme > Works fine for systems lacking the system preference, but where's the UPDATE > for those which already have it set? Addressed. Also adds the missing edits to opac.pref.
Created attachment 40949 [details] [review] Bug 13291 - remove prog as fallback theme The 'prog' theme for OPAC was removed from Koha. The OPACFallback syspref was added in 3.18.0 with prog|bootstrap options. This patch proposes to remove the prog option, but still maintain the syspref for reasons outlined in Bug 13291 comment #1. TEST PLAN --------- 1) back up DB 2) in sql client: > select * from systempreferences where variable like '%OPACFallback%'; -- 1 record, should have bootstrap and prog choices, and likely prog fallback. 3) at command line: ./installer/data/mysql/updatedatabase.pl 4) in sql client: > select * from systempreferences where variable like '%OPACFallback%'; -- 1 record, should have bootstrap and bootstrap values now. 5) in sql client: > delete from systempreferences where variable like '%OPACFallback%'; -- now there should be no OPACFallback system preference. 6) at command line: ./installer/data/mysql/updatedatabase.pl 7) in sql client: > select * from systempreferences where variable like '%OPACFallback%'; -- 1 record, should have bootstrap and bootstrap values now. 8) in sql client (substitute your koha database name!): > drop database koha_library; > create database koha_library; > use koha_library; 9) Run web installer 10) in sql client: > select * from systempreferences where variable like '%OPACFallback%'; -- 1 record, should have bootstrap and bootstrap values now. 11) run koha qa test tools Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
I am not sure a syspref with 1 option is really useful. The fallback could be done automatically on the bootstrap theme, don't you think?
Hm, thinking about this, you are probably right. If you have your own set of templates, hardcoding the fallback to the only available OPAC template seems right. OTH if we want to allow people to add other fallbacks and avoid changing files in Koha, this should not be a pull down but a text field? Hm, I feel like I am missing something.
(In reply to Jonathan Druart from comment #9) > I am not sure a syspref with 1 option is really useful. we already have a related syspref with 1 option - opacthemes > The fallback could be done automatically on the bootstrap theme, don't you > think? We seem to have that already in C4/Templates.pm in the form of my $hardcoded_theme = ($interface eq 'intranet') ? 'prog' : 'bootstrap';
(In reply to Katrin Fischer from comment #10) > Hm, thinking about this, you are probably right. If you have your own set of > templates, hardcoding the fallback to the only available OPAC template seems > right. OTH if we want to allow people to add other fallbacks and avoid > changing files in Koha, this should not be a pull down but a text field? Sounds like a fair argument - like say bootstrap|prog|custom_theme_name The ordering of these could then drive the selection in C4::Templates::themelanguage() Not that it would address Tomas's observation in line 243 about the hardcoding in case of an unpopulated db
(In reply to Indranil Das Gupta from comment #12) > (In reply to Katrin Fischer from comment #10) > > OTH if we want to allow people to add other fallbacks and avoid > > changing files in Koha, this should not be a pull down but a text field? > > Sounds like a fair argument - like say bootstrap|prog|custom_theme_name Yuck, no. Let's not do that. Drop downs prevent people messing up data. If people wish to install custom themes, it has to go in a known directory. Can't we calculate the drop down from the directory, rather than require the list be updated in the opac.pref file? Yes, I know the system preference code might get ugly looking. But I rather have ugly code than bad, hard to debug data.
(In reply to Jonathan Druart from comment #9) > I am not sure a syspref with 1 option is really useful. > The fallback could be done automatically on the bootstrap theme, don't you > think? Though 1 is kind of silly now, it does allow for bringing back multiple themes in the future.
(In reply to M. Tompsett from comment #13) > (In reply to Indranil Das Gupta from comment #12) > > (In reply to Katrin Fischer from comment #10) > > > OTH if we want to allow people to add other fallbacks and avoid > > > changing files in Koha, this should not be a pull down but a text field? > > > > Sounds like a fair argument - like say bootstrap|prog|custom_theme_name > > Yuck, no. Let's not do that. Drop downs prevent people messing up data. If > people wish to install custom themes, it has to go in a known directory. > Can't we calculate the drop down from the directory, rather than require the > list be updated in the opac.pref file? Yes, I know the system preference > code might get ugly looking. But I rather have ugly code than bad, hard to > debug data. I think that would be overengineering this a bit. We have other prefs that should not really be changed without knowing what you do. And I wonder what would happen if you enter something wrong - we could always make it fall back to the standard templates.
Four years later, no more themes to choose from. Perhaps we could remove this syspref?
(In reply to Nick Clemens from comment #16) > Four years later, no more themes to choose from. Perhaps we could remove > this syspref? I believe our naming/id'ing of objects has really moved us away from themes via separate files, and towards CSS-based themes only. As such, I agree this "In Discussion" should probably be changed to a "Let's scrap the Fallback. Here's a patch. Needs Sign off". Anyone object?
I know that themes are still used in libraries, even some around here. Does it really hurt us to keep it? Hea shows also shows use of opacthemes, but OpacFallback seems not to be recorded.