[Bug 10325] New: Allow system preferences to be overridable from koha-httpd.conf
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 Bug ID: 10325 Summary: Allow system preferences to be overridable from koha-httpd.conf Classification: Unclassified Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: kyle.m.hall@gmail.com For Koha installations with multiple OPAC URLs, It would be nice to be able to override systeprefs from the http conf file. Case in point, a library wants to have two separate opacs, one the is only viewable from within the library that allows patrons to place holds, and a second public one that does not. In this case, overriding the system preference RequestOnOpac would accomplish this simply, and with no ill affects. This feature would of course be should only be used to override cosmetic effects on the system, and should not be used for system preferences such as CircControl, but would be great for preferences such as OpacStarRatings, opacuserjs, OpacHighlightedWords and many others! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 18374 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18374&action=edit Bug 10325 - Allow system preferences to be overridable from koha-httpd.conf For Koha installations with multiple OPAC URLs, It would be nice to be able to override systeprefs from the http conf file. Case in point, a library wants to have two separate opacs, one the is only viewable from within the library that allows patrons to place holds, and a second public one that does not. In this case, overriding the system preference RequestOnOpac would accomplish this simply, and with no ill affects. This feature would of course be should only be used to override cosmetic effects on the system, and should not be used for system preferences such as CircControl, but would be great for preferences such as OpacStarRatings, opacuserjs, OpacHighlightedWords and many others! Test Plan: 1) Apply this patch 2) Disable the system pref OpacHighlightedWords 3) Do a seach in the OPAC, not the term is not highlighted 4) Edit your koha-http.conf file, add the line SetEnv OVERRIDE_SYSPREF_OpacHighlightedWords "1" to your koha-http.conf file's OPAC section. 5) Restart your web server, or just reload it's config 6) Do a seach, now your search term should be highlighted! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |kyle.m.hall@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #2 from Galen Charlton <gmcharlt@gmail.com> --- Comment on attachment 18374 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18374 Bug 10325 - Allow system preferences to be overridable from koha-httpd.conf Review of attachment 18374: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=10325&attachment=18374) ----------------------------------------------------------------- I can see the utility of this, and it would have applications for the command line, too. I can imagine an edge case, though -- there's nothing stopping one from using this in the *intranet* virtual host definitions (and I could imagine a Koha administrator wanting to do this to allow local admins access to system preferences while locking down ones such as 'marcflavour' that should never be changed). But that could really confuse somebody who is wondering why their system preference change isn't having the desired effect. Perhaps the system preference editor should be taught how to recognize overriden sysprefs? ::: C4/Context.pm @@ +548,5 @@
my $dbh = C4::Context->dbh or return 0;
+ my $value; + if ( defined $ENV{"OVERRIDE_SYSPREF_$var"} ) {
The test should be 'exists', no? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gmcharlt@gmail.com --- Comment #3 from Galen Charlton <gmcharlt@gmail.com> --- Another thing that occurred to me -- some system preference names contain hyphens and colons, neither of which can be used as part of environment variable names. Not a huge deal, but perhaps the override mechanism should do some normalization of the syspref name before checking the environment. For example, it could normalize hyphens and colons to underscores, so that $context->preference('ILS-DI:AuthorisedIPs') would check for an environment variable named OVERRIDE_SYSPREF_ILS_DI_AuthorisedIPs. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #4 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to comment #3)
Another thing that occurred to me -- some system preference names contain hyphens and colons, neither of which can be used as part of environment variable names. Not a huge deal, but perhaps the override mechanism should do some normalization of the syspref name before checking the environment.
I would not be sad if we got rid of these special chars in pref names btw :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion --- Comment #5 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to comment #2)
I can imagine an edge case, though -- there's nothing stopping one from using this in the *intranet* virtual host definitions (and I could imagine a Koha administrator wanting to do this to allow local admins access to system preferences while locking down ones such as 'marcflavour' that should never be changed). But that could really confuse somebody who is wondering why their system preference change isn't having the desired effect.
Perhaps the system preference editor should be taught how to recognize overriden sysprefs?
I think that this feature is indeed very interesting, but dangerous at the other side. Galen's idea to make it visible in the pref editor is a good one too. Why not e.g. disable editing the pref when it is overridden? Other color? But it will be very difficult to say which prefs should be excluded from overriding. You could start with Version, marcflavour, etc. but will become very arbitrary at the end. And yet another idea: If you have a pref, yes.., AllowOverridingPrefValues or something, you could say that the admin at least knows which adventure he started?
+ if ( defined $ENV{"OVERRIDE_SYSPREF_$var"} ) {
The test should be 'exists', no?
If it does not exist, it is undefined? It could even exist and be undefined (pure theoretically).. Setting status to In Discussion. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #6 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to comment #5)
And yet another idea: If you have a pref, yes.., AllowOverridingPrefValues or something, you could say that the admin at least knows which adventure he started?
Ideas come and go: A pref value that consists of the separated names of preferences that you allow to be overridden from the environment? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #7 from Galen Charlton <gmcharlt@gmail.com> --- (In reply to comment #5)
But it will be very difficult to say which prefs should be excluded from overriding. You could start with Version, marcflavour, etc. but will become very arbitrary at the end.
To be clear, I wasn't calling for the creation of a list of system preferences that should not be overridden; what I was proposing was that a system administrator might choose to override marcflavour to prevent staff who only have access to the staff interface from changing its value.
And yet another idea: If you have a pref, yes.., AllowOverridingPrefValues or something, you could say that the admin at least knows which adventure he started?
I don't think this is necessary. To use the proposed mechanism, one has to have root access in order to change the Apache configuration, which is itself a higher bar to mischief than what YASP would provide. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #8 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to comment #7)
To be clear, I wasn't calling for the creation of a list of system preferences that should not be overridden; what I was proposing was that a system administrator might choose to override marcflavour to prevent staff who only have access to the staff interface from changing its value.
If these are two individuals, you will not always know who should prevent who from doing what..
I don't think this is necessary. To use the proposed mechanism, one has to have root access in order to change the Apache configuration, which is itself a higher bar to mischief than what YASP would provide.
Agreed, it is not essential but changing the Apache config and restarting http services will perhaps be not as easy as 'stop overriding this pref' by changing a koha preference.. What we do need in addition to this patch, is a way to make very clear in the preferences editor that a specific preference has been overridden. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #9 from Galen Charlton <gmcharlt@gmail.com> --- (In reply to comment #8)
(In reply to comment #7)
To be clear, I wasn't calling for the creation of a list of system preferences that should not be overridden; what I was proposing was that a system administrator might choose to override marcflavour to prevent staff who only have access to the staff interface from changing its value.
If these are two individuals, you will not always know who should prevent who from doing what..
Well, ultimately that's up to the Koha consortium or library to work out. In practice, however, I suspect most consortial users of Koha would be hesitant to let local library sysadmins have access to system preference until such time as Koha support per-library syspref values.
What we do need in addition to this patch, is a way to make very clear in the preferences editor that a specific preference has been overridden.
Agreed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #10 from Galen Charlton <gmcharlt@gmail.com> --- An observation: one situation where this would be *very* useful is setting OPACBaseURL and staffClientBaseURL from the Apache virtual host definition. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Failed QA --- Comment #11 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Kyle: Reading the comments now, we seem to reach consensus that this patch just needs an additional change in the pref editor to make visible that the pref is overridden. Changing status to reflect that. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #12 from Kyle M Hall <kyle@bywatersolutions.com> --- I don't think that is possible, at least not to my knowledge. What you are asking for is for one website ( the intranet ) to know the internal settings of another website ( the opac ). The only thing I can think of is to add an callable ajax function for the opac and intranet that can be passed each system preference and return true if it is overridden and false otherwise. We'd need to add a section to koha to list all the opacs and intranets in the system, and then for each system preference iterate though the list of opacs and intranets and check each one to see if the system preference is overridden. If it is, then we display some kind of not by the preference with the opac's url and new preference value. This seems terribly and overly complicated just to see which system preferences are overridden. Is there a simpler way I am not seeing? (In reply to M. de Rooy from comment #11)
Kyle: Reading the comments now, we seem to reach consensus that this patch just needs an additional change in the pref editor to make visible that the pref is overridden.
Changing status to reflect that.
-- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #13 from M. de Rooy <m.de.rooy@rijksmuseum.nl> ---
This seems terribly and overly complicated just to see which system preferences are overridden. Is there a simpler way I am not seeing?
When initializing the form, you could check %ENV for all OVERRIDE_SYSPREF keys and mark what you find in a special way? Would that be simpler? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #14 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Too fast: yes too simple. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to M. de Rooy from comment #13)
This seems terribly and overly complicated just to see which system preferences are overridden. Is there a simpler way I am not seeing?
When initializing the form, you could check %ENV for all OVERRIDE_SYSPREF keys and mark what you find in a special way? Would that be simpler?
Yes, the would unfortunately only work for staff intranet the preference editor is being used on. Since we have a minimum of two virtual hosts per koha installation ( one opac, one staff ) it won't work. I'm wondering if maybe a simpler/better solution would be to add a parameter to koha-conf.xml to control if this feature is enabled or not, and if it is enabled, display a nice big warning at the top of the system preferences edit along the lines of "Warning: You have the system preferences override feature enabled, please check your apache configuration files before assuming a change in a system preference will affect your installation." -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #16 from Galen Charlton <gmcharlt@gmail.com> --- My proposal, as I agree that it's impossible for one vhost to be 100% sure about the configuration of another one: - only check the environment passed to the staff interface virtual host to determine if a syspref is overridden - add a (commented-out) example to koha-httpd.conf that alerts the the sysadmin that they probably want to override any syspref in an OPAC vhost in the staff interface vhost as well. Since overriding sysprefs via the Apache configuration would be an "advanced" feature, having a couple rough edges is not a showstopper IMO as long as we document them. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #17 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Yes, I think that checking all possible opac and staff vhosts is overkill. Parsing the apache conf is also not really what you want to do in Koha. We could just introduce e.g. an environment var for the staff vhost like OVERRIDE_SYSPREF_NAMES. This could just be a comma separated list of all sysprefs that are overridden in one or more of the corresponding opac vhosts. (As a variant of what Galen also suggests.) No further checking; using this feature wisely requires skills. Note that seeing a few overridden prefs should trigger the Koha admin to expect one more too.. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18374|0 |1 is obsolete| | --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 20228 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20228&action=edit Bug 10325 - Allow system preferences to be overridable from koha-httpd.conf For Koha installations with multiple OPAC URLs, It would be nice to be able to override systeprefs from the http conf file. Case in point, a library wants to have two separate opacs, one the is only viewable from within the library that allows patrons to place holds, and a second public one that does not. In this case, overriding the system preference RequestOnOpac would accomplish this simply, and with no ill affects. This feature would of course be should only be used to override cosmetic effects on the system, and should not be used for system preferences such as CircControl, but would be great for preferences such as OpacStarRatings, opacuserjs, OpacHighlightedWords and many others! Test Plan: 1) Apply this patch 2) Disable the system pref OpacHighlightedWords 3) Do a seach in the OPAC, not the term is not highlighted 4) Edit your koha-http.conf file, add the line SetEnv OVERRIDE_SYSPREF_OpacHighlightedWords "1" to your koha-http.conf file's OPAC section. Also add the line SetEnv OVERRIDE_SYSPREF_NAMES "OpacHighlightedWords" to the Intranet section 5) Restart your web server, or just reload it's config 6) Do a seach, now your search term should be highlighted! 7) From the intranet preference editor, view the pref, You should see a warning the this preference has been overridden. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20228|0 |1 is obsolete| | --- Comment #19 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 20229 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20229&action=edit Bug 10325 - Allow system preferences to be overridable from koha-httpd.conf For Koha installations with multiple OPAC URLs, It would be nice to be able to override systeprefs from the http conf file. Case in point, a library wants to have two separate opacs, one the is only viewable from within the library that allows patrons to place holds, and a second public one that does not. In this case, overriding the system preference RequestOnOpac would accomplish this simply, and with no ill affects. This feature would of course be should only be used to override cosmetic effects on the system, and should not be used for system preferences such as CircControl, but would be great for preferences such as OpacStarRatings, opacuserjs, OpacHighlightedWords and many others! Test Plan: 1) Apply this patch 2) Disable the system pref OpacHighlightedWords 3) Do a seach in the OPAC, not the term is not highlighted 4) Edit your koha-http.conf file, add the line SetEnv OVERRIDE_SYSPREF_OpacHighlightedWords "1" to your koha-http.conf file's OPAC section. Also add the line SetEnv OVERRIDE_SYSPREF_NAMES "OpacHighlightedWords" to the Intranet section 5) Restart your web server, or just reload it's config 6) Do a seach, now your search term should be highlighted! 7) From the intranet preference editor, view the pref, You should see a warning the this preference has been overridden. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20229|0 |1 is obsolete| | --- Comment #20 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 20230 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20230&action=edit Bug 10325 - Allow system preferences to be overridable from koha-httpd.conf For Koha installations with multiple OPAC URLs, It would be nice to be able to override systeprefs from the http conf file. Case in point, a library wants to have two separate opacs, one the is only viewable from within the library that allows patrons to place holds, and a second public one that does not. In this case, overriding the system preference RequestOnOpac would accomplish this simply, and with no ill affects. This feature would of course be should only be used to override cosmetic effects on the system, and should not be used for system preferences such as CircControl, but would be great for preferences such as OpacStarRatings, opacuserjs, OpacHighlightedWords and many others! Test Plan: 1) Apply this patch 2) Disable the system pref OpacHighlightedWords 3) Do a seach in the OPAC, not the term is not highlighted 4) Edit your koha-http.conf file, add the line SetEnv OVERRIDE_SYSPREF_OpacHighlightedWords "1" to your koha-http.conf file's OPAC section. Also add the line SetEnv OVERRIDE_SYSPREF_NAMES "OpacHighlightedWords" to the Intranet section 5) Restart your web server, or just reload it's config 6) Do a seach, now your search term should be highlighted! 7) From the intranet preference editor, view the pref, You should see a warning the this preference has been overridden. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20230|0 |1 is obsolete| | --- Comment #21 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 20360 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20360&action=edit Bug 10325 - Allow system preferences to be overridable from koha-httpd.conf For Koha installations with multiple OPAC URLs, It would be nice to be able to override systeprefs from the http conf file. Case in point, a library wants to have two separate opacs, one the is only viewable from within the library that allows patrons to place holds, and a second public one that does not. In this case, overriding the system preference RequestOnOpac would accomplish this simply, and with no ill affects. This feature would of course be should only be used to override cosmetic effects on the system, and should not be used for system preferences such as CircControl, but would be great for preferences such as OpacStarRatings, opacuserjs, OpacHighlightedWords and many others! Test Plan: 1) Apply this patch 2) Disable the system pref OpacHighlightedWords 3) Do a seach in the OPAC, not the term is not highlighted 4) Edit your koha-http.conf file, add the line SetEnv OVERRIDE_SYSPREF_OpacHighlightedWords "1" to your koha-http.conf file's OPAC section. Also add the line SetEnv OVERRIDE_SYSPREF_NAMES "OpacHighlightedWords" to the Intranet section 5) Restart your web server, or just reload it's config 6) Do a seach, now your search term should be highlighted! 7) From the intranet preference editor, view the pref, You should see a warning the this preference has been overridden. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #22 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 20361 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20361&action=edit Bug 10325: QA Followup for lowercase in pref hash and few typos Restoring the lowercase behavior for entries in pref hash in Context.pm. Textual change in preferences.tt: $pref been -> value for $pref may have been. Typo in koha-httpd.conf: SevEnv -> SetEnv. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #23 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- QA Comment: Fine addition! I added a small (trivial) QA followup. Code looks good to me. No complaints from koha-qa. Before passing QA on this one, I have still two (simple) requests in connection with case insensitive pref names in koha-httpd.conf (easier use): 1) Would it be possible to make the check in Context.pm: defined $ENV{"OVERRIDE_SYSPREF_$var"} case insensitive somehow? Replace it perhaps by something like: my @k= grep {/OVERRIDE_SYSPREF_$var/i} keys %ENV; if ( @k ) { $value = $ENV{$k[0]}; 2) Check in preferences.pl with any: Could this be case insensitive too? any { lc $name eq lc $_ } @override_syspref_names ? If you could add a followup for this, I will be happy to sign it off too :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #24 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- my @k= grep {/OVERRIDE_SYSPREF_$var/i} keys %ENV; Safer variant: my @k= grep {/^OVERRIDE_SYSPREF_${var}$/i} keys %ENV; -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20361|0 |1 is obsolete| | --- Comment #25 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 20375 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20375&action=edit Bug 10325 - QA Followup - Fix typos -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #26 from Kyle M Hall <kyle@bywatersolutions.com> --- Marcel, I've been talking to Galen about the case insensitivity for system preferences. We are both of the opinion that it's not good and it should go away. It's extremely odd in that the preference value is looked up in a case sensitive fashion, but cached in all lowercase. This situation where the first call to C4::Context->preference() *must* be correctly cased, but any further calls do not have to be. I had thought that it was possible that somewhere in the code base this situation might occur, so I wrote a script to extract the preference strings from all the calls to C4::Context->preference() anywhere in the code base. The script proved to me that there are no incorrectly cased calls to preference(). I will attach it for your perusal. So, in summary: 1) Galen and I both agree your followup to add case insensitivity for this particular feature, as per your request, is not necessary. 2) Galen and I both agree we should not cache the preferences in a case sensitive fashion. 3) I have written a script that proves no system preferences are called in an incorrectly cased manner. 3) My patch on it's own removes the case insensitivity from caching, while your qa followup re-adds it. Do to points 2 and 3, I think it's best that we do not use your followup ( except for the typo correction ), Though I do thank your for thinking of it and providing a followup! If that was the only issue of note, I believe it's now safe to pass QA on this! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20375|0 |1 is obsolete| | --- Comment #27 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 20376 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20376&action=edit Bug 10325 - QA Followup - Fix typos -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #28 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 20377 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20377&action=edit Script to test for badly cases preference calls To run, copy this to the root directory of your git Koha clone and execute it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #29 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Kyle M Hall from comment #26)
I've been talking to Galen about the case insensitivity for system preferences. We are both of the opinion that it's not good and it should go away. It's extremely odd in that the preference value is looked up in a case sensitive fashion, but cached in all lowercase.
It is not a matter of life and death, of course. But I would opt for case insensitive. For me, you may look it up in uppercase or whatever if you like that better :) So I do not simply agree with "It is not good and it should go away" (without further reasons given). But no problem for me to pass qa on this patch either. Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 --- Comment #30 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to M. de Rooy from comment #29)
(In reply to Kyle M Hall from comment #26)
I've been talking to Galen about the case insensitivity for system preferences. We are both of the opinion that it's not good and it should go away. It's extremely odd in that the preference value is looked up in a case sensitive fashion, but cached in all lowercase.
It is not a matter of life and death, of course. But I would opt for case insensitive. For me, you may look it up in uppercase or whatever if you like that better :)
So I do not simply agree with "It is not good and it should go away" (without further reasons given).
But no problem for me to pass qa on this patch either.
Passed QA
Excellent. Thanks Marcel! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #31 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master, along with a follow-up that adds a test case and another that fixes typos. Thanks, Kyle! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10325 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org