Update legislation drafts translatable fields

Updating it required reorganizing the form so translatable fields are
together.

We also needed to add a `hint` option to the form label and input
methods so the hint wouldn't show up for every language.

Finally, the markdown editor needed to use the same globalize attributes
as inputs, labels and hints, which adds a bit of duplication.

app/assets/javascripts/markdown_editor.js.coffee

@@ -3,12 +3,12 @@ App.MarkdownEditor =
   refresh_preview: (element, md) ->
     textarea_content = App.MarkdownEditor.find_textarea(element).val()
     result = md.render(textarea_content)
-    element.find('#markdown-preview').html(result)
+    element.find('.markdown-preview').html(result)
 
   # Multi-locale (translatable) form fields work by hiding inputs of locales
   # which are not "active".
   find_textarea: (editor) ->
-    editor.find('textarea:visible')
+    editor.find('textarea')
 
   initialize: ->
     $('.markdown-editor').each ->
@@ -26,7 +26,7 @@ App.MarkdownEditor =
         return
 
       editor.find('textarea').on 'scroll', ->
-        $('#markdown-preview').scrollTop($(this).scrollTop())
+        editor.find('.markdown-preview').scrollTop($(this).scrollTop())
 
       editor.find('.fullscreen-toggle').on 'click', ->
         editor.toggleClass('fullscreen')

app/controllers/admin/legislation/draft_versions_controller.rb

@@ -41,13 +41,9 @@ class Admin::Legislation::DraftVersionsController < Admin::Legislation::BaseCont
 
     def draft_version_params
       params.require(:legislation_draft_version).permit(
-        :title,
-        :changelog,
         :status,
         :final_version,
-        :body,
+        translation_params(Legislation::DraftVersion)
-        :body_html,
-        *translation_params(Legislation::DraftVersion)
       )
     end
 

app/helpers/translatable_form_helper.rb

@@ -10,11 +10,6 @@ module TranslatableFormHelper
       class: "#{options[:class]} js-globalize-attribute".strip,
       style: "#{options[:style]} #{display_translation?(locale)}".strip,
       data:  options.fetch(:data, {}).merge(locale: locale),
-      label_options: {
-        class: "#{options.dig(:label_options, :class)} js-globalize-attribute".strip,
-        style: "#{options.dig(:label_options, :style)} #{display_translation?(locale)}".strip,
-        data:  (options.dig(:label_options, :data) || {}) .merge(locale: locale)
-      }
     )
   end
 
@@ -56,14 +51,43 @@ module TranslatableFormHelper
   class TranslationsFieldsBuilder < FoundationRailsHelper::FormBuilder
     %i[text_field text_area cktext_area].each do |field|
       define_method field do |attribute, options = {}|
-        super attribute, translations_options(options)
+        final_options = translations_options(options)
+
+        custom_label(attribute, final_options[:label], final_options[:label_options]) +
+          help_text(final_options[:hint]) +
+          super(attribute, final_options.merge(label: false, hint: false))
       end
     end
 
+    def locale
+      @object.locale
+    end
+
+    def label(attribute, text = nil, options = {})
+      label_options = options.merge(
+        class: "#{options[:class]} js-globalize-attribute".strip,
+        style: "#{options[:style]} #{@template.display_translation?(locale)}".strip,
+        data:  (options[:data] || {}) .merge(locale: locale)
+      )
+
+      hint = label_options.delete(:hint)
+      super(attribute, text, label_options) + help_text(hint)
+    end
+
     private
+      def help_text(text)
+        if text
+          content_tag :span, text,
+                      class: "help-text js-globalize-attribute",
+                      data: { locale: locale },
+                      style: @template.display_translation?(locale)
+        else
+          ""
+        end
+      end
 
       def translations_options(options)
-        @template.merge_translatable_field_options(options, @object.locale)
+        @template.merge_translatable_field_options(options, locale)
       end
   end
 end

app/models/legislation/draft_version.rb

@@ -10,12 +10,16 @@ class Legislation::DraftVersion < ActiveRecord::Base
   translates :body_html, touch: true
   translates :toc_html,  touch: true
   globalize_accessors
+  accepts_nested_attributes_for :translations, allow_destroy: true
 
   belongs_to :process, class_name: 'Legislation::Process', foreign_key: 'legislation_process_id'
   has_many :annotations, class_name: 'Legislation::Annotation', foreign_key: 'legislation_draft_version_id', dependent: :destroy
 
-  validates :title, presence: true
+  translation_class.instance_eval do
-  validates :body, presence: true
+    validates :title, presence: true
+    validates :body, presence: true
+  end
+
   validates :status, presence: true, inclusion: { in: VALID_STATUSES }
 
   scope :published, -> { where(status: 'published').order('id DESC') }

app/views/admin/legislation/draft_versions/_form.html.erb

@@ -15,19 +15,59 @@
     </div>
   <% end %>
 
-  <div class="small-12 medium-9 column">
+  <%= f.translatable_fields do |translations_form| %>
-    <%= f.translatable_text_field :title,
+    <div class="small-12 medium-9 column">
-                                  placeholder: t("admin.legislation.draft_versions.form.title_placeholder") %>
+      <%= translations_form.text_field :title,
-  </div>
+                                       placeholder: t("admin.legislation.draft_versions.form.title_placeholder") %>
+    </div>
 
-  <div class="small-12 medium-9 column">
+    <div class="small-12 medium-9 column">
-    <%= f.label :changelog %>
+      <%= translations_form.text_area :changelog,
-    <span class="help-text"><%= t("admin.legislation.draft_versions.form.use_markdown") %></span>
+                                      hint: t("admin.legislation.draft_versions.form.use_markdown"),
-    <%= f.translatable_text_area :changelog,
+                                      rows: 5,
-                                 label: false,
+                                      placeholder: t("admin.legislation.draft_versions.form.changelog_placeholder") %>
-                                 rows: 5,
+    </div>
-                                 placeholder: t("admin.legislation.draft_versions.form.changelog_placeholder") %>
+
-  </div>
+    <div class="small-12 medium-4 column">
+      <%= translations_form.label :body, nil, hint: t("admin.legislation.draft_versions.form.use_markdown") %>
+    </div>
+
+    <%= content_tag :div,
+                    class: "markdown-editor clear js-globalize-attribute",
+                    data: { locale: translations_form.locale },
+                    style: display_translation?(translations_form.locale) do %>
+
+      <div class="small-12 medium-8 column fullscreen-container">
+        <div class="markdown-editor-header truncate">
+          <%= t("admin.legislation.draft_versions.form.title_html",
+                 draft_version_title: @draft_version.title,
+                 process_title: @process.title ) %>
+        </div>
+
+        <div class="markdown-editor-buttons">
+          <%= f.submit(class: "button", value: t("admin.legislation.draft_versions.#{admin_submit_action(@draft_version)}.submit_button")) %>
+        </div>
+
+        <%= link_to "#", class: 'fullscreen-toggle' do %>
+          <span data-closed-text="<%= t("admin.legislation.draft_versions.form.launch_text_editor")%>"
+                data-open-text="<%= t("admin.legislation.draft_versions.form.close_text_editor")%>">
+            <strong><%= t("admin.legislation.draft_versions.form.launch_text_editor")%></strong>
+          </span>
+
+        <% end %>
+      </div>
+
+      <div class="small-12 medium-6 column markdown-area">
+        <%= translations_form.text_area :body,
+                                        label: false,
+                                        rows: 10,
+                                        placeholder: t("admin.legislation.draft_versions.form.body_placeholder") %>
+      </div>
+
+      <div class="small-12 medium-6 column markdown-preview">
+      </div>
+    <% end %>
+  <% end %>
 
   <div class="small-12 medium-9 column">
     <%= f.label :status %>
@@ -45,40 +85,6 @@
     <span class="help-text"><%= t("admin.legislation.draft_versions.form.hints.final_version") %></span>
   </div>
 
-  <div class="small-12 medium-4 column">
-    <%= f.label :body %>
-    <span class="help-text"><%= t("admin.legislation.draft_versions.form.use_markdown") %></span>
-  </div>
-
-  <div class="markdown-editor clear">
-    <div class="small-12 medium-8 column fullscreen-container">
-      <div class="markdown-editor-header truncate">
-        <%= t("admin.legislation.draft_versions.form.title_html",
-               draft_version_title: @draft_version.title,
-               process_title: @process.title ) %>
-      </div>
-
-      <div class="markdown-editor-buttons">
-        <%= f.submit(class: "button", value: t("admin.legislation.draft_versions.#{admin_submit_action(@draft_version)}.submit_button")) %>
-      </div>
-
-      <%= link_to "#", class: 'fullscreen-toggle' do %>
-        <span data-closed-text="<%= t("admin.legislation.draft_versions.form.launch_text_editor")%>"
-              data-open-text="<%= t("admin.legislation.draft_versions.form.close_text_editor")%>">
-          <strong><%= t("admin.legislation.draft_versions.form.launch_text_editor")%></strong>
-        </span>
-      <% end %>
-    </div>
-    <div class="small-12 medium-6 column markdown-area">
-      <%= f.translatable_text_area :body,
-                                   label: false,
-                                   placeholder: t("admin.legislation.draft_versions.form.body_placeholder") %>
-    </div>
-
-    <div id="markdown-preview" class="small-12 medium-6 column markdown-preview">
-    </div>
-  </div>
-
   <div class="small-12 medium-3 column clear end margin-top">
     <%= f.submit(class: "button success expanded", value: t("admin.legislation.draft_versions.#{admin_submit_action(@draft_version)}.submit_button")) %>
   </div>

config/locales/en/activerecord.yml

@@ -231,6 +231,10 @@ en:
         changelog: Changes
         status: Status
         final_version: Final version
+      legislation/draft_version/translation:
+        title: Version title
+        body: Text
+        changelog: Changes
       legislation/question:
         title: Title
         question_options: Options

config/locales/es/activerecord.yml

@@ -231,6 +231,10 @@ es:
         changelog: Cambios
         status: Estado
         final_version: Versión final
+      legislation/draft_version/translation:
+        title: Título de la version
+        body: Texto
+        changelog: Cambios
       legislation/question:
         title: Título
         question_options: Respuestas

spec/features/admin/legislation/draft_versions_spec.rb

@@ -10,7 +10,8 @@ feature 'Admin legislation draft versions' do
   it_behaves_like "translatable",
                   "legislation_draft_version",
                   "edit_admin_legislation_process_draft_version_path",
-                  %w[title changelog]
+                  %w[title changelog],
+                  { "body" => :markdownit }
 
   context "Feature flag" do
 
@@ -58,9 +59,9 @@ feature 'Admin legislation draft versions' do
 
       click_link 'Create version'
 
-      fill_in 'legislation_draft_version_title_en', with: 'Version 3'
+      fill_in 'Version title', with: 'Version 3'
-      fill_in 'legislation_draft_version_changelog_en', with: 'Version 3 changes'
+      fill_in 'Changes', with: 'Version 3 changes'
-      fill_in 'legislation_draft_version_body_en', with: 'Version 3 body'
+      fill_in 'Text', with: 'Version 3 body'
 
       within('.end') do
         click_button 'Create version'
@@ -91,11 +92,11 @@ feature 'Admin legislation draft versions' do
 
       click_link 'Version 1'
 
-      fill_in 'legislation_draft_version_title_en', with: 'Version 1b'
+      fill_in 'Version title', with: 'Version 1b'
 
       click_link 'Launch text editor'
 
-      fill_in 'legislation_draft_version_body_en', with: '# Version 1 body\r\n\r\nParagraph\r\n\r\n>Quote'
+      fill_in 'Text', with: '# Version 1 body\r\n\r\nParagraph\r\n\r\n>Quote'
 
       within('.fullscreen') do
         click_link 'Close text editor'
@@ -106,31 +107,4 @@ feature 'Admin legislation draft versions' do
       expect(page).to have_content 'Version 1b'
     end
   end
-
-  context "Special translation behaviour" do
-
-    let!(:draft_version) { create(:legislation_draft_version) }
-
-    scenario 'Add body translation through markup editor', :js do
-      edit_path = edit_admin_legislation_process_draft_version_path(draft_version.process, draft_version)
-
-      visit edit_path
-
-      select "Français", from: "translation_locale"
-
-      click_link 'Launch text editor'
-
-      fill_in 'legislation_draft_version_body_fr', with: 'Texte en Français'
-
-      click_link 'Close text editor'
-      click_button "Save changes"
-
-      visit edit_path
-
-      click_link "Français"
-      click_link 'Launch text editor'
-
-      expect(page).to have_field('legislation_draft_version_body_fr', with: 'Texte en Français')
-    end
-  end
 end

spec/models/legislation/draft_version_spec.rb

@@ -17,6 +17,7 @@ RSpec.describe Legislation::DraftVersion, type: :model do
   end
 
   it "renders and saves the html from the markdown body field with alternative translation" do
+    legislation_draft_version.title_fr = "Français"
     legislation_draft_version.body_fr = body_markdown
 
     legislation_draft_version.save!

spec/shared/features/translatable.rb

@@ -1,4 +1,4 @@
-shared_examples "translatable" do |factory_name, path_name, fields|
+shared_examples "translatable" do |factory_name, path_name, input_fields, textarea_fields = {}|
   let(:language_texts) do
     {
       es:      "en español",
@@ -10,6 +10,11 @@ shared_examples "translatable" do |factory_name, path_name, fields|
 
   let(:translatable_class) { build(factory_name).class }
 
+  let(:input_fields) { input_fields } # So it's accessible by methods
+  let(:textarea_fields) { textarea_fields } # So it's accessible by methods
+
+  let(:fields) { input_fields + textarea_fields.keys }
+
   let(:attributes) do
     fields.product(%i[en es]).map do |field, locale|
       [:"#{field}_#{locale}", text_for(field, locale)]
@@ -41,23 +46,19 @@ shared_examples "translatable" do |factory_name, path_name, fields|
       visit path
 
       select "Français", from: "translation_locale"
-
+      fields.each { |field| fill_in_field field, :fr, with: text_for(field, :fr) }
-      fields.each do |field|
-        fill_in field_for(field, :fr), with: text_for(field, :fr)
-      end
-
       click_button update_button_text
 
       visit path
       field = fields.sample
 
-      expect(page).to have_field(field_for(field, :en), with: text_for(field, :en))
+      expect_page_to_have_translatable_field field, :en, with: text_for(field, :en)
 
       click_link "Español"
-      expect(page).to have_field(field_for(field, :es), with: text_for(field, :es))
+      expect_page_to_have_translatable_field field, :es, with: text_for(field, :es)
 
       click_link "Français"
-      expect(page).to have_field(field_for(field, :fr), with: text_for(field, :fr))
+      expect_page_to_have_translatable_field field, :fr, with: text_for(field, :fr)
     end
 
     scenario "Add an invalid translation", :js do
@@ -66,15 +67,16 @@ shared_examples "translatable" do |factory_name, path_name, fields|
       field = required_fields.sample
 
       visit path
+
       select "Français", from: "translation_locale"
-      fill_in field_for(field, :fr), with: ""
+      fill_in_field field, :fr, with: ""
       click_button update_button_text
 
       expect(page).to have_css "#error_explanation"
 
       click_link "Français"
 
-      expect(page).to have_field(field_for(field, :fr), with: "")
+      expect_page_to_have_translatable_field field, :fr, with: ""
     end
 
     scenario "Update a translation", :js do
@@ -84,17 +86,17 @@ shared_examples "translatable" do |factory_name, path_name, fields|
       field = fields.sample
       updated_text = "Corrección de #{text_for(field, :es)}"
 
-      fill_in field_for(field, :es), with: updated_text
+      fill_in_field field, :es, with: updated_text
 
       click_button update_button_text
 
       visit path
 
-      expect(page).to have_field(field_for(field, :en), with: text_for(field, :en))
+      expect_page_to_have_translatable_field field, :en, with: text_for(field, :en)
 
       select('Español', from: 'locale-switcher')
 
-      expect(page).to have_field(field_for(field, :es), with: updated_text)
+      expect_page_to_have_translatable_field field, :es, with: updated_text
     end
 
     scenario "Update a translation with invalid data", :js do
@@ -105,16 +107,16 @@ shared_examples "translatable" do |factory_name, path_name, fields|
       visit path
       click_link "Español"
 
-      expect(page).to have_field(field_for(field, :es), with: text_for(field, :es))
+      expect_page_to_have_translatable_field field, :es, with: text_for(field, :es)
 
-      fill_in field_for(field, :es), with: ""
+      fill_in_field field, :es, with: ""
       click_button update_button_text
 
       expect(page).to have_css "#error_explanation"
 
       click_link "Español"
 
-      expect(page).to have_field(field_for(field, :es), with: "")
+      expect_page_to_have_translatable_field field, :es, with: ""
     end
 
     scenario "Remove a translation", :js do
@@ -142,17 +144,17 @@ shared_examples "translatable" do |factory_name, path_name, fields|
       click_link "Remove language"
 
       click_link "English"
-      fill_in field_for(field, :en), with: ""
+      fill_in_field field, :en, with: ""
       click_button update_button_text
 
       expect(page).to have_css "#error_explanation"
-      expect(page).to have_field(field_for(field, :en), with: "")
+      expect_page_to_have_translatable_field field, :en, with: ""
       expect(page).not_to have_link "Español"
 
       visit path
       click_link "Español"
 
-      expect(page).to have_field(field_for(field, :es), with: text_for(field, :es))
+      expect_page_to_have_translatable_field field, :es, with: text_for(field, :es)
     end
 
     scenario 'Change value of a translated field to blank', :js do
@@ -161,24 +163,20 @@ shared_examples "translatable" do |factory_name, path_name, fields|
       field = optional_fields.sample
 
       visit path
-      expect(page).to have_field(field_for(field, :en), with: text_for(field, :en))
+      expect_page_to_have_translatable_field field, :en, with: text_for(field, :en)
 
-      fill_in field_for(field, :en), with: ''
+      fill_in_field field, :en, with: ''
       click_button update_button_text
 
       visit path
-      expect(page).to have_field(field_for(field, :en), with: '')
+      expect_page_to_have_translatable_field field, :en, with: ''
     end
 
     scenario "Add a translation for a locale with non-underscored name", :js do
       visit path
 
       select "Português brasileiro", from: "translation_locale"
-
+      fields.each { |field| fill_in_field field, :"pt-BR", with: text_for(field, :"pt-BR") }
-      fields.each do |field|
-        fill_in field_for(field, :"pt-BR"), with: text_for(field, :"pt-BR")
-      end
-
       click_button update_button_text
 
       visit path
@@ -186,7 +184,7 @@ shared_examples "translatable" do |factory_name, path_name, fields|
       select('Português brasileiro', from: 'locale-switcher')
 
       field = fields.sample
-      expect(page).to have_field(field_for(field, :"pt-BR"), with: text_for(field, :"pt-BR"))
+      expect_page_to_have_translatable_field field, :"pt-BR", with: text_for(field, :"pt-BR")
     end
   end
 
@@ -215,11 +213,11 @@ shared_examples "translatable" do |factory_name, path_name, fields|
       visit path
       field = fields.sample
 
-      expect(page).to have_field(field_for(field, :en), with: text_for(field, :en))
+      expect_page_to_have_translatable_field field, :en, with: text_for(field, :en)
 
       click_link "Español"
 
-      expect(page).to have_field(field_for(field, :es), with: text_for(field, :es))
+      expect_page_to_have_translatable_field field, :es, with: text_for(field, :es)
     end
 
     scenario "Select a locale and add it to the form", :js do
@@ -231,7 +229,7 @@ shared_examples "translatable" do |factory_name, path_name, fields|
 
       click_link "Français"
 
-      expect(page).to have_field(field_for(fields.sample, :fr))
+      expect_page_to_have_translatable_field fields.sample, :fr, with: ""
     end
   end
 end
@@ -250,6 +248,36 @@ def field_for(field, locale)
   end
 end
 
+def fill_in_field(field, locale, with:)
+  if input_fields.include?(field)
+    fill_in field_for(field, locale), with: with
+  else
+    fill_in_textarea(field, textarea_fields[field], locale, with: with)
+  end
+end
+
+def fill_in_textarea(field, textarea_type, locale, with:)
+  if textarea_type == :markdownit
+    click_link class: "fullscreen-toggle"
+    fill_in field_for(field, locale), with: with
+    click_link class: "fullscreen-toggle"
+  end
+end
+
+def expect_page_to_have_translatable_field(field, locale, with:)
+  if input_fields.include?(field)
+    expect(page).to have_field field_for(field, locale), with: with
+  else
+    textarea_type = textarea_fields[field]
+
+    if textarea_type == :markdownit
+      click_link class: "fullscreen-toggle"
+      expect(page).to have_field field_for(field, locale), with: with
+      click_link class: "fullscreen-toggle"
+    end
+  end
+end
+
 # FIXME: button texts should be consistent. Right now, buttons don't
 # even share the same colour.
 def update_button_text