324783 - The chartgroup, insidergroup and timetrackinggroup groups in editparams.cgi should be listed in a dropdown menu

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[Frédéric Buclin]

This doesn't make sense to "guess" the group names. We already moved from a text field to a dropdown menu in editflagtypes.cgi, we should do the same here. It's boring to manually enter the group names and then realize this doesn't work because you did a typo.

I would even tend to take it for 2.22, but that's probably too late.

Comment 1

[Frédéric Buclin]

Note also that there is no check about entered groups. editparams.cgi should at least make sure the groups you entered are valid ones. In the same way, editgroups.cgi should:
1) update the group in editparams.cgi when a group used by it is renamed;
2) reject to delete a group if the group is used by editparams.cgi.

I will fill a new bug for editgroups.cgi. And let fix editparams.cgi here.

Comment 2

[Frédéric Buclin]

Attachment: patch, v1

Comment 3

[Colin Ogilvie [:cso]]

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

>Index: Bugzilla/Config/Common.pm
>+sub check_group {
>+    my $group_name = shift;
>+    my $group = new Bugzilla::Group({'name' => $group_name});
>+    unless (defined $group) {
>+        return "Must be an existing group name";
>+    }
>+    return "";
>+}
>+

Nit:

It may be personal, but I'd prefer:

  return "Must be an existing group name" unless defined $group;

Comment 4

[Frédéric Buclin]

Checking in Bugzilla/Group.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Group.pm,v  <--  Group.pm
new revision: 1.13; previous revision: 1.12
done
Checking in Bugzilla/Config/Common.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config/Common.pm,v  <--  Common.pm
new revision: 1.9; previous revision: 1.8
done
Checking in Bugzilla/Config/GroupSecurity.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config/GroupSecurity.pm,v  <--  GroupSecurity.pm
new revision: 1.4; previous revision: 1.3
done