244239 - Add group-based pronouns to query

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, enhancement, P3)

Description

[Joel Peshkin]

Add the ability to specify %group.groupname% in queries to represent all members
of a group.

Comment 1

[Joel Peshkin]

Regarding Vlad's note in bug 240325, comment 1....

  I cannot think of a reason to be concerned if a user wants to narrow the
search for bugs that he can already see by criteria relating to groups of which
he is not a member.

  The one concern I do have is that someone who is a member of the
"CustomerPepsi" group could use a query (with debug=1) to confirm if a group
called "CustomerCoke" exists as well.  Assuming that this site is
bugzilla.acmebottlecap.com and wants to keep its customers isolated from each
other, we would not want this confirmation to reveal any information.  However,
the acmebottlecap.com account manager probably does want to query on bugs
reported by anyone at Pepsi without including himself in that group.

Comment 2

[Dave Miller [:justdave]]

right, that's what I was concerned about, too.  The validation of the group
parameter should be done in such a way that the lack of membership in a group
turns up the same error that the lack of a group does.

Comment 3

[Joel Peshkin]

That still doesn't cover the case where someone wants to use the group as a way
of clumping a bunch of users without having to include himself (like the account
manager example).  I'd like to find another way to determine if a user is
allowed to know of a group without having to be a member himserlf.

The criteria that covers the applications I can think of is probably a bit too
wierd to use.  I might be able to query on a group if I am a member of every
group that is used for bugs in which that group is included.  That means that I
can reference a group if it is not itself used for bugs AND members of that
group can never see anything I cannot.  That solves the account manager problem
nicely, but it would be hell to explain.  (Unless I can use the term "magic" in
the documentation)

Comment 4

[Dave Miller [:justdave]]

That sounds like it would be hell to write SQL to discover, too.

How much of a presumption would it be making to assume that said hypothetical
account manager might have bless rights for the group in question?  Perhaps we
can just check if they're either a member of or have bless rights for the group.

Comment 5

[Joel Peshkin]

In any decent size company, the account manager would certainly not have bless
permissions.  That said, any of the following would work....

1) Place people who are allowed to ue group-based pronouns in yet another system
group.
2) Add another user_group_map relationship defining who is allowed to know of
the existence of the group.


I'd really like to find a simpler solution, though.

Comment 6

[Joel Peshkin]

OK, here we go...

you can use a group pronoun if ANY of the following are true...

1) You are a member of the group
2) You can bless the group
3) You are a member of bz_use_group_pronouns

Comment 7

[Joel Peshkin]

OK, bug 251837 makes this a lot simpler...

You can use a group pronoun if you are in a group or if group_group_map says it
is visible to you.

We'll have to enforce this at the time Search.pm executes the query.  Initially,
it will be OK to use the permissions of the user under whose permissions the
query is being run. However, we will eventually need to make it possible for a
query to facter in the visibility rules fo the query creator even if it is being
run for its recipient.

Comment 8

[Joel Peshkin]

Attachment: group pronouns

Comment 9

[Joel Peshkin]

This makes it possible to use terms like
%group.groupname% in boolean charts 

with equals, notequals, or anyexact operators on reporter, assignee, qa_contact,
and CC fields.

Subsequent patches will add this capability for commenter and activity records

Comment 10

[Joel Peshkin]

One other note.  To use this, you must turn "usevisibilitygroups" on and then
visit editgroups and enable some groups to see each other.  If the person
running the query is not allowed to "see" the group, they will get an error just
as if the group does not exist.

Comment 11

[Erik Stambaugh]

Comment on attachment 154531 [details] [diff] [review]
group pronouns


>Index: Bugzilla/Search.pm
>===================================================================
>+             $groupid || ThrowUserError('invalid_group_name',{name => $group});

'invalid_group_name' is not defined in user-error.html.tmpl

Also, though I don't think this was introduced by this specific patch, I had
trouble taking an existing group and giving it visibility to itself. 
editgroups.cgi complained that I didn't change anything.  It is ignoring my
attempts to make a group capable of seeing itself, even if I try slipping it in
next to other group changes.

I tried creating two groups, putting a new user in both groups, and making the
groups capable of seeing each other, and it's still trying to throw an
invalid_group_name error.

Comment 12

[Joel Peshkin]

Attachment: v2

Comment 13

[Erik Stambaugh]

Comment on attachment 155237 [details] [diff] [review]
v2

Looks good.

Comment 14

[Erik Stambaugh]

Actually, I do have one little nit.  Should the ValidateGroupName function be
located somewhere besides Search.pm?  I'd like to call it in my patch for bug
253721, and I wonder if it belongs in a different module.

Comment 15

[Dave Miller [:justdave]]

Comment on attachment 155237 [details] [diff] [review]
v2

it's a little disappointing that it doesn't work in the Email and Numbering
section, but that can come later. :)  (Probably means that Email and Numbering
isn't getting converted to chart format properly internally, which would be a
different bug)

Seems to work pretty good.

Comment 16

[Dave Miller [:justdave]]

(In reply to comment #14)
> Actually, I do have one little nit.  Should the ValidateGroupName function be
> located somewhere besides Search.pm?  I'd like to call it in my patch for bug
> 253721, and I wonder if it belongs in a different module.

Yeah, for lack of a better place, maybe in Bugzilla::Util...

Comment 17

[Joel Peshkin]

We'll move the ValidateGroupName function as part of bug 253721


Also, I'll look at converting the email/numbering queries to use the same
charting code after some of the other Search.pm corrections are done.  I think
it is not using the standard charting code today because the code was broken
(one of the many LEFT JOIN issues)

Comment 18

[Joel Peshkin]

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

Comment 19

[timeless]

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

Comment 20

[victory <[e-mail]>]

i won't do anything about this :-(

Comment 21

[Frédéric Buclin]

Attachment: doc patch, v1

Comment 22

[Frédéric Buclin]

tip:

Checking in docs/xml/using.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v  <--  using.xml
new revision: 1.79; previous revision: 1.78
done

3.0.3:

Checking in docs/xml/using.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v  <--  using.xml
new revision: 1.64.2.12; previous revision: 1.64.2.11
done

2.22.3:

Checking in docs/xml/using.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v  <--  using.xml
new revision: 1.37.2.26; previous revision: 1.37.2.25
done

2.20.5:

Checking in docs/xml/using.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v  <--  using.xml
new revision: 1.33.2.27; previous revision: 1.33.2.26
done