clone_attributes_from: update translations in place, not delete+recreate

Every promotion (autosave creation, draft creation, draft save) ran
every locale's translation through delete-all-then-recreate
unconditionally, giving every locale a fresh created_at/updated_at
regardless of whether its content actually changed. Confirmed via
Node.find(546).draft.translations -- both DE and EN shared one
identical timestamp despite only EN having been edited.

This silently defeated Page.find_with_outdated_translations' whole
staleness comparison: every save reset every locale back into
lockstep, so timestamps could never actually drift apart the way
the feature depends on. The existing test for it only ever passed
by manually backdating a timestamp with record_timestamps = false,
bypassing normal save flow entirely -- not something that happens
through ordinary editing.

Now updates a matching locale's translation only when its own
attributes actually differ, creates one genuinely new, and removes
one genuinely gone from the source -- same end state, but real
per-locale timestamps survive an unrelated save. search_vector is
excluded from both the comparison and the copied attributes: it's
DB-trigger-maintained from title/abstract, not real content, and
comparing a precomputed tsvector risked a false 'changed' from
representation noise alone.

Also reloads the source's translations association, not just
self's -- clone_attributes_from previously only guaranteed a fresh
self, leaving any caller holding a page reference across an earlier
mutation vulnerable to reading a stale cached association.

1 files changed, 53 insertions, 12 deletions

diff --git a/app/models/page.rb b/app/models/page.rb
index f796228..e5a8d9d 100644
--- a/app/models/page.rb
+++ b/app/models/page.rb
@@ -176,18 +176,34 @@ class Page < ApplicationRecord
     return nil unless page
 
     self.reload
+    page.translations.reload
 
     # Clone untranslated attributes
     self.tag_list         = page.tag_list
     self.template_name  ||= page.template_name
     self.published_at     = page.published_at
 
-    # Getting rid of the auto-generated empty translations
-    self.translations.delete_all
+    # Clone translated attributes -- update each locale in place rather
+    # than delete-and-recreate, so a locale whose content is genuinely
+    # unchanged keeps its real created_at/updated_at instead of looking
+    # freshly touched on every single save (which was silently defeating
+    # Page.find_with_outdated_translations' whole staleness comparison).
+    # search_vector is excluded deliberately: it's DB-trigger-maintained
+    # from title/abstract, not real content, and comparing a precomputed
+    # tsvector risked a false "changed" from representation noise alone.
+    source_locales = page.translations.map(&:locale)
+    self.translations.where.not(:locale => source_locales).destroy_all
 
-    # Clone translated attributes
-    page.translations.each do |translation|
-      self.translations.create!(translation.attributes.except("id", "page_id", "created_at", "updated_at"))
+    page.translations.each do |source_translation|
+      attrs = source_translation.attributes.except("id", "page_id", "created_at", "updated_at", "search_vector")
+      mine  = self.translations.find_by(:locale => source_translation.locale)
+
+      if mine
+        changed_attrs = attrs.reject { |k, v| mine.public_send(k) == v }
+        mine.update!(changed_attrs) if changed_attrs.any?
+      else
+        self.translations.create!(attrs)
+      end
     end
 
     # Clone asset references
@@ -196,19 +212,28 @@ class Page < ApplicationRecord
     self.save
   end
 
-  def diff_against other, view: :inline
+  def diff_against other, view: :inline, locale: nil
+    if locale
+      mine, theirs = translations.find_by(:locale => locale), other.translations.find_by(:locale => locale)
+      my_title, my_abstract, my_body          = mine&.title,   mine&.abstract,   mine&.body
+      their_title, their_abstract, their_body = theirs&.title, theirs&.abstract, theirs&.body
+    else
+      my_title, my_abstract, my_body          = title, abstract, body
+      their_title, their_abstract, their_body = other.title, other.abstract, other.body
+    end
+
     text_diffs =
       if view == :side_by_side
         {
-          title:    HtmlWordDiff.side_by_side(other.title.to_s,    title.to_s),
-          abstract: HtmlWordDiff.side_by_side(other.abstract.to_s, abstract.to_s),
-          body:     HtmlWordDiff.side_by_side(other.body.to_s,     body.to_s)
+          title:    HtmlWordDiff.side_by_side(their_title.to_s,    my_title.to_s),
+          abstract: HtmlWordDiff.side_by_side(their_abstract.to_s, my_abstract.to_s),
+          body:     HtmlWordDiff.side_by_side(their_body.to_s,     my_body.to_s)
         }
       else
         {
-          title:    HtmlWordDiff.inline(other.title.to_s,    title.to_s),
-          abstract: HtmlWordDiff.inline(other.abstract.to_s, abstract.to_s),
-          body:     HtmlWordDiff.inline(other.body.to_s,     body.to_s)
+          title:    HtmlWordDiff.inline(their_title.to_s,    my_title.to_s),
+          abstract: HtmlWordDiff.inline(their_abstract.to_s, my_abstract.to_s),
+          body:     HtmlWordDiff.inline(their_body.to_s,     my_body.to_s)
         }
       end
 
@@ -219,6 +244,22 @@ class Page < ApplicationRecord
     )
   end
 
+  def locale_diff_summary other
+    (translated_locales | other.translated_locales).sort_by(&:to_s).map do |locale|
+      mine   = translations.find_by(:locale => locale)
+      theirs = other.translations.find_by(:locale => locale)
+      {
+        :locale       => locale,
+        :exists_here  => mine.present?,
+        :exists_there => theirs.present?,
+        :changed      => mine.present? != theirs.present? ||
+                          mine&.title != theirs&.title ||
+                          mine&.abstract != theirs&.abstract ||
+                          mine&.body != theirs&.body
+      }
+    end
+  end
+
   def public?
     published_at.nil? ? true : published_at < Time.now
   end