Bug 863686: "Congratulations" email received for first approval+ instead of first review+ (part two)

4 files changed, 41 insertions, 87 deletions

diff --git a/extensions/BMO/Extension.pm b/extensions/BMO/Extension.pm
index 030dcbeab..311c3fe6a 100644
--- a/extensions/BMO/Extension.pm
+++ b/extensions/BMO/Extension.pm
@@ -924,7 +924,7 @@ sub _log_sent_email {
     } else {
         $message_type = $email->header('X-Bugzilla-Watch-Reason');
     }
-    $message_type ||= '?';
+    $message_type ||= $type || '?';
 
     $subject =~ s/[\[\(]Bug \d+[\]\)]\s*//;
 
diff --git a/extensions/ContributorEngagement/Extension.pm b/extensions/ContributorEngagement/Extension.pm
index 13956c3e8..e41635e69 100644
--- a/extensions/ContributorEngagement/Extension.pm
+++ b/extensions/ContributorEngagement/Extension.pm
@@ -19,22 +19,20 @@ use Bugzilla::Install::Util qw(indicate_progress);
 
 use Bugzilla::Extension::ContributorEngagement::Constants;
 
-our $VERSION = '1.0';
+our $VERSION = '2.0';
 
 BEGIN {
-    *Bugzilla::User::first_patch_approved_id = \&_first_patch_approved_id;
+    *Bugzilla::User::first_patch_reviewed_id = \&_first_patch_reviewed_id;
 }
 
-sub _first_patch_approved_id { return $_[0]->{'first_patch_approved_id'}; }
+sub _first_patch_reviewed_id { return $_[0]->{'first_patch_reviewed_id'}; }
 
 sub install_update_db {
     my ($self) = @_;
     my $dbh = Bugzilla->dbh;
 
-    if (!$dbh->bz_column_info('profiles', 'first_patch_approved_id')) {
-        $dbh->bz_add_column('profiles', 'first_patch_approved_id', 
-                            { TYPE => 'INT3' });
-        _migrate_first_approved_ids();
+    if ($dbh->bz_column_info('profiles', 'first_patch_approved_id')) {
+        $dbh->bz_drop_column('profiles', 'first_patch_approved_id');
     }
     if (!$dbh->bz_column_info('profiles', 'first_patch_reviewed_id')) {
         $dbh->bz_add_column('profiles', 'first_patch_reviewed_id', { TYPE => 'INT3' });
@@ -66,42 +64,11 @@ sub _populate_first_reviewed_ids {
     print "done\n";
 }
 
-sub _migrate_first_approved_ids {
-    my $dbh = Bugzilla->dbh;
-
-    my $sth = $dbh->prepare('UPDATE profiles SET first_patch_approved_id = ? WHERE userid = ?');
-    my $ra = $dbh->selectall_arrayref("SELECT attachments.submitter_id, 
-                                              attachments.attach_id, 
-                                              flagtypes.name 
-                                         FROM attachments 
-                                              INNER JOIN flags ON attachments.attach_id = flags.attach_id
-                                              INNER JOIN flagtypes ON flags.type_id = flagtypes.id
-                                        WHERE flags.status = '+'
-                                     ORDER BY flags.modification_date");
-    my $count = 1;
-    my $total = scalar @$ra;
-    my %user_seen;
-    foreach my $ra_row (@$ra) {
-        my ($user_id, $attach_id, $flag_name) = @$ra_row;
-        next if $user_seen{$user_id};
-        my $found_flag = 0;
-        foreach my $flag_re (FLAG_REGEXES) {
-            $found_flag = 1 if ($flag_name =~ $flag_re);
-        }
-        next if !$found_flag;
-        indicate_progress({ current => $count++, total => $total, every => 25 });
-        $sth->execute($attach_id, $user_id);
-        $user_seen{$user_id} = 1;
-    } 
-
-    print "done\n";   
-}
-
 sub object_columns {
     my ($self, $args) = @_;
     my ($class, $columns) = @$args{qw(class columns)};
     if ($class->isa('Bugzilla::User')) {
-        push(@$columns, 'first_patch_approved_id');
+        push(@$columns, 'first_patch_reviewed_id');
     }
 }
 
@@ -109,42 +76,37 @@ sub flag_end_of_update {
     my ($self, $args) = @_;
     my ($object, $timestamp, $new_flags) = @$args{qw(object timestamp new_flags)};
 
-    if ($object->isa('Bugzilla::Attachment') 
-        && @$new_flags 
-        && grep($_ eq $object->bug->product, ENABLED_PRODUCTS)
-        && !$object->attacher->first_patch_approved_id) 
+    if ($object->isa('Bugzilla::Attachment')
+        && @$new_flags
+        && !$object->attacher->first_patch_reviewed_id
+        && grep($_ eq $object->bug->product, ENABLED_PRODUCTS))
     {
         my $attachment = $object;
 
-        # Glob: Borrowed this code from your push extension :)
         foreach my $change (@$new_flags) {
             $change =~ s/^[^:]+://; # get rid of setter
             $change =~ s/\([^\)]+\)$//; # get rid of requestee
             my ($name, $value) = $change =~ /^(.+)(.)$/;
 
-            # Only interested in flags set to +
-            next if $value ne '+';
-
-            my $found_flag = 0; 
-            foreach my $flag_re (FLAG_REGEXES) {
-                $found_flag = 1 if ($name =~ $flag_re);
-            }
-            next if !$found_flag;
-        
-            _send_approval_mail($attachment, $timestamp);
-            
+            # Only interested in review flags set to +
+            next unless $name =~ /^review/ && $value eq '+';
+
+            _send_mail($attachment, $timestamp);
+
+            Bugzilla->dbh->do("UPDATE profiles SET first_patch_reviewed_id = ? WHERE userid = ?",
+                              undef, $attachment->id, $attachment->attacher->id);
             last;
         }
     }
 }
 
-sub _send_approval_mail {
+sub _send_mail {
     my ($attachment, $timestamp) = @_;
-    
-    my $vars = { 
-        date      => format_time($timestamp, '%a, %d %b %Y %T %z', 'UTC'), 
-        to_user   => $attachment->attacher->email, 
-        from_user => EMAIL_FROM,  
+
+    my $vars = {
+        date      => format_time($timestamp, '%a, %d %b %Y %T %z', 'UTC'),
+        to_user   => $attachment->attacher->email,
+        from_user => EMAIL_FROM,
     };
 
     my $msg;
@@ -153,10 +115,6 @@ sub _send_approval_mail {
         || ThrowTemplateError($template->error());
 
     MessageToMTA($msg);
-
-    # Make sure we don't do this again
-    Bugzilla->dbh->do("UPDATE profiles SET first_patch_approved_id = ? WHERE userid = ?", 
-                      undef, $attachment->id, $attachment->attacher->id);
 }
 
 __PACKAGE__->NAME;
diff --git a/extensions/ContributorEngagement/lib/Constants.pm b/extensions/ContributorEngagement/lib/Constants.pm
index 851c0dbc2..346e00c35 100644
--- a/extensions/ContributorEngagement/lib/Constants.pm
+++ b/extensions/ContributorEngagement/lib/Constants.pm
@@ -14,23 +14,18 @@ use base qw(Exporter);
 our @EXPORT = qw(
     EMAIL_FROM
     ENABLED_PRODUCTS
-    FLAG_REGEXES 
 );
 
 use constant EMAIL_FROM => 'bugzilla-daemon@mozilla.org';
 
 use constant ENABLED_PRODUCTS => (
     "Core",
-    "Firefox for Android", 
     "Firefox",
-    "Testing", 
-    "Toolkit", 
-    "Mozilla Services", 
-    "TestProduct", 
-);
-
-use constant FLAG_REGEXES => (
-    qr/^approval/
+    "Firefox for Android",
+    "Firefox for Metro",
+    "Mozilla Services",
+    "Testing",
+    "Toolkit",
 );
 
 1;
diff --git a/extensions/ContributorEngagement/template/en/default/contributor/email.txt.tmpl b/extensions/ContributorEngagement/template/en/default/contributor/email.txt.tmpl
index b403a4bfb..d4601542d 100644
--- a/extensions/ContributorEngagement/template/en/default/contributor/email.txt.tmpl
+++ b/extensions/ContributorEngagement/template/en/default/contributor/email.txt.tmpl
@@ -10,32 +10,33 @@ From: [% from_user FILTER none %]
 To: [% to_user FILTER none %]
 Subject: Congratulations on having your first patch approved
 Date: [% date FILTER none %]
+X-Bugzilla-Type: contributor-engagement
 
-Congratulations on having your first patch approved, and thank you 
-for your contribution to Mozilla.
+Congratulations on having your first patch approved, and thank you for your
+contribution to Mozilla.
 
-The next step is to get the patch actually checked in to our repository.
-For more information about how to make that happen, check out this post:
+The next step is to get the patch actually checked in to our repository. For
+more information about how to make that happen, check out this post:
 
-https://developer.mozilla.org/en/Creating_a_patch_that_can_be_checked_in
+https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
 
-While you are going through those final steps, if you're looking for a 
-new project to take on, have a look at our list of 'mentored' [% terms.bugs %] ([% terms.bugs %] where 
+While you are going through those final steps, if you're looking for a new
+project to take on, have a look at our list of 'mentored' [% terms.bugs %] ([% terms.bugs %] where
 someone is specifically available to help you):
 
 https://bugzil.la/sw:mentor
 
 Alternatively, you could join us on our IRC chat server in the #introduction
-channel and ask for suggestions  about what would be a good [% terms.bugs %] to work on. 
+channel and ask for suggestions about what would be a good [% terms.bugs %] to work on.
 There's more about using our chat server at:
 
 http://irc.mozilla.org/
 
 If you haven't done so already, this is also a good time to sign up to the
-Mozilla Contributor Directory and create a profile for yourself.  Doing this 
+Mozilla Contributor Directory and create a profile for yourself. Doing this
 will give you access to community members' profiles so you can reach out and
-connect with other Mozillians. You will need someone to 'vouch for' your 
-profile; if you don't know any other Mozillians well, why not contact the 
+connect with other Mozillians. You will need someone to 'vouch for' your
+profile; if you don't know any other Mozillians well, why not contact the
 person who approved your patch?
 
 The directory is here: