Bug 469196 - WebService function to update a user's group membership

r=glob, a=glob

2 files changed, 40 insertions, 3 deletions

diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 2631a5eae..55a967796 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -25,6 +25,7 @@ use DateTime::TimeZone;
 use List::Util qw(max);
 use List::MoreUtils qw(any);
 use Scalar::Util qw(blessed);
+use Storable qw(dclone);
 use URI;
 use URI::QueryParam;
 
@@ -374,7 +375,8 @@ sub _set_groups {
            WHERE user_id = ? AND isbless = ? AND grant_type = ?},
         undef, $self->id, $is_bless, GRANT_DIRECT);
 
-    my $new_groups = my $current_groups = Bugzilla::Group->new_from_list($ids);
+    my $current_groups = Bugzilla::Group->new_from_list($ids);
+    my $new_groups = dclone($current_groups);
 
     # Record the changes
     if (exists $changes->{set}) {
@@ -390,10 +392,10 @@ sub _set_groups {
         }
     }
     else {
-        foreach my $group (@{$changes->{removed} // []}) {
+        foreach my $group (@{$changes->{remove} // []}) {
             @$new_groups = grep { $_->id ne $group->id } @$new_groups;
         }
-        foreach my $group (@{$changes->{added} // []}) {
+        foreach my $group (@{$changes->{add} // []}) {
             push @$new_groups, $group
                 unless grep { $_->id eq $group->id } @$new_groups;
         }
diff --git a/Bugzilla/WebService/User.pm b/Bugzilla/WebService/User.pm
index f8358f78d..f05b2b247 100644
--- a/Bugzilla/WebService/User.pm
+++ b/Bugzilla/WebService/User.pm
@@ -314,6 +314,10 @@ sub update {
             # stays consistent for things that can become empty.
             $change->[0] = '' if !defined $change->[0];
             $change->[1] = '' if !defined $change->[1];
+            # We also flatten arrays (used by groups and blessed_groups)
+            $change->[0] = join(',', @{$change->[0]}) if ref $change->[0];
+            $change->[1] = join(',', @{$change->[1]}) if ref $change->[1];
+
             $hash{changes}{$field} = {
                 removed => $self->type('string', $change->[0]),
                 added   => $self->type('string', $change->[1]) 
@@ -733,6 +737,37 @@ C<string> A text field that holds the reason for disabling a user from logging
 into bugzilla, if empty then the user account is enabled otherwise it is
 disabled/closed.
 
+=item C<groups>
+
+C<hash> These specify the groups that this user is directly a member of.
+To set these, you should pass a hash as the value. The hash may contain
+the following fields:
+
+=over
+
+=item C<add> An array of C<int>s or C<string>s. The group ids or group names
+that the user should be added to.
+
+=item C<remove> An array of C<int>s or C<string>s. The group ids or group names
+that the user should be removed from.
+
+=item C<set> An array of C<int>s or C<string>s. An exact set of group ids
+and group names that the user should be a member of. NOTE: This does not
+remove groups from the user where the person making the change does not
+have the bless privilege for.
+
+If you specify C<set>, then C<add> and C<remove> will be ignored. A group in
+both the C<add> and C<remove> list will be added. Specifying a group that the
+user making the change does not have bless rights will generate an error.
+
+=back
+
+=item C<bless_groups>
+
+C<hash> - This is the same as groups, but affects what groups a user
+has direct membership to bless that group. It takes the same inputs as
+groups.
+
 =back
 
 =item B<Returns>