275608 - editflag group selection should be a listbox w/ --- meaning no restriction

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[timeless]

i went to try to restrict a flag to a list of people (it seemed logical). unfortunately it turns out you can't 
select people. if the list of choices is in fact a *single closed list*, then it should be a listbox.

otherwise you're expecting someone who may not even have editgroups (i certainly don't) to remember 
some random string, or if they happen to be gods, to go back to editgroups to find out.

Comment 1

[Frédéric Buclin]

Do we want users with editcomponents privs to see groups? Should we define a new
kind of groups dedicated to flag types and which could be used by users having
editcomponents privs (bug 275610)?

Comment 2

[Frédéric Buclin]

I already see many complains coming if this is not fixed before we release 2.20!

Comment 3

[Dave Miller [:justdave]]

although this would be way cool to have, we have several other places where we
ask for groups as a textbox, so at least this is consistant.  I'll take it for
2.20 but I won't block the release on it.

as for implementation, do we need to consider usevisibilitygroups here?

Comment 4

[Frédéric Buclin]

Attachment: patch, v1

The main changes are in admin/flag-type/edit.html.tmpl. Note that this patch
requires the patch from bug 306271 to be applied.

Comment 5

[GavinS]

Comment on attachment 194134 [details] [diff] [review]
patch, v1

Yes, tested, all works OK.

Nits/Comments

1) Problems if a group is called '(No Group)', but we can't do anything sane
about that, so we can live with it

2) 80 character per line exceeded in the new select BLOCK

3) It would be nice to pass in the selected value to the select BLOCK, but that
can wait till we have a generic solution

4) I'm sure that some bits of the code would be simpler if we used group ids as
the option values in the select -- but I don't think that enough bits of the
code would be simpler to ask for it to be changed now

Comment 6

[Frédéric Buclin]

(In reply to comment #5)
> 1) Problems if a group is called '(No Group)', but we can't do anything sane
> about that, so we can live with it

The distinction between both is made thanks to the value of the option which is
"" when no group restriction is chosen and "(no group)" for the '(no group)' group.


> 4) I'm sure that some bits of the code would be simpler if we used group ids as
> the option values in the select -- but I don't think that enough bits of the
> code would be simpler to ask for it to be changed now

Actually not, because validateGroups() in editflagtypes.cgi in all cases has to
make sure that the ID/name is valid. So the only change would be SELECT ... FROM
groups WHERE id = ... instead of WHERE name = ... .

Comment 7

[Frédéric Buclin]

Checking in editflagtypes.cgi;
/cvsroot/mozilla/webtools/bugzilla/editflagtypes.cgi,v  <--  editflagtypes.cgi
new revision: 1.26; previous revision: 1.25
done
Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v 
<--  filterexceptions.pl
new revision: 1.49; previous revision: 1.48
done
Checking in template/en/default/admin/flag-type/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/flag-type/edit.html.tmpl,v
 <--  edit.html.tmpl
new revision: 1.12; previous revision: 1.11
done