Bug 1067410 - Modification time wrong for deleted flags in review schema

3 files changed, 29 insertions, 16 deletions

diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index ea83ae5d9..f0cb830b5 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -484,7 +484,7 @@ sub update {
     }
 
     # BMO - provide a hook which passes the flag object
-    Bugzilla::Hook::process('flag_updated', {flag => $self, changes => $changes});
+    Bugzilla::Hook::process('flag_updated', {flag => $self, changes => $changes, timestamp => $timestamp});
 
     return $changes;
 }
@@ -540,6 +540,10 @@ sub update_flags {
     # These flags have been deleted.
     foreach my $old_flag (values %old_flags) {
         $class->notify(undef, $old_flag, $self, $timestamp);
+
+        # BMO - provide a hook which passes the timestamp,
+        # because that isn't passed to remove_from_db().
+        Bugzilla::Hook::process('flag_deleted', {flag => $old_flag, timestamp => $timestamp});
         $old_flag->remove_from_db();
     }
 
@@ -637,6 +641,11 @@ sub force_retarget {
             # Track deleted attachment flags.
             push(@removed, $class->snapshot([$flag])) if $flag->attach_id;
             $class->notify(undef, $flag, $bug || $flag->bug);
+
+            # BMO - provide a hook which passes the timestamp,
+            # because that isn't passed to remove_from_db().
+            my ($timestamp) = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
+            Bugzilla::Hook::process('flag_deleted', {flag => $flag, timestamp => $timestamp});
             $flag->remove_from_db();
         }
     }
diff --git a/extensions/Review/Extension.pm b/extensions/Review/Extension.pm
index d5d96daa9..f6a3bf743 100644
--- a/extensions/Review/Extension.pm
+++ b/extensions/Review/Extension.pm
@@ -320,7 +320,7 @@ sub object_end_of_create {
         _adjust_request_count($object, +1);
     }
     if (_is_countable_flag($object)) {
-        $self->_log_flag_state_activity($object, $object->status);
+        $self->_log_flag_state_activity($object, $object->status, $object->modification_date);
     }
 }
 
@@ -393,25 +393,27 @@ sub object_end_of_update {
 
 sub flag_updated {
     my ($self, $args) = @_;
-    my $flag = $args->{flag};
-    my $changes = $args->{changes};
+    my $flag      = $args->{flag};
+    my $timestamp = $args->{timestamp};
+    my $changes   = $args->{changes};
 
     return unless scalar(keys %$changes);
     if (_is_countable_flag($flag)) {
-        $self->_log_flag_state_activity( $flag, $flag->status );
+        $self->_log_flag_state_activity($flag, $flag->status, $timestamp);
     }
 }
 
-sub object_before_delete {
+sub flag_deleted {
     my ($self, $args) = @_;
-    my $object = $args->{object};
+    my $flag      = $args->{flag};
+    my $timestamp = $args->{timestamp};
 
-    if (_is_countable_flag($object) && $object->requestee_id && $object->status eq '?') {
-        _adjust_request_count($object, -1);
+    if (_is_countable_flag($flag) && $flag->requestee_id && $flag->status eq '?') {
+        _adjust_request_count($flag, -1);
     }
 
-    if (_is_countable_flag($object)) {
-        $self->_log_flag_state_activity($object, 'X');
+    if (_is_countable_flag($flag)) {
+        $self->_log_flag_state_activity($flag, 'X', $timestamp, Bugzilla->user->id);
     }
 }
 
@@ -423,17 +425,19 @@ sub _is_countable_flag {
 }
 
 sub _log_flag_state_activity {
-    my ($self, $flag, $status) = @_;
+    my ($self, $flag, $status, $timestamp, $setter_id) = @_;
+
+    $setter_id //= $flag->setter_id;
 
     Bugzilla::Extension::Review::FlagStateActivity->create({
-        flag_when     => $flag->modification_date,
+        flag_when     => $timestamp,
+        setter_id     => $setter_id,
+        status        => $status,
         type_id       => $flag->type_id,
         flag_id       => $flag->id,
-        setter_id     => $flag->setter_id,
         requestee_id  => $flag->requestee_id,
         bug_id        => $flag->bug_id,
         attachment_id => $flag->attach_id,
-        status        => $status,
     });
 }
 
diff --git a/extensions/Review/lib/WebService.pm b/extensions/Review/lib/WebService.pm
index b3baf509a..c61c358df 100644
--- a/extensions/Review/lib/WebService.pm
+++ b/extensions/Review/lib/WebService.pm
@@ -366,7 +366,7 @@ GET /rest/review/flag_activity/setter/C<setter>
 
 GET /rest/review/flag_activity/type_id/C<type_id>
 
-GET /rest/review/flag_activity/type/C<type_name>
+GET /rest/review/flag_activity/type_name/C<type_name>
 
 GET /rest/review/flag_activity