Bug 556167: Move the setting of dependson/blocked from process_bug.cgi

into Bugzilla::Bug::set_all
r=dkl, a=mkanat

2 files changed, 41 insertions, 26 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index ee53a0aae..26e71c4f8 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -1854,6 +1854,36 @@ sub set_all {
     my $self = shift;
     my ($params) = @_;
 
+    if (exists $params->{'dependson'} or exists $params->{'blocked'}) {
+        my %set_deps;
+        foreach my $name (qw(dependson blocked)) {
+            my @dep_ids = @{ $self->$name };
+            # If only one of the two fields was passed in, then we need to
+            # retain the current value for the other one.
+            if (!exists $params->{$name}) {
+                $set_deps{$name} = \@dep_ids;
+                next;
+            }
+
+            # Explicitly setting them to a particular value overrides
+            # add/remove.
+            if (exists $params->{$name}->{set}) {
+                $set_deps{$name} = $params->{$name}->{set};
+                next;
+            }
+
+            foreach my $add (@{ $params->{$name}->{add} || [] }) {
+                push(@dep_ids, $add) if !grep($_ == $add, @dep_ids);
+            }
+            foreach my $remove (@{ $params->{$name}->{remove} || [] }) {
+                @dep_ids = grep($_ != $remove, @dep_ids);
+            }
+            $set_deps{$name} = \@dep_ids;
+        }
+
+        $self->set_dependencies($set_deps{'dependson'}, $set_deps{'blocked'});
+    }
+
     if (exists $params->{'keywords'}) {
         $self->modify_keywords($params->{'keywords'},
                                $params->{'keywords_action'});
diff --git a/process_bug.cgi b/process_bug.cgi
index 0753773df..bac09450f 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -280,32 +280,6 @@ if ($cgi->param('id')) {
     $first_bug->set_flags($flags, $new_flags);
 }
 
-if ($cgi->param('id') && (defined $cgi->param('dependson')
-                          || defined $cgi->param('blocked')) )
-{
-    $first_bug->set_dependencies(scalar $cgi->param('dependson'),
-                                 scalar $cgi->param('blocked'));
-}
-elsif (should_set('dependson') || should_set('blocked')) {
-    foreach my $bug (@bug_objects) {
-        my %temp_deps;
-        foreach my $type (qw(dependson blocked)) {
-            $temp_deps{$type} = { map { $_ => 1 } @{$bug->$type} };
-            if (should_set($type) && $cgi->param($type . '_action') =~ /^(add|remove)$/) {
-                foreach my $id (split(/[,\s]+/, $cgi->param($type))) {
-                    if ($cgi->param($type . '_action') eq 'remove') {
-                        delete $temp_deps{$type}{$id};
-                    }
-                    else {
-                        $temp_deps{$type}{$id} = 1;
-                    }
-                }
-            }
-        }
-        $bug->set_dependencies([ keys %{$temp_deps{'dependson'}} ], [ keys %{$temp_deps{'blocked'}} ]);
-    }
-}
-
 # Component, target_milestone, and version are in here just in case
 # the 'product' field wasn't defined in the CGI. It doesn't hurt to set
 # them twice.
@@ -348,6 +322,17 @@ if (should_set('see_also')) {
 if (should_set('remove_see_also')) {
     $set_all_fields{'see_also'}->{remove} = [$cgi->param('remove_see_also')];
 }
+foreach my $dep_field (qw(dependson blocked)) {
+    if (should_set($dep_field)) {
+        if (my $dep_action = $cgi->param("${dep_field}_action")) {
+            $set_all_fields{$dep_field}->{$dep_action} =
+                [split(/\s,/, $cgi->param($dep_field))];
+        }
+        else {
+            $set_all_fields{$dep_field}->{set} = $cgi->param($dep_field);
+        }
+    }
+}
 
 my @custom_fields = Bugzilla->active_custom_fields;
 foreach my $field (@custom_fields) {