126571 - param removal is shown to users in userprefs.cgi

Status: RESOLVED FIXED

(Bugzilla :: User Accounts, defect, P3)

Description

[Dave Miller [:justdave]]

I cvs updated a local install today...  among the changes picked up was the
removal of the strictvaluechecks param.

I visited userprefs.cgi while logged in with my account, and was told just below
the tabs and above the password change form that the strictvalue checks param
was no longer used and had been removed from my params file.

Why is this being shown to users on the prefs page?  This is an admin thing, and
end-users shouldn't see it.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: fixes problem; has issues

This patch moves the parameter deletion code into checksetup.pl so that it
happens on upgrade rather than the first time the parameters file gets
re-written afterwards.	I had to require globals.pl to use defparams.pl's
WriteParams function to write the new parameters out to disk, which meant
removing redundant code from checksetup.pl and changing the name of the
"GroupExists" function in checksetup.pl to avoid conflicts with the same-named
but differently-functioning equivalent in globals.pl

With this patch, checksetup.pl's `which mysql

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: fixes problem; has issues

This patch moves the parameter deletion code into checksetup.pl so that it
happens on upgrade rather than the first time the parameters file gets
re-written afterwards.	I had to require globals.pl to use defparams.pl's
WriteParams function to write the new parameters out to disk, which meant
removing redundant code from checksetup.pl and changing the name of the
"GroupExists" function in checksetup.pl to avoid conflicts with the same-named
but differently-functioning equivalent in globals.pl

With this patch, checksetup.pl's `which mysql` call gives the error "which: no
mysql in ((null))".  Not sure why this is happening.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 70899 [details] [diff] [review]
patch v1: fixes problem; has issues

Duplicate

Comment 4

[Andreas Franke (gone)]

Not sure if this is a problem, bug I remember some trouble with checksetup.pl
requiring globals.pl because globals.pl calls "do 'localconfig';", whereas the
initial localconfig is only created by checksetup.pl.   

Comment 5

[Bradley Baetz (:bbaetz)]

You get that error because globals.pl removes $ENV{'PATH'}....

Comment 6

[Dave Miller [:justdave]]

The main issue was polluting the namespace within checksetup.pl.  Creating the
variables in localconfig depends on those variables not already existing after a
do localconfig.  Since globals.pl provides defaults for some of the variables in
localconfig in case localconfig is missing/broken, those variables wind up
existing in checksetup.pl if globals.pl gets required, and a blank or missing
localconfig won't get completed correctly.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2: resolves issues

Moving "require 'globals.pl';" below the "localconfig" stuff solves the
localconfig variable definition problem as well as the mysql path problem.

Comment 8

[Matthew Tuck [:CodeMachine]]

Moving to 2.16 to keep this flowing, but this might not happen for 2.16.

Comment 9

[Gervase Markham [:gerv]]

Comment on attachment 72432 [details] [diff] [review]
patch v2: resolves issues

Why the change from GroupExists to GroupDoesExist? This seems arbitrary.

It all looks good apart from that.

Gerv

Comment 10

[Myk Melez [:myk] [@mykmelez]]

>Why the change from GroupExists to GroupDoesExist? This seems arbitrary.

Because another function called "GroupExists" already exists.

Comment 11

[Gervase Markham [:gerv]]

Comment on attachment 72432 [details] [diff] [review]
patch v2: resolves issues

2xr=gerv.

Gerv

Comment 12

[Myk Melez [:myk] [@mykmelez]]

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.137; previous revision: 1.136
done