Closed Bug 133276 Opened 21 years ago Closed 21 years ago

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

Categories

(Bugzilla :: Creating/Changing Bugs, defect)

2.15
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 2.16

People

(Reporter: ddkilzer, Assigned: ddkilzer)

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

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.
Target -> 2.16.  Keywords: patch, regression, review.  Add CCs.
Reassign to me.
Assignee: myk → ddkilzer
Keywords: patch, regression, review
Target Milestone: --- → Bugzilla 2.16
Note that without this fix, the text for "Only users in the
selected groups can view this bug" appears with no checkboxes
below it.
Status: NEW → ASSIGNED
Attached patch Patch v.1 (obsolete) — Splinter Review
Changes |IF groups| to |IF groups.size > 0|.
Attached patch Patch v.1Splinter Review
Changes |IF groups| to |IF groups.size > 0|.
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 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.
Attachment #75975 - Attachment is obsolete: true
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
Attachment #75976 - Flags: review+
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 ?
Bradley - your last comment makes no sense to me :-)

Gerv
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.
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
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.
Also, 'usebuggroups' and 'usebuggroupsentry' are both off, and 
there are NO groups defined (editgroups.cgi lists no groups) in
my local bugzilla.
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 on attachment 75976 [details] [diff] [review]
Patch v.1

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

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
Status: ASSIGNED → RESOLVED
Closed: 21 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.