Closed Bug 150829 Opened 22 years ago Closed 22 years ago

'My Votes' link missing from footer


(Bugzilla :: Bugzilla-General, defect)

Not set



Bugzilla 2.16


(Reporter: bbaetz, Assigned: bbaetz)



(Keywords: regression)


(1 file, 1 obsolete file)

The 'My Votes' link isn't in the footer. This is because the condition is:

[% IF user.login AND use_votes %]

use_votes is set in to $::anyvotesallowed.

$::anyvotesallowed is read in from teh versioncache, which hasn't been loaded at
this point. Oops... uses the user_votes var name, too; doesn't this conflict?
regression; -> 2.16
Blocks: 150783
Keywords: regression
Target Milestone: --- → Bugzilla 2.16
Eventually, all these horrible globals can go away and become part of the One
Great Global, $vars. In the mean time, would it be an acceptable fix to make it so:

$::vars{'use_votes'} = $::anyvotesallowed;

appeared at the end of data/versioncache?

As for the clash: you are correct. I think we can just remove the instance from; currently, the My Votes link would only appear on the bug view if
the product the bug is in allowed voting. This seems counter-intuitive. It can
use the global flag like everything else.


Ug. What about making GetVersionTable() set this?
Given this should be on all logged in footers, and it requires information in
the version cache, it seems a possibility that calling GetVersionCache should be
done during quietly_check_login or confirm_login.

Yeah, you're right - we do need to call GetVersionCache all the time.

How did the old cod edeal with this/
*** Bug 151420 has been marked as a duplicate of this bug. ***
Putting in an always-called call to GetVersionTable() would also be a fix; it
has built-in protection about being called twice. This could go at the end of

The only problem is we want to move all GetVersionTables to after
quietly_check_logins.  Perhaps the eventual solution there is to do a
quietly_check_login automatically as well.
Why do we want to move GetVersionTable to after we get the logins?

I did it that way incase it returned an error, but the current code never errors
out anyway. And if it did, the installation would have more problems that
working out if the footer was correct.
The whole situation of what code to always run need a serious tidy up, it's
true. But we need to work out the minimum risk fix for 2.16. I'd say that was to
call GetVersionTable() as roughly the first thing we do in If we end
up calling it twice, fine (although we could remove all the other calls
trivially, as well.)

OTOH, The 'my votes' link only comes up if you are logged in.

The only question I have about putting it into is 'Will this break
programs running from contrib/?' I suspect that the answer is, for some of them,
yes, but OTOH they may alrady be broken.


a) add it to quietly_check_login, for 2.16
b) Work out what parts of the versioncache we actually still need/want/etc
c) Remove the uneeded bits
d) _Then_ work out where to put it

Our trick of removing the file when certain changes are made, and letting it get
regenerated later, won't work under mod_perl by default. Nor will our fussing
with the global namespace.
OK. Go for it :-)

Attached patch v1 (obsolete) — Splinter Review
This adds it to the user hash on the basis that:

a) We don't conflict with the var for, this way
b) We may want to make it depend on if the user has any votes - the page isn't
very useful otherwise, plus it would solve the problem of when to call
c) Its a simple, quick, easy, fix, which we need for 2.16
> This adds it to the user hash on the basis that:
> a) We don't conflict with the var for, this way

But it should "conflict" - the two are supposed to be the same. If they aren't,
that's a bug.

> b) We may want to make it depend on if the user has any votes - the page isn't
> very useful otherwise, plus it would solve the problem of when to call
> GetVersionTable

If the user has no votes, the page should say "You have no votes", and perhaps
link to the page explaining voting.

> c) Its a simple, quick, easy, fix, which we need for 2.16

If you are going to put it in the user hash, call it "has_votes", not
"use_votes". But I believe it makes no sense there. Put it in the root of $vars,
and remove the other version in

a) No, they're not the same. Remember, votes are per product. The footer should
show if there are any votes, anywhere, but we should only let people vote on a
bug if that particular product allows votes.

b) Well, thats what the page says

c) Yeah, but we can't easily do that, because of ordering issues.
IMHO, the patch works fine, tested ok here. When there are no votes it simply
says " You are currently not voting on any bugs. " 
bbaetz: I'll concede the other points, but what are "ordering issues"?

The ordering issue is that we set this when we create the template object, but
thats done before we call GetVersionTable, which is needed. If we call
GetVersionTable earlier (and always, in, then its called before the
code whch checks for stuff like shutdownhtml. If we move that arround (and,
IIRC, that means changing files, which may create other oridering issues), then
that causes other problems. Its not impossible to solve, but its not a simple
change either.

Since we don't use a wrapper func to process a template (yet) we can't do it there.
Comment on attachment 88121 [details] [diff] [review]

r=gerv. Whatever - if it works, let's get it into 2.16 :-)

Attachment #88121 - Flags: review+
Comment on attachment 88121 [details] [diff] [review]

This patch needs-work, because quey.cgi needs to check this var too, even if
we're not logged in.

Attachment #88121 - Flags: review-
-> patch author bbaetz
Assignee: justdave → bbaetz
bbaetz: this is your patch, and you marked it needs-work on yourself.  Status?
So, we need a solution to this.

use_votes has to be made available even when noone has logged in at all, because
of bug 155031. I think I should change the bug_form vote param name, since its
conceptually different.

However, I'm not sure where to put this check, any more. Maybe query.cgi should
handle it differently?

Or maybe use_votes should really be a param. Thats not happening for 2.16, though.

Blocks: 155031
Attached patch v2Splinter Review
Making it a param isn't that hard...

 - Not everywhere is checked (eg votes.cgi doesn't check). Votes.cgi doesn't
check currently, though. A lot of the use* stuff isn't checked that well. We
should tidy it up at some stage.
 - Its not automatic. It is, however, more predictable.

 - Its a lot simpler
 - It works
Attachment #88121 - Attachment is obsolete: true
Comment on attachment 90462 [details] [diff] [review]

Looks good, works.  r=myk. Needs a second review.
Attachment #90462 - Flags: review+
Comment on attachment 90462 [details] [diff] [review]

Attachment #90462 - Flags: review+
Checked in, trunk + branch
Closed: 22 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.