304936 - checksetup.pl should inform the user to go to editparams.cgi if they haven't yet

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, enhancement)

Description

[timeless]

checksetup.pl should check for "you-havent-visited" in the urlbase and suggest 
you visit editparams.cgi upon successful completion if it's there

Comment 1

[Max Kanat-Alexander]

Yeah, I agree -- that's a good idea.

Comment 2

[Frédéric Buclin]

see bug 263340 comment 3

*** This bug has been marked as a duplicate of 263340 ***

Comment 3

[Max Kanat-Alexander]

These aren't duplicates. This bug is about checksetup, and your bug is about editparams.cgi.

This bug is about checksetup being able to figure out "Oh, the user hasn't gone to editparams.cgi yet, we should tell them to go there."

Comment 4

[Colin Ogilvie [:cso]]

Attachment: Patch v1

Patch v1

Comment 5

[Frédéric Buclin]

Comment on attachment 202741 [details] [diff] [review]
Patch v1

>Index: checksetup.pl

>+# Check if the default paramater for urlbase is still set, and if so, give

Nit: s/paramater/parameter/


>+if (Param('urlbase') eq 'http://you-havent-visited-editparams.cgi-yet/') {

The default value should not be hardcoded. Use the one from Bugzilla/Config/Core.pm.

Comment 6

[Colin Ogilvie [:cso]]

Attachment: Patch v2

Fixes nit and the hardcoding.

Comment 7

[Frédéric Buclin]

Comment on attachment 202930 [details] [diff] [review]
Patch v2

>+require Bugzilla::Config;
>+import Bugzilla::Config qw(Param);

Useless. These lines already exist at line 466, and Param() can already be used everywhere in checksetup.pl.


>+    print "Now that you have installed Bugzilla, you should visit the " .
>+          "'Edit Parameters' page as the Administrator to ensure it is " .
>+          "set up as you wish\n";

Nit: Instead of 'Edit Parameters', you should either say 'editparams.cgi' or 'the "Parameters" link in the footer' as it could be unclear which page you are talking about. Please make clear that the 'urlbase' parameter really has to be correctly set. Else the admin could change some other params, rerun checksetup.pl and wonder why this message appears again despite he went to editparams.cgi already.

Moreover, please break this long sentence by adding carriage returns (this doesn't look nice when your screen is large).

Comment 8

[Colin Ogilvie [:cso]]

(In reply to comment #7)
> Moreover, please break this long sentence by adding carriage returns (this
> doesn't look nice when your screen is large).

Indeed, Carriage Returns don't look nice when your screen is large. Hence, I am completely confused by this sentence...

Comment 9

[Colin Ogilvie [:cso]]

Attachment: Patch v3

While not following what lpsolit meant, I have added the carriage returns that he asked for and re-worded the message.

Comment 10

[Colin Ogilvie [:cso]]

Attachment: Patch v3.1

Add 'unless $silent' to not print when checksetup.pl should be silent.

Comment 11

[Frédéric Buclin]

Comment on attachment 202989 [details] [diff] [review]
Patch v3.1

r=LpSolit

Comment 12

[Frédéric Buclin]

Comment on attachment 202989 [details] [diff] [review]
Patch v3.1

>+# Check if the default parameter for urlbase is still set, and if so, give
>+# notification that they should go and visit editparamns.cgi 

Nit: s/editparamns/editparams/

Comment 13

[Colin Ogilvie [:cso]]

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

(Fixed the typo, too)