Destroy all translations after model is destroyed (KeyValue backend)

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,21 +79,31 @@ 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
           send(association_name).select { |t| t.value.blank? }.each do |translation|
             send(association_name).destroy(translation)
           end
         end
+        after_destroy :mobility_destroy_key_value_translations
 
         mod = Module.new do
           define_method :i18n do
             @mobility_scope ||= super().extending(QueryMethods.new(attributes, 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

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

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 }

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 }