675366 - Use Bugzilla::Object's remove_from_db when deleting a Component

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[Edmund Yan]

If I'm not mistaken there are no existing hooks that are triggered when a component is deleted.  I feel that there are 2 solutions to this:

1. Make Component::remove_from_db() call Object:remove_from_db() and thus make using the 'object_before_delete' hook possible
2. Add a hook before the database queries in Component::remove_from_db called 'component_before_delete'

I am not sure which method is preferred by you guys.  Let me know and I'd be more than happy to write a patch.

Comment 1

[Frédéric Buclin]

Our goal is to use Bugzilla::Object as much as possible, which we already did in bug 339380 for other Bugzilla::Component methods. So we will follow your suggestion #1.

Comment 2

[Edmund Yan]

the remove_from_db() on Product is also missing a call to the Object's method.  Should I file a separate bug for that or can I just use this one?

Comment 3

[Max Kanat-Alexander]

We have a bug for using remove_from_db everywhere--either make this a blocker or find an already-filed bug for adding the right remove_from_db method.

I would do each one separately unless the patches are 3 or 4 lines per object.

Comment 4

[Edmund Yan]

Attachment: Patch to Components.pm to call Object's remove_from_db()

This patch is only for Components.pm, will submit future patches on bug 641428.

Comment 5

[Max Kanat-Alexander]

Comment on attachment 551297 [details] [diff] [review]
Patch to Components.pm to call Object's remove_from_db()

  Hey Edmund. Thanks for the patch! In the future, make sure to ask for review, so that your patch can get checked in. Here's a bit more about our process, if you'd like to know:

  http://wiki.mozilla.org/Bugzilla:Developers

  So, the remove_from_db subroutine in Bugzilla::Component that you're editing was written before we had foreign keys. could you remove all the DELETE statements there and replace them all with just a call to SUPER::remove_from_db? The database will automatically handle all the other deletions.

Comment 6

[Edmund Yan]

Attachment: Patch v2

Revised patch and requested code review.

Comment 7

[Frédéric Buclin]

Comment on attachment 553390 [details] [diff] [review]
Patch v2

r=LpSolit

Comment 8

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Component.pm
Committed revision 7924.