diff --git a/lib/mobility/backend/active_record/key_value.rb b/lib/mobility/backend/active_record/key_value.rb index 5177b5699..b8485d69e 100644 --- a/lib/mobility/backend/active_record/key_value.rb +++ b/lib/mobility/backend/active_record/key_value.rb @@ -1,3 +1,5 @@ +# frozen-string-literal: true + module Mobility module Backend =begin @@ -54,7 +56,7 @@ def write(locale, value, **_) def self.configure!(options) super type = options[:type] - options[:class_name] ||= Mobility::ActiveRecord.const_get("#{type.capitalize}Translation") + options[:class_name] ||= Mobility::ActiveRecord.const_get("#{type.capitalize}Translation".freeze) options[:class_name] = options[:class_name].constantize if options[:class_name].is_a?(String) options[:association_name] ||= options[:class_name].table_name.to_sym %i[type association_name].each { |key| options[key] = options[key].to_sym } @@ -77,7 +79,6 @@ def self.configure!(options) has_many association_name, ->{ where key: association_attributes }, as: :translatable, class_name: translations_class, - dependent: :destroy, inverse_of: :translatable, autosave: true before_save do @@ -85,6 +86,7 @@ def self.configure!(options) send(association_name).destroy(translation) end end + after_destroy :mobility_destroy_key_value_translations mod = Module.new do define_method :i18n do @@ -92,6 +94,16 @@ def self.configure!(options) end end extend mod + + private + + # Clean up *all* leftover translations of this model, only once. + def mobility_destroy_key_value_translations + [:string, :text].freeze.each do |type| + Mobility::ActiveRecord.const_get("#{type.capitalize}Translation".freeze). + where(translatable: self).destroy_all + end + end unless private_instance_methods(false).include?(:mobility_destroy_key_value_translations) end # @!group Cache Methods diff --git a/lib/mobility/backend/sequel/key_value.rb b/lib/mobility/backend/sequel/key_value.rb index 3d720e5f6..60dd9d2e7 100644 --- a/lib/mobility/backend/sequel/key_value.rb +++ b/lib/mobility/backend/sequel/key_value.rb @@ -1,3 +1,5 @@ +# frozen-string-literal: true + module Mobility module Backend =begin @@ -52,7 +54,7 @@ def self.configure!(options) super raise CacheRequired, "Cache required for Sequel::KeyValue backend" if options[:cache] == false type = options[:type] -options[:class_name] ||= Mobility::Sequel.const_get("#{type.capitalize}Translation") + options[:class_name] ||= Mobility::Sequel.const_get("#{type.capitalize}Translation".freeze) options[:class_name] = options[:class_name].constantize if options[:class_name].is_a?(String) options[:association_name] ||= options[:class_name].table_name.to_sym %i[type association_name].each { |key| options[key] = options[key].to_sym } @@ -77,8 +79,6 @@ def self.configure!(options) clearer: proc { send(:"#{association_name}_dataset").update(translatable_id: nil, translatable_type: nil) }, class: translations_class - plugin :association_dependencies, association_name => :destroy - callback_methods = Module.new do define_method :before_save do super() @@ -99,6 +99,22 @@ def self.configure!(options) extend extension include Mobility::Sequel::ColumnChanges.new(attributes) + + private + + # Clean up *all* leftover translations of this model, only once. + def self.mobility_key_value_callbacks_module + @mobility_key_value_destroy_callbacks_module ||= Module.new do + def after_destroy + super + [:string, :text].freeze.each do |type| + Mobility::Sequel.const_get("#{type.capitalize}Translation".freeze). + where(translatable_id: id, translatable_type: self.class.name).destroy + end + end + end + end unless respond_to?(:mobility_key_value_callbacks_module, true) + include mobility_key_value_callbacks_module end # @!group Cache Methods diff --git a/spec/mobility/backend/active_record/key_value_spec.rb b/spec/mobility/backend/active_record/key_value_spec.rb index 0bf96bf35..08171eb5c 100644 --- a/spec/mobility/backend/active_record/key_value_spec.rb +++ b/spec/mobility/backend/active_record/key_value_spec.rb @@ -236,6 +236,37 @@ end end + describe "after destroy" do + # In case we change the translated attributes on a model, we need to make + # sure we clean them up when the model is destroyed. + it "cleans up all associated translations, regardless of key" do + article = Article.create(title: "foo title", content: "foo content") + Mobility.with_locale(:ja) { article.update_attributes(title: "あああ", content: "ばばば") } + article.save + + # Create translations on another model, to check they do not get destroyed + Post.create(title: "post title", content: "post content") + + expect(Mobility::ActiveRecord::StringTranslation.count).to eq(1) + expect(Mobility::ActiveRecord::TextTranslation.count).to eq(5) + + Mobility::ActiveRecord::TextTranslation.create!(translatable: article, key: "key1", value: "value1", locale: "de") + Mobility::ActiveRecord::StringTranslation.create!(translatable: article, key: "key2", value: "value2", locale: "fr") + expect(Mobility::ActiveRecord::TextTranslation.count).to eq(6) + expect(Mobility::ActiveRecord::StringTranslation.count).to eq(2) + + article.destroy! + expect(Mobility::ActiveRecord::TextTranslation.count).to eq(1) + expect(Mobility::ActiveRecord::StringTranslation.count).to eq(1) + end + + it "only destroys translations once when cleaning up" do + article = Article.create(title: "foo title", content: "foo content") + expect(article).to receive(:mobility_destroy_key_value_translations).once.and_call_original + article.destroy! + end + end + describe ".configure!" do it "sets association_name and class_name from string type" do options = { type: :string } diff --git a/spec/mobility/backend/sequel/key_value_spec.rb b/spec/mobility/backend/sequel/key_value_spec.rb index e357d2bf8..db1586ce1 100644 --- a/spec/mobility/backend/sequel/key_value_spec.rb +++ b/spec/mobility/backend/sequel/key_value_spec.rb @@ -272,6 +272,41 @@ def after_save end end + describe "after destroy" do + # In case we change the translated attributes on a model, we need to make + # sure we clean them up when the model is destroyed. + it "cleans up all associated translations, regardless of key" do + article = Article.create(title: "foo title", content: "foo content") + Mobility.with_locale(:ja) { article.update(title: "あああ", content: "ばばば") } + article.save + + # Create translations on another model, to check they do not get destroyed + Post.create(title: "post title", content: "post content") + + expect(Mobility::Sequel::StringTranslation.count).to eq(1) + expect(Mobility::Sequel::TextTranslation.count).to eq(5) + + Mobility::Sequel::TextTranslation.create(translatable: article, key: "key1", value: "value1", locale: "de") + Mobility::Sequel::StringTranslation.create(translatable: article, key: "key2", value: "value2", locale: "fr") + expect(Mobility::Sequel::TextTranslation.count).to eq(6) + expect(Mobility::Sequel::StringTranslation.count).to eq(2) + + article.destroy + expect(Mobility::Sequel::TextTranslation.count).to eq(1) + expect(Mobility::Sequel::StringTranslation.count).to eq(1) + end + + it "only destroys translations once when cleaning up" do + article = Article.create(title: "foo title", content: "foo content") + # This is an ugly way to check that we are not destroying all + # translations twice. Since the actual callback is included in a module, + # it's hard to get at this directly. + expect(Mobility::Sequel::TextTranslation).to receive(:where).once.and_call_original + expect(Mobility::Sequel::StringTranslation).to receive(:where).once.and_call_original + article.destroy + end + end + describe ".configure!" do it "sets association_name and class_name from string type" do options = { type: :string }