#569: Don't create empty translations on read

lib/mobility/backends/active_record/table.rb

@@ -292,10 +292,25 @@ def visit_Mobility_Plugins_Arel_Attribute(object)
         end
       end
 
+      # @!macro backend_writer
+      def write(locale, value, **options)
+        super
+        if @cache_name
+          cache.values.each do |translation|
+            self.translations.push(translation) unless self.translations.include?(translation)
+          end
+        end
+      end
+
       # Returns translation for a given locale, or builds one if none is present.
+      # @param [Boolean] for_read will be set if trying to read a value, in which case blank
+      #   translation records will not be attached to the parent record.
       # @param [Symbol] locale
-      def translation_for(locale, **)
+      def translation_for(locale, for_read: false, **)
         translation = translations.in_locale(locale)
+        if for_read
+          return translations.klass.new(locale: locale) if translation.nil?
+        end
         translation ||= translations.build(locale: locale)
         translation
       end

lib/mobility/backends/table.rb

@@ -85,7 +85,7 @@ module Table
       # @!group Backend Accessors
       # @!macro backend_reader
       def read(locale, **options)
-        translation_for(locale, **options).send(attribute)
+        translation_for(locale, for_read: true, **options).send(attribute)
       end
 
       # @!macro backend_writer
@@ -131,12 +131,12 @@ def table_alias(locale)
       # Simple hash cache to memoize translations as a hash so they can be
       # fetched quickly.
       module Cache
-        def translation_for(locale, **options)
+        def translation_for(locale, for_read: false, **options)
           return super(locale, options) if options.delete(:cache) == false
           if cache.has_key?(locale)
             cache[locale]
           else
-            cache[locale] = super(locale, **options)
+            cache[locale] = super(locale, for_read: for_read, **options)
           end
         end
 

spec/mobility/backends/active_record/table_spec.rb

@@ -90,7 +90,7 @@
     describe "cleaning up blank translations" do
       let(:title_backend) { backend_for(article, :title) }
 
-      it "builds nil translations when reading but does not save them" do
+      it "does not build nil translations" do
         Mobility.locale = :en
         article = Article.new(title: "New Article")
         association_name = article.mobility_backends[:title].association_name
@@ -102,7 +102,70 @@
         article.title
 
         aggregate_failures do
-          expect(article.send(association_name).size).to eq(3)
+          expect(article.send(association_name).size).to eq(1)
+          article.save
+          expect(article.title).to be_nil
+          expect(article.reload.send(association_name).size).to eq(1)
+        end
+      end
+
+      it "removes nil translations when saving persisted record" do
+        Mobility.locale = :en
+        article = Article.create(title: "New Article")
+        association_name = article.mobility_backends[:title].association_name
+
+        aggregate_failures do
+          expect(article.send(association_name).size).to eq(1)
+
+          Mobility.locale = :ja
+          article.title = "新規記事"
+          expect(article.send(association_name).size).to eq(2)
+
+          article.save
+          expect(article.send(association_name).size).to eq(2)
+
+          article.title = nil
+          article.save
+          article.reload
+          expect(article.send(association_name).size).to eq(1)
+        end
+      end
+    end
+  end
+
+  describe "translations association with cache" do
+    plugins :active_record, :reader, :writer, :cache
+    before { translates Article, :title, :content, backend: :table }
+
+    describe "cleaning up blank translations" do
+      let(:title_backend) { backend_for(article, :title) }
+
+      it "saves correctly by writing after reads" do
+        Mobility.locale = :en
+        article = Article.new
+        association_name = article.mobility_backends[:title].association_name
+
+        article.title # cached read
+        article.title = "New Article"
+
+        expect(article.send(association_name).size).to eq(1)
+        article.save
+        expect(article.send(association_name).size).to eq(1)
+      end
+
+      it "does not build nil translations" do
+        Mobility.locale = :en
+        article = Article.new(title: "New Article")
+        association_name = article.mobility_backends[:title].association_name
+
+        Mobility.locale = :ja
+        article.title
+
+        Mobility.locale = :fr
+        article.title
+
+        aggregate_failures do
+          expect(article.send(association_name).size).to eq(1)
           article.save
           expect(article.title).to be_nil
           expect(article.reload.send(association_name).size).to eq(1)
@@ -166,10 +229,10 @@
         expect(title_backend.read(:de)).to eq(nil)
       end
 
-      it "builds translation if no translation exists" do
+      it "does not build a translation if no translation exists" do
         expect {
           title_backend.read(:de)
-        }.to change(subject.send(title_backend.association_name), :size).by(1)
+        }.not_to change(subject.send(title_backend.association_name), :size)
       end
 
       describe "reading back written attributes" do