diff options
3 files changed, 47 insertions, 8 deletions
diff --git a/extensions/Review/Extension.pm b/extensions/Review/Extension.pm index eda12f734..4e5fd35a6 100644 --- a/extensions/Review/Extension.pm +++ b/extensions/Review/Extension.pm @@ -21,10 +21,15 @@ use Bugzilla::Install::Filesystem; use Bugzilla::Search; use Bugzilla::User; use Bugzilla::User::Setting; -use Bugzilla::Util qw(clean_text); +use Bugzilla::Util qw(clean_text datetime_from); use constant UNAVAILABLE_RE => qr/\b(?:unavailable|pto|away)\b/i; +# This extension prevents you from assigning reviews to inactive reviewers, +# with this constant defining 'inactive'. Set the value to 0 to disable this +# prevention. +use constant MAX_REVIEWER_LAST_SEEN_DAYS_AGO => 60; + # # monkey-patched methods # @@ -42,6 +47,7 @@ BEGIN { *Bugzilla::Bug::set_bug_mentors = \&_bug_set_bug_mentors; *Bugzilla::User::review_count = \&_user_review_count; *Bugzilla::User::reviews_blocked = \&_user_reviews_blocked; + *Bugzilla::User::is_active = \&_user_is_active; } # @@ -54,12 +60,12 @@ sub _component_reviewers { _reviewers($_[0], 'component', $_[1]) } sub _component_reviewers_objs { _reviewers_objs($_[0], 'component', $_[1]) } sub _reviewers { - my ($object, $type, $include_disabled) = @_; - return join(', ', map { $_->login } @{ _reviewers_objs($object, $type, $include_disabled) }); + my ($object, $type, $unfiltered) = @_; + return join(', ', map { $_->login } @{ _reviewers_objs($object, $type, $unfiltered) }); } sub _reviewers_objs { - my ($object, $type, $include_disabled) = @_; + my ($object, $type, $unfiltered) = @_; if (!$object->{reviewers}) { my $dbh = Bugzilla->dbh; my $user_ids = $dbh->selectcol_arrayref( @@ -72,9 +78,10 @@ sub _reviewers_objs { my $users = Bugzilla::User->new_from_list($user_ids); my %user_map = map { $_->id => $_ } @$users; my @reviewers = map { $user_map{$_} } @$user_ids; - if (!$include_disabled) { + if (!$unfiltered) { @reviewers = grep { $_->is_enabled + && $_->is_active && $_->name !~ UNAVAILABLE_RE && !$_->reviews_blocked } @reviewers; @@ -84,6 +91,18 @@ sub _reviewers_objs { return $object->{reviewers}; } +sub _user_is_active { + my ($self) = @_; + + return 0 if !defined($self->last_seen_date); + return 1 if MAX_REVIEWER_LAST_SEEN_DAYS_AGO == 0; # feature disabled + + my $dt = datetime_from($self->last_seen_date); + my $days_ago = $dt->delta_days(DateTime->now())->in_units('days'); + + return $days_ago <= MAX_REVIEWER_LAST_SEEN_DAYS_AGO; +} + sub _user_review_count { my ($self) = @_; if (!exists $self->{review_count}) { @@ -576,7 +595,6 @@ sub flag_end_of_update { my ($self, $args) = @_; my ($object, $new_flags) = @$args{qw(object new_flags)}; my $bug = $object->isa('Bugzilla::Attachment') ? $object->bug : $object; - return unless $bug->product_obj->reviewer_required; foreach my $orig_change (@$new_flags) { my $change = $orig_change; # work on a copy @@ -587,8 +605,22 @@ sub flag_end_of_update { } my ($name, $value) = $change =~ /^(.+)(.)$/; - if ($name eq 'review' && $value eq '?' && $reviewer eq '') { - ThrowUserError('reviewer_required'); + if ($name eq 'review' && $value eq '?') { + if ($reviewer eq '' && $bug->product_obj->reviewer_required) { + ThrowUserError('reviewer_required'); + } + + my $reviewer_obj = Bugzilla::User->new({ + name => $reviewer, + cache => 1 + }); + + if (!$reviewer_obj->is_active) { + ThrowUserError('reviewer_inactive', { + reviewer => $reviewer_obj, + timeout => MAX_REVIEWER_LAST_SEEN_DAYS_AGO + }); + } } } } diff --git a/extensions/Review/template/en/default/hook/global/header-start.html.tmpl b/extensions/Review/template/en/default/hook/global/header-start.html.tmpl index a1ced8108..8bc72ecdb 100644 --- a/extensions/Review/template/en/default/hook/global/header-start.html.tmpl +++ b/extensions/Review/template/en/default/hook/global/header-start.html.tmpl @@ -43,6 +43,7 @@ review_suggestions = { _mentors: [ [% FOREACH u = mentors %] + [% NEXT IF NOT u.is_active %] [% PROCESS reviewer %][% "," UNLESS loop.last %] [% END %] ], diff --git a/extensions/Review/template/en/default/hook/global/user-error-errors.html.tmpl b/extensions/Review/template/en/default/hook/global/user-error-errors.html.tmpl index 112751076..aafdb5445 100644 --- a/extensions/Review/template/en/default/hook/global/user-error-errors.html.tmpl +++ b/extensions/Review/template/en/default/hook/global/user-error-errors.html.tmpl @@ -10,6 +10,12 @@ [% title = "Reviewer Required" %] You must provide a reviewer for review requests. +[% ELSIF error == "reviewer_inactive" %] + [% title = "Reviewer Is Inactive" %] + You can't ask <em>[% reviewer.identity FILTER html %]</em> for a review + because they have not logged in to [% terms.Bugzilla %] within the last + [%+ timeout FILTER none %] days. + [% ELSIF error == "reviewer_suggestions_param_required" %] [% title = "Parameter Required" %] You must provide either a bug_id, or a product (and optionally a |