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

RESOLVED FIXED in Bugzilla 4.4

Status

()

Bugzilla
Administration
--
enhancement
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Edmund Yan, Assigned: Edmund Yan)

Tracking

unspecified
Bugzilla 4.4
Bug Flags:
approval +

Details

Attachments

(1 attachment, 1 obsolete attachment)

719 bytes, patch
Frédéric Buclin
: review+
Details | Diff | Splinter Review
(Assignee)

Description

7 years ago
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

7 years ago
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.
(Assignee)

Comment 2

7 years ago
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

7 years ago
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.
Assignee: extensions → edmundhyan

Updated

7 years ago
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
Summary: Adding a hook when deleting a component → Use Bugzilla::Object's remove_from_db when deleting a Component
(Assignee)

Comment 4

6 years ago
Created attachment 551297 [details] [diff] [review]
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

6 years ago
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.
Attachment #551297 - Flags: review-

Updated

6 years ago
Target Milestone: --- → Bugzilla 5.0
(Assignee)

Comment 6

6 years ago
Created attachment 553390 [details] [diff] [review]
Patch v2

Revised patch and requested code review.
Attachment #551297 - Attachment is obsolete: true
Attachment #553390 - Flags: review?(mkanat)

Comment 7

6 years ago
Comment on attachment 553390 [details] [diff] [review]
Patch v2

r=LpSolit
Attachment #553390 - Flags: review?(mkanat) → review+

Updated

6 years ago
Status: NEW → ASSIGNED
Flags: approval+

Comment 8

6 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Component.pm
Committed revision 7924.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Component: Extensions → Administration
Resolution: --- → FIXED

Updated

6 years ago
Blocks: 679516
You need to log in before you can comment on or make changes to this bug.