922482 - Change all bugs with [e-mail] assignee to [e-mail]

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Administration, task, P2)

Comment 1

[:glob ✱]

jdm - is it ok to just touch open bugs (about 3k) instead of all bugs (about 10k)?

we'll have to write a script to update the database directly to avoid triggering a torrent of bugmail, and in order to the records to be sane we have to touch the bug's last-modified timestamp.  doing that for closed bugs has proven to be painful, as people with dashboards watching the component are flooded with unnecessary "this bug has been updated" notifications.

Comment 2

[Josh Matthews [:jdm]]

Yes, open bugs are fine.

Comment 3

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-922482-v1.patch

I believe this script reassigns open bugs from one user to another, and updates the bugs_activity table, and does not cause bugmail. It updates the delta_ts and so on like contrib/reorg-tools/migrate_crash_signatures.pl.

Tests pass, and I am happily now the owner of a few thousands bugs on my local BMO install.

Comment 4

[:glob ✱]

Comment on attachment 8439716 [details] [diff] [review]
bug-922482-v1.patch

Review of attachment 8439716 [details] [diff] [review]:
-----------------------------------------------------------------

::: contrib/reorg-tools/reassign-open-bugs.pl
@@ +8,5 @@
> +
> +use 5.10.1;
> +use strict;
> +use warnings;
> +use lib qw(. lib);

while most reorg script don't current do this, i think it's nicer to allow these scripts to work regardless of the cwd:

use FindBin '$RealBin';
use lib "$ReadBin/../..", "$RealBin/../../lib";

@@ +29,5 @@
> +pod2usage(1) unless defined $from && defined $to;
> +
> +my $dbh = Bugzilla->dbh;
> +
> +$dbh->bz_start_transaction;

move start_transaction to before the foreach - you need to keep the transaction scope as small as possible

@@ +34,5 @@
> +
> +my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
> +my $field     = Bugzilla::Field->new({name => 'assigned_to', cache => 1});
> +my $from_user = Bugzilla::User->new({name => $from, cache => 1});
> +my $to_user   = Bugzilla::User->new({name => $to, cache => 1});

all of these should be ->check instead of ->new, so an error will be thrown if you provide an invalid user instead of returning undef

@@ +42,5 @@
> +                                      LEFT JOIN bug_status
> +                                             ON bug_status.value = bugs.bug_status
> +                                      WHERE bug_status.is_open = 1
> +                                        AND bugs.assigned_to = ?},
> +    { Slice => {} }, $from_user->id);

as you don't need to return or use assigned_to (it's always going to be $from_user), remove the Slice and use selectcol_arrayref instead

@@ +71,5 @@
> +
> +
> +=head1 AUTHOR
> +
> +Dylan William Hardison <[e-mail]>

remove the author section

Comment 5

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-922482-v2.patch

Revised and cleaned patch.

Comment 6

[:glob ✱]

Comment on attachment 8454851 [details] [diff] [review]
bug-922482-v2.patch

Review of attachment 8454851 [details] [diff] [review]:
-----------------------------------------------------------------

please rename this file to reassign_open_bugs.pl to match existing scripts (use underscore not hyphen).

::: contrib/reorg-tools/reassign-open-bugs.pl
@@ +1,1 @@
> +#!/usr/bin/perl -wT

nit: don't need both -w and "use warnings"; remove -w from here.

@@ +11,5 @@
> +use warnings;
> +
> +use FindBin '$RealBin';
> +BEGIN { ($RealBin) = $RealBin =~ /^(.+)$/ }
> +use lib "$RealBin/../..", "$RealBin/../../lib";

as it's a command line tool with a single source of user input, i'm not convinced the acrobatics required to work around taint issues are worth enabling of taint checking.
i'll leave it up to you if you want to leave taint mode enabled.

@@ +43,5 @@
> +                                      LEFT JOIN bug_status
> +                                             ON bug_status.value = bugs.bug_status
> +                                      WHERE bug_status.is_open = 1
> +                                        AND bugs.assigned_to = ?}, undef, $from_user->id);
> +$dbh->bz_start_transaction;

for consistency we should add a confirmation prompt here (as per movebugs)

@@ +45,5 @@
> +                                      WHERE bug_status.is_open = 1
> +                                        AND bugs.assigned_to = ?}, undef, $from_user->id);
> +$dbh->bz_start_transaction;
> +foreach my $bug_id (@$bugs) {
> +    warn "Updating bug $bug_id, ${\$from_user->id} -> ${\$to_user->id}...\n";

nit: no need to note the from/to user ids, as it's the same for every bug

@@ +52,5 @@
> +                undef, $bug_id, $to_user->id, $timestamp, $field->id, $from_user->login, $to_user->login);
> +    $dbh->do(q{UPDATE bugs SET assigned_to = ?, delta_ts = ?, lastdiffed = ? WHERE bug_id = ?},
> +        undef, $to_user->id, $timestamp, $timestamp, $bug_id);
> +}
> +$dbh->bz_commit_transaction;

because you're changing something which is tracked by the user profile extension, we need to tell it to recalculate the statistics for the two users:

  $from_user->clear_last_statistics_ts();
  $to_user->clear_last_statistics_ts();

as these methods are monkeypatched in by the UserProfile extension, for this to work the extensions need to be loaded first:
  use Bugzilla;
  BEGIN { Bugzilla->extensions() }

Comment 7

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-922482-v3.patch

> please rename this file to reassign_open_bugs.pl to match existing scripts (use underscore not hyphen).
Done.

> nit: don't need both -w and "use warnings"; remove -w from here.
> as it's a command line tool with a single source of user input, i'm not convinced the acrobatics required to work around taint issues are worth enabling of taint checking.
> i'll leave it up to you if you want to leave taint mode enabled.

Removed -T and -w

> for consistency we should add a confirmation prompt here (as per movebugs)
Done.

> nit: no need to note the from/to user ids, as it's the same for every bug
Done.

> because you're changing something which is tracked by the user profile extension, we need to tell it to recalculate the statistics for the two users:
>
>   $from_user->clear_last_statistics_ts();
>   $to_user->clear_last_statistics_ts();
>
> as these methods are monkeypatched in by the UserProfile extension, for this to work the extensions need to be loaded first:
>   use Bugzilla;
>   BEGIN { Bugzilla->extensions() }

And done.

Comment 8

[:glob ✱]

Comment on attachment 8455389 [details] [diff] [review]
bug-922482-v3.patch

Review of attachment 8455389 [details] [diff] [review]:
-----------------------------------------------------------------

r=glob

Comment 9

[Dylan Hardison [:dylan] (he/him)]

To [e-mail]:webtools/bmo/bugzilla.git
   27f4072..7dc9415  master -> master