Destroy all translations after model is destroyed (KeyValue backend) #15
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The KeyValue backend uses a polymorphic association where key columns store attributes, and we limit the association to only the keys defined as translated attributes for the model (for performance, etc.) This has the consequence that although we may have a dependent: destroy on the model, translations which were previously stored with different keys (for example, if we changed attribute names in the model) will be left behind, because they are not seen by the association (anymore). Having them left behind is unavoidable, but at minimum we should destroy them all when the model is destroyed to avoid them begin associated with other, new models. To do this we add an after destroy callback which destroys *all* string and text translations associated with the model, regardless of their attribute name (key). This ensures that when we destroy a translated record, we are guaranteed that the entire record of the record is wiped from the database.
shioyama
changed the title
|
I have a model with When a record is soft deleted, it remains in the database, but the translations are gone. Is it possible to keep the translations? |
|
Hmm... well, this could easily be made an option for the KeyValue backend. Say: class Post < ApplicationRecord
translates :foo, dependent_destroy: false
endOr something like this. Can you make a feature request (issue) for it? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a small but important change to ensure data integrity. When you translate a model using the KeyValue backend with some attribute, say
title, then later decide to change the name of the attribute to something else, your earlier translations will disappear. This is natural since they were stored with the keytitle, and Mobility is now looking for a different key on the association.This means that if you change your attribute name back, those earlier translations will again become available, because they were never destroyed (the change of attribute name was made at the code level, not the data level). I consider this a good thing.
However, currently they will also remain in the db even after you destroy the model, although translations with keys currently defined on the model will be destroyed. The reason is that there is a
dependent: :destroyoption on the translations association, but the association itself is limited with awhere(key: attributes)scope. The scope is important for efficiency, to avoid looping through "dead" translations that are no longer used, but it also results in these translations not being seen as dependencies.To fix this, I've added after destroy callbacks for ActiveRecord and Sequel which destroys all translations associated with this model, regardless of their key value. This means that provided you have at least one KeyValue attribute defined on the model when you destroy it, all translations will be destroyed as you would expect. I think this is reasonable and the best we can do.