679516 - Use Bugzilla::Object's remove_from_db when deleting any object

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[Edmund Yan]

Not all objects, when deleted, are using Bugzilla::Object's remove_from_db() method.  This is part of the goal to audit every deletion in Bugzilla, bug 641428.  In addition, this allows users to use the object_before_delete extension hook when any object is deleted.

Component.pm has already been fixed in bug 675366.  This bug deals with all other objects.

Comment 1

[Frédéric Buclin]

Let's fix them all at once, please. No need to file one bug per module.

Comment 2

[Edmund Yan]

Attachment: Patch for Products, Field, and Milestone

This patch fixes the remove_from_db() of the following:
Bugzilla::Products
Bugzilla::Field
Bugzilla::Milestone

There is one object left that is not fixed, and that is Bugzilla::Attachment.  Although it uses a method called remove_from_db(), it's somewhat of a misnomer.  The file is indeed deleted from the database, but a record is kept in the 'attachments' table and marked obsolete.

Comment 3

[Edmund Yan]

Attachment: Patch for Products, Field, Milestone, and Status

After talking to LpSolit on irc I am adding the clean-up of Bugzilla::Status as part of one patch dealing with remove_from_db() in general.

Comment 4

[Frédéric Buclin]

Comment on attachment 553605 [details] [diff] [review]
Patch for Products, Field, Milestone, and Status

>=== modified file 'Bugzilla/Status.pm'

>     my $id = $self->id;

$id is no longer in use.


>     $dbh->bz_start_transaction();
>     $self->SUPER::remove_from_db();
>     $dbh->bz_commit_transaction();

We don't need a transaction for this single line. Bugzilla::Object already does it for us. Just call remove_from_db.


Otherwise looks good.

Comment 5

[Edmund Yan]

Attachment: Patch for Products, Field, Milestone, and Status

Comment 6

[Frédéric Buclin]

Comment on attachment 553611 [details] [diff] [review]
Patch for Products, Field, Milestone, and Status

>=== modified file 'Bugzilla/Status.pm'

> sub remove_from_db {
>     my $self = shift;
>     my $dbh = Bugzilla->dbh;
>     $self->SUPER::remove_from_db();
>     delete Bugzilla->request_cache->{status_bug_state_open};
> }

$dbh is unused. No need to define it. This line should be removed on checkin. r=LpSolit

Comment 7

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Field.pm
modified Bugzilla/Milestone.pm
modified Bugzilla/Product.pm
modified Bugzilla/Status.pm
Committed revision 7938.