133276 - Check for data in 'groups' variable in show_bug.cgi template doesn't work

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[David D. Kilzer (ddk)]

The check for existance of data in the 'groups' variable doesn't
work in the show_bug.html.tmpl template.  It should use
|IF groups.size > 0| instead of simply |IF groups|.  Patch to
follow shortly.

Bug 110012 is the original show_bug.cgi templatization bug.

Comment 1

[David D. Kilzer (ddk)]

Target -> 2.16.  Keywords: patch, regression, review.  Add CCs.
Reassign to me.

Comment 2

[David D. Kilzer (ddk)]

Note that without this fix, the text for "Only users in the
selected groups can view this bug" appears with no checkboxes
below it.

Comment 3

[David D. Kilzer (ddk)]

Attachment: Patch v.1

Changes |IF groups| to |IF groups.size > 0|.

Comment 4

[David D. Kilzer (ddk)]

Attachment: Patch v.1

Changes |IF groups| to |IF groups.size > 0|.

Comment 5

[David D. Kilzer (ddk)]

How in blazes did I get TWO IDENTICAL patches attached to this
bug at the same time?!  I swear I only hit 'Submit' once!

Comment 6

[David D. Kilzer (ddk)]

Comment on attachment 75975 [details] [diff] [review]
Patch v.1

No clue how I got two identical patches attached to this bug.

Marking the first one obsolete.

Comment 7

[Gervase Markham [:gerv]]

Comment on attachment 75976 [details] [diff] [review]
Patch v.1

2xr=gerv. You're right - groups.size doesn't work for complex arrays.

I'll check this in if ddk doesn't beat me to it.

Gerv

Comment 8

[Bradley Baetz (:bbaetz)]

Comment on attachment 75976 [details] [diff] [review]
Patch v.1

The real problem is that the groups var is being pushed to the template when it
shouldn't be.

You have:
   # Groups
    if ($::usergroupset ne '0' || $bug{'groupset'} ne '0') {	  

Don't you want == instead ?

Comment 9

[Gervase Markham [:gerv]]

Bradley - your last comment makes no sense to me :-)

Gerv

Comment 10

[Bradley Baetz (:bbaetz)]

Sorry - had to run.

In bug_form.pl, you have those two lines, and you push $vars{groups} inside that
if. So you should only get groups if the user is in a group, or the bug is in a
group. that way you don't have to worry about the array being empty, and you
don't do  uneeded db queries.

The only part of that if statement which looks wrong is that you use eq instead
of ==. I didn't debug it though.

Comment 11

[Gervase Markham [:gerv]]

Ah, right. So you are saying the groups var is being pushed when it shouldn't
be, but you don't know why that is, given that the code seems to do the right thing?

Perhaps switching to "== 0" would be a smart move, too. Let's do that as well.

Gerv

Comment 12

[David D. Kilzer (ddk)]

In my case, the code is being run because:

  $::usergroupset = '9223372036854775807';
  $bug{'groupset'} = '0';

I can't figure out where $::usergroupset is being set to 'all
64 bits', though.

Comment 13

[David D. Kilzer (ddk)]

Also, 'usebuggroups' and 'usebuggroupsentry' are both off, and 
there are NO groups defined (editgroups.cgi lists no groups) in
my local bugzilla.

Comment 14

[Bradley Baetz (:bbaetz)]

Hmm. thats odd.

I have groups, and when I view a bug as a user who isn' tin those groups, I see
the header, but no groups. If usergroupset was all 64 buts, I'd expect to see
those grousp, too. I'm not at home, but I'll check later

/me thinks.

Oh. HAng on, you're right. The user can be in a group without being in a
buggroup, because of canconfirm.

So the original patch is correct. Oops

Comment 15

[Bradley Baetz (:bbaetz)]

Comment on attachment 75976 [details] [diff] [review]
Patch v.1

r=bbaetz, too

Comment 16

[Gervase Markham [:gerv]]

OK, so the next action is to check in Patch v.1, which is the second version. :-)

Gerv

Comment 17

[Gervase Markham [:gerv]]

Fixed.

Checking in template/default/show/show_bug.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/default/show/show_bug.html.tmpl,v 
<--  show_bug.html.tmpl
new revision: 1.3; previous revision: 1.2
done

Gerv