Bug 1169131: add-mozreview-children.pl production run issues

2 files changed, 148 insertions, 60 deletions

diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index e87f1f674..830c60a57 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -81,6 +81,8 @@ use constant AUDIT_REMOVES => 0;
 
 use constant SKIP_REQUESTEE_ON_ERROR => 1;
 
+our $disable_flagmail = 0;
+
 sub DB_COLUMNS {
     my $dbh = Bugzilla->dbh;
     return qw(
@@ -977,6 +979,10 @@ or deleted.
 sub notify {
     my ($class, $flag, $old_flag, $obj, $timestamp) = @_;
 
+    if ($disable_flagmail) {
+        return;
+    }
+
     my ($bug, $attachment);
     if (blessed($obj) && $obj->isa('Bugzilla::Attachment')) {
         $attachment = $obj;
@@ -1124,6 +1130,10 @@ sub _flag_types {
     return $flag_types;
 }
 
+1;
+
+__END__
+
 =head1 B<Methods in need of POD>
 
 =over
@@ -1149,5 +1159,3 @@ sub _flag_types {
 =item update
 
 =back
-
-1;
diff --git a/extensions/MozReview/bin/add-mozreview-children.pl b/extensions/MozReview/bin/add-mozreview-children.pl
index fd990d92c..b93a95a94 100755
--- a/extensions/MozReview/bin/add-mozreview-children.pl
+++ b/extensions/MozReview/bin/add-mozreview-children.pl
@@ -27,13 +27,19 @@ Bugzilla->usage_mode(USAGE_MODE_CMDLINE);
 use Bugzilla::Attachment;
 use Bugzilla::Bug;
 use Bugzilla::Constants;
+use Bugzilla::Flag;
+use Bugzilla::FlagType;
 use JSON;
 use LWP::Simple qw( get $ua );
 
+$Bugzilla::Flag::disable_flagmail = 1;
+
 if (my $proxy = Bugzilla->params->{proxy_url}) {
     $ua->proxy('https', $proxy);
 }
 
+my $MOZREVIEW_MIMETYPE = 'text/x-review-board-request';
+
 # Disable the "cannot ask for review" so we can reassign their flags to
 # the new attachments.
 Bugzilla->params->{max_reviewer_last_seen} = 0;
@@ -41,26 +47,32 @@ Bugzilla->params->{max_reviewer_last_seen} = 0;
 my $rb_host = shift or die "syntax: $0 review-board-url\n";
 $rb_host =~ s#/$##;
 
-my $dbh = Bugzilla->dbh;
+sub rr_url {
+    my ($rrid) = @_;
+    return $rb_host . "/r/" . $rrid . "/";
+}
 
-$dbh->bz_start_transaction();
+my $dbh = Bugzilla->dbh;
 
 my $bugs_query = "SELECT distinct bug_id FROM attachments WHERE mimetype='text/x-review-board-request' AND isobsolete=0";
 my $bug_ids = $dbh->selectcol_arrayref($bugs_query);
-my $bug_count = scalar @$bug_ids;
-$bug_count or die "No bugs were found.\n";
+my $total_bugs = scalar @$bug_ids;
+$total_bugs or die "No bugs were found.\n";
+my $bug_count = 0;
 
 print <<EOF;
-About to convert MozReview attachments for $bug_count bugs.
+About to convert MozReview attachments for $total_bugs bugs.
 
 Press <Ctrl-C> to stop or <Enter> to continue...
 EOF
 getc();
 
 foreach my $bug_id (@$bug_ids) {
+    $dbh->bz_start_transaction();
     my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
+    my $bug_changed = 0;
     my $bug = Bugzilla::Bug->new($bug_id);
-    print "Bug " . $bug->id . "\n";
+    print "Bug " . $bug->id . " (" . ++$bug_count  . "/" . $total_bugs . ")\n";
 
     my $url = $rb_host . "/api/extensions/mozreview.extension.MozReviewExtension/summary/?bug=" . $bug->id;
     print "  Fetching reviews from $url...\n";
@@ -68,84 +80,152 @@ foreach my $bug_id (@$bug_ids) {
     die "Error fetching review requests for bug " . $bug->id
         unless defined $body;
 
-    my $attachments = Bugzilla::Attachment->get_attachments_by_bug($bug);
-    my %families;
     my $data = from_json($body);
     my $summaries = $data->{"review_request_summaries"};
-    foreach my $summary (@$summaries) {
-        $families{$summary->{"parent"}->{"id"}} = $summary;
+    my $attachments = Bugzilla::Attachment->get_attachments_by_bug($bug);
+    my %attach_map;
+
+    my $flag_types = Bugzilla::FlagType::match({
+        'target_type'  => 'attachment',
+        'product_id'   => $bug->product_id,
+        'component_id' => $bug->component_id,
+        'is_active'    => 1});
+    my $flag_type;
+
+    foreach my $ft (@$flag_types) {
+        if ($ft->is_active && $ft->name eq "review") {
+            $flag_type = $ft;
+            last;
+        }
     }
+
+    if (!defined($flag_type)) {
+        print "      Couldn't find flag type for attachments on this bug!\n";
+        $dbh->bz_rollback_transaction();
+        next;
+    }
+
     foreach my $attachment (@$attachments) {
         next if ($attachment->isobsolete
-                 || $attachment->contenttype ne 'text/x-review-board-request');
+                 || $attachment->contenttype ne $MOZREVIEW_MIMETYPE);
+
         print "  Attachment " . $attachment->id . ": " . $attachment->data . "\n";
         my ($rrid) = $attachment->data =~ m#/r/(\d+)/?$#;
         if (!defined($rrid)) {
             print "    Malformed or missing reviewboard URL\n";
             next;
         }
-        my $family = $families{$rrid};
-        if (!defined($family)) {
-            print "    Cannot find family with parent $rrid associated with bug " . $bug->id . "\n";
-            next;
+
+        $attach_map{$attachment->data} = $attachment;
+    }
+
+    foreach my $summary (@$summaries) {
+        my $parent = $summary->{"parent"};
+        my $attacher = Bugzilla::User->new({ id => $parent->{"submitter_bmo_id"},
+                                             cache => 1 });
+        Bugzilla->set_user($attacher);
+        print "  Parent review request " . $parent->{"id"} . "\n";
+
+        # %parent_flags is used to keep track of review flags related to
+        # reviewers.  It maps requestee => status if status is "?" or
+        # setter => status otherwise.
+        my %parent_flags;
+
+        my $parent_url = rr_url($parent->{"id"});
+        my $parent_attach = $attach_map{$parent_url};
+        if (defined($parent_attach)) {
+            print "    Parent attachment has ID " . $parent_attach->id . ". Obsoleting it.\n";
+            foreach my $flag (@{ $parent_attach->flags }) {
+                if ($flag->type->name eq "review") {
+                    if ($flag->status eq "?") {
+                        $parent_flags{$flag->requestee->id} = $flag->status;
+                    } else {
+                        $parent_flags{$flag->setter->id} = $flag->status;
+                    }
+                }
+            }
+            $parent_attach->set_is_obsolete(1);
+            $parent_attach->update($timestamp);
+            print "    Posting comment.\n";
+            $bug->add_comment('',
+                { isprivate  => 0,
+                  type       => CMT_ATTACHMENT_UPDATED,
+                  extra_data => $parent_attach->id });
+            $bug_changed = 1;
+        } else {
+            print "    Parent attachment not found.\n";
         }
-        my @children = @{ $family->{"children"} };
-        print "  Commit requests:\n";
+
+        my @children = @{ $summary->{"children"} };
         foreach my $child (@children) {
-            print "    Review request " . $child->{"id"} . ": " . $child->{"summary"} . "\n";
-            Bugzilla->set_user($attachment->attacher);
+            print "    Child review request " . $child->{"id"} . "\n";
+            my $child_url = rr_url($child->{"id"});
+            my $child_attach = $attach_map{$child_url};
+            if (defined($child_attach)) {
+                print "      Found attachment.\n";
+                next;
+            }
+
+            print "      No attachment found for child " . $child_url . "\n";
             my %child_attach_params = (
                 bug => $bug,
                 data => $rb_host . "/r/" . $child->{"id"} . "/",
                 description => "MozReview Request: " . $child->{"summary"},
                 filename => "reviewboard-" . $child->{"id"} . "-url.txt",
-                mimetype => $attachment->contenttype,
+                mimetype => $MOZREVIEW_MIMETYPE,
             );
-            my $flag_type;
-            foreach my $ft (@{ $attachment->flag_types }) {
-                if ($ft->is_active && $ft->name eq "review") {
-                    $flag_type = $ft;
-                    last;
-                }
-            }
-            my $child_attach = Bugzilla::Attachment->create(\%child_attach_params);
+            $child_attach = Bugzilla::Attachment->create(\%child_attach_params);
             print "      New attachment id: " . $child_attach->id . "\n";
+            $bug_changed = 1;
+
+            # Set flags.  If there was a parent, check it for flags by the
+            # requestee.  Otherwise, set an r? flag.
+
             foreach my $reviewer_id (@{ $child->{"reviewers_bmo_ids"} }) {
                 my $reviewer = Bugzilla::User->new({ id => $reviewer_id,
                                                      cache => 1 });
-                print "      Adding reviewer " . $reviewer->login . "\n";
-                my $child_flag = Bugzilla::Flag->set_flag($child_attach, {
-                    type_id => $flag_type->id,
-                    status => "?",
-                    requestee => $reviewer->login,
-                    setter => $attachment->attacher,
-                });
-            }
-            $child_attach->update($timestamp);
+                print "      Reviewer " . $reviewer->login . " (" . $reviewer->id . ")\n";
+                $reviewer->settings->{block_reviews}->{value} = 'off';
+                my $flag_status = $parent_flags{$reviewer->id};
+                if (defined($flag_status)) {
+                    print "      Flag for reviewer " . $reviewer->id . ": " . $flag_status . "\n";
+                    my %params = (
+                        type_id => $flag_type->id,
+                        status  => $flag_status
+                    );
+
+                    if ($flag_status eq "?") {
+                        $params{'requestee'} = $reviewer->login;
+                        $params{'setter'} = $attacher;
+                    } else {
+                        $params{'setter'} = $reviewer;
+                    }
+
+                    my $child_flag = Bugzilla::Flag->set_flag($child_attach, \%params);
+                } else {
+                    # No flag on the parent; this probably means the reviewer
+                    # cancelled the review, so don't set r?.
+                    print "      No review flag for reviewer " . $reviewer->id . "\n";
+                }
 
-            print "      Posting comment.\n";
-            $bug->add_comment('',
-                { isprivate  => 0,
-                  type       => CMT_ATTACHMENT_CREATED,
-                  extra_data => $child_attach->id });
+                $child_attach->update($timestamp);
+                print "      Posting comment.\n";
+                $bug->add_comment('',
+                    { isprivate  => 0,
+                      type       => CMT_ATTACHMENT_CREATED,
+                      extra_data => $child_attach->id });
+            }
         }
-        print "    Obsoleting parent attachment.\n";
-        $attachment->set_is_obsolete(1);
-        $attachment->update($timestamp);
-        print "    Posting comment.\n";
-        $bug->add_comment('',
-            { isprivate  => 0,
-              type       => CMT_ATTACHMENT_UPDATED,
-              extra_data => $attachment->id });
     }
-    print "    Updating bug.\n";
-    $bug->update($timestamp);
-    $dbh->do("UPDATE bugs SET lastdiffed = ?, delta_ts = ? WHERE bug_id = ?",
-             undef, $timestamp, $timestamp, $bug_id);
-}
-
-$dbh->bz_commit_transaction();
 
-Bugzilla->memcached->clear_all();
+    if ($bug_changed) {
+        print "    Updating bug.\n";
+        $bug->update($timestamp);
+        $dbh->do("UPDATE bugs SET lastdiffed = ?, delta_ts = ? WHERE bug_id = ?",
+                 undef, $timestamp, $timestamp, $bug_id);
+    }
+    $dbh->bz_commit_transaction();
+    Bugzilla->memcached->clear_all();
+}
 
 print "Done.\n";