372700 - Make Bugzilla::Bug do bug updating for moving in process_bug.cgi

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, enhancement)

Description

[Max Kanat-Alexander]

The code that does bug-moving in process_bug is pretty simple, and the best candidate for the start of $bug->update().

Comment 1

[Max Kanat-Alexander]

Attachment: v1

Okay, here we go. I've tested this on landfill and it works.

_check_bug_status and _check_resolution aren't *fully* implemented--they're only implemented as much as we need for this change, plus a little bit extra for everconfirmed.

I'll flesh them out in future bugs.

Comment 2

[Frédéric Buclin]

Comment on attachment 257392 [details] [diff] [review]
v1

The status and resolution are not updated when moving bugs. Only the comment is added.

Comment 3

[Max Kanat-Alexander]

Attachment: v2

Ah, thanks for catching that. :-) Bugzilla::Bug was missing an "id" method, so it was trying to update bug_id = NULL. :-)

Comment 4

[Frédéric Buclin]

Comment on attachment 258102 [details] [diff] [review]
v2

>Index: process_bug.cgi

>-        if ($bug->bug_status ne 'RESOLVED') {
>-            LogActivityEntry($id, 'bug_status', $bug->bug_status,
>-                             'RESOLVED', $whoid, $timestamp);
>-        }
>-        if ($bug->resolution ne 'MOVED') {
>-            LogActivityEntry($id, 'resolution', $bug->resolution,
>-                             'MOVED', $whoid, $timestamp);
>-        }

Deleting them prevents the bug activity table from being updated. Now this should be done by Bug->update() itself (which should loop over $changes and write changes to the DB). Either that or let Bug->set_foo() push data into some array, and update() would then loop over this array and commit changes.


>+        $bug->add_comment($cgi->param('comment'), 
>+                          { type => CMT_MOVED_TO, extra_data => $user->login });

'scalar' in front of $cgi->param('comment') is mandatory as it's the first argument passed to add_comment (if the comment is undefined, the hash would be seen by add_comment as the first argument).



>Index: Bugzilla/Bug.pm

>+        my $dup_id = $self->dup_id;
>+        $dbh->do("DELETE FROM duplicates WHERE dupe = ?", undef, $self->bug_id);

Why defining $dup_id if you don't use it here? :)


>+sub id { return $_[0]->{'bug_id'} }

You should fix Object->id instead, and make it return $self->{$self->ID_FIELD}. This means you would have to fix User.pm too (a one-liner, probably). This makes even more sense that Object->update() uses $id_field = $self->ID_FIELD, so using ID_FIELD as field but 'id' as value doesn't make sense.

Comment 5

[Max Kanat-Alexander]

Attachment: v3

Thanks for catching all those things!

Note that I do use dup_id, for $changes below. I wanted $changes to be accurate.

Comment 6

[Frédéric Buclin]

Comment on attachment 258106 [details] [diff] [review]
v3

Works fine. r=LpSolit

Comment 7

[Max Kanat-Alexander]

Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.355; previous revision: 1.354
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.175; previous revision: 1.174
done
Checking in Bugzilla/Object.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Object.pm,v  <--  Object.pm
new revision: 1.13; previous revision: 1.12
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.149; previous revision: 1.148
done