Fix SourceSynchronizer silently dropping edits to translated attributes

app/services/source_synchronizer.rb

@@ -167,7 +167,7 @@ def find_record(src_id)
   # rubocop:disable Metrics/MethodLength
   def compare_and_sync(record, incoming_attrs, src_at, report)
     base  = base_attrs(record)
-    local = record.attributes.slice(*@source_attributes)
+    local = local_attrs(record)
 
     local_digest    = canonical_digest(local)
     incoming_digest = canonical_digest(incoming_attrs)
@@ -245,6 +245,27 @@ def compare_and_sync(record, incoming_attrs, src_at, report)
   end
   # rubocop:enable Metrics/MethodLength
 
+  # Local state for the source-managed attributes.
+  #
+  # Deliberately NOT record.attributes.slice(*@source_attributes). Mobility is
+  # configured with `backend :container` and without the `attribute_methods`
+  # plugin (config/initializers/mobility.rb), so translated attributes
+  # (description, uses, cultivation, ...) live inside the `translations` jsonb
+  # and never appear in #attributes. Slicing there yields {} for them, so local
+  # reads as permanently different from the stored snapshot: an unchanged
+  # re-sync is scored `locally_modified` and a genuine upstream edit is silently
+  # dropped rather than applied. Reading through the public reader treats
+  # column-backed and translated attributes identically.
+  #
+  # Note on fallbacks: the Mobility `fallbacks` plugin means reading a missing
+  # locale returns the :en value. Sync writes and reads within the same locale,
+  # so the comparison stays symmetric.
+  def local_attrs(record)
+    @source_attributes.each_with_object({}) do |attr, acc|
+      acc[attr] = record.public_send(attr) if record.respond_to?(attr)
+    end
+  end
+
   # base = last accepted source snapshot, sliced to source_attributes
   def base_attrs(record)
     snap = record.source_snapshot
@@ -287,7 +308,7 @@ def handle_content_conflict(record, base, local, incoming_attrs, report)
   end
 
   def handle_source_deletion(record, report)
-    local = record.attributes.slice(*@source_attributes)
+    local = local_attrs(record)
 
     existing = SyncConflict.where(
       syncable: record,

spec/services/source_synchronizer_translated_spec.rb

@@ -0,0 +1,91 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+# Verification of the suspected defect recorded as risk R5 in the plant-data
+# ownership migration roadmap.
+#
+# SourceSynchronizer#compare_and_sync reads local state with
+#
+#   local = record.attributes.slice(*@source_attributes)
+#
+# `record.attributes` returns ActiveRecord's column-backed attributes. Mobility
+# is configured with `backend :container` and WITHOUT the `attribute_methods`
+# plugin (config/initializers/mobility.rb), so translated attributes such as
+# `description` live inside the `translations` jsonb and never appear in
+# `attributes`. For a translated source attribute the local side therefore reads
+# as {} on every run.
+#
+# The existing suite never catches this: SOURCE_ATTRS is
+# %w[scientific_name family_names], both real columns, and the one spec that
+# passes %w[description] exercises the invalid-payload path, which returns
+# before compare_and_sync.
+#
+# Predicted consequence: a re-sync of unchanged data is scored as
+# `locally_modified` rather than `synced`, and a genuine upstream change is
+# never applied. Translated fields would silently stop syncing after creation.
+RSpec.describe SourceSynchronizer, 'with a Mobility-translated source attribute' do
+  let(:org)         { create(:organization, :real) }
+  let(:data_source) { create(:data_source, organization: org) }
+  let(:run_id)      { SecureRandom.hex(8) }
+
+  def sync(attrs)
+    SourceSynchronizer.new(
+      data_source: data_source,
+      model: Plant,
+      source_attributes: attrs,
+      run_id: run_id
+    )
+  end
+
+  def row(description:, source_record_id: 'tr-1', source_updated_at: 1.day.ago)
+    {
+      source_record_id: source_record_id,
+      deleted: false,
+      attributes: { 'description' => description },
+      source_updated_at: source_updated_at
+    }
+  end
+
+  it 'confirms the mechanism: a translated attribute is absent from #attributes' do
+    plant = create(:plant, owner_organization_id: org.id, source_organization_id: org.id,
+                           description: 'Some description')
+
+    expect(plant.description).to eq 'Some description'
+    expect(plant.attributes).to have_key('translations')
+    expect(plant.attributes).not_to have_key('description'),
+                                    'if this fails, Mobility now exposes translated attrs and the defect is gone'
+    expect(plant.attributes.slice('description')).to eq({})
+  end
+
+  it 'scores an identical re-run as synced, not locally_modified' do
+    batch = [row(description: 'A stable description')]
+
+    first = sync(%w[description]).apply(batch)
+    expect(first.created).to eq(1), 'expected the first run to create the record'
+
+    second = sync(%w[description]).apply(batch)
+
+    plant = Plant.find_by(source_record_id: 'tr-1')
+    aggregate_failures do
+      expect(second.locally_modified).to eq(0),
+                                         'nobody edited this record, so it must not be scored locally_modified'
+      expect(second.synced).to eq(1)
+      expect(plant.sync_state).to eq 'synced'
+    end
+  end
+
+  it 'applies a genuine upstream change to a translated attribute' do
+    sync(%w[description]).apply([row(description: 'Original text')])
+
+    changed = sync(%w[description]).apply(
+      [row(description: 'Updated upstream text', source_updated_at: 1.hour.ago)]
+    )
+
+    plant = Plant.find_by(source_record_id: 'tr-1')
+    aggregate_failures do
+      expect(changed.applied).to eq(1), 'an upstream edit must reach the record'
+      expect(plant.description).to eq 'Updated upstream text'
+    end
+  end
+end