Closed
Bug 513962
Opened 15 years ago
Closed 15 years ago
Deleting a collection does not delete its strings from the translation table
Categories
(addons.mozilla.org Graveyard :: Collections, defect, P3)
addons.mozilla.org Graveyard
Collections
Tracking
(Not tracked)
VERIFIED
FIXED
5.3
People
(Reporter: smccammon, Assigned: wenzel)
References
Details
Attachments
(1 file)
4.10 KB,
patch
|
clouserw
:
review+
|
Details | Diff | Splinter Review |
Querying the translations table after deleting a collection shows all the deleted collection's name and description strings still present.
Expected behavior:
Deleting a collection also deletes translations referenced by name and description foreign keys.
Theoretically there is no rule preventing these translations from being referenced by other foreign keys, so deleting these rows could potentially fail due to foreign key constraints (which is perfectly ok).
Updated•15 years ago
|
Priority: -- → P3
Target Milestone: --- → 5.1
Assignee | ||
Comment 1•15 years ago
|
||
The solution for this should go into the app_model's beforeDelete() callback, and iterate over the model's translated_fields array.
Updated•15 years ago
|
Target Milestone: 5.1 → 5.2
Updated•15 years ago
|
Assignee: nobody → fwenzel
Assignee | ||
Comment 2•15 years ago
|
||
(I have a feeling everytime I drop random thoughts in a bug, I end up having to fix it ;) )
Assignee | ||
Comment 3•15 years ago
|
||
This deletes translations before deleting the actual collection. We should (still) do this in the app_model, but for that we'll need to change all translation foreign keys to ON DELETE SET NULL which is out of scope here. But I shall file another bug for it.
Attachment #405081 -
Flags: review?(clouserw)
Assignee | ||
Updated•15 years ago
|
Status: NEW → ASSIGNED
Comment 4•15 years ago
|
||
Comment on attachment 405081 [details] [diff] [review]
Patch, rev. 1
I don't like running the DELETE in a loop. It would be cool to have some kind of a delayed SQL processor where we could just drop queries in a bucket and this other thing runs them when it gets a chance.
Attachment #405081 -
Flags: review?(clouserw) → review+
Comment 5•15 years ago
|
||
(In reply to comment #4)
> (From update of attachment 405081 [details] [diff] [review])
> I don't like running the DELETE in a loop. It would be cool to have some kind
> of a delayed SQL processor where we could just drop queries in a bucket and
> this other thing runs them when it gets a chance.
I've discussed this briefly with davedash, since he wants to do some addon post-save processing that needs to be fancier than trigger sql.
Hooking up gearman would be perfect here. The web request would shoot a message to gearmand and keep going, and a queue processor on some other machine would pick up the message and do some work.
Assignee | ||
Comment 6•15 years ago
|
||
That doesn't seem like a bad idea, this way we could have these requests processed asynchronously.
I'll land this for now, then we can improve it later. Aren't we having a maintenance release one of these milestones?
r53309.
Updated•15 years ago
|
Keywords: push-needed
Comment 7•15 years ago
|
||
from internet relay chat:
> you added ON DELETE SET NULL to the translations foreign keys in collections
> but that field is NOT NULL in the translations table
> so mysql won't let me create a new database
> and I don't know what's going to happen when that on delete trigger is hit
> it seems like ON DELETE CASCADE is what we want
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 5.2 → 5.3
Assignee | ||
Comment 8•15 years ago
|
||
(In reply to comment #7)
> > it seems like ON DELETE CASCADE is what we want
I don't think so. If you have two tables, a and b, and a references b ON DELETE CASCADE, then *a* is deleted when the corresponding entry in *b* is removed. In other words, ON DELETE CASCADE would delete the *collection* when the corresponding *translation* is deleted, not vice-versa. It would be nice if we could define a "backwards ON DELETE CASCADE" but since that doesn't (seem to?) exist, ON DELETE SET NULL is as good as it gets: It sets the "name" (etc.) field NULL if the corresponding translation is deleted.
I will try to import our test data from scratch and make sure that works as expected. Maybe something is missing there.
Assignee | ||
Comment 9•15 years ago
|
||
Indeed, the test data was borken :( Fixed in r53944.
Status: REOPENED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
Comment 10•15 years ago
|
||
(In reply to comment #8)
> (In reply to comment #7)
> > > it seems like ON DELETE CASCADE is what we want
>
> I don't think so. If you have two tables, a and b, and a references b ON DELETE
> CASCADE, then *a* is deleted when the corresponding entry in *b* is removed. In
> other words, ON DELETE CASCADE would delete the *collection* when the
> corresponding *translation* is deleted, not vice-versa. It would be nice if we
> could define a "backwards ON DELETE CASCADE" but since that doesn't (seem to?)
> exist, ON DELETE SET NULL is as good as it gets: It sets the "name" (etc.)
> field NULL if the corresponding translation is deleted.
Ah, thanks. The docs talk about parent and child and had me befuddled until I saw an example schema. Your patch makes everything load happily for me.
Status: RESOLVED → VERIFIED
Updated•9 years ago
|
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•