80183 - configurable index page (using Template Toolkit)

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect, P1)

Description

[Matt Jones]

This is an enhancement request (with patch providing the functionality) to add a
new 'index.cgi' script that can replace or supplement the existing index.html
static html file.  By having the main index page be a cgi program rather than
static html, we can take advantage of the configuration params (like bannerhtml)
which makes it far easier for sites to customize the HTML associated with their
deployed version of Bugzilla.  I will attach to this bug two files: a new
"index.cgi" file, and a patch to the defparams.pl file that adds a new
"index-template" param that contains the html to be displayed on the index page.

In addition, the attached index.cgi file can (optionally) display a table of the
products that are part of this Bugzilla instance.  If that table is shown, the
product names can optionally be links to a query for all of the bugs for that
product.  These optional features are turned off by default in order to emulate
the original behavior of Bugzilla as closely as possible.

Although this is not even close to a solution for separating UI from application
logic, it does help a bit by making it easier to upgrade Bugzilla without having
to reimplement HTML changes in the index.html file.  A solution more along the
lines of Bug 49225 in which XML is converted to HTML using a user-specified set
of XSLT stylesheets would be preferable but more disruptive to implement.

See bug 2900 and bug 51100 for related issues.

Comment 1

[Matt Jones]

Attachment: Perl CGI script that can be entry point for Bugzilla

Comment 2

[Matt Jones]

Attachment: Patch with new params for defparams.pl

Comment 3

[Dave Miller [:justdave]]

Just for comparison when we're reviewing this...

See also:
Bug 69807: Make "My Bugs" the default starting page.
Bug 51100: configurable "home page" per engineer and per team
Bug 2900: PATCH to synchronize static html file with 'bannerhtml' param

I'm not arguing against an index.cgi...  I'm all for it in fact.  I think it's 
been generally accepted that we need to go that route.  There has always been a 
lot of arguments as to how to do it (see the above bugs).

Comment 4

[Christopher Aillon (sabbatical, not receiving bugmail)]

->New (old by now?) product Bugzilla 

Comment 5

[Jacob Steenhagen]

*** Bug 102689 has been marked as a duplicate of this bug. ***

Comment 6

[Jacob Steenhagen]

Attachment: index.cgi - Uses a template

Comment 7

[Jacob Steenhagen]

Attachment: The aforementioned template

Comment 8

[Jacob Steenhagen]

-> me

Comment 9

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

Making this bug block the templatization tracker bug.

Comment 10

[Christian Reis]

Jake, this looks pretty good. I have some questions:

 # Colon-separated list of directories containing templates.
    INCLUDE_PATH => "template/default" ,

Is this correct? IOW, if the user wants to override this, does he have to hack
the file or it automatic? (Sorry for being clueless)

  $vars->{'username'} = $::COOKIE{'Bugzilla_login'} || '';   
  $vars->{'subst'} = { 'userid' => $::COOKIE{'Bugzilla_login'} };

Why do you check for an invalid cookie first, and on the second line, ignore the
test? Any specific reason? Why not

  $vars->{'subst'} = { 'userid' => $vars->{'username'} }; 

perhaps?

  <SCRIPT LANGUAGE="JavaScript">

This should be <SCRIPT TYPE="application/x-javascript"> 

   var rv = window.confirm ("This page is enhanced for use with Netscape 6.  "   
     + "Would you like to upgrade now?");

ROTFL Why not mention Mozilla?

Looks good to me, if these are problems and are fixed r=kiko

Comment 11

[Christian Reis]

(spam, sorry, stupid: vote for bug 34488 to solve this once and for all)

Comment 12

[Jacob Steenhagen]

>Is this correct? IOW, if the user wants to override this, does he have to hack
>the file or it automatic? (Sorry for being clueless)

Well, I copied that from attachment.cgi :)

>Why do you check for an invalid cookie first, and on the second line, ignore the
>test? Any specific reason? Why not

Not sure what I was thinking...

>This should be <SCRIPT TYPE="application/x-javascript"> 
...
>ROTFL Why not mention Mozilla?

I took that bit from Netscape's web site w/only minor modifications.  I assume
the first is for HTML 4 compliance?  As for mentioning Mozilla, I guess I have
no opionion either way... I've heard it said that Mozilla is really for the
devolopers and end users are better off w/a distribution (such as Netscape 6).

Comment 13

[Jacob Steenhagen]

Attachment: index.cgi - v2

Comment 14

[Jacob Steenhagen]

Attachment: index.atml - v2

Comment 15

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

Comment on attachment 53096 [details]
index.cgi - v2

>    INCLUDE_PATH => "template/default" ,

This should be:

    INCLUDE_PATH => "template/custom:template/default" , 

The way it is in attachment.cgi is a mistake.

Comment 16

[Gervase Markham [:gerv]]

Personally I think this is annoying, because I find it very useful to have a
static page available. This helps me see whether problems during setup are
webserver permissions problems or execute permissions problems or SQL server
problems or what.

Do we ship other static HTML files I could use for this instead?

Gerv

Comment 17

[Jacob Steenhagen]

Gerv, there are numerous static files... the entire docs/ sub-tree for example
(there are also many in the "bugzilla root" directory.

Comment 18

[Jacob Steenhagen]

Attachment: index.cgi - v3

Comment 19

[Jacob Steenhagen]

*** Bug 15967 has been marked as a duplicate of this bug. ***

Comment 20

[Gervase Markham [:gerv]]

Comment on attachment 53395 [details]
index.cgi - v3

># Suppress silly "used only once" warnings
>sub index_cgi_silliness {
>    my $zz;
>    $zz = %::COOKIE;
>}

This is not necessary - there are several bugs, with patches attached, that show 
how to use "use vars" instead.

And some comments on the template:

> Forget the currently stored login

Can't we just say "Logout" like everyone else?

> ...explaining all about bugzilla.

_B_ugzilla :-)

You main page doesn't look quite like the default one; e.g. the black border around 
the bug image, which makes it look a lot neater. And some of the words are missing
from the top. Why is that?

Gerv

Comment 21

[Dave Miller [:justdave]]

I tried to change several of those to use "use vars" in the past, and was never
able to get "use vars" to work with global variables.

Comment 22

[Jacob Steenhagen]

IIRC, I also tried to "use vars" and it balked.

> Can't we just say "Logout" like everyone else?

Um, sure :)

> the black border around the bug image, which makes it look a lot neater

Well, ok...

> And some of the words are missing from the top. Why is that?

The "This is Bugzilla" words we duplicated less than half a screen down (in the
page footer) when using defaults, so I didn't put them at the top of the main page.

Comment 23

[Jacob Steenhagen]

Attachment: index.atml - v3

Comment 24

[Christian Reis]

Comment on attachment 53395 [details]
index.cgi - v3

r=kiko
Thanks Jake.

Comment 25

[Christian Reis]

Comment on attachment 53632 [details]
index.atml - v3

r=kiko
Let's get this in and fix anything that falls in later :)

Comment 26

[Jacob Steenhagen]

Attachment: Optionally create an index.html to redirect to index.cgi

Comment 27

[Gervase Markham [:gerv]]

Comment on attachment 53395 [details]
index.cgi - v3

r=gerv.

Gerv

Comment 28

[Gervase Markham [:gerv]]

Comment on attachment 53632 [details]
index.atml - v3

r=gerv.

Gerv

Comment 29

[Jacob Steenhagen]

Gerv/Chistrian, in order some of the things in this patch refer to QuickSearch
patch or the sidebar.cgi from bug 37339... can I get review on that, too?

Comment 30

[Dave Miller [:justdave]]

Comment on attachment 54155 [details] [diff] [review]
Optionally create an index.html to redirect to index.cgi

I like what's in this patch so far, but I want more! :)

People who are updating via cvs may suddenly be surprised when their existing 
index.html file disappears when they cvs update after we cvs remove it...

I believe cvs will move their existing file to ".#index.html.(version)" where 
(version) is the RCS revision number for the last version they had from cvs 
before they updated.  The checksetup.pl script should, *before* checking 
localconfig vars, look for the presense of a file by such a name, and 
indicate that their old index.html file has been moved there if it finds one.
Also indicate that they can remove that file to get rid of the warning every 
time they run checksetup.pl.

Comment 31

[Dave Miller [:justdave]]

ok....  ignore everything I said above...  that's now how it works.  I did some
testing...

on installation 1 I added a file called "testfile" which simply contained a
single line of text.

> cvs add textfile
> cvs commit
Checking in testfile;
/cvsroot/syndiboard2/testfile,v  <--  testfile
initial revision: 1.1
done

on installation 2, I did a cvs update, then removed the file:

> cvs up
cvs update: Updating .
U testfile
> rm testfile 
> cvs remove testfile
cvs remove: scheduling `testfile' for removal
cvs remove: use 'cvs commit' to remove this file permanently
> cvs commit
Removing testfile;
/cvsroot/syndiboard2/testfile,v  <--  testfile
new revision: delete; previous revision: 1.1
done

OK, back to installation 1...  I modified testfile so it had different text in
it than was there when I originally committed it...  then did a cvs update:

> cvs up
cvs server: Updating .
RCS file: /cvsroot/syndiboard2/Attic/testfile,v
retrieving revision 1.1
retrieving revision 1.2
Merging differences between 1.1 and 1.2 into testfile
testfile already contains the differences between 1.1 and 1.2

> ls -l testfile
-rw-------   1 dave  unknown     30 Oct 25 23:11 testfile

it's still there....

> cvs diff -u testfile
cvs server: testfile is a new entry, no comparison available
> cvs up
cvs server: Updating .
M testfile

Comment 32

[Jacob Steenhagen]

I did an expiriment very similar to what you did as far as having two
installations...  This is what happened when README.txt was locally modified:

[intranet@webserv3 spit]$ cvs -q update -dP
cvs server: conflict: README.txt is modified but no longer in the repository
C README.txt
[intranet@webserv3 spit]$ ll
total 12
drwxrwxrwx    2 intranet nobody       4096 Oct 26 08:33 CVS
-rw-r--r--    1 intranet nobody       1021 Oct 26 08:37 README.txt
drwxrwxrwx    4 intranet nobody       4096 Oct 26 08:37 Vb6
[intranet@webserv3 spit]$ cd CVS
[intranet@webserv3 CVS]$ cat Entries
D/Vb6////
/README.txt/1.11/Fri Oct 26 12:33:28 2001//
[intranet@webserv3 CVS]$

And when it wasn't:

[intranet@webserv3 spit]$ cvs -q update -dP
cvs server: warning: README.txt is not (any longer) pertinent
[intranet@webserv3 spit]$ ll
total 8
drwxrwxrwx    2 intranet nobody       4096 Oct 26 08:38 CVS
drwxrwxrwx    4 intranet nobody       4096 Oct 26 08:38 Vb6
[intranet@webserv3 spit]$ cd CVS
[intranet@webserv3 CVS]$ cat Entries
D/Vb6////
[intranet@webserv3 CVS]$

Comment 33

[Jacob Steenhagen]

Attachment 55231 [details] [diff] on bug 37339 contains my attempt to detect the situation of
index.html not being removed because it's modified and warn the user.  What it
does is look for "index.cgi" in the .html file and if it doesn't find it, warns
the user (we dont' want to warn them if they chose to have checksetup.pl create
the index.html file to redirect them to index.cgi :)

Comment 34

[Matthew Tuck [:CodeMachine]]

The CGI and page looks OK on a quick scan, but the CGI should run in taint mode.

Comment 35

[Bradley Baetz (:bbaetz)]

> Attachment 55231 [details] [diff] on bug 37339 contains my attempt to detect the situation of
> index.html not being removed because it's modified and warn the user.  What it
> does is look for "index.cgi" in the .html file and if it doesn't find it, warns
> the user (we dont' want to warn them if they chose to have checksetup.pl create
> the index.html file to redirect them to index.cgi :)

If CVS/Entries exists, can't you just grep for ^index.html in that? It leaves
some text in there so that you get teh conflict message.

Comment 36

[Jacob Steenhagen]

>If CVS/Entries exists, can't you just grep for ^index.html in that? It leaves
>some text in there so that you get teh conflict message.

I thought about doing that, but then I relized that if the did:
cvs update
./checksetup.pl
<some work to put index.html changes into template/custom/index.atml>
rm index.html
vi localconfig
<change $index_html to 1>
./checksetup.pl

Then index.html will still exist in CVS/Entries even though they've now properly
set it up to use the template (but because of their webserver configuration they
needed the index.html redirect page).

Of course, I suppose in this situation cvs would still complain about the
localally modified index.html.

The other situation is kinda the opposit, where checking CVS/Entires wouldn't
catch the fact that an error should be thrown.  If the user grabs the 2.16 (I'm
assuming) tarball, untars it and copies index.html from their 2.14 (or older)
install, then index.html will no longer exist in CVS/Entires, but they still
wouldn't be using index.cgi.

Comment 37

[Bradley Baetz (:bbaetz)]

Hmm. We could just hack .htaccess to prefer index.cgi, but that won't get all cases.

Comment 38

[Kevin Brannen]

Ahhh, bannerHTML and footerHTML automatically... :-)

I did something like this with my install.  I think I took index.cgi from a
patch of a bug long ago--Dave Lawarance did it IIRC (I could submit it if anyone
cared, but it looks like y'all are pretty far along on this already).

Anyway, I might add that I think it's a good idea not to totally kill
index.html, that might make life a little harder on some people unneccessarily.
 (Yes I know I could avoid the work by just adding another file name to the
default files list [DirectoryIndex for Apache], but why should I?)

I avoided the problem by changing index.html to:
<html>
<meta http-equiv="refresh" content="0; URL=index.cgi">
</html>

Because of that, no bookmarks by anyone were affected, no Apache config files
had to be changed, and no animals were sacrified. ;-)

Comment 39

[Jacob Steenhagen]

Kevin, the idea is that checksetup.pl will optionally create an index.html that
will redirect to index.cgi.  The latest patches for this are on bug 37339
(because of the interdependencies... which is the while reason I started work on
this bug :)

Comment 40

[Dave Miller [:justdave]]

blocks a blocker, so it's a blocker.

Comment 41

[Jacob Steenhagen]

Bug 37339 had been checked in which included the fix for this bug in the patch.