diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index a2bbd3224a..f264849a58 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -747,6 +747,12 @@ use constant ABSTRACT_SCHEMA => { NOTNULL => 1, REFERENCES => {TABLE => 'profiles', COLUMN => 'userid'}, }, + # The user who made the change. Not always the setter: redirecting a + # request keeps the original setter. Nullable until all pods write it. + who => { + TYPE => 'INT3', + REFERENCES => {TABLE => 'profiles', COLUMN => 'userid'}, + }, requestee_id => { TYPE => 'INT3', REFERENCES => {TABLE => 'profiles', COLUMN => 'userid'}, @@ -767,6 +773,7 @@ use constant ABSTRACT_SCHEMA => { flag_activity_type_id_idx => ['type_id'], flag_activity_bug_id_idx => ['bug_id'], flag_activity_status_when_idx => ['status', 'flag_when'], + flag_activity_who_idx => ['who', 'flag_when'], ], }, diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index f582bfcaa0..7779512597 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -491,6 +491,7 @@ sub create { Bugzilla::FlagActivity->create({ flag_when => $timestamp, setter_id => $flag->setter_id, + who => Bugzilla->user->id || $flag->setter_id, status => $flag->status, type_id => $flag->type_id, flag_id => $flag->id, @@ -519,6 +520,7 @@ sub update { Bugzilla::FlagActivity->create({ flag_when => $timestamp, setter_id => $self->setter_id, + who => Bugzilla->user->id || $self->setter_id, status => $self->status, type_id => $self->type_id, flag_id => $self->id, @@ -595,6 +597,7 @@ sub update_flags { Bugzilla::FlagActivity->create({ flag_when => $timestamp, setter_id => Bugzilla->user->id || $old_flag->setter_id, + who => Bugzilla->user->id || $old_flag->setter_id, status => 'X', type_id => $old_flag->type_id, flag_id => $old_flag->id, @@ -716,6 +719,7 @@ sub force_retarget { Bugzilla::FlagActivity->create({ flag_when => $timestamp, setter_id => Bugzilla->user->id || $flag->setter_id, + who => Bugzilla->user->id || $flag->setter_id, status => 'X', type_id => $flag->type_id, flag_id => $flag->id, diff --git a/Bugzilla/FlagActivity.pm b/Bugzilla/FlagActivity.pm index da41a79980..4ed09aa76d 100644 --- a/Bugzilla/FlagActivity.pm +++ b/Bugzilla/FlagActivity.pm @@ -29,6 +29,7 @@ use constant DB_COLUMNS => qw( type_id flag_id setter_id + who requestee_id bug_id attachment_id @@ -69,6 +70,7 @@ use constant VALIDATORS => { type_id => _check_param_required('type_id'), flag_id => _check_param_required('flag_id'), setter_id => _check_param_required('setter_id'), + who => _check_param_required('who'), bug_id => _check_param_required('bug_id'), status => \&_check_status, }; @@ -77,6 +79,7 @@ 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 who { return $_[0]->{who} } sub bug_id { return $_[0]->{bug_id} } sub requestee_id { return $_[0]->{requestee_id} } sub attachment_id { return $_[0]->{attachment_id} } diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index 5147beaa07..dbda8e1faa 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -844,6 +844,10 @@ sub update_table_definitions { # Bug 1949556 - dkl@mozilla.com $dbh->bz_add_index('audit_log', 'audit_log_object_id_idx', ['object_id']); + # Bug 2088705 - xavier.lhour@gmail.com + # Before _migrate_flag_state_activity, which copies into this column. + _add_flag_activity_who(); + # Bug 1806896 - xavier.lhour@gmail.com _migrate_flag_state_activity(); @@ -4531,16 +4535,34 @@ sub _migrate_flag_state_activity { # id is not carried over: nothing references flag_activity.id externally. $dbh->do( 'INSERT INTO flag_activity - (flag_when, type_id, flag_id, setter_id, requestee_id, bug_id, + (flag_when, type_id, flag_id, setter_id, who, requestee_id, bug_id, attachment_id, status) - SELECT flag_when, type_id, flag_id, setter_id, requestee_id, bug_id, - attachment_id, status + SELECT flag_when, type_id, flag_id, setter_id, setter_id, requestee_id, + bug_id, attachment_id, status FROM flag_state_activity' ); $dbh->bz_drop_table('flag_state_activity'); } +# Nullable for now: during the rolling deploy, pods still on the previous code +# insert rows without it. Bug 2085398 makes it NOT NULL once all pods write it. +sub _add_flag_activity_who { + my $dbh = Bugzilla->dbh; + + $dbh->bz_add_column('flag_activity', 'who', + {TYPE => 'INT3', REFERENCES => {TABLE => 'profiles', COLUMN => 'userid'}}); + + # For the user activity report and the changedby search. + $dbh->bz_add_index('flag_activity', 'flag_activity_who_idx', + [qw(who flag_when)]); + + # Rows from before this column, or from pods on the previous code, don't + # record who made the change: the setter is the best value available. Runs + # every time, on the index above. + $dbh->do('UPDATE flag_activity SET who = setter_id WHERE who IS NULL'); +} + sub _remove_duo_recovery_codes { my $dbh = Bugzilla->dbh; diff --git a/scripts/remove-non-public-data.pl b/scripts/remove-non-public-data.pl index c4cffc202a..29d9915387 100755 --- a/scripts/remove-non-public-data.pl +++ b/scripts/remove-non-public-data.pl @@ -94,7 +94,7 @@ BEGIN ], flag_activity => [ qw( - id flag_when type_id flag_id setter_id requestee_id bug_id + id flag_when type_id flag_id setter_id who requestee_id bug_id attachment_id status ) ], @@ -268,6 +268,7 @@ BEGIN AND (SELECT COUNT(*) FROM flags WHERE flags.setter_id = profiles.userid) = 0 AND (SELECT COUNT(*) FROM flags WHERE flags.requestee_id = profiles.userid) = 0 AND (SELECT COUNT(*) FROM flag_activity WHERE flag_activity.setter_id = profiles.userid) = 0 + AND (SELECT COUNT(*) FROM flag_activity WHERE flag_activity.who = profiles.userid) = 0 AND (SELECT COUNT(*) FROM flag_activity WHERE flag_activity.requestee_id = profiles.userid) = 0 "); diff --git a/t/flag-activity-who.t b/t/flag-activity-who.t new file mode 100644 index 0000000000..2dc6109712 --- /dev/null +++ b/t/flag-activity-who.t @@ -0,0 +1,96 @@ +#!/usr/bin/env perl +# 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. +use strict; +use warnings; +use 5.10.1; +use lib qw( . lib local/lib/perl5 ); + +BEGIN { + $ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf'; + $ENV{BUGZILLA_DISABLE_HOSTAGE} = 1; +} + +use Bugzilla::Test::MockLocalconfig (urlbase => 'http://bmo.test'); +use Bugzilla::Test::MockDB; +use Bugzilla::Test::MockParams (mail_delivery_method => 'None'); +use Bugzilla::Test::Util qw(create_bug create_user); +use Bugzilla::Bug; +use Bugzilla::Install::DB; +use Bugzilla::User; +use Test2::V0; + +# Redirecting a request keeps the original setter, but flag_activity.who +# records the user who redirected it. The column stays nullable while pods on +# the previous code still insert rows without it. + +create_user('automation@bmo.tld', '*'); +Bugzilla->set_user(Bugzilla::User->super_user); +my $dbh = Bugzilla->dbh; + +my $requester = create_user('requester@bmo.test', '*'); +my $redirector = create_user('redirector@bmo.test', '*'); +create_user($_, '*') for qw(bob@bmo.test carol@bmo.test); + +my $bug_id = create_bug( + short_desc => 'Flag redirect test bug', + comment => 'Bug for the flag_activity who tests', + bug_type => 'defect', + assigned_to => 'nobody@mozilla.org', +)->id; +my ($needinfo_id) + = $dbh->selectrow_array("SELECT id FROM flagtypes WHERE name = 'needinfo'"); + +Bugzilla->set_user($requester); +my $bug = Bugzilla::Bug->new($bug_id); +$bug->set_flags([], + [{type_id => $needinfo_id, status => '?', requestee => 'bob@bmo.test'}]); +$bug->update('2026-01-01 10:00:00'); + +Bugzilla->set_user($redirector); +$bug = Bugzilla::Bug->new($bug_id); +my ($flag) = @{$bug->flags}; +$bug->set_flags( + [{id => $flag->id, status => '?', requestee => 'carol@bmo.test'}], []); +$bug->update('2026-01-01 11:00:00'); + +$flag = Bugzilla::Flag->new($flag->id); +is($flag->setter_id, $requester->id, 'the setter is kept on redirect'); + +my $rows = $dbh->selectall_arrayref( + 'SELECT setter_id, who FROM flag_activity WHERE flag_id = ? ORDER BY id', + {Slice => {}}, + $flag->id +); +is( + $rows, + [ + {setter_id => $requester->id, who => $requester->id}, + {setter_id => $requester->id, who => $redirector->id}, + ], + 'who is the user who made each change' +); + +# A pod on the previous code inserts a row without who: it's saved, and the +# next checksetup fills it with the setter. +$dbh->do( + 'INSERT INTO flag_activity (flag_when, type_id, flag_id, setter_id, bug_id, + status) + VALUES (?, ?, ?, ?, ?, ?)', undef, '2026-01-01 12:00:00', $needinfo_id, + $flag->id, $requester->id, $bug_id, 'X' +); +Bugzilla::Install::DB::_add_flag_activity_who(); +my ($who) + = $dbh->selectrow_array( + "SELECT who FROM flag_activity WHERE flag_id = ? AND status = 'X'", + undef, $flag->id); +is($who, $requester->id, + 'a row inserted without who gets the setter on the next checksetup'); +ok(!$dbh->bz_column_info('flag_activity', 'who')->{NOTNULL}, + 'the column stays nullable'); + +done_testing;