Bug 687725: Adding a local bug ID in the See Also field isn't logged in the bug history of that bug

r/a=LpSolit

2 files changed, 34 insertions, 66 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 515fabb90..483a2a865 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -966,6 +966,9 @@ sub update {
                                 join(', ', map { $_->name } @$added_see)];
     }
 
+    $_->update foreach @{ $self->{_update_ref_bugs} || [] };
+    delete $self->{_update_ref_bugs};
+
     # Log bugs_activity items
     # XXX Eventually, when bugs_activity is able to track the dupe_id,
     # this code should go below the duplicates-table-updating code below.
@@ -2835,7 +2838,7 @@ sub remove_group {
 }
 
 sub add_see_also {
-    my ($self, $input) = @_;
+    my ($self, $input, $skip_recursion) = @_;
 
     # This is needed by xt/search.t.
     $input = $input->name if blessed($input);
@@ -2851,15 +2854,6 @@ sub add_see_also {
     $uri = $field_values->{value};
     $field_values->{value} = $uri->as_string;
 
-    # If this is a link to a local bug then save the
-    # ref bug id for sending changes email.
-    if ($class->isa('Bugzilla::BugUrl::Bugzilla::Local')) {
-        my $ref_bug = $field_values->{ref_bug};
-        my $self_url = $class->local_uri($self->id);
-        push @{ $self->{see_also_changes} }, $ref_bug->id
-            if !grep { $_->name eq $self_url } @{ $ref_bug->see_also };
-    }
-
     # We only add the new URI if it hasn't been added yet. URIs are
     # case-sensitive, but most of our DBs are case-insensitive, so we do
     # this check case-insensitively.
@@ -2873,13 +2867,22 @@ sub add_see_also {
                                                newvalue => $value,
                                                privs    => $privs });
         }
-
+        # If this is a link to a local bug then save the
+        # ref bug id for sending changes email.
+        my $ref_bug = delete $field_values->{ref_bug};
+        if ($class->isa('Bugzilla::BugUrl::Bugzilla::Local')
+            and !$skip_recursion)
+        {
+            $ref_bug->add_see_also($self->id, 'skip_recursion');
+            push @{ $self->{_update_ref_bugs} }, $ref_bug;
+            push @{ $self->{see_also_changes} }, $ref_bug->id;
+        }
         push @{ $self->{see_also} }, bless ($field_values, $class);
     }
 }
 
 sub remove_see_also {
-    my ($self, $url) = @_;
+    my ($self, $url, $skip_recursion) = @_;
     my $see_also = $self->see_also;
 
     # This is needed by xt/search.t.
@@ -2887,17 +2890,6 @@ sub remove_see_also {
 
     my ($removed_bug_url, $new_see_also) =
         part { lc($_->name) ne lc($url) } @$see_also;
- 
-    # Since we remove also the url from the referenced bug,
-    # we need to notify changes for that bug too.
-    $removed_bug_url = $removed_bug_url->[0];
-    if ($removed_bug_url
-        and $removed_bug_url->isa('Bugzilla::BugUrl::Bugzilla::Local')
-        and defined $removed_bug_url->ref_bug_url)
-    {
-        push @{ $self->{see_also_changes} },
-             $removed_bug_url->ref_bug_url->bug_id;
-    }
 
     my $privs;
     my $can = $self->check_can_change_field('see_also', $see_also, $new_see_also, \$privs);
@@ -2907,6 +2899,23 @@ sub remove_see_also {
                                            privs    => $privs });
     }
 
+    # Since we remove also the url from the referenced bug,
+    # we need to notify changes for that bug too.
+    $removed_bug_url = $removed_bug_url->[0];
+    if (!$skip_recursion and $removed_bug_url
+        and $removed_bug_url->isa('Bugzilla::BugUrl::Bugzilla::Local'))
+    {
+        my $ref_bug
+            = Bugzilla::Bug->check($removed_bug_url->ref_bug_url->bug_id);
+
+        if (Bugzilla->user->can_edit_product($ref_bug->product_id)) {
+            my $self_url = $removed_bug_url->local_uri($self->id);
+            $ref_bug->remove_see_also($self_url, 'skip_recursion');
+            push @{ $self->{_update_ref_bugs} }, $ref_bug;
+            push @{ $self->{see_also_changes} }, $ref_bug->id;
+        }
+    }
+
     $self->{see_also} = $new_see_also || [];
 }
 
diff --git a/Bugzilla/BugUrl/Bugzilla/Local.pm b/Bugzilla/BugUrl/Bugzilla/Local.pm
index bdfae2835..99f944fa6 100644
--- a/Bugzilla/BugUrl/Bugzilla/Local.pm
+++ b/Bugzilla/BugUrl/Bugzilla/Local.pm
@@ -42,56 +42,15 @@ sub ref_bug_url {
 
     if (!exists $self->{ref_bug_url}) {
         my $ref_bug_id = new URI($self->name)->query_param('id');
+        my $ref_bug = Bugzilla::Bug->check($ref_bug_id);
         my $ref_value = $self->local_uri($self->bug_id);
         $self->{ref_bug_url} =
-            new Bugzilla::BugUrl::Bugzilla::Local({ bug_id => $ref_bug_id,
+            new Bugzilla::BugUrl::Bugzilla::Local({ bug_id => $ref_bug->id,
                                                     value => $ref_value });
     }
     return $self->{ref_bug_url};
 }
 
-sub insert_create_data {
-    my ($class, $field_values) = @_;
-
-    my $ref_bug = delete $field_values->{ref_bug};
-    my $bug_url = $class->SUPER::insert_create_data($field_values);
-    my $url = $class->local_uri($bug_url->bug_id);
-
-    # Check if the ref bug has already the url and then,
-    # update the ref bug to point to the current bug.
-    if (!grep { $_->name eq $url } @{ $ref_bug->see_also }) {
-        $class->SUPER::insert_create_data({ value  => $url,
-                                            bug_id => $ref_bug->id,
-                                            class  => ref($class) || $class });
-    }
-
-    return $bug_url;
-}
-
-sub remove_from_db {
-    my $self = shift;
-
-    my $dbh = Bugzilla->dbh;
-    my $ref_bug_url = $self->ref_bug_url;
-
-    $dbh->bz_start_transaction();
-
-    # We remove the current see also first so then we
-    # avoid infinite loop later.
-    $self->SUPER::remove_from_db();
-
-    # We also remove the referenced bug url.
-    if (defined $ref_bug_url) {
-        my $ref_bug = Bugzilla::Bug->check($ref_bug_url->bug_id);
-        my $product = $ref_bug->product_obj;
-        if (Bugzilla->user->can_edit_product($product->id)) {
-            $ref_bug_url->remove_from_db();
-        }
-    }
-
-    $dbh->bz_commit_transaction();
-}
-
 sub should_handle {
     my ($class, $uri) = @_;