From 0b6ae26f3b4c3c8278e569fa6644a544e996c25d Mon Sep 17 00:00:00 2001 From: erdgeist Date: Mon, 13 Jul 2026 16:36:38 +0200 Subject: Add per-node translation management: list, edit, destroy, compare Replaces the old locale-switch-and-edit-the-same-screen workflow, which conflated presentation locale with content locale and let an editor silently drift into editing the wrong language with no persistent signal that anything had changed. Non-default-locale content now has its own explicit routes and screens, never sharing a route param with the ambient chrome locale. - PageTranslationsController: index/show/edit/update/destroy, scoped to Page.non_default_locales; update handles first-time creation too, so there's no separate new/create step. - Reads go through the actual PageTranslation row (Page#translation_summary), never through the Globalize fallback-bearing accessor -- fallback is correct for public rendering but wrong for editing, where a missing translation needs to look empty, not borrowed from another locale. - Translations ride on the page's own draft/head cycle; no independent publish state. - nodes#show gains a Translations section (per-locale Lock+Edit / Create+Lock, Destroy, a link into the read-only Compare view) and a locale indicator on its own default-locale content; nodes#edit, nodes#update, and nodes#autosave are pinned to the default locale via Globalize.with_locale regardless of the ambient route locale. - nodes#show no longer double-loads the node or calls wipe_draft! on every view (see previous commit for why that's now safe). - .preview_link_row is renamed .aligned_action_row now that it has a second real consumer. --- app/controllers/nodes_controller.rb | 12 ++- app/controllers/page_translations_controller.rb | 98 +++++++++++++++++++++++++ app/models/page.rb | 21 ++++++ app/views/nodes/show.html.erb | 49 +++++++++++-- app/views/page_translations/edit.html.erb | 39 ++++++++++ app/views/page_translations/index.html.erb | 19 +++++ app/views/page_translations/show.html.erb | 27 +++++++ 7 files changed, 257 insertions(+), 8 deletions(-) create mode 100644 app/controllers/page_translations_controller.rb create mode 100644 app/views/page_translations/edit.html.erb create mode 100644 app/views/page_translations/index.html.erb create mode 100644 app/views/page_translations/show.html.erb (limited to 'app') diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb index c1468be..249602d 100644 --- a/app/controllers/nodes_controller.rb +++ b/app/controllers/nodes_controller.rb @@ -16,6 +16,8 @@ class NodesController < ApplicationController :revert ] + around_action :pin_to_default_locale, :only => [:show, :edit, :update, :autosave] + def index @nodes = Node.root.descendants.includes(:head, :draft) .order('id DESC') @@ -59,9 +61,9 @@ class NodesController < ApplicationController end def show - node = Node.find(params[:id]) - node.wipe_draft! - @page = node.draft || node.head + @page = @node.draft || @node.head + @default_translation = @page.translations.find_by(:locale => I18n.default_locale) + @translations = @page.translation_summary end def edit @@ -248,6 +250,10 @@ class NodesController < ApplicationController end end + def pin_to_default_locale + Globalize.with_locale(I18n.default_locale) { yield } + end + def descendant_counts_for(ordered_with_level) counts = Hash.new(0) stack = [] # [node, level, index] diff --git a/app/controllers/page_translations_controller.rb b/app/controllers/page_translations_controller.rb new file mode 100644 index 0000000..47f01f7 --- /dev/null +++ b/app/controllers/page_translations_controller.rb @@ -0,0 +1,98 @@ +class PageTranslationsController < ApplicationController + layout 'admin' + + before_action :login_required + before_action :find_node + before_action :find_locale, :only => [:show, :edit, :update, :destroy] + + def index + page = @node.draft || @node.head + @translations = page ? page.translation_summary : [] + end + + def show + @page = @node.draft || @node.head + @translation = @page.translations.find_by(:locale => @locale) + @default_translation = @page.translations.find_by(:locale => I18n.default_locale) + end + + def edit + @node.lock_for_editing!(current_user) + @page = @node.draft || @node.head + @translation = @page.translations.find_by(:locale => @locale) + rescue LockedByAnotherUser => e + flash[:error] = e.message + redirect_to node_path(@node) + end + + def update + draft = ensure_editable_draft + Globalize.with_locale(@locale) { draft.update!(translation_params) } + flash[:notice] = "#{@locale.upcase} translation saved. Publish the draft to make it live." + + if params[:commit] == "Save + Unlock + Exit" + @node.unlock! + redirect_to node_path(@node) + else + redirect_to edit_node_translation_path(@node, @locale) + end + rescue LockedByAnotherUser => e + flash[:error] = e.message + redirect_to node_path(@node) + end + + def destroy + base = @node.draft || @node.head + unless base && base.translated_locales.include?(@locale) + flash[:error] = "No #{@locale.to_s.upcase} translation exists to remove." + return redirect_to node_path(@node) + end + + if (base.translated_locales - [@locale]).empty? + flash[:error] = "Can't remove the only remaining translation." + return redirect_to node_path(@node) + end + + draft = ensure_editable_draft + draft.translations.where(:locale => @locale).delete_all + draft.reload + + flash[:notice] = "#{@locale.upcase} translation removed from the draft. Publish to make this permanent." + redirect_to node_path(@node) + rescue LockedByAnotherUser => e + flash[:error] = e.message + redirect_to node_path(@node) + end + + private + + def find_node + @node = Node.find(params[:node_id]) + end + + # Every remaining action's :translation_locale is already + # router-constrained to non-default locales, so this should never + # actually fire in practice -- it's a deliberate second check, kept + # as a drift detector in case the route constraint and this list + # (both driven by Page.non_default_locales) ever disagree. + def find_locale + candidate = params[:translation_locale].to_s.to_sym + unless Page.non_default_locales.include?(candidate) + raise ActionController::RoutingError, "Unknown or default locale" + end + @locale = candidate + end + + # Never mutates head directly. Locks first (same guard as the + # primary editor), then reuses an existing draft or creates one -- + # deliberately not going through the autosave buffer: this is a + # plain, explicit Save, not a live-typing surface. + def ensure_editable_draft + @node.lock_for_editing!(current_user) + @node.draft || @node.create_new_draft(current_user) + end + + def translation_params + params.fetch(:page, {}).permit(:title, :abstract, :body) + end +end diff --git a/app/models/page.rb b/app/models/page.rb index db5b688..f796228 100644 --- a/app/models/page.rb +++ b/app/models/page.rb @@ -87,6 +87,27 @@ class Page < ApplicationRecord end end + def self.non_default_locales + I18n.available_locales - [:root, I18n.default_locale] + end + + # One row per non-default locale, read from the actual translation + # row -- never through the locale-dependent accessor, so a locale + # with no real translation yet reports as absent rather than quietly + # showing a fallback value borrowed from another locale. + def translation_summary + Page.non_default_locales.map do |locale| + translation = translations.find_by(:locale => locale) + { + :locale => locale, + :exists => translation.present?, + :title => translation&.title, + :updated_at => translation&.updated_at, + :outdated => translation.present? && outdated_translations?(:locale => locale) + } + end + end + def self.untranslated(options = {:locale => :de}) PageTranslation.all.group_by(&:page_id).select do |k,v| v.size == 1 && v.map{|x| x.locale}.include?(options[:locale]) diff --git a/app/views/nodes/show.html.erb b/app/views/nodes/show.html.erb index 54bb38f..e6a922b 100644 --- a/app/views/nodes/show.html.erb +++ b/app/views/nodes/show.html.erb @@ -1,6 +1,6 @@ <% locked_by_other = @node.locked? && @node.lock_owner != current_user %>
-

<%= title_for_node(@node) %>

+

<%= title_for_node(@node) %> (<%= I18n.default_locale.to_s.upcase %>)

Status
@@ -70,6 +70,46 @@ <% end %>
+
Translations
+
+
+
+ <%= I18n.default_locale.to_s.upcase %> (default) + <%= @page.title %> — updated <%= @default_translation&.updated_at || @page.updated_at %> +
+ <% @translations.each do |t| %> +
+ <%= t[:locale].to_s.upcase %> +
+ <% if t[:exists] %> + <%= link_to t[:title], node_translation_path(@node, t[:locale]) %> — updated <%= t[:updated_at] %> + <% if t[:outdated] %> + may be outdated + <% end %> + <% else %> + Not yet translated. + <% end %> + <% if locked_by_other %> + <%= t[:exists] ? "Lock + Edit" : "Create translation + Lock" %> + <% else %> + <%= link_to edit_node_translation_path(@node, t[:locale]), class: "action_button" do %> + <%= icon(t[:exists] ? "edit" : "language", library: "tabler", "aria-hidden": true) %> + <%= t[:exists] ? "Lock + Edit" : "Create translation + Lock" %> + <% end %> + <% if t[:exists] %> + <%= button_to node_translation_path(@node, t[:locale]), method: :delete, + form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } do %> + <%= icon("trash", library: "tabler", "aria-hidden": true) %> + Destroy Translation + <% end %> + <% end %> + <% end %> +
+
+ <% end %> +
+
+
People
@@ -122,7 +162,7 @@
Public Preview <% if @node.draft.preview_token.present? %> -
+
<%= link_to shared_preview_path(token: @node.draft.preview_token), shared_preview_url(token: @node.draft.preview_token), target: "_blank", rel: "noopener" %> <%= button_to 'Revoke', revoke_shared_preview_node_path(@node), method: :put, form: { data: { confirm: "Revoke this preview link? Anyone currently using it will immediately lose access." }, class: 'button_to state_changing' } %>
@@ -205,12 +245,11 @@
<% end %> -
Abstract
+
Abstract (<%= I18n.default_locale.to_s.upcase %>)
<%= sanitize(@page.abstract) %>
-
Body
+
Body (<%= I18n.default_locale.to_s.upcase %>)
<%= sanitize(@page.body) %>
- <%# Assets not yet addressed - no confirmed model/association seen for this page's attached assets %>
diff --git a/app/views/page_translations/edit.html.erb b/app/views/page_translations/edit.html.erb new file mode 100644 index 0000000..7371a42 --- /dev/null +++ b/app/views/page_translations/edit.html.erb @@ -0,0 +1,39 @@ +
+ Editing the <%= @locale.to_s.upcase %> translation of <%= title_for_node(@node) %>. +
+ +

+ Metadata such as images, tags, template, and author belong to the page + as a whole, not to a single translation — change those from the + <%= link_to 'default-locale editor', edit_node_path(@node) %>. +

+ +
+ <%= button_to 'Unlock + Back', unlock_node_path(@node), method: :put, + form: { class: 'button_to state_changing' }, + disabled: @node.autosave.present? %> + <%= submit_tag "Save #{@locale.to_s.upcase} translation", form: "translation_edit_form" %> + <%= submit_tag "Save + Unlock + Exit", form: "translation_edit_form" %> + <%= link_to "Preview ↗", preview_page_path(@page, :locale => @locale), target: "_blank", rel: "noopener", class: "preview_link" %> +
+ +
+ <%= form_with url: node_translation_path(@node, @locale), method: :patch, local: true, id: "translation_edit_form" do |f| %> +
+
Title
+
+ <%= text_field_tag "page[title]", @translation&.title %> +
+ +
Abstract
+
+ <%= text_area_tag "page[abstract]", @translation&.abstract %> +
+ +
Body
+
+ <%= text_area_tag "page[body]", @translation&.body, :class => 'with_editor' %> +
+
+ <% end %> +
diff --git a/app/views/page_translations/index.html.erb b/app/views/page_translations/index.html.erb new file mode 100644 index 0000000..a8c4f5d --- /dev/null +++ b/app/views/page_translations/index.html.erb @@ -0,0 +1,19 @@ +

Translations — <%= title_for_node(@node) %>

+ +
+
+ <% @translations.each do |t| %> +
+ <%= t[:locale].to_s.upcase %> + <% if t[:exists] %> + <%= t[:title] %> — updated <%= t[:updated_at] %> + <% if t[:outdated] %>may be outdated<% end %> + <%= link_to 'Edit', edit_node_translation_path(@node, t[:locale]) %> + <% else %> + Not yet translated. + <%= link_to 'Add', edit_node_translation_path(@node, t[:locale]) %> + <% end %> +
+ <% end %> +
+
diff --git a/app/views/page_translations/show.html.erb b/app/views/page_translations/show.html.erb new file mode 100644 index 0000000..c5bd151 --- /dev/null +++ b/app/views/page_translations/show.html.erb @@ -0,0 +1,27 @@ +

Compare — <%= title_for_node(@node) %>

+ +
+
+

<%= @locale.to_s.upcase %>

+
Title
+
<%= @translation&.title %>
+ +
Abstract
+
<%= sanitize(@translation&.abstract.to_s) %>
+ +
Body
+
<%= sanitize(@translation&.body.to_s) %>
+
+ +
+

<%= I18n.default_locale.to_s.upcase %> (default)

+
Title
+
<%= @default_translation&.title %>
+ +
Abstract
+
<%= sanitize(@default_translation&.abstract.to_s) %>
+ +
Body
+
<%= sanitize(@default_translation&.body.to_s) %>
+
+
-- cgit v1.3