diff options
Diffstat (limited to 'extensions/Review')
| -rw-r--r-- | extensions/Review/Config.pm | 2 | ||||
| -rw-r--r-- | extensions/Review/Extension.pm | 1651 | ||||
| -rwxr-xr-x | extensions/Review/bin/migrate_mentor_from_whiteboard.pl | 268 | ||||
| -rwxr-xr-x | extensions/Review/bin/review_requests_rebuild.pl | 6 | ||||
| -rw-r--r-- | extensions/Review/lib/FlagStateActivity.pm | 110 | ||||
| -rw-r--r-- | extensions/Review/lib/Util.pm | 71 | ||||
| -rw-r--r-- | extensions/Review/lib/WebService.pm | 468 |
7 files changed, 1230 insertions, 1346 deletions
diff --git a/extensions/Review/Config.pm b/extensions/Review/Config.pm index ea7e8a725..aa9a8c32d 100644 --- a/extensions/Review/Config.pm +++ b/extensions/Review/Config.pm @@ -11,7 +11,7 @@ use 5.10.1; use strict; use warnings; -use constant NAME => 'Review'; +use constant NAME => 'Review'; use constant REQUIRED_MODULES => []; use constant OPTIONAL_MODULES => []; diff --git a/extensions/Review/Extension.pm b/extensions/Review/Extension.pm index a918a5ca5..975857cf7 100644 --- a/extensions/Review/Extension.pm +++ b/extensions/Review/Extension.pm @@ -36,95 +36,96 @@ use constant MENTOR_LIMIT => 10; # BEGIN { - *Bugzilla::Product::reviewers = \&_product_reviewers; - *Bugzilla::Product::reviewers_objs = \&_product_reviewers_objs; - *Bugzilla::Product::reviewer_required = \&_product_reviewer_required; - *Bugzilla::Component::reviewers = \&_component_reviewers; - *Bugzilla::Component::reviewers_objs = \&_component_reviewers_objs; - *Bugzilla::Bug::mentors = \&_bug_mentors; - *Bugzilla::Bug::bug_mentors = \&_bug_mentors; - *Bugzilla::Bug::bug_mentor = \&_bug_mentors; - *Bugzilla::Bug::is_mentor = \&_bug_is_mentor; - *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; + *Bugzilla::Product::reviewers = \&_product_reviewers; + *Bugzilla::Product::reviewers_objs = \&_product_reviewers_objs; + *Bugzilla::Product::reviewer_required = \&_product_reviewer_required; + *Bugzilla::Component::reviewers = \&_component_reviewers; + *Bugzilla::Component::reviewers_objs = \&_component_reviewers_objs; + *Bugzilla::Bug::mentors = \&_bug_mentors; + *Bugzilla::Bug::bug_mentors = \&_bug_mentors; + *Bugzilla::Bug::bug_mentor = \&_bug_mentors; + *Bugzilla::Bug::is_mentor = \&_bug_is_mentor; + *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; } # # monkey-patched methods # -sub _product_reviewers { _reviewers($_[0], 'product', $_[1]) } -sub _product_reviewers_objs { _reviewers_objs($_[0], 'product', $_[1]) } -sub _component_reviewers { _reviewers($_[0], 'component', $_[1]) } -sub _component_reviewers_objs { _reviewers_objs($_[0], 'component', $_[1]) } +sub _product_reviewers { _reviewers($_[0], 'product', $_[1]) } +sub _product_reviewers_objs { _reviewers_objs($_[0], 'product', $_[1]) } +sub _component_reviewers { _reviewers($_[0], 'component', $_[1]) } +sub _component_reviewers_objs { _reviewers_objs($_[0], 'component', $_[1]) } sub _reviewers { - my ($object, $type, $unfiltered) = @_; - return join(', ', map { $_->login } @{ _reviewers_objs($object, $type, $unfiltered) }); + my ($object, $type, $unfiltered) = @_; + return + join(', ', map { $_->login } @{_reviewers_objs($object, $type, $unfiltered)}); } sub _reviewers_objs { - my ($object, $type, $unfiltered) = @_; - if (!$object->{reviewers}) { - my $dbh = Bugzilla->dbh; - my $user_ids = $dbh->selectcol_arrayref( - "SELECT user_id FROM ${type}_reviewers WHERE ${type}_id = ? ORDER BY sortkey", - undef, - $object->id, - ); - # new_from_list always sorts according to the object's definition, - # so we have to reorder the list - my $users = Bugzilla::User->new_from_list($user_ids); - my %user_map = map { $_->id => $_ } @$users; - my @reviewers = map { $user_map{$_} } @$user_ids; - if (!$unfiltered) { - @reviewers = grep { - $_->is_enabled - && $_->is_active - && $_->name !~ UNAVAILABLE_RE - && !$_->reviews_blocked - } @reviewers; - } - $object->{reviewers} = \@reviewers; + my ($object, $type, $unfiltered) = @_; + if (!$object->{reviewers}) { + my $dbh = Bugzilla->dbh; + my $user_ids + = $dbh->selectcol_arrayref( + "SELECT user_id FROM ${type}_reviewers WHERE ${type}_id = ? ORDER BY sortkey", + undef, $object->id,); + + # new_from_list always sorts according to the object's definition, + # so we have to reorder the list + my $users = Bugzilla::User->new_from_list($user_ids); + my %user_map = map { $_->id => $_ } @$users; + my @reviewers = map { $user_map{$_} } @$user_ids; + if (!$unfiltered) { + @reviewers = grep { + $_->is_enabled + && $_->is_active + && $_->name !~ UNAVAILABLE_RE + && !$_->reviews_blocked + } @reviewers; } - return $object->{reviewers}; + $object->{reviewers} = \@reviewers; + } + return $object->{reviewers}; } sub _user_is_active { - my ($self) = @_; + my ($self) = @_; - # never consider .bugs or .tld addresses as inactive. - return 1 if $self->login =~ /\.(?:bugs|tld)$/; - return 1 unless Bugzilla->params->{max_reviewer_last_seen}; - return 0 if !defined($self->last_seen_date); + # never consider .bugs or .tld addresses as inactive. + return 1 if $self->login =~ /\.(?:bugs|tld)$/; + return 1 unless Bugzilla->params->{max_reviewer_last_seen}; + return 0 if !defined($self->last_seen_date); - my $dt = datetime_from($self->last_seen_date); - my $days_ago = $dt->delta_days(DateTime->now())->in_units('days'); + my $dt = datetime_from($self->last_seen_date); + my $days_ago = $dt->delta_days(DateTime->now())->in_units('days'); - return $days_ago <= Bugzilla->params->{max_reviewer_last_seen}; + return $days_ago <= Bugzilla->params->{max_reviewer_last_seen}; } sub _user_review_count { - my ($self) = @_; - if (!exists $self->{review_count}) { - my $dbh = Bugzilla->dbh; - ($self->{review_count}) = $dbh->selectrow_array( - "SELECT COUNT(*) + my ($self) = @_; + if (!exists $self->{review_count}) { + my $dbh = Bugzilla->dbh; + ($self->{review_count}) = $dbh->selectrow_array( + "SELECT COUNT(*) FROM flags INNER JOIN flagtypes ON flagtypes.id = flags.type_id WHERE flags.requestee_id = ? - AND " . $dbh->sql_in('flagtypes.name', [ "'review'", "'feedback'" ]), - undef, - $self->id, - ); - } - return $self->{review_count}; + AND " + . $dbh->sql_in('flagtypes.name', ["'review'", "'feedback'"]), undef, + $self->id, + ); + } + return $self->{review_count}; } sub _user_reviews_blocked { - return $_[0]->settings->{block_reviews}->{value} eq 'on'; + return $_[0]->settings->{block_reviews}->{value} eq 'on'; } # @@ -132,148 +133,143 @@ sub _user_reviews_blocked { # sub _bug_mentors { - my ($self, $options) = @_; - $options //= {}; - my $dbh = Bugzilla->dbh; - if (!$self->{bug_mentors}) { - my $mentor_ids = $dbh->selectcol_arrayref(" - SELECT user_id FROM bug_mentors WHERE bug_id = ?", - undef, - $self->id); - $self->{bug_mentors} = []; - foreach my $mentor_id (@$mentor_ids) { - push(@{ $self->{bug_mentors} }, Bugzilla::User->new({ id => $mentor_id, cache => 1 })); - } - $self->{bug_mentors} = [ - sort { $a->login cmp $b->login } @{ $self->{bug_mentors} } - ]; - } - my @result = @{ $self->{bug_mentors} }; - if ($options->{exclude_needinfo_blocked}) { - @result = grep { !$_->needinfo_blocked } @result; - } - if ($options->{exclude_review_blocked}) { - @result = grep { !$_->reviews_blocked } @result; + my ($self, $options) = @_; + $options //= {}; + my $dbh = Bugzilla->dbh; + if (!$self->{bug_mentors}) { + my $mentor_ids = $dbh->selectcol_arrayref(" + SELECT user_id FROM bug_mentors WHERE bug_id = ?", undef, $self->id); + $self->{bug_mentors} = []; + foreach my $mentor_id (@$mentor_ids) { + push( + @{$self->{bug_mentors}}, + Bugzilla::User->new({id => $mentor_id, cache => 1}) + ); } - return \@result; + $self->{bug_mentors} + = [sort { $a->login cmp $b->login } @{$self->{bug_mentors}}]; + } + my @result = @{$self->{bug_mentors}}; + if ($options->{exclude_needinfo_blocked}) { + @result = grep { !$_->needinfo_blocked } @result; + } + if ($options->{exclude_review_blocked}) { + @result = grep { !$_->reviews_blocked } @result; + } + return \@result; } sub _bug_is_mentor { - my ($self, $user) = @_; - my $user_id = ($user || Bugzilla->user)->id; - return (grep { $_->id == $user_id} @{ $self->mentors }) ? 1 : 0; + my ($self, $user) = @_; + my $user_id = ($user || Bugzilla->user)->id; + return (grep { $_->id == $user_id } @{$self->mentors}) ? 1 : 0; } sub _bug_set_bug_mentors { - my ($self, $value) = @_; - $self->set('bug_mentors', $value); + my ($self, $value) = @_; + $self->set('bug_mentors', $value); } sub object_validators { - my ($self, $args) = @_; - return unless $args->{class} eq 'Bugzilla::Bug'; - $args->{validators}->{bug_mentors} = \&_bug_check_bug_mentors; + my ($self, $args) = @_; + return unless $args->{class} eq 'Bugzilla::Bug'; + $args->{validators}->{bug_mentors} = \&_bug_check_bug_mentors; } sub _bug_check_bug_mentors { - my ($self, $value) = @_; - my %seen; - my $mentors = [ - grep { !$seen{$_->id}++ } - map { Bugzilla::User->check({ name => $_, cache => 1 }) } - ref($value) ? @$value : ($value) - ]; - if (scalar(@$mentors) > MENTOR_LIMIT) { - ThrowUserError('mentor_limit_exceeded', { limit => MENTOR_LIMIT }); - } - return $mentors; + my ($self, $value) = @_; + my %seen; + my $mentors + = [grep { !$seen{$_->id}++ } + map { Bugzilla::User->check({name => $_, cache => 1}) } + ref($value) ? @$value : ($value)]; + if (scalar(@$mentors) > MENTOR_LIMIT) { + ThrowUserError('mentor_limit_exceeded', {limit => MENTOR_LIMIT}); + } + return $mentors; } sub bug_user_match_fields { - my ($self, $args) = @_; - $args->{fields}->{bug_mentors} = { type => 'multi' }; + my ($self, $args) = @_; + $args->{fields}->{bug_mentors} = {type => 'multi'}; } sub bug_before_create { - my ($self, $args) = @_; - my $params = $args->{params}; - my $stash = $args->{stash}; - $stash->{bug_mentors} = delete $params->{bug_mentors}; + my ($self, $args) = @_; + my $params = $args->{params}; + my $stash = $args->{stash}; + $stash->{bug_mentors} = delete $params->{bug_mentors}; } sub bug_end_of_create { - my ($self, $args) = @_; - my $bug = $args->{bug}; - my $stash = $args->{stash}; - if (my $mentors = $stash->{bug_mentors}) { - $self->_update_user_table({ - object => $bug, - old_users => [], - new_users => $self->_bug_check_bug_mentors($mentors), - table => 'bug_mentors', - id_field => 'bug_id', - }); - } + my ($self, $args) = @_; + my $bug = $args->{bug}; + my $stash = $args->{stash}; + if (my $mentors = $stash->{bug_mentors}) { + $self->_update_user_table({ + object => $bug, + old_users => [], + new_users => $self->_bug_check_bug_mentors($mentors), + table => 'bug_mentors', + id_field => 'bug_id', + }); + } } sub _update_user_table { - my ($self, $args) = @_; - my ($object, $old_users, $new_users, $table, $id_field, $has_sortkey, $return) = - @$args{qw(object old_users new_users table id_field has_sortkey return)}; - my $dbh = Bugzilla->dbh; - my (@removed, @added); - - # remove deleted users - foreach my $old_user (@$old_users) { - if (!grep { $_->id == $old_user->id } @$new_users) { - $dbh->do( - "DELETE FROM $table WHERE $id_field = ? AND user_id = ?", - undef, - $object->id, $old_user->id, - ); - push @removed, $old_user; - } + my ($self, $args) = @_; + my ($object, $old_users, $new_users, $table, $id_field, $has_sortkey, $return) + = @$args{qw(object old_users new_users table id_field has_sortkey return)}; + my $dbh = Bugzilla->dbh; + my (@removed, @added); + + # remove deleted users + foreach my $old_user (@$old_users) { + if (!grep { $_->id == $old_user->id } @$new_users) { + $dbh->do("DELETE FROM $table WHERE $id_field = ? AND user_id = ?", + undef, $object->id, $old_user->id,); + push @removed, $old_user; } - # add new users - foreach my $new_user (@$new_users) { - if (!grep { $_->id == $new_user->id } @$old_users) { - $dbh->do( - "INSERT INTO $table ($id_field, user_id) VALUES (?, ?)", - undef, - $object->id, $new_user->id, - ); - push @added, $new_user; - } + } + + # add new users + foreach my $new_user (@$new_users) { + if (!grep { $_->id == $new_user->id } @$old_users) { + $dbh->do("INSERT INTO $table ($id_field, user_id) VALUES (?, ?)", + undef, $object->id, $new_user->id,); + push @added, $new_user; } + } - return unless @removed || @added; - - if ($has_sortkey) { - # update the sortkey for all users - for (my $i = 0; $i < scalar(@$new_users); $i++) { - $dbh->do( - "UPDATE $table SET sortkey=? WHERE $id_field = ? AND user_id = ?", - undef, - ($i + 1) * 10, $object->id, $new_users->[$i]->id, - ); - } - } + return unless @removed || @added; - if (!$return) { - return undef; - } - elsif ($return eq 'diff') { - return [ - @removed ? join(', ', map { $_->login } @removed) : undef, - @added ? join(', ', map { $_->login } @added) : undef, - ]; - } - elsif ($return eq 'old-new') { - return [ - @$old_users ? join(', ', map { $_->login } @$old_users) : '', - @$new_users ? join(', ', map { $_->login } @$new_users) : '', - ]; + if ($has_sortkey) { + + # update the sortkey for all users + for (my $i = 0; $i < scalar(@$new_users); $i++) { + $dbh->do( + "UPDATE $table SET sortkey=? WHERE $id_field = ? AND user_id = ?", + undef, ($i + 1) * 10, + $object->id, $new_users->[$i]->id, + ); } + } + + if (!$return) { + return undef; + } + elsif ($return eq 'diff') { + return [ + @removed ? join(', ', map { $_->login } @removed) : undef, + @added ? join(', ', map { $_->login } @added) : undef, + ]; + } + elsif ($return eq 'old-new') { + return [ + @$old_users ? join(', ', map { $_->login } @$old_users) : '', + @$new_users ? join(', ', map { $_->login } @$new_users) : '', + ]; + } } # @@ -283,44 +279,46 @@ sub _update_user_table { sub _product_reviewer_required { $_[0]->{reviewer_required} } sub object_columns { - my ($self, $args) = @_; - my ($class, $columns) = @$args{qw(class columns)}; - if ($class->isa('Bugzilla::Product')) { - my $dbh = Bugzilla->dbh; - my @new_columns = qw(reviewer_required); - push @$columns, grep { $dbh->bz_column_info($class->DB_TABLE, $_) } @new_columns; - } - elsif ($class->isa('Bugzilla::User')) { - my $dbh = Bugzilla->dbh; - my @new_columns = qw(review_request_count feedback_request_count needinfo_request_count); - push @$columns, grep { $dbh->bz_column_info($class->DB_TABLE, $_) } @new_columns; - } + my ($self, $args) = @_; + my ($class, $columns) = @$args{qw(class columns)}; + if ($class->isa('Bugzilla::Product')) { + my $dbh = Bugzilla->dbh; + my @new_columns = qw(reviewer_required); + push @$columns, + grep { $dbh->bz_column_info($class->DB_TABLE, $_) } @new_columns; + } + elsif ($class->isa('Bugzilla::User')) { + my $dbh = Bugzilla->dbh; + my @new_columns + = qw(review_request_count feedback_request_count needinfo_request_count); + push @$columns, + grep { $dbh->bz_column_info($class->DB_TABLE, $_) } @new_columns; + } } sub object_update_columns { - my ($self, $args) = @_; - my ($object, $columns) = @$args{qw(object columns)}; - if ($object->isa('Bugzilla::Product')) { - push @$columns, 'reviewer_required'; - } - elsif ($object->isa('Bugzilla::User')) { - push @$columns, qw(review_request_count feedback_request_count needinfo_request_count); - } + my ($self, $args) = @_; + my ($object, $columns) = @$args{qw(object columns)}; + if ($object->isa('Bugzilla::Product')) { + push @$columns, 'reviewer_required'; + } + elsif ($object->isa('Bugzilla::User')) { + push @$columns, + qw(review_request_count feedback_request_count needinfo_request_count); + } } sub _new_users_from_input { - my ($field) = @_; - my $input_params = Bugzilla->input_params; - return undef unless exists $input_params->{$field}; - return [] unless $input_params->{$field}; - Bugzilla::User::match_field({ $field => {'type' => 'multi'} });; - my $value = $input_params->{$field}; - my %seen; - return [ - grep { !$seen{$_->id}++ } - map { Bugzilla::User->check({ name => $_, cache => 1 }) } - ref($value) ? @$value : ($value) - ]; + my ($field) = @_; + my $input_params = Bugzilla->input_params; + return undef unless exists $input_params->{$field}; + return [] unless $input_params->{$field}; + Bugzilla::User::match_field({$field => {'type' => 'multi'}}); + my $value = $input_params->{$field}; + my %seen; + return [grep { !$seen{$_->id}++ } + map { Bugzilla::User->check({name => $_, cache => 1}) } + ref($value) ? @$value : ($value)]; } # @@ -328,311 +326,329 @@ sub _new_users_from_input { # sub object_before_create { - my ($self, $args) = @_; - my ($class, $params) = @$args{qw(class params)}; - return unless $class->isa('Bugzilla::Product'); + my ($self, $args) = @_; + my ($class, $params) = @$args{qw(class params)}; + return unless $class->isa('Bugzilla::Product'); - $params->{reviewer_required} = Bugzilla->cgi->param('reviewer_required') ? 1 : 0; + $params->{reviewer_required} + = Bugzilla->cgi->param('reviewer_required') ? 1 : 0; } sub object_end_of_set_all { - my ($self, $args) = @_; - my ($object, $params) = @$args{qw(object params)}; - return unless $object->isa('Bugzilla::Product'); + my ($self, $args) = @_; + my ($object, $params) = @$args{qw(object params)}; + return unless $object->isa('Bugzilla::Product'); - $object->set('reviewer_required', Bugzilla->cgi->param('reviewer_required') ? 1 : 0); + $object->set('reviewer_required', + Bugzilla->cgi->param('reviewer_required') ? 1 : 0); } sub object_end_of_create { - my ($self, $args) = @_; - my ($object, $params) = @$args{qw(object params)}; - - if ($object->isa('Bugzilla::Product')) { - $self->_update_user_table({ - object => $object, - old_users => [], - new_users => _new_users_from_input('reviewers'), - table => 'product_reviewers', - id_field => 'product_id', - has_sortkey => 1, - }); - } - elsif ($object->isa('Bugzilla::Component')) { - $self->_update_user_table({ - object => $object, - old_users => [], - new_users => _new_users_from_input('reviewers'), - table => 'component_reviewers', - id_field => 'component_id', - has_sortkey => 1, - }); - } - elsif (_is_countable_flag($object) && $object->requestee_id && $object->status eq '?') { - _check_requestee($object); - _adjust_request_count($object, +1); - } - if (_is_countable_flag($object)) { - $self->_log_flag_state_activity($object, $object->status, $object->modification_date); - } + my ($self, $args) = @_; + my ($object, $params) = @$args{qw(object params)}; + + if ($object->isa('Bugzilla::Product')) { + $self->_update_user_table({ + object => $object, + old_users => [], + new_users => _new_users_from_input('reviewers'), + table => 'product_reviewers', + id_field => 'product_id', + has_sortkey => 1, + }); + } + elsif ($object->isa('Bugzilla::Component')) { + $self->_update_user_table({ + object => $object, + old_users => [], + new_users => _new_users_from_input('reviewers'), + table => 'component_reviewers', + id_field => 'component_id', + has_sortkey => 1, + }); + } + elsif (_is_countable_flag($object) + && $object->requestee_id + && $object->status eq '?') + { + _check_requestee($object); + _adjust_request_count($object, +1); + } + if (_is_countable_flag($object)) { + $self->_log_flag_state_activity($object, $object->status, + $object->modification_date); + } } sub object_end_of_update { - my ($self, $args) = @_; - my ($object, $old_object, $changes) = @$args{qw(object old_object changes)}; - - if ($object->isa('Bugzilla::Product') && exists Bugzilla->input_params->{reviewers}) { - my $diff = $self->_update_user_table({ - object => $object, - old_users => $old_object->reviewers_objs(1), - new_users => _new_users_from_input('reviewers'), - table => 'product_reviewers', - id_field => 'product_id', - has_sortkey => 1, - return => 'old-new', - }); - $changes->{reviewers} = $diff if $diff; + my ($self, $args) = @_; + my ($object, $old_object, $changes) = @$args{qw(object old_object changes)}; + + if ($object->isa('Bugzilla::Product') + && exists Bugzilla->input_params->{reviewers}) + { + my $diff = $self->_update_user_table({ + object => $object, + old_users => $old_object->reviewers_objs(1), + new_users => _new_users_from_input('reviewers'), + table => 'product_reviewers', + id_field => 'product_id', + has_sortkey => 1, + return => 'old-new', + }); + $changes->{reviewers} = $diff if $diff; + } + elsif ($object->isa('Bugzilla::Component')) { + my $diff = $self->_update_user_table({ + object => $object, + old_users => $old_object->reviewers_objs(1), + new_users => _new_users_from_input('reviewers'), + table => 'component_reviewers', + id_field => 'component_id', + has_sortkey => 1, + return => 'old-new', + }); + $changes->{reviewers} = $diff if $diff; + } + elsif ($object->isa('Bugzilla::Bug')) { + my $diff = $self->_update_user_table({ + object => $object, + old_users => $old_object->mentors, + new_users => $object->mentors, + table => 'bug_mentors', + id_field => 'bug_id', + return => 'diff', + }); + $changes->{bug_mentor} = $diff if $diff; + } + elsif (_is_countable_flag($object)) { + my ($old_status, $new_status) = ($old_object->status, $object->status); + if ($old_status ne '?' && $new_status eq '?') { + + # setting flag to ? + _adjust_request_count($object, +1); + if ($object->requestee_id) { + _check_requestee($object); + } } - elsif ($object->isa('Bugzilla::Component')) { - my $diff = $self->_update_user_table({ - object => $object, - old_users => $old_object->reviewers_objs(1), - new_users => _new_users_from_input('reviewers'), - table => 'component_reviewers', - id_field => 'component_id', - has_sortkey => 1, - return => 'old-new', - }); - $changes->{reviewers} = $diff if $diff; + elsif ($old_status eq '?' && $new_status ne '?') { + + # setting flag from ? + _adjust_request_count($old_object, -1); } - elsif ($object->isa('Bugzilla::Bug')) { - my $diff = $self->_update_user_table({ - object => $object, - old_users => $old_object->mentors, - new_users => $object->mentors, - table => 'bug_mentors', - id_field => 'bug_id', - return => 'diff', - }); - $changes->{bug_mentor} = $diff if $diff; + elsif ($old_object->requestee_id && !$object->requestee_id) { + + # removing requestee + _adjust_request_count($old_object, -1); } - elsif (_is_countable_flag($object)) { - my ($old_status, $new_status) = ($old_object->status, $object->status); - if ($old_status ne '?' && $new_status eq '?') { - # setting flag to ? - _adjust_request_count($object, +1); - if ($object->requestee_id) { - _check_requestee($object); - } - } - elsif ($old_status eq '?' && $new_status ne '?') { - # setting flag from ? - _adjust_request_count($old_object, -1); - } - elsif ($old_object->requestee_id && !$object->requestee_id) { - # removing requestee - _adjust_request_count($old_object, -1); - } - elsif (!$old_object->requestee_id && $object->requestee_id) { - # setting requestee - _check_requestee($object); - _adjust_request_count($object, +1); - } - elsif ($old_object->requestee_id && $object->requestee_id - && $old_object->requestee_id != $object->requestee_id) - { - # changing requestee - _check_requestee($object); - _adjust_request_count($old_object, -1); - _adjust_request_count($object, +1); - } + elsif (!$old_object->requestee_id && $object->requestee_id) { + + # setting requestee + _check_requestee($object); + _adjust_request_count($object, +1); } + elsif ($old_object->requestee_id + && $object->requestee_id + && $old_object->requestee_id != $object->requestee_id) + { + # changing requestee + _check_requestee($object); + _adjust_request_count($old_object, -1); + _adjust_request_count($object, +1); + } + } } sub flag_updated { - my ($self, $args) = @_; - 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, $timestamp); - } + my ($self, $args) = @_; + 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, $timestamp); + } } sub flag_deleted { - my ($self, $args) = @_; - my $flag = $args->{flag}; - my $timestamp = $args->{timestamp}; + my ($self, $args) = @_; + my $flag = $args->{flag}; + my $timestamp = $args->{timestamp}; - if (_is_countable_flag($flag) && $flag->requestee_id && $flag->status eq '?') { - _adjust_request_count($flag, -1); - } + if (_is_countable_flag($flag) && $flag->requestee_id && $flag->status eq '?') { + _adjust_request_count($flag, -1); + } - if (_is_countable_flag($flag)) { - $self->_log_flag_state_activity($flag, 'X', $timestamp, Bugzilla->user->id); - } + if (_is_countable_flag($flag)) { + $self->_log_flag_state_activity($flag, 'X', $timestamp, Bugzilla->user->id); + } } sub _is_countable_flag { - my ($object) = @_; - return unless $object->isa('Bugzilla::Flag'); - my $type_name = $object->type->name; - return $type_name eq 'review' || $type_name eq 'feedback' || $type_name eq 'needinfo'; + my ($object) = @_; + return unless $object->isa('Bugzilla::Flag'); + my $type_name = $object->type->name; + return + $type_name eq 'review' + || $type_name eq 'feedback' + || $type_name eq 'needinfo'; } sub _check_requestee { - my ($flag) = @_; - return unless $flag->type->name eq 'review' || $flag->type->name eq 'feedback'; - if ($flag->requestee->reviews_blocked) { - ThrowUserError('reviews_blocked', - { requestee => $flag->requestee, flagtype => $flag->type->name }); - } + my ($flag) = @_; + return unless $flag->type->name eq 'review' || $flag->type->name eq 'feedback'; + if ($flag->requestee->reviews_blocked) { + ThrowUserError('reviews_blocked', + {requestee => $flag->requestee, flagtype => $flag->type->name}); + } } sub _log_flag_state_activity { - my ($self, $flag, $status, $timestamp, $setter_id) = @_; - - $setter_id //= $flag->setter_id; - - Bugzilla::Extension::Review::FlagStateActivity->create({ - flag_when => $timestamp, - setter_id => $setter_id, - status => $status, - type_id => $flag->type_id, - flag_id => $flag->id, - requestee_id => $flag->requestee_id, - bug_id => $flag->bug_id, - attachment_id => $flag->attach_id, - }); + my ($self, $flag, $status, $timestamp, $setter_id) = @_; + + $setter_id //= $flag->setter_id; + + Bugzilla::Extension::Review::FlagStateActivity->create({ + flag_when => $timestamp, + setter_id => $setter_id, + status => $status, + type_id => $flag->type_id, + flag_id => $flag->id, + requestee_id => $flag->requestee_id, + bug_id => $flag->bug_id, + attachment_id => $flag->attach_id, + }); } sub _adjust_request_count { - my ($flag, $add) = @_; - return unless my $requestee_id = $flag->requestee_id; - my $field = $flag->type->name . '_request_count'; - - # update the current user's object so things are display correctly on the - # post-processing page - my $user = Bugzilla->user; - if ($requestee_id == $user->id) { - $user->{$field} += $add; - } - - # update database directly to avoid creating audit_log entries - $add = $add == -1 ? ' - 1' : ' + 1'; - Bugzilla->dbh->do( - "UPDATE profiles SET $field = $field $add WHERE userid = ?", - undef, - $requestee_id - ); - Bugzilla->memcached->clear({ table => 'profiles', id => $requestee_id }); + my ($flag, $add) = @_; + return unless my $requestee_id = $flag->requestee_id; + my $field = $flag->type->name . '_request_count'; + + # update the current user's object so things are display correctly on the + # post-processing page + my $user = Bugzilla->user; + if ($requestee_id == $user->id) { + $user->{$field} += $add; + } + + # update database directly to avoid creating audit_log entries + $add = $add == -1 ? ' - 1' : ' + 1'; + Bugzilla->dbh->do("UPDATE profiles SET $field = $field $add WHERE userid = ?", + undef, $requestee_id); + Bugzilla->memcached->clear({table => 'profiles', id => $requestee_id}); } # bugzilla's handling of requestee matching when creating bugs is "if it's # wrong, or matches too many, default to empty", which breaks mandatory # reviewer requirements. instead we just throw an error. sub post_bug_attachment_flags { - my ($self, $args) = @_; - $self->_check_review_flag($args); + my ($self, $args) = @_; + $self->_check_review_flag($args); } sub create_attachment_flags { - my ($self, $args) = @_; - $self->_check_review_flag($args); + my ($self, $args) = @_; + $self->_check_review_flag($args); } sub _check_review_flag { - my ($self, $args) = @_; - my ($bug, $attachment) = @$args{qw( bug attachment )}; - my $cgi = Bugzilla->cgi; - - # extract the set flag-types - my @flagtype_ids = map { /^flag_type-(\d+)$/ ? $1 : () } $cgi->param(); - @flagtype_ids = grep { $cgi->param("flag_type-$_") eq '?' } @flagtype_ids; - return unless scalar(@flagtype_ids); - - # find valid review flagtypes - my $flag_types = Bugzilla::FlagType::match({ - product_id => $bug->product_id, - component_id => $bug->component_id, - is_active => 1 - }); - foreach my $flag_type (@$flag_types) { - next unless $flag_type->name eq 'review' - && $flag_type->target_type eq 'attachment'; - my $type_id = $flag_type->id; - next unless scalar(grep { $_ == $type_id } @flagtype_ids); - - my $reviewers = clean_text($cgi->param("requestee_type-$type_id") || ''); - if ($reviewers eq '' && $bug->product_obj->reviewer_required) { - ThrowUserError('reviewer_required'); - } - - foreach my $reviewer (split(/[,;]+/, $reviewers)) { - # search on the reviewer - my $users = Bugzilla::User::match($reviewer, 2, 1); - - # no matches - if (scalar(@$users) == 0) { - ThrowUserError('user_match_failed', { name => $reviewer }); - } - - # more than one match, throw error - if (scalar(@$users) > 1) { - ThrowUserError('user_match_too_many', { fields => [ 'review' ] }); - } - - # we want to throw an error if the requestee does not have access - # to the bug. bugzilla's default behaviour is to sliently drop the - # requestee, which results in a confusing 'reviewer required' - # error. - # fake it by creating a flag and try to set the requestee. - # bugzilla's flags don't have a normal constructor or property - # setters, so we have to bless it directly then call the internal - # check_requestee method. urgh. - my $flag = bless({ - type_id => $flag_type->id, - status => '?', - bug_id => $bug->id, - attach_id => $attachment->id - }, 'Bugzilla::Flag'); - $flag->_check_requestee($users->[0]->login, $bug, $attachment); - } + my ($self, $args) = @_; + my ($bug, $attachment) = @$args{qw( bug attachment )}; + my $cgi = Bugzilla->cgi; + + # extract the set flag-types + my @flagtype_ids = map { /^flag_type-(\d+)$/ ? $1 : () } $cgi->param(); + @flagtype_ids = grep { $cgi->param("flag_type-$_") eq '?' } @flagtype_ids; + return unless scalar(@flagtype_ids); + + # find valid review flagtypes + my $flag_types = Bugzilla::FlagType::match({ + product_id => $bug->product_id, + component_id => $bug->component_id, + is_active => 1 + }); + foreach my $flag_type (@$flag_types) { + next + unless $flag_type->name eq 'review' + && $flag_type->target_type eq 'attachment'; + my $type_id = $flag_type->id; + next unless scalar(grep { $_ == $type_id } @flagtype_ids); + + my $reviewers = clean_text($cgi->param("requestee_type-$type_id") || ''); + if ($reviewers eq '' && $bug->product_obj->reviewer_required) { + ThrowUserError('reviewer_required'); } + + foreach my $reviewer (split(/[,;]+/, $reviewers)) { + + # search on the reviewer + my $users = Bugzilla::User::match($reviewer, 2, 1); + + # no matches + if (scalar(@$users) == 0) { + ThrowUserError('user_match_failed', {name => $reviewer}); + } + + # more than one match, throw error + if (scalar(@$users) > 1) { + ThrowUserError('user_match_too_many', {fields => ['review']}); + } + + # we want to throw an error if the requestee does not have access + # to the bug. bugzilla's default behaviour is to sliently drop the + # requestee, which results in a confusing 'reviewer required' + # error. + # fake it by creating a flag and try to set the requestee. + # bugzilla's flags don't have a normal constructor or property + # setters, so we have to bless it directly then call the internal + # check_requestee method. urgh. + my $flag = bless( + { + type_id => $flag_type->id, + status => '?', + bug_id => $bug->id, + attach_id => $attachment->id + }, + 'Bugzilla::Flag' + ); + $flag->_check_requestee($users->[0]->login, $bug, $attachment); + } + } } sub flag_end_of_update { - my ($self, $args) = @_; - my ($object, $old_flags, $new_flags) = @$args{qw(object old_flags new_flags)}; - my $bug = $object->isa('Bugzilla::Attachment') ? $object->bug : $object; - - my (undef, $added) = diff_arrays($old_flags, $new_flags); - foreach my $change (@$added) { - $change =~ s/^[^:]+://; - my $reviewer = ''; - if ($change =~ s/\(([^\)]+)\)$//) { - $reviewer = $1; - } - my ($name, $value) = $change =~ /^(.+)(.)$/; - - if ($name eq 'review' && $value eq '?') { - if ($reviewer eq '') { - ThrowUserError('reviewer_required') if $bug->product_obj->reviewer_required; - } - else { - my $reviewer_obj = Bugzilla::User->check({ - name => $reviewer, - cache => 1 - }); - - ThrowUserError('reviewer_inactive', { - reviewer => $reviewer_obj, - timeout => Bugzilla->params->{max_reviewer_last_seen} - }) unless $reviewer_obj->is_active; - } - } + my ($self, $args) = @_; + my ($object, $old_flags, $new_flags) = @$args{qw(object old_flags new_flags)}; + my $bug = $object->isa('Bugzilla::Attachment') ? $object->bug : $object; + + my (undef, $added) = diff_arrays($old_flags, $new_flags); + foreach my $change (@$added) { + $change =~ s/^[^:]+://; + my $reviewer = ''; + if ($change =~ s/\(([^\)]+)\)$//) { + $reviewer = $1; + } + my ($name, $value) = $change =~ /^(.+)(.)$/; + + if ($name eq 'review' && $value eq '?') { + if ($reviewer eq '') { + ThrowUserError('reviewer_required') if $bug->product_obj->reviewer_required; + } + else { + my $reviewer_obj = Bugzilla::User->check({name => $reviewer, cache => 1}); + + ThrowUserError( + 'reviewer_inactive', + { + reviewer => $reviewer_obj, + timeout => Bugzilla->params->{max_reviewer_last_seen} + } + ) unless $reviewer_obj->is_active; + } } + } } # @@ -640,44 +656,45 @@ sub flag_end_of_update { # sub buglist_columns { - my ($self, $args) = @_; - my $dbh = Bugzilla->dbh; - my $columns = $args->{columns}; - $columns->{bug_mentor} = { title => 'Mentor' }; - if (Bugzilla->user->id) { - $columns->{bug_mentor}->{name} - = $dbh->sql_group_concat('map_mentors_names.login_name'); - } - else { - $columns->{bug_mentor}->{name} - = $dbh->sql_group_concat('map_mentors_names.realname'); - - } + my ($self, $args) = @_; + my $dbh = Bugzilla->dbh; + my $columns = $args->{columns}; + $columns->{bug_mentor} = {title => 'Mentor'}; + if (Bugzilla->user->id) { + $columns->{bug_mentor}->{name} + = $dbh->sql_group_concat('map_mentors_names.login_name'); + } + else { + $columns->{bug_mentor}->{name} + = $dbh->sql_group_concat('map_mentors_names.realname'); + + } } sub buglist_column_joins { - my ($self, $args) = @_; - my $column_joins = $args->{column_joins}; - $column_joins->{bug_mentor} = { - as => 'map_mentors', - table => 'bug_mentors', - then_to => { - as => 'map_mentors_names', - table => 'profiles', - from => 'map_mentors.user_id', - to => 'userid', - }, + my ($self, $args) = @_; + my $column_joins = $args->{column_joins}; + $column_joins->{bug_mentor} = { + as => 'map_mentors', + table => 'bug_mentors', + then_to => { + as => 'map_mentors_names', + table => 'profiles', + from => 'map_mentors.user_id', + to => 'userid', }, + }, + ; } sub search_operator_field_override { - my ($self, $args) = @_; - my $operators = $args->{operators}; - $operators->{bug_mentor} = { - _non_changed => sub { - Bugzilla::Search::_user_nonchanged(@_) - } - }; + my ($self, $args) = @_; + my $operators = $args->{operators}; + $operators->{bug_mentor} = { + _non_changed => sub { + Bugzilla::Search::_user_nonchanged(@_); + } + }; } # @@ -685,105 +702,103 @@ sub search_operator_field_override { # sub webservice { - my ($self, $args) = @_; - my $dispatch = $args->{dispatch}; - $dispatch->{Review} = "Bugzilla::Extension::Review::WebService"; + my ($self, $args) = @_; + my $dispatch = $args->{dispatch}; + $dispatch->{Review} = "Bugzilla::Extension::Review::WebService"; } sub user_preferences { - my ($self, $args) = @_; - return unless - $args->{current_tab} eq 'account' - && $args->{save_changes}; - - my $input = Bugzilla->input_params; - my $settings = Bugzilla->user->settings; - - my $value = $input->{block_reviews} ? 'on' : 'off'; - $settings->{block_reviews}->validate_value($value); - $settings->{block_reviews}->set($value); - clear_settings_cache(Bugzilla->user->id); + my ($self, $args) = @_; + return unless $args->{current_tab} eq 'account' && $args->{save_changes}; + + my $input = Bugzilla->input_params; + my $settings = Bugzilla->user->settings; + + my $value = $input->{block_reviews} ? 'on' : 'off'; + $settings->{block_reviews}->validate_value($value); + $settings->{block_reviews}->set($value); + clear_settings_cache(Bugzilla->user->id); } sub page_before_template { - my ($self, $args) = @_; - - if ($args->{page_id} eq 'review_suggestions.html') { - $self->review_suggestions_report($args); - } - elsif ($args->{page_id} eq 'review_requests_rebuild.html') { - $self->review_requests_rebuild($args); - } - elsif ($args->{page_id} eq 'review_history.html') { - $self->review_history($args); - } + my ($self, $args) = @_; + + if ($args->{page_id} eq 'review_suggestions.html') { + $self->review_suggestions_report($args); + } + elsif ($args->{page_id} eq 'review_requests_rebuild.html') { + $self->review_requests_rebuild($args); + } + elsif ($args->{page_id} eq 'review_history.html') { + $self->review_history($args); + } } sub review_suggestions_report { - my ($self, $args) = @_; - - my $user = Bugzilla->login(LOGIN_REQUIRED); - my $products = []; - my @products = sort { lc($a->name) cmp lc($b->name) } - @{ Bugzilla->user->get_accessible_products }; - foreach my $product_obj (@products) { - my $has_reviewers = 0; - my $product = { - name => $product_obj->name, - components => [], - reviewers => $product_obj->reviewers_objs(1), - }; - $has_reviewers = scalar @{ $product->{reviewers} }; - - foreach my $component_obj (@{ $product_obj->components }) { - my $component = { - name => $component_obj->name, - reviewers => $component_obj->reviewers_objs(1), - }; - if (@{ $component->{reviewers} }) { - push @{ $product->{components} }, $component; - $has_reviewers = 1; - } - } - - if ($has_reviewers) { - push @$products, $product; - } + my ($self, $args) = @_; + + my $user = Bugzilla->login(LOGIN_REQUIRED); + my $products = []; + my @products = sort { lc($a->name) cmp lc($b->name) } + @{Bugzilla->user->get_accessible_products}; + foreach my $product_obj (@products) { + my $has_reviewers = 0; + my $product = { + name => $product_obj->name, + components => [], + reviewers => $product_obj->reviewers_objs(1), + }; + $has_reviewers = scalar @{$product->{reviewers}}; + + foreach my $component_obj (@{$product_obj->components}) { + my $component = { + name => $component_obj->name, + reviewers => $component_obj->reviewers_objs(1), + }; + if (@{$component->{reviewers}}) { + push @{$product->{components}}, $component; + $has_reviewers = 1; + } + } + + if ($has_reviewers) { + push @$products, $product; } - $args->{vars}->{products} = $products; + } + $args->{vars}->{products} = $products; } sub review_requests_rebuild { - my ($self, $args) = @_; - - Bugzilla->user->in_group('admin') - || ThrowUserError('auth_failure', { group => 'admin', - action => 'run', - object => 'review_requests_rebuild' }); - if (Bugzilla->cgi->param('rebuild')) { - my $processed_users = 0; - rebuild_review_counters(sub { - my ($count, $total) = @_; - $processed_users = $total; - }); - $args->{vars}->{rebuild} = 1; - $args->{vars}->{total} = $processed_users; - } + my ($self, $args) = @_; + + Bugzilla->user->in_group('admin') + || ThrowUserError('auth_failure', + {group => 'admin', action => 'run', object => 'review_requests_rebuild'}); + if (Bugzilla->cgi->param('rebuild')) { + my $processed_users = 0; + rebuild_review_counters(sub { + my ($count, $total) = @_; + $processed_users = $total; + }); + $args->{vars}->{rebuild} = 1; + $args->{vars}->{total} = $processed_users; + } } sub review_history { - my ($self, $args) = @_; - - my $user = Bugzilla->login(LOGIN_REQUIRED); - - Bugzilla::User::match_field({ 'requestee' => { 'type' => 'single' } }); - my $requestee = Bugzilla->input_params->{requestee}; - if ($requestee) { - $args->{vars}{requestee} = Bugzilla::User->check({ name => $requestee, cache => 1 }); - } - else { - $args->{vars}{requestee} = $user; - } + my ($self, $args) = @_; + + my $user = Bugzilla->login(LOGIN_REQUIRED); + + Bugzilla::User::match_field({'requestee' => {'type' => 'single'}}); + my $requestee = Bugzilla->input_params->{requestee}; + if ($requestee) { + $args->{vars}{requestee} + = Bugzilla::User->check({name => $requestee, cache => 1}); + } + else { + $args->{vars}{requestee} = $user; + } } # @@ -791,282 +806,178 @@ sub review_history { # sub db_schema_abstract_schema { - my ($self, $args) = @_; - $args->{'schema'}->{'product_reviewers'} = { - FIELDS => [ - id => { - TYPE => 'MEDIUMSERIAL', - NOTNULL => 1, - PRIMARYKEY => 1, - }, - user_id => { - TYPE => 'INT3', - NOTNULL => 1, - REFERENCES => { - TABLE => 'profiles', - COLUMN => 'userid', - DELETE => 'CASCADE', - } - }, - display_name => { - TYPE => 'VARCHAR(64)', - }, - product_id => { - TYPE => 'INT2', - NOTNULL => 1, - REFERENCES => { - TABLE => 'products', - COLUMN => 'id', - DELETE => 'CASCADE', - } - }, - sortkey => { - TYPE => 'INT2', - NOTNULL => 1, - DEFAULT => 0, - }, - ], - INDEXES => [ - product_reviewers_idx => { - FIELDS => [ 'user_id', 'product_id' ], - TYPE => 'UNIQUE', - }, - ], - }; - $args->{'schema'}->{'component_reviewers'} = { - FIELDS => [ - id => { - TYPE => 'MEDIUMSERIAL', - NOTNULL => 1, - PRIMARYKEY => 1, - }, - user_id => { - TYPE => 'INT3', - NOTNULL => 1, - REFERENCES => { - TABLE => 'profiles', - COLUMN => 'userid', - DELETE => 'CASCADE', - } - }, - display_name => { - TYPE => 'VARCHAR(64)', - }, - component_id => { - TYPE => 'INT2', - NOTNULL => 1, - REFERENCES => { - TABLE => 'components', - COLUMN => 'id', - DELETE => 'CASCADE', - } - }, - sortkey => { - TYPE => 'INT2', - NOTNULL => 1, - DEFAULT => 0, - }, - ], - INDEXES => [ - component_reviewers_idx => { - FIELDS => [ 'user_id', 'component_id' ], - TYPE => 'UNIQUE', - }, - ], - }; - - $args->{'schema'}->{'flag_state_activity'} = { - FIELDS => [ - id => { - TYPE => 'MEDIUMSERIAL', - NOTNULL => 1, - PRIMARYKEY => 1, - }, - - flag_when => { - TYPE => 'DATETIME', - NOTNULL => 1, - }, - - type_id => { - TYPE => 'INT2', - NOTNULL => 1, - REFERENCES => { - TABLE => 'flagtypes', - COLUMN => 'id', - DELETE => 'CASCADE' - } - }, - - flag_id => { - TYPE => 'INT3', - NOTNULL => 1, - }, - - setter_id => { - TYPE => 'INT3', - NOTNULL => 1, - REFERENCES => { - TABLE => 'profiles', - COLUMN => 'userid', - }, - }, - - requestee_id => { - TYPE => 'INT3', - REFERENCES => { - TABLE => 'profiles', - COLUMN => 'userid', - }, - }, - - bug_id => { - TYPE => 'INT3', - NOTNULL => 1, - REFERENCES => { - TABLE => 'bugs', - COLUMN => 'bug_id', - DELETE => 'CASCADE' - } - }, - - attachment_id => { - TYPE => 'INT3', - REFERENCES => { - TABLE => 'attachments', - COLUMN => 'attach_id', - DELETE => 'CASCADE' - } - }, - - status => { - TYPE => 'CHAR(1)', - NOTNULL => 1, - }, - ], - }; - - $args->{'schema'}->{'bug_mentors'} = { - FIELDS => [ - bug_id => { - TYPE => 'INT3', - NOTNULL => 1, - REFERENCES => { - TABLE => 'bugs', - COLUMN => 'bug_id', - DELETE => 'CASCADE', - }, - }, - user_id => { - TYPE => 'INT3', - NOTNULL => 1, - REFERENCES => { - TABLE => 'profiles', - COLUMN => 'userid', - DELETE => 'CASCADE', - } - }, - ], - INDEXES => [ - bug_mentors_idx => { - FIELDS => [ 'bug_id', 'user_id' ], - TYPE => 'UNIQUE', - }, - bug_mentors_bug_id_idx => [ 'bug_id' ], - ], - }; - - $args->{'schema'}->{'bug_mentors'} = { - FIELDS => [ - bug_id => { - TYPE => 'INT3', - NOTNULL => 1, - REFERENCES => { - TABLE => 'bugs', - COLUMN => 'bug_id', - DELETE => 'CASCADE', - }, - }, - user_id => { - TYPE => 'INT3', - NOTNULL => 1, - REFERENCES => { - TABLE => 'profiles', - COLUMN => 'userid', - DELETE => 'CASCADE', - } - }, - ], - INDEXES => [ - bug_mentors_idx => { - FIELDS => [ 'bug_id', 'user_id' ], - TYPE => 'UNIQUE', - }, - bug_mentors_bug_id_idx => [ 'bug_id' ], - ], - }; + my ($self, $args) = @_; + $args->{'schema'}->{'product_reviewers'} = { + FIELDS => [ + id => {TYPE => 'MEDIUMSERIAL', NOTNULL => 1, PRIMARYKEY => 1,}, + user_id => { + TYPE => 'INT3', + NOTNULL => 1, + REFERENCES => {TABLE => 'profiles', COLUMN => 'userid', DELETE => 'CASCADE',} + }, + display_name => {TYPE => 'VARCHAR(64)',}, + product_id => { + TYPE => 'INT2', + NOTNULL => 1, + REFERENCES => {TABLE => 'products', COLUMN => 'id', DELETE => 'CASCADE',} + }, + sortkey => {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0,}, + ], + INDEXES => [ + product_reviewers_idx => + {FIELDS => ['user_id', 'product_id'], TYPE => 'UNIQUE',}, + ], + }; + $args->{'schema'}->{'component_reviewers'} = { + FIELDS => [ + id => {TYPE => 'MEDIUMSERIAL', NOTNULL => 1, PRIMARYKEY => 1,}, + user_id => { + TYPE => 'INT3', + NOTNULL => 1, + REFERENCES => {TABLE => 'profiles', COLUMN => 'userid', DELETE => 'CASCADE',} + }, + display_name => {TYPE => 'VARCHAR(64)',}, + component_id => { + TYPE => 'INT2', + NOTNULL => 1, + REFERENCES => {TABLE => 'components', COLUMN => 'id', DELETE => 'CASCADE',} + }, + sortkey => {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0,}, + ], + INDEXES => [ + component_reviewers_idx => + {FIELDS => ['user_id', 'component_id'], TYPE => 'UNIQUE',}, + ], + }; + + $args->{'schema'}->{'flag_state_activity'} = { + FIELDS => [ + id => {TYPE => 'MEDIUMSERIAL', NOTNULL => 1, PRIMARYKEY => 1,}, + + flag_when => {TYPE => 'DATETIME', NOTNULL => 1,}, + + type_id => { + TYPE => 'INT2', + NOTNULL => 1, + REFERENCES => {TABLE => 'flagtypes', COLUMN => 'id', DELETE => 'CASCADE'} + }, + + flag_id => {TYPE => 'INT3', NOTNULL => 1,}, + + setter_id => { + TYPE => 'INT3', + NOTNULL => 1, + REFERENCES => {TABLE => 'profiles', COLUMN => 'userid',}, + }, + + requestee_id => + {TYPE => 'INT3', REFERENCES => {TABLE => 'profiles', COLUMN => 'userid',},}, + + bug_id => { + TYPE => 'INT3', + NOTNULL => 1, + REFERENCES => {TABLE => 'bugs', COLUMN => 'bug_id', DELETE => 'CASCADE'} + }, + + attachment_id => { + TYPE => 'INT3', + REFERENCES => + {TABLE => 'attachments', COLUMN => 'attach_id', DELETE => 'CASCADE'} + }, + + status => {TYPE => 'CHAR(1)', NOTNULL => 1,}, + ], + }; + + $args->{'schema'}->{'bug_mentors'} = { + FIELDS => [ + bug_id => { + TYPE => 'INT3', + NOTNULL => 1, + REFERENCES => {TABLE => 'bugs', COLUMN => 'bug_id', DELETE => 'CASCADE',}, + }, + user_id => { + TYPE => 'INT3', + NOTNULL => 1, + REFERENCES => {TABLE => 'profiles', COLUMN => 'userid', DELETE => 'CASCADE',} + }, + ], + INDEXES => [ + bug_mentors_idx => {FIELDS => ['bug_id', 'user_id'], TYPE => 'UNIQUE',}, + bug_mentors_bug_id_idx => ['bug_id'], + ], + }; + + $args->{'schema'}->{'bug_mentors'} = { + FIELDS => [ + bug_id => { + TYPE => 'INT3', + NOTNULL => 1, + REFERENCES => {TABLE => 'bugs', COLUMN => 'bug_id', DELETE => 'CASCADE',}, + }, + user_id => { + TYPE => 'INT3', + NOTNULL => 1, + REFERENCES => {TABLE => 'profiles', COLUMN => 'userid', DELETE => 'CASCADE',} + }, + ], + INDEXES => [ + bug_mentors_idx => {FIELDS => ['bug_id', 'user_id'], TYPE => 'UNIQUE',}, + bug_mentors_bug_id_idx => ['bug_id'], + ], + }; } sub install_update_db { - my $dbh = Bugzilla->dbh; - $dbh->bz_add_column( - 'products', - 'reviewer_required', { TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE' } - ); - $dbh->bz_add_column( - 'profiles', - 'review_request_count', { TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0 } - ); - $dbh->bz_add_column( - 'profiles', - 'feedback_request_count', { TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0 } - ); - $dbh->bz_add_column( - 'profiles', - 'needinfo_request_count', { TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0 } - ); - - my $field = Bugzilla::Field->new({ name => 'bug_mentor' }); - if (!$field) { - Bugzilla::Field->create({ - name => 'bug_mentor', - description => 'Mentor', - mailhead => 1 - }); - } - elsif (!$field->in_new_bugmail) { - $field->set_in_new_bugmail(1); - $field->update(); - } + my $dbh = Bugzilla->dbh; + $dbh->bz_add_column('products', 'reviewer_required', + {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'}); + $dbh->bz_add_column('profiles', 'review_request_count', + {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0}); + $dbh->bz_add_column('profiles', 'feedback_request_count', + {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0}); + $dbh->bz_add_column('profiles', 'needinfo_request_count', + {TYPE => 'INT2', NOTNULL => 1, DEFAULT => 0}); + + my $field = Bugzilla::Field->new({name => 'bug_mentor'}); + if (!$field) { + Bugzilla::Field->create({ + name => 'bug_mentor', description => 'Mentor', mailhead => 1 + }); + } + elsif (!$field->in_new_bugmail) { + $field->set_in_new_bugmail(1); + $field->update(); + } } sub install_filesystem { - my ($self, $args) = @_; - my $files = $args->{files}; - my $extensions_dir = bz_locations()->{extensionsdir}; - $files->{"$extensions_dir/Review/bin/review_requests_rebuild.pl"} = { - perms => Bugzilla::Install::Filesystem::OWNER_EXECUTE - }; + my ($self, $args) = @_; + my $files = $args->{files}; + my $extensions_dir = bz_locations()->{extensionsdir}; + $files->{"$extensions_dir/Review/bin/review_requests_rebuild.pl"} + = {perms => Bugzilla::Install::Filesystem::OWNER_EXECUTE}; } sub install_before_final_checks { - my ($self, $args) = @_; - add_setting({ - name => 'block_reviews', - options => ['on', 'off'], - default => 'off', - category => 'Reviews and Needinfo' - }); + my ($self, $args) = @_; + add_setting({ + name => 'block_reviews', + options => ['on', 'off'], + default => 'off', + category => 'Reviews and Needinfo' + }); } sub config_modify_panels { - my ($self, $args) = @_; - push @{ $args->{panels}->{advanced}->{params} }, { - name => 'max_reviewer_last_seen', - type => 't', - default => '', - default => 0, - checker => \&check_numeric, + my ($self, $args) = @_; + push @{$args->{panels}->{advanced}->{params}}, + { + name => 'max_reviewer_last_seen', + type => 't', + default => '', + default => 0, + checker => \&check_numeric, }; } @@ -1075,46 +986,44 @@ sub config_modify_panels { # sub webservice_user_get { - my ($self, $args) = @_; - my ($webservice, $params, $users) = @$args{qw(webservice params users)}; + my ($self, $args) = @_; + my ($webservice, $params, $users) = @$args{qw(webservice params users)}; - return unless filter_wants($params, 'requests'); + return unless filter_wants($params, 'requests'); - my $ids = [ - map { blessed($_->{id}) ? $_->{id}->value : $_->{id} } - grep { exists $_->{id} } - @$users - ]; + my $ids = [map { blessed($_->{id}) ? $_->{id}->value : $_->{id} } + grep { exists $_->{id} } @$users]; - return unless @$ids; - - my %user_map = map { $_->id => $_ } @{ Bugzilla::User->new_from_list($ids) }; - - foreach my $user (@$users) { - my $id = blessed($user->{id}) ? $user->{id}->value : $user->{id}; - my $user_obj = $user_map{$id}; - - $user->{requests} = { - review => { - blocked => $webservice->type('boolean', $user_obj->reviews_blocked), - pending => $webservice->type('int', $user_obj->{review_request_count}), - }, - feedback => { - # reviews_blocked includes feedback as well - blocked => $webservice->type('boolean', $user_obj->reviews_blocked), - pending => $webservice->type('int', $user_obj->{feedback_request_count}), - }, - needinfo => { - blocked => $webservice->type('boolean', $user_obj->needinfo_blocked), - pending => $webservice->type('int', $user_obj->{needinfo_request_count}), - }, - }; - } + return unless @$ids; + + my %user_map = map { $_->id => $_ } @{Bugzilla::User->new_from_list($ids)}; + + foreach my $user (@$users) { + my $id = blessed($user->{id}) ? $user->{id}->value : $user->{id}; + my $user_obj = $user_map{$id}; + + $user->{requests} = { + review => { + blocked => $webservice->type('boolean', $user_obj->reviews_blocked), + pending => $webservice->type('int', $user_obj->{review_request_count}), + }, + feedback => { + + # reviews_blocked includes feedback as well + blocked => $webservice->type('boolean', $user_obj->reviews_blocked), + pending => $webservice->type('int', $user_obj->{feedback_request_count}), + }, + needinfo => { + blocked => $webservice->type('boolean', $user_obj->needinfo_blocked), + pending => $webservice->type('int', $user_obj->{needinfo_request_count}), + }, + }; + } } sub webservice_user_suggest { - my ($self, $args) = @_; - $self->webservice_user_get($args); + my ($self, $args) = @_; + $self->webservice_user_get($args); } __PACKAGE__->NAME; diff --git a/extensions/Review/bin/migrate_mentor_from_whiteboard.pl b/extensions/Review/bin/migrate_mentor_from_whiteboard.pl index debf173a7..19837779e 100755 --- a/extensions/Review/bin/migrate_mentor_from_whiteboard.pl +++ b/extensions/Review/bin/migrate_mentor_from_whiteboard.pl @@ -36,11 +36,11 @@ EOF <>; # we need to be logged in to do user searching and update bugs -my $nobody = Bugzilla::User->check({ name => Bugzilla->params->{'nobody_user'} }); -$nobody->{groups} = [ Bugzilla::Group->get_all ]; +my $nobody = Bugzilla::User->check({name => Bugzilla->params->{'nobody_user'}}); +$nobody->{groups} = [Bugzilla::Group->get_all]; Bugzilla->set_user($nobody); -my $mentor_field = Bugzilla::Field->check({ name => 'bug_mentor' }); +my $mentor_field = Bugzilla::Field->check({name => 'bug_mentor'}); my $dbh = Bugzilla->dbh; # fix broken migration @@ -54,48 +54,40 @@ my $sth = $dbh->prepare(" $sth->execute($mentor_field->id); my %pair; while (my $row = $sth->fetchrow_hashref) { - if ($row->{added} && $row->{removed}) { - %pair = (); - next; - } - if ($row->{added}) { - $pair{bug_id} = $row->{bug_id}; - $pair{bug_when} = $row->{bug_when}; - $pair{who} = $row->{added}; - next; - } - if (!$pair{bug_id}) { - next; - } - if ($row->{removed}) { - if ($row->{bug_id} == $pair{bug_id} - && $row->{bug_when} eq $pair{bug_when} - && $row->{removed} eq $pair{who}) - { - print "Fixing mentor on bug $row->{bug_id}\n"; - my $user = Bugzilla::User->check({ name => $row->{removed} }); - $dbh->bz_start_transaction; - $dbh->do( - "DELETE FROM bugs_activity WHERE id = ?", - undef, - $row->{id} - ); - my ($exists) = $dbh->selectrow_array( - "SELECT 1 FROM bug_mentors WHERE bug_id = ? AND user_id = ?", - undef, - $row->{bug_id}, $user->id - ); - if (!$exists) { - $dbh->do( - "INSERT INTO bug_mentors (bug_id, user_id) VALUES (?, ?)", - undef, - $row->{bug_id}, $user->id, - ); - } - $dbh->bz_commit_transaction; - %pair = (); - } + if ($row->{added} && $row->{removed}) { + %pair = (); + next; + } + if ($row->{added}) { + $pair{bug_id} = $row->{bug_id}; + $pair{bug_when} = $row->{bug_when}; + $pair{who} = $row->{added}; + next; + } + if (!$pair{bug_id}) { + next; + } + if ($row->{removed}) { + if ( $row->{bug_id} == $pair{bug_id} + && $row->{bug_when} eq $pair{bug_when} + && $row->{removed} eq $pair{who}) + { + print "Fixing mentor on bug $row->{bug_id}\n"; + my $user = Bugzilla::User->check({name => $row->{removed}}); + $dbh->bz_start_transaction; + $dbh->do("DELETE FROM bugs_activity WHERE id = ?", undef, $row->{id}); + my ($exists) + = $dbh->selectrow_array( + "SELECT 1 FROM bug_mentors WHERE bug_id = ? AND user_id = ?", + undef, $row->{bug_id}, $user->id); + if (!$exists) { + $dbh->do("INSERT INTO bug_mentors (bug_id, user_id) VALUES (?, ?)", + undef, $row->{bug_id}, $user->id,); + } + $dbh->bz_commit_transaction; + %pair = (); } + } } # migrate remaining bugs @@ -110,119 +102,95 @@ my $bug_ids = $dbh->selectcol_arrayref(" print "Bugs found: " . scalar(@$bug_ids) . "\n"; my $bugs = Bugzilla::Bug->new_from_list($bug_ids); foreach my $bug (@$bugs) { - my $whiteboard = $bug->status_whiteboard; - my $orig_whiteboard = $whiteboard; - my ($mentors, $errors) = extract_mentors($whiteboard); - - printf "%7s %s\n", $bug->id, $whiteboard; - foreach my $error (@$errors) { - print " $error\n"; + my $whiteboard = $bug->status_whiteboard; + my $orig_whiteboard = $whiteboard; + my ($mentors, $errors) = extract_mentors($whiteboard); + + printf "%7s %s\n", $bug->id, $whiteboard; + foreach my $error (@$errors) { + print " $error\n"; + } + foreach my $user (@$mentors) { + print " Mentor: " . $user->identity . "\n"; + } + next if @$errors; + $whiteboard =~ s/\[mentor=[^\]]+\]//g; + + my $migrated + = $dbh->selectcol_arrayref("SELECT user_id FROM bug_mentors WHERE bug_id = ?", + undef, $bug->id); + if (@$migrated) { + foreach my $migrated_id (@$migrated) { + $mentors = [grep { $_->id != $migrated_id } @$mentors]; } - foreach my $user (@$mentors) { - print " Mentor: " . $user->identity . "\n"; + if (!@$mentors) { + print " mentor(s) already migrated\n"; + next; } - next if @$errors; - $whiteboard =~ s/\[mentor=[^\]]+\]//g; - - my $migrated = $dbh->selectcol_arrayref( - "SELECT user_id FROM bug_mentors WHERE bug_id = ?", - undef, - $bug->id - ); - if (@$migrated) { - foreach my $migrated_id (@$migrated) { - $mentors = [ - grep { $_->id != $migrated_id } - @$mentors - ]; - } - if (!@$mentors) { - print " mentor(s) already migrated\n"; - next; - } - } - - my $delta_ts = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)'); - $dbh->bz_start_transaction; - $dbh->do( - "UPDATE bugs SET status_whiteboard=? WHERE bug_id=?", - undef, - $whiteboard, $bug->id - ); - Bugzilla::Bug::LogActivityEntry( - $bug->id, - 'status_whiteboard', - $orig_whiteboard, - $whiteboard, - $nobody->id, - $delta_ts, - ); - foreach my $mentor (@$mentors) { - $dbh->do( - "INSERT INTO bug_mentors (bug_id, user_id) VALUES (?, ?)", - undef, - $bug->id, $mentor->id, - ); - Bugzilla::Bug::LogActivityEntry( - $bug->id, - 'bug_mentor', - '', - $mentor->login, - $nobody->id, - $delta_ts, - ); - } - $dbh->do( - "UPDATE bugs SET lastdiffed = delta_ts WHERE bug_id = ?", - undef, - $bug->id, - ); - $dbh->bz_commit_transaction; + } + + my $delta_ts = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)'); + $dbh->bz_start_transaction; + $dbh->do("UPDATE bugs SET status_whiteboard=? WHERE bug_id=?", + undef, $whiteboard, $bug->id); + Bugzilla::Bug::LogActivityEntry($bug->id, 'status_whiteboard', + $orig_whiteboard, $whiteboard, $nobody->id, $delta_ts,); + foreach my $mentor (@$mentors) { + $dbh->do("INSERT INTO bug_mentors (bug_id, user_id) VALUES (?, ?)", + undef, $bug->id, $mentor->id,); + Bugzilla::Bug::LogActivityEntry($bug->id, 'bug_mentor', '', $mentor->login, + $nobody->id, $delta_ts,); + } + $dbh->do("UPDATE bugs SET lastdiffed = delta_ts WHERE bug_id = ?", + undef, $bug->id,); + $dbh->bz_commit_transaction; } sub extract_mentors { - my ($whiteboard) = @_; - - my (@mentors, @errors); - my $logout = 0; - while ($whiteboard =~ /\[mentor=([^\]]+)\]/g) { - my $mentor_string = $1; - $mentor_string =~ s/(^\s+|\s+$)//g; - if ($mentor_string =~ /\@/) { - # assume it's a full username if it contains an @ - my $user = Bugzilla::User->new({ name => $mentor_string }); - if (!$user) { - push @errors, "'$mentor_string' failed to match any users"; - } else { - push @mentors, $user; - } - } else { - # otherwise assume it's a : prefixed nick - - $mentor_string =~ s/^://; - my $matches = find_users(":$mentor_string"); - if (!@$matches) { - $matches = find_users($mentor_string); - } - - if (!$matches || !@$matches) { - push @errors, "'$mentor_string' failed to match any users"; - } elsif (scalar(@$matches) > 1) { - push @errors, "'$mentor_string' matches more than one user: " . - join(', ', map { $_->identity } @$matches); - } else { - push @mentors, $matches->[0]; - } - } + my ($whiteboard) = @_; + + my (@mentors, @errors); + my $logout = 0; + while ($whiteboard =~ /\[mentor=([^\]]+)\]/g) { + my $mentor_string = $1; + $mentor_string =~ s/(^\s+|\s+$)//g; + if ($mentor_string =~ /\@/) { + + # assume it's a full username if it contains an @ + my $user = Bugzilla::User->new({name => $mentor_string}); + if (!$user) { + push @errors, "'$mentor_string' failed to match any users"; + } + else { + push @mentors, $user; + } + } + else { + # otherwise assume it's a : prefixed nick + + $mentor_string =~ s/^://; + my $matches = find_users(":$mentor_string"); + if (!@$matches) { + $matches = find_users($mentor_string); + } + + if (!$matches || !@$matches) { + push @errors, "'$mentor_string' failed to match any users"; + } + elsif (scalar(@$matches) > 1) { + push @errors, "'$mentor_string' matches more than one user: " + . join(', ', map { $_->identity } @$matches); + } + else { + push @mentors, $matches->[0]; + } } - return (\@mentors, \@errors); + } + return (\@mentors, \@errors); } sub find_users { - my ($query) = @_; - my $matches = Bugzilla::User::match("*$query*", 2); - return [ - grep { $_->name =~ /:?\Q$query\E\b/i } - @$matches - ]; + my ($query) = @_; + my $matches = Bugzilla::User::match("*$query*", 2); + return [grep { $_->name =~ /:?\Q$query\E\b/i } @$matches]; } diff --git a/extensions/Review/bin/review_requests_rebuild.pl b/extensions/Review/bin/review_requests_rebuild.pl index 03d25d045..8bda4119c 100755 --- a/extensions/Review/bin/review_requests_rebuild.pl +++ b/extensions/Review/bin/review_requests_rebuild.pl @@ -24,7 +24,7 @@ use Bugzilla::Extension::Review::Util; Bugzilla->usage_mode(USAGE_MODE_CMDLINE); -rebuild_review_counters(sub{ - my ($count, $total) = @_; - indicate_progress({ current => $count, total => $total, every => 5 }); +rebuild_review_counters(sub { + my ($count, $total) = @_; + indicate_progress({current => $count, total => $total, every => 5}); }); diff --git a/extensions/Review/lib/FlagStateActivity.pm b/extensions/Review/lib/FlagStateActivity.pm index 35da42351..92efb6c02 100644 --- a/extensions/Review/lib/FlagStateActivity.pm +++ b/extensions/Review/lib/FlagStateActivity.pm @@ -24,65 +24,58 @@ 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 + 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; - }, + my ($param) = @_; + + return sub { + my ($invocant, $value) = @_; + $value = trim($value) or ThrowCodeError('param_required', {param => $param}); + return $value; + },; } sub _check_date { - my ($invocant, $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; + $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; + 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, + 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} } @@ -95,30 +88,33 @@ 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 }); + 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 }); + 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 }); + 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 }); + 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 }); + my ($self) = @_; + return $self->{attachment} + //= Bugzilla::Attachment->new({id => $self->attachment_id, cache => 1}); } 1; diff --git a/extensions/Review/lib/Util.pm b/extensions/Review/lib/Util.pm index a8744079d..61d4e9117 100644 --- a/extensions/Review/lib/Util.pm +++ b/extensions/Review/lib/Util.pm @@ -17,12 +17,12 @@ use Bugzilla; our @EXPORT = qw( rebuild_review_counters ); sub rebuild_review_counters { - my ($callback) = @_; - my $dbh = Bugzilla->dbh; + my ($callback) = @_; + my $dbh = Bugzilla->dbh; - $dbh->bz_start_transaction; + $dbh->bz_start_transaction; - my $rows = $dbh->selectall_arrayref(" + my $rows = $dbh->selectall_arrayref(" SELECT flags.requestee_id AS user_id, flagtypes.name AS flagtype, COUNT(*) as count @@ -32,55 +32,48 @@ sub rebuild_review_counters { WHERE flags.status = '?' AND flagtypes.name IN ('review', 'feedback', 'needinfo') GROUP BY flags.requestee_id, flagtypes.name - ", { Slice => {} }); + ", {Slice => {}}); - my ($count, $total, $current) = (1, scalar(@$rows), { id => 0 }); - foreach my $row (@$rows) { - $callback->($count++, $total) if $callback; - if ($row->{user_id} != $current->{id}) { - _update_profile($dbh, $current) if $current->{id}; - $current = { id => $row->{user_id} }; - } - $current->{$row->{flagtype}} = $row->{count}; + my ($count, $total, $current) = (1, scalar(@$rows), {id => 0}); + foreach my $row (@$rows) { + $callback->($count++, $total) if $callback; + if ($row->{user_id} != $current->{id}) { + _update_profile($dbh, $current) if $current->{id}; + $current = {id => $row->{user_id}}; } - _update_profile($dbh, $current) if $current->{id}; + $current->{$row->{flagtype}} = $row->{count}; + } + _update_profile($dbh, $current) if $current->{id}; - foreach my $field (qw( review feedback needinfo )) { - _fix_negatives($dbh, $field); - } + foreach my $field (qw( review feedback needinfo )) { + _fix_negatives($dbh, $field); + } - $dbh->bz_commit_transaction; + $dbh->bz_commit_transaction; } sub _fix_negatives { - my ($dbh, $field) = @_; - my $user_ids = $dbh->selectcol_arrayref( - "SELECT userid FROM profiles WHERE ${field}_request_count < 0" - ); - return unless @$user_ids; - $dbh->do( - "UPDATE profiles SET ${field}_request_count = 0 WHERE " . $dbh->sql_in('userid', $user_ids) - ); - foreach my $user_id (@$user_ids) { - Bugzilla->memcached->clear({ table => 'profiles', id => $user_id }); - } + my ($dbh, $field) = @_; + my $user_ids = $dbh->selectcol_arrayref( + "SELECT userid FROM profiles WHERE ${field}_request_count < 0"); + return unless @$user_ids; + $dbh->do("UPDATE profiles SET ${field}_request_count = 0 WHERE " + . $dbh->sql_in('userid', $user_ids)); + foreach my $user_id (@$user_ids) { + Bugzilla->memcached->clear({table => 'profiles', id => $user_id}); + } } sub _update_profile { - my ($dbh, $data) = @_; - $dbh->do(" + my ($dbh, $data) = @_; + $dbh->do(" UPDATE profiles SET review_request_count = ?, feedback_request_count = ?, needinfo_request_count = ? - WHERE userid = ?", - undef, - $data->{review} || 0, - $data->{feedback} || 0, - $data->{needinfo} || 0, - $data->{id} - ); - Bugzilla->memcached->clear({ table => 'profiles', id => $data->{id} }); + WHERE userid = ?", undef, $data->{review} || 0, $data->{feedback} || 0, + $data->{needinfo} || 0, $data->{id}); + Bugzilla->memcached->clear({table => 'profiles', id => $data->{id}}); } 1; diff --git a/extensions/Review/lib/WebService.pm b/extensions/Review/lib/WebService.pm index 0c54d725a..79843cf2c 100644 --- a/extensions/Review/lib/WebService.pm +++ b/extensions/Review/lib/WebService.pm @@ -20,277 +20,295 @@ use Bugzilla::Util qw(detaint_natural trick_taint); use Bugzilla::WebService::Util 'filter'; use constant PUBLIC_METHODS => qw( - flag_activity - suggestions + flag_activity + suggestions ); sub suggestions { - my ($self, $params) = @_; - my $dbh = Bugzilla->switch_to_shadow_db(); - - my ($bug, $product, $component); - if (exists $params->{bug_id}) { - $bug = Bugzilla::Bug->check($params->{bug_id}); - $product = $bug->product_obj; - $component = $bug->component_obj; - } - elsif (exists $params->{product}) { - $product = Bugzilla::Product->check($params->{product}); - if (exists $params->{component}) { - $component = Bugzilla::Component->check({ - product => $product, name => $params->{component} - }); - } - } - else { - ThrowUserError("reviewer_suggestions_param_required"); + my ($self, $params) = @_; + my $dbh = Bugzilla->switch_to_shadow_db(); + + my ($bug, $product, $component); + if (exists $params->{bug_id}) { + $bug = Bugzilla::Bug->check($params->{bug_id}); + $product = $bug->product_obj; + $component = $bug->component_obj; + } + elsif (exists $params->{product}) { + $product = Bugzilla::Product->check($params->{product}); + if (exists $params->{component}) { + $component + = Bugzilla::Component->check({ + product => $product, name => $params->{component} + }); } + } + else { + ThrowUserError("reviewer_suggestions_param_required"); + } - my @reviewers; - if ($bug) { - # we always need to be authentiated to perform user matching - my $user = Bugzilla->user; - if (!$user->id) { - Bugzilla->set_user(Bugzilla::User->check({ name => Bugzilla->params->{'nobody_user'} })); - push @reviewers, @{ $bug->mentors }; - Bugzilla->set_user($user); - } else { - push @reviewers, @{ $bug->mentors }; - } - } - if ($component) { - push @reviewers, @{ $component->reviewers_objs }; - } - if (!$component || !@{ $component->reviewers_objs }) { - push @reviewers, @{ $product->reviewers_objs }; - } + my @reviewers; + if ($bug) { - my @result; - foreach my $reviewer (@reviewers) { - push @result, { - id => $self->type('int', $reviewer->id), - email => $self->type('email', $reviewer->login), - name => $self->type('string', $reviewer->name), - review_count => $self->type('int', $reviewer->review_count), - }; + # we always need to be authentiated to perform user matching + my $user = Bugzilla->user; + if (!$user->id) { + Bugzilla->set_user(Bugzilla::User->check( + {name => Bugzilla->params->{'nobody_user'}})); + push @reviewers, @{$bug->mentors}; + Bugzilla->set_user($user); + } + else { + push @reviewers, @{$bug->mentors}; } - return \@result; + } + if ($component) { + push @reviewers, @{$component->reviewers_objs}; + } + if (!$component || !@{$component->reviewers_objs}) { + push @reviewers, @{$product->reviewers_objs}; + } + + my @result; + foreach my $reviewer (@reviewers) { + push @result, + { + id => $self->type('int', $reviewer->id), + email => $self->type('email', $reviewer->login), + name => $self->type('string', $reviewer->name), + review_count => $self->type('int', $reviewer->review_count), + }; + } + return \@result; } sub flag_activity { - my ($self, $params) = @_; - my $dbh = Bugzilla->switch_to_shadow_db(); - my %match_criteria; + my ($self, $params) = @_; + my $dbh = Bugzilla->switch_to_shadow_db(); + my %match_criteria; - if (my $flag_id = $params->{flag_id}) { - detaint_natural($flag_id) - or ThrowUserError('invalid_flag_id', { flag_id => $flag_id }); + if (my $flag_id = $params->{flag_id}) { + detaint_natural($flag_id) + or ThrowUserError('invalid_flag_id', {flag_id => $flag_id}); - $match_criteria{flag_id} = $flag_id; - } - - if (my $flag_ids = $params->{flag_ids}) { - foreach my $flag_id (@$flag_ids) { - detaint_natural($flag_id) - or ThrowUserError('invalid_flag_id', { flag_id => $flag_id }); - } + $match_criteria{flag_id} = $flag_id; + } - $match_criteria{flag_id} = $flag_ids; + if (my $flag_ids = $params->{flag_ids}) { + foreach my $flag_id (@$flag_ids) { + detaint_natural($flag_id) + or ThrowUserError('invalid_flag_id', {flag_id => $flag_id}); } - if (my $type_id = $params->{type_id}) { - detaint_natural($type_id) - or ThrowUserError('invalid_flag_type_id', { type_id => $type_id }); + $match_criteria{flag_id} = $flag_ids; + } - $match_criteria{type_id} = $type_id; - } + if (my $type_id = $params->{type_id}) { + detaint_natural($type_id) + or ThrowUserError('invalid_flag_type_id', {type_id => $type_id}); - if (my $type_name = $params->{type_name}) { - trick_taint($type_name); - my $flag_types = Bugzilla::FlagType::match({ name => $type_name }); - $match_criteria{type_id} = [map { $_->id } @$flag_types]; - } + $match_criteria{type_id} = $type_id; + } - foreach my $user_field (qw( requestee setter )) { - if (my $user_name = $params->{$user_field}) { - my $user = Bugzilla::User->check({ name => $user_name, cache => 1, _error => 'invalid_username' }); + if (my $type_name = $params->{type_name}) { + trick_taint($type_name); + my $flag_types = Bugzilla::FlagType::match({name => $type_name}); + $match_criteria{type_id} = [map { $_->id } @$flag_types]; + } - $match_criteria{ $user_field . "_id" } = $user->id; - } - } + foreach my $user_field (qw( requestee setter )) { + if (my $user_name = $params->{$user_field}) { + my $user = Bugzilla::User->check( + {name => $user_name, cache => 1, _error => 'invalid_username'}); - foreach my $field (qw( bug_id status )) { - if (exists $params->{$field}) { - $match_criteria{$field} = $params->{$field}; - } + $match_criteria{$user_field . "_id"} = $user->id; } + } - ThrowCodeError('param_required', { param => 'limit', function => 'Review.flag_activity()' }) - if defined $params->{offset} && !defined $params->{limit}; - - my $limit = delete $params->{limit}; - my $offset = delete $params->{offset}; - my $after = delete $params->{after}; - my $before = delete $params->{before}; - my $max_results = Bugzilla->params->{max_search_results}; - - if (!$limit || $limit > $max_results) { - $limit = $max_results; + foreach my $field (qw( bug_id status )) { + if (exists $params->{$field}) { + $match_criteria{$field} = $params->{$field}; } - - if ($after && $after =~ /^(\d{4}-\d{1,2}-\d{1,2})$/) { - $after = $1; - } - else { - my $now = DateTime->now; - $now->subtract(days => 30); - $after = $now->ymd('-'); - } - - if ($before && $before =~ /^(\d{4}-\d{1,2}-\d{1,2})$/) { - $before = $1; - } - else { - my $now = DateTime->now; - $before = $now->ymd('-'); - } - - $match_criteria{LIMIT} = $limit; - $match_criteria{OFFSET} = $offset if defined $offset; - $match_criteria{WHERE} = { 'date(flag_when) BETWEEN ? AND ?' => [$after, $before] }; - - # Throw error if no other parameters have been passed other than limit and offset - if (!grep(!/^(LIMIT|OFFSET)$/, keys %match_criteria)) { - ThrowUserError('flag_activity_parameters_required'); - } - - my $matches = Bugzilla::Extension::Review::FlagStateActivity->match(\%match_criteria); - my $user = Bugzilla->user; - $user->visible_bugs([ map { $_->bug_id } @$matches ]); - my @results = map { $self->_flag_state_activity_to_hash($_, $params) } - grep { $user->can_see_bug($_->bug_id) && _can_see_attachment($user, $_) } - @$matches; - return \@results; + } + + ThrowCodeError('param_required', + {param => 'limit', function => 'Review.flag_activity()'}) + if defined $params->{offset} && !defined $params->{limit}; + + my $limit = delete $params->{limit}; + my $offset = delete $params->{offset}; + my $after = delete $params->{after}; + my $before = delete $params->{before}; + my $max_results = Bugzilla->params->{max_search_results}; + + if (!$limit || $limit > $max_results) { + $limit = $max_results; + } + + if ($after && $after =~ /^(\d{4}-\d{1,2}-\d{1,2})$/) { + $after = $1; + } + else { + my $now = DateTime->now; + $now->subtract(days => 30); + $after = $now->ymd('-'); + } + + if ($before && $before =~ /^(\d{4}-\d{1,2}-\d{1,2})$/) { + $before = $1; + } + else { + my $now = DateTime->now; + $before = $now->ymd('-'); + } + + $match_criteria{LIMIT} = $limit; + $match_criteria{OFFSET} = $offset if defined $offset; + $match_criteria{WHERE} + = {'date(flag_when) BETWEEN ? AND ?' => [$after, $before]}; + + # Throw error if no other parameters have been passed other than limit and offset + if (!grep(!/^(LIMIT|OFFSET)$/, keys %match_criteria)) { + ThrowUserError('flag_activity_parameters_required'); + } + + my $matches + = Bugzilla::Extension::Review::FlagStateActivity->match(\%match_criteria); + my $user = Bugzilla->user; + $user->visible_bugs([map { $_->bug_id } @$matches]); + my @results + = map { $self->_flag_state_activity_to_hash($_, $params) } + grep { $user->can_see_bug($_->bug_id) && _can_see_attachment($user, $_) } + @$matches; + return \@results; } sub _can_see_attachment { - my ($user, $flag_state_activity) = @_; + my ($user, $flag_state_activity) = @_; - return 1 if !$flag_state_activity->attachment_id; - return 0 if $flag_state_activity->attachment->isprivate && !$user->is_insider; - return 1; + return 1 if !$flag_state_activity->attachment_id; + return 0 if $flag_state_activity->attachment->isprivate && !$user->is_insider; + return 1; } sub rest_resources { - return [ - # bug-id - qr{^/review/suggestions/(\d+)$}, { - GET => { - method => 'suggestions', - params => sub { - return { bug_id => $_[0] }; - }, - }, - }, - # product/component - qr{^/review/suggestions/([^/]+)/(.+)$}, { - GET => { - method => 'suggestions', - params => sub { - return { product => $_[0], component => $_[1] }; - }, - }, + return [ + # bug-id + qr{^/review/suggestions/(\d+)$}, + { + GET => { + method => 'suggestions', + params => sub { + return {bug_id => $_[0]}; }, - # just product - qr{^/review/suggestions/([^/]+)$}, { - GET => { - method => 'suggestions', - params => sub { - return { product => $_[0] }; - }, - }, + }, + }, + + # product/component + qr{^/review/suggestions/([^/]+)/(.+)$}, + { + GET => { + method => 'suggestions', + params => sub { + return {product => $_[0], component => $_[1]}; }, - # named parameters - qr{^/review/suggestions$}, { - GET => { - method => 'suggestions', - }, + }, + }, + + # just product + qr{^/review/suggestions/([^/]+)$}, + { + GET => { + method => 'suggestions', + params => sub { + return {product => $_[0]}; }, - # flag activity by flag id - qr{^/review/flag_activity/(\d+)$}, { - GET => { - method => 'flag_activity', - params => sub { - return { flag_id => $_[0] } - }, - }, + }, + }, + + # named parameters + qr{^/review/suggestions$}, + {GET => {method => 'suggestions',},}, + + # flag activity by flag id + qr{^/review/flag_activity/(\d+)$}, + { + GET => { + method => 'flag_activity', + params => sub { + return {flag_id => $_[0]}; }, - qr{^/review/flag_activity/type_name/(\w+)$}, { - GET => { - method => 'flag_activity', - params => sub { - return { type_name => $_[0] } - }, - }, + }, + }, + qr{^/review/flag_activity/type_name/(\w+)$}, + { + GET => { + method => 'flag_activity', + params => sub { + return {type_name => $_[0]}; }, - # flag activity by user - qr{^/review/flag_activity/(requestee|setter|type_id)/(.*)$}, { - GET => { - method => 'flag_activity', - params => sub { - return { $_[0] => $_[1] }; - }, - }, + }, + }, + + # flag activity by user + qr{^/review/flag_activity/(requestee|setter|type_id)/(.*)$}, + { + GET => { + method => 'flag_activity', + params => sub { + return {$_[0] => $_[1]}; }, - # flag activity with only query strings - qr{^/review/flag_activity$}, { - GET => { method => 'flag_activity' }, - }, - ]; + }, + }, + + # flag activity with only query strings + qr{^/review/flag_activity$}, + {GET => {method => 'flag_activity'},}, + ]; } sub _flag_state_activity_to_hash { - my ($self, $fsa, $params) = @_; - - my %flag = ( - id => $self->type('int', $fsa->id), - 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; - $flag{flag_id} = $self->type('int', $fsa->flag_id) unless $params->{flag_id}; - - return filter($params, \%flag); + my ($self, $fsa, $params) = @_; + + my %flag = ( + id => $self->type('int', $fsa->id), + 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; + $flag{flag_id} = $self->type('int', $fsa->flag_id) unless $params->{flag_id}; + + 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), - }; + 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) = @_; + my ($self, $user) = @_; - return { - id => $self->type('int', $user->id), - real_name => $self->type('string', $user->name), - name => $self->type('email', $user->login), - }; + return { + id => $self->type('int', $user->id), + real_name => $self->type('string', $user->name), + name => $self->type('email', $user->login), + }; } 1; |