Bug 956229: develop a system to track the lifetime of review/feedback/needinfo requests

5 files changed, 232 insertions, 3 deletions

diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index 6ef7be81e..c3f7a2df7 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -482,6 +482,10 @@ sub update {
         $self->{'modification_date'} = format_time($timestamp,  '%Y.%m.%d %T');
         Bugzilla->memcached->clear({ table => 'flags', id => $self->id });
     }
+
+    # BMO - provide a hook which passes the flag object
+    Bugzilla::Hook::process('flag_updated', {flag => $self, changes => $changes});
+
     return $changes;
 }
 
diff --git a/extensions/Review/Extension.pm b/extensions/Review/Extension.pm
index b495c9ecd..8d069e8e6 100644
--- a/extensions/Review/Extension.pm
+++ b/extensions/Review/Extension.pm
@@ -15,6 +15,7 @@ our $VERSION = '1';
 use Bugzilla;
 use Bugzilla::Constants;
 use Bugzilla::Error;
+use Bugzilla::Extension::Review::FlagStateActivity;
 use Bugzilla::Extension::Review::Util;
 use Bugzilla::Install::Filesystem;
 use Bugzilla::User;
@@ -196,6 +197,9 @@ sub object_end_of_create {
     elsif (_is_countable_flag($object) && $object->requestee_id && $object->status eq '?') {
         _adjust_request_count($object, +1);
     }
+    if (_is_countable_flag($object)) {
+        $self->_log_flag_state_activity($object, $object->status);
+    }
 }
 
 sub object_end_of_update {
@@ -238,6 +242,17 @@ sub object_end_of_update {
     }
 }
 
+sub flag_updated {
+    my ( $self, $args ) = @_;
+    my $flag = $args->{flag};
+    my $changes = $args->{changes};
+
+    return unless scalar(keys %$changes);
+    if ( _is_countable_flag($flag) ) {
+        $self->_log_flag_state_activity( $flag, $flag->status );
+    }
+}
+
 sub object_before_delete {
     my ($self, $args) = @_;
     my $object = $args->{object};
@@ -245,6 +260,10 @@ sub object_before_delete {
     if (_is_countable_flag($object) && $object->requestee_id && $object->status eq '?') {
         _adjust_request_count($object, -1);
     }
+
+    if (_is_countable_flag($object)) {
+        $self->_log_flag_state_activity($object, 'X');
+    }
 }
 
 sub _is_countable_flag {
@@ -254,6 +273,21 @@ sub _is_countable_flag {
     return $type_name eq 'review' || $type_name eq 'feedback' || $type_name eq 'needinfo';
 }
 
+sub _log_flag_state_activity {
+    my ($self, $flag, $status) = @_;
+
+    Bugzilla::Extension::Review::FlagStateActivity->create({
+        flag_when     => $flag->modification_date,
+        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,
+    });
+}
+
 sub _adjust_request_count {
     my ($flag, $add) = @_;
     return unless my $requestee_id = $flag->requestee_id;
@@ -626,7 +660,6 @@ sub db_schema_abstract_schema {
             },
         ],
     };
-
 }
 
 sub install_update_db {
diff --git a/extensions/Review/lib/FlagStateActivity.pm b/extensions/Review/lib/FlagStateActivity.pm
new file mode 100644
index 000000000..46e9300a5
--- /dev/null
+++ b/extensions/Review/lib/FlagStateActivity.pm
@@ -0,0 +1,122 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
+
+package Bugzilla::Extension::Review::FlagStateActivity;
+use strict;
+use warnings;
+
+use Bugzilla::Error qw(ThrowUserError);
+use Bugzilla::Util qw(trim datetime_from);
+use List::MoreUtils qw(none);
+
+use base qw( Bugzilla::Object );
+
+use constant DB_TABLE      => 'flag_state_activity';
+use constant LIST_ORDER    => 'id';
+use constant AUDIT_CREATES => 0;
+use constant AUDIT_UPDATES => 0;
+use constant AUDIT_REMOVES => 0;
+
+use constant DB_COLUMNS => qw(
+    id
+    flag_when
+    type_id
+    flag_id
+    setter_id
+    requestee_id
+    bug_id
+    attachment_id
+    status
+);
+
+
+sub _check_param_required {
+    my ($param) = @_;
+
+    return sub {
+        my ($invocant, $value) = @_;
+        $value = trim($value)
+            or ThrowCodeError('param_required', {param => $param});
+        return $value;
+    },
+}
+
+sub _check_date {
+    my ($invocant, $date) = @_;
+
+    $date = trim($date);
+    datetime_from($date)
+        or ThrowUserError('illegal_date', { date   => $date,
+                                            format => 'YYYY-MM-DD HH24:MI:SS' });
+    return $date;
+}
+
+sub _check_status {
+    my ($self, $status) = @_;
+
+    # - Make sure the status is valid.
+    # - Make sure the user didn't request the flag unless it's requestable.
+    #   If the flag existed and was requested before it became unrequestable,
+    #   leave it as is.
+    if (none { $status eq $_ } qw( X + - ? )) {
+        ThrowUserError(
+            'flag_status_invalid',
+            {
+                id     => $self->id,
+                status => $status
+            }
+        );
+    }
+    return $status;
+}
+
+use constant VALIDATORS => {
+    flag_when => \&_check_date,
+    type_id   => _check_param_required('type_id'),
+    flag_id   => _check_param_required('flag_id'),
+    setter_id => _check_param_required('setter_id'),
+    bug_id    => _check_param_required('bug_id'),
+    status    => \&_check_status,
+};
+
+sub flag_when     { return $_[0]->{flag_when} }
+sub type_id       { return $_[0]->{type_id} }
+sub flag_id       { return $_[0]->{flag_id} }
+sub setter_id     { return $_[0]->{setter_id} }
+sub bug_id        { return $_[0]->{bug_id} }
+sub requestee_id  { return $_[0]->{requestee_id} }
+sub attachment_id { return $_[0]->{attachment_id} }
+sub status        { return $_[0]->{status} }
+
+sub type {
+    my ($self) = @_;
+    return $self->{type} //= Bugzilla::FlagType->new({ id => $self->type_id, cache => 1 });
+}
+
+sub setter {
+    my ($self) = @_;
+    return $self->{setter} //= Bugzilla::User->new({ id => $self->setter_id, cache => 1 });
+}
+
+sub requestee {
+    my ($self) = @_;
+    return undef unless defined $self->requestee_id;
+    return $self->{requestee} //= Bugzilla::User->new({ id => $self->requestee_id, cache => 1 });
+}
+
+sub bug {
+    my ($self) = @_;
+    return $self->{bug} //= Bugzilla::Bug->new({ id => $self->bug_id, cache => 1 });
+}
+
+sub attachment {
+    my ($self) = @_;
+    return $self->{attachment} //=
+        Bugzilla::Attachment->new({ id => $self->attachment_id, cache => 1 });
+}
+
+1;
diff --git a/extensions/Review/lib/WebService.pm b/extensions/Review/lib/WebService.pm
index 4cb3d48d8..17ab51864 100644
--- a/extensions/Review/lib/WebService.pm
+++ b/extensions/Review/lib/WebService.pm
@@ -15,6 +15,8 @@ use base qw(Bugzilla::WebService);
 use Bugzilla::Bug;
 use Bugzilla::Component;
 use Bugzilla::Error;
+use Bugzilla::Util qw(detaint_natural);
+use Bugzilla::WebService::Util 'filter';
 
 sub suggestions {
     my ($self, $params) = @_;
@@ -69,6 +71,20 @@ sub suggestions {
     return \@result;
 }
 
+sub flag_activity {
+    my ( $self, $params ) = @_;
+    my $dbh = Bugzilla->switch_to_shadow_db();
+
+    my $flag_id = $params->{flag_id};
+
+    detaint_natural($flag_id)
+        or ThrowUserError('invalid_flag_id', { flag_id => $flag_id });
+
+    my $matches = Bugzilla::Extension::Review::FlagStateActivity->match({flag_id => $flag_id});
+    my @results = map { $self->_flag_state_activity_to_hash($_, $params) } @$matches;
+    return \@results;
+}
+
 sub rest_resources {
     return [
         # bug-id
@@ -104,11 +120,61 @@ sub rest_resources {
                 method => 'suggestions',
             },
         },
+        # flag activity by flag id
+        qr{^/review/flag_activity/(\d+)$}, {
+            GET => {
+                method => 'flag_activity',
+                params => sub {
+                    return {flag_id => $_[0]}
+                },
+            },
+        },
     ];
-};
+}
 
-1;
+sub _flag_state_activity_to_hash {
+    my ($self, $fsa, $params) = @_;
+
+    my %flag = (
+        creation_time => $self->type('string', $fsa->flag_when),
+        type          => $self->_flagtype_to_hash($fsa->type),
+        setter        => $self->_user_to_hash($fsa->setter),
+        bug_id        => $self->type('int',    $fsa->bug_id),
+        attachment_id => $self->type('int',    $fsa->attachment_id),
+        status        => $self->type('string', $fsa->status),
+    );
+
+    $flag{requestee} = $self->_user_to_hash($fsa->requestee) if $fsa->requestee;
 
+    return filter($params, \%flag);
+}
+
+sub _flagtype_to_hash {
+    my ($self, $flagtype) = @_;
+    my $user = Bugzilla->user;
+
+    return {
+        id               => $self->type('int',     $flagtype->id),
+        name             => $self->type('string',  $flagtype->name),
+        description      => $self->type('string',  $flagtype->description),
+        type             => $self->type('string',  $flagtype->target_type),
+        is_active        => $self->type('boolean', $flagtype->is_active),
+        is_requesteeble  => $self->type('boolean', $flagtype->is_requesteeble),
+        is_multiplicable => $self->type('boolean', $flagtype->is_multiplicable),
+    };
+}
+
+sub _user_to_hash {
+    my ($self, $user) = @_;
+
+    return {
+        id        => $self->type('int',    $user->id),
+        real_name => $self->type('string', $user->name),
+        name      => $self->type('email',  $user->login),
+    };
+}
+
+1;
 __END__
 =head1 NAME
 
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 46e181409..bd8e57a69 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -991,6 +991,10 @@
     Invalid datasets <em>[% datasets.join(":") FILTER html %]</em>. Only digits,
     letters and colons are allowed.
 
+  [% ELSIF error == "invalid_flag_id" %]
+    [% title = "Invalid Flag ID" %]
+    The flag id [% flag_id FILTER html %] is invalid.
+
   [% ELSIF error == "invalid_format" %]
     [% title = "Invalid Format" %]
     The format "[% format FILTER html %]" is invalid (must be one of