Merge pull request #2903 from consul/backport-refactor-translatable-specs

[Backport] Refactor translatable specs

app/controllers/notifications_controller.rb

@@ -36,19 +36,11 @@ class NotificationsController < ApplicationController
   end
 
   private
-
     def linkable_resource_path(notification)
-      case notification.linkable_resource.class.name
+      if notification.linkable_resource.is_a?(AdminNotification)
-      when "Budget::Investment"
+        notification.linkable_resource.link || notifications_path
-        budget_investment_path @notification.linkable_resource.budget, @notification.linkable_resource
-      when "Topic"
-        community_topic_path @notification.linkable_resource.community, @notification.linkable_resource
       else
-        if @notification.linkable_resource.is_a?(AdminNotification)
+        polymorphic_hierarchy_path(notification.linkable_resource)
-          @notification.linkable_resource.link || notifications_path
-        else
-          url_for @notification.linkable_resource
-        end
       end
     end
 

app/helpers/comments_helper.rb

@@ -41,22 +41,7 @@ module CommentsHelper
   end
 
   def commentable_path(comment)
-    commentable = comment.commentable
+    polymorphic_hierarchy_path(comment.commentable)
-
-    case comment.commentable_type
-    when "Budget::Investment"
-      budget_investment_path(commentable.budget_id, commentable)
-    when "Legislation::Question"
-      legislation_process_question_path(commentable.process, commentable)
-    when "Legislation::Annotation"
-      legislation_process_draft_version_annotation_path(commentable.draft_version.process, commentable.draft_version, commentable)
-    when "Topic"
-      community_topic_path(commentable.community, commentable)
-    when "Legislation::Proposal"
-      legislation_process_proposal_path(commentable.legislation_process_id, commentable)
-    else
-      commentable
-    end
   end
 
   def user_level_class(comment)

config/initializers/routes_hierarchy.rb

@@ -0,0 +1,60 @@
+# This module is expanded in order to make it easier to use polymorphic
+# routes with nested resources.
+# HACK: is there a way to avoid monkey-patching here? Using helpers is
+# a similar use of a global namespace too...
+module ActionDispatch::Routing::UrlFor
+  def resource_hierarchy_for(resource)
+    case resource.class.name
+    when "Budget::Investment"
+      [resource.budget, resource]
+    when "Budget::Investment::Milestone"
+      [resource.investment.budget, resource.investment, resource]
+    when "Legislation::Annotation"
+      [resource.draft_version.process, resource.draft_version, resource]
+    when "Legislation::Proposal", "Legislation::Question"
+      [resource.process, resource]
+    when "Topic"
+      [resource.community, resource]
+    else
+      resource
+    end
+  end
+
+  def polymorphic_hierarchy_path(resource)
+    # Unfortunately, we can't use polymorphic routes because there
+    # are cases where polymorphic_path doesn't get the named routes properly.
+    # Example:
+    #
+    # polymorphic_path([legislation_proposal.process, legislation_proposal])
+    #
+    # That line tries to find legislation_process_legislation_proposal_path
+    # while the correct route would be legislation_process_proposal_path
+    #
+    # We probably need to define routes differently in order to be able to use
+    # polymorphic_path which might be possible with Rails 5.1 `direct` and
+    # `resolve` methods.
+
+    resources = resource_hierarchy_for(resource)
+
+    case resource.class.name
+    when "Budget::Investment"
+      # polymorphic_path would return budget_budget_investment_path
+      budget_investment_path(*resources)
+    when "Legislation::Annotation"
+      # polymorphic_path would return:
+      # "legislation_process_legislation_draft_version_legislation_annotation_path"
+      legislation_process_draft_version_annotation_path(*resources)
+    when "Legislation::Proposal"
+      # polymorphic_path would return legislation_process_legislation_proposal_path
+      legislation_process_proposal_path(*resources)
+    when "Legislation::Question"
+      # polymorphic_path would return legislation_process_legislation_question_path
+      legislation_process_question_path(*resources)
+    when "Poll::Question"
+      # polymorphic_path would return poll_question_path
+      question_path(*resources)
+    else
+      polymorphic_path(resources)
+    end
+  end
+end

spec/features/admin/banners_spec.rb

@@ -6,6 +6,11 @@ feature 'Admin banners magement' do
     login_as(create(:administrator).user)
   end
 
+  it_behaves_like "translatable",
+                  "banner",
+                  "edit_admin_banner_path",
+                  %w[title description]
+
   context "Index" do
     background do
       @banner1 = create(:banner, title: "Banner number one",
@@ -183,123 +188,4 @@ feature 'Admin banners magement' do
     visit admin_root_path
     expect(page).not_to have_content 'Ugly banner'
   end
-
-  context "Translations" do
-
-    let(:banner) { create(:banner, title_en: "Title in English",
-                                   title_es: "Título en Español",
-                                   target_url: 'http://url.com',
-                                   description_en: "Description in English",
-                                   description_es: "Descripción en Español") }
-
-    before do
-      @edit_banner_url = edit_admin_banner_path(banner)
-    end
-
-    scenario "Add a translation", :js do
-      visit @edit_banner_url
-
-      select "Français", from: "translation_locale"
-      fill_in 'banner_title_fr', with: 'Titre en Français'
-      fill_in 'banner_description_fr', with: 'Description en Français'
-
-      click_button 'Save changes'
-
-      visit @edit_banner_url
-      expect(page).to have_field('banner_description_en', with: 'Description in English')
-
-      click_link "Español"
-      expect(page).to have_field('banner_description_es', with: 'Descripción en Español')
-
-      click_link "Français"
-      expect(page).to have_field('banner_description_fr', with: 'Description en Français')
-    end
-
-    scenario "Update a translation", :js do
-      banner.update_attributes(target_url:  'http://www.url.com',
-                               post_started_at: (Time.current - 4.days),
-                               post_ended_at:   (Time.current + 10.days))
-
-      section = create(:web_section, name: 'debates')
-      create(:banner_section, web_section: section, banner_id: banner.id)
-
-      visit @edit_banner_url
-
-      click_link "Español"
-      fill_in 'banner_title_es', with: 'Título correcto en Español'
-
-      click_button 'Save changes'
-
-      visit debates_path
-
-      within('.banner') do
-        expect(page).to have_content("Description in English")
-      end
-
-      select('Español', from: 'locale-switcher')
-
-      within('.banner') do
-        expect(page).to have_content('Título correcto en Español')
-      end
-    end
-
-    scenario "Remove a translation", :js do
-
-      visit @edit_banner_url
-
-      click_link "Español"
-      click_link "Remove language"
-
-      expect(page).not_to have_link "Español"
-
-      click_button "Save changes"
-      visit @edit_banner_url
-      expect(page).not_to have_link "Español"
-    end
-
-    context "Globalize javascript interface" do
-
-      scenario "Highlight current locale", :js do
-        visit @edit_banner_url
-
-        expect(find("a.js-globalize-locale-link.is-active")).to have_content "English"
-
-        select('Español', from: 'locale-switcher')
-
-        expect(find("a.js-globalize-locale-link.is-active")).to have_content "Español"
-      end
-
-      scenario "Highlight selected locale", :js do
-        visit @edit_banner_url
-
-        expect(find("a.js-globalize-locale-link.is-active")).to have_content "English"
-
-        click_link "Español"
-
-        expect(find("a.js-globalize-locale-link.is-active")).to have_content "Español"
-      end
-
-      scenario "Show selected locale form", :js do
-        visit @edit_banner_url
-
-        expect(page).to have_field('banner_description_en', with: 'Description in English')
-
-        click_link "Español"
-
-        expect(page).to have_field('banner_description_es', with: 'Descripción en Español')
-      end
-
-      scenario "Select a locale and add it to the banner form", :js do
-        visit @edit_banner_url
-
-        select "Français", from: "translation_locale"
-
-        expect(page).to have_link "Français"
-
-        click_link "Français"
-
-        expect(page).to have_field('banner_description_fr')
-      end
-    end
-  end
 end

spec/features/admin/budget_investment_milestones_spec.rb

@@ -9,6 +9,11 @@ feature 'Admin budget investment milestones' do
     @investment = create(:budget_investment)
   end
 
+  it_behaves_like "translatable",
+                  "budget_investment_milestone",
+                  "edit_admin_budget_budget_investment_budget_investment_milestone_path",
+                  %w[description]
+
   context "Index" do
     scenario 'Displaying milestones' do
       milestone = create(:budget_investment_milestone, investment: @investment)

spec/features/admin/site_customization/information_texts_spec.rb

@@ -7,6 +7,11 @@ feature "Admin custom information texts" do
     login_as(admin.user)
   end
 
+  it_behaves_like "translatable",
+                  "i18n_content",
+                  "admin_site_customization_information_texts_path",
+                  %w[value]
+
   scenario 'page is correctly loaded' do
     visit admin_site_customization_information_texts_path
 
@@ -113,60 +118,6 @@ feature "Admin custom information texts" do
       expect(debate_text.value_en).to eq('Custom debate text')
       expect(debate_title.value_en).to eq('Custom debate title')
     end
-
-    context "Javascript interface" do
-
-      scenario "Highlight current locale", :js do
-        visit admin_site_customization_information_texts_path
-
-        expect(find("a.js-globalize-locale-link.is-active")).to have_content "English"
-
-        select('Español', from: 'locale-switcher')
-
-        expect(find("a.js-globalize-locale-link.is-active")).to have_content "Español"
-      end
-
-      scenario "Highlight selected locale", :js do
-        key = "debates.form.debate_title"
-        content = create(:i18n_content, key: key, value_es: 'Título')
-
-        visit admin_site_customization_information_texts_path
-
-        expect(find("a.js-globalize-locale-link.is-active")).to have_content "English"
-
-        click_link "Español"
-
-        expect(find("a.js-globalize-locale-link.is-active")).to have_content "Español"
-      end
-
-      scenario "Show selected locale form", :js do
-        key = "debates.form.debate_title"
-        content = create(:i18n_content, key: key,
-                                        value_en: 'Title',
-                                        value_es: 'Título')
-
-        visit admin_site_customization_information_texts_path
-
-        expect(page).to have_field("contents_content_#{key}values_value_en", with: 'Title')
-
-        click_link "Español"
-
-        expect(page).to have_field("contents_content_#{key}values_value_es", with: 'Título')
-      end
-
-      scenario "Select a locale and add it to the form", :js do
-        key = "debates.form.debate_title"
-
-        visit admin_site_customization_information_texts_path
-        select "Français", from: "translation_locale"
-
-        expect(page).to have_link "Français"
-
-        click_link "Français"
-        expect(page).to have_field("contents_content_#{key}values_value_fr")
-      end
-    end
-
   end
 
 end

spec/features/translations_spec.rb

@@ -1,153 +0,0 @@
-require 'rails_helper'
-
-feature "Translations" do
-
-  context "Milestones" do
-
-    let(:investment) { create(:budget_investment) }
-    let(:milestone) { create(:budget_investment_milestone,
-                              investment: investment,
-                              description_en: "Description in English",
-                              description_es: "Descripción en Español") }
-
-    background do
-      admin = create(:administrator)
-      login_as(admin.user)
-    end
-
-    before do
-      @edit_milestone_url = edit_admin_budget_budget_investment_budget_investment_milestone_path(investment.budget, investment, milestone)
-    end
-
-    scenario "Add a translation", :js do
-      visit @edit_milestone_url
-
-      select "Français", from: "translation_locale"
-      fill_in 'budget_investment_milestone_description_fr', with: 'Description en Français'
-
-      click_button 'Update milestone'
-      expect(page).to have_content "Milestone updated successfully"
-
-      visit @edit_milestone_url
-      expect(page).to have_field('budget_investment_milestone_description_en', with: 'Description in English')
-
-      click_link "Español"
-      expect(page).to have_field('budget_investment_milestone_description_es', with: 'Descripción en Español')
-
-      click_link "Français"
-      expect(page).to have_field('budget_investment_milestone_description_fr', with: 'Description en Français')
-    end
-
-    scenario "Update a translation", :js do
-      visit @edit_milestone_url
-
-      click_link "Español"
-      fill_in 'budget_investment_milestone_description_es', with: 'Descripción correcta en Español'
-
-      click_button 'Update milestone'
-      expect(page).to have_content "Milestone updated successfully"
-
-      visit budget_investment_path(investment.budget, investment)
-
-      click_link("Milestones (1)")
-      expect(page).to have_content("Description in English")
-
-      select('Español', from: 'locale-switcher')
-      click_link("Seguimiento (1)")
-
-      expect(page).to have_content("Descripción correcta en Español")
-    end
-
-    scenario "Remove a translation", :js do
-      visit @edit_milestone_url
-
-      click_link "Español"
-      click_link "Remove language"
-
-      expect(page).not_to have_link "Español"
-
-      click_button "Update milestone"
-      visit @edit_milestone_url
-      expect(page).not_to have_link "Español"
-    end
-
-    scenario 'Change value of a translated field to blank' do
-      visit @edit_milestone_url
-
-      fill_in 'budget_investment_milestone_description_en', with: ''
-
-      click_button "Update milestone"
-      expect(page).to have_content "Milestone updated successfully"
-
-      expect(page).to have_content "Milestone updated successfully"
-      expect(page).not_to have_content "Description in English"
-    end
-
-    scenario "Add a translation for a locale with non-underscored name", :js do
-      visit @edit_milestone_url
-
-      select "Português", from: "translation_locale"
-      fill_in 'budget_investment_milestone_description_pt_br', with: 'Description in pt-BR'
-
-      click_button 'Update milestone'
-      expect(page).to have_content "Milestone updated successfully"
-
-      visit budget_investment_path(investment.budget, investment)
-
-      click_link("Milestones (1)")
-      expect(page).to have_content("Description in English")
-
-      select('Português', from: 'locale-switcher')
-      click_link("Milestones (1)")
-
-      expect(page).to have_content('Description in pt-BR')
-    end
-
-    context "Globalize javascript interface" do
-
-      scenario "Highlight current locale", :js do
-        visit @edit_milestone_url
-
-        expect(find("a.js-globalize-locale-link.is-active")).to have_content "English"
-
-        select('Español', from: 'locale-switcher')
-
-        expect(find("a.js-globalize-locale-link.is-active")).to have_content "Español"
-      end
-
-      scenario "Highlight selected locale", :js do
-        visit @edit_milestone_url
-
-        expect(find("a.js-globalize-locale-link.is-active")).to have_content "English"
-
-        click_link "Español"
-
-        expect(find("a.js-globalize-locale-link.is-active")).to have_content "Español"
-      end
-
-      scenario "Show selected locale form", :js do
-        visit @edit_milestone_url
-
-        expect(page).to have_field('budget_investment_milestone_description_en', with: 'Description in English')
-
-        click_link "Español"
-
-        expect(page).to have_field('budget_investment_milestone_description_es', with: 'Descripción en Español')
-      end
-
-      scenario "Select a locale and add it to the milestone form", :js do
-        visit @edit_milestone_url
-
-        select "Français", from: "translation_locale"
-
-        expect(page).to have_link "Français"
-
-        click_link "Français"
-
-        expect(page).to have_field('budget_investment_milestone_description_fr')
-      end
-    end
-
-  end
-
-end

spec/shared/features/translatable.rb

@@ -0,0 +1,192 @@
+shared_examples "translatable" do |factory_name, path_name, fields|
+  let(:language_texts) do
+    {
+      es:      "en español",
+      en:      "in English",
+      fr:      "en Français",
+      "pt-BR": "Português"
+    }
+  end
+
+  let(:translatable_class) { build(factory_name).class }
+
+  let(:attributes) do
+    fields.product(%i[en es]).map do |field, locale|
+      [:"#{field}_#{locale}", text_for(field, locale)]
+    end.to_h
+  end
+
+  let(:translatable) { create(factory_name, attributes) }
+  let(:path) { send(path_name, *resource_hierarchy_for(translatable)) }
+  before { login_as(create(:administrator).user) }
+
+  context "Manage translations" do
+    before do
+      if translatable_class.name == "I18nContent"
+        skip "Translation handling is different for site customizations"
+      end
+    end
+
+    scenario "Add a translation", :js do
+      visit path
+
+      select "Français", from: "translation_locale"
+
+      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))
+
+      click_link "Español"
+      expect(page).to have_field(field_for(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))
+    end
+
+    scenario "Update a translation", :js do
+      visit path
+
+      click_link "Español"
+      field = fields.sample
+      updated_text = "Corrección de #{text_for(field, :es)}"
+
+      fill_in field_for(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))
+
+      select('Español', from: 'locale-switcher')
+
+      expect(page).to have_field(field_for(field, :es), with: updated_text)
+    end
+
+    scenario "Remove a translation", :js do
+      visit path
+
+      click_link "Español"
+      click_link "Remove language"
+
+      expect(page).not_to have_link "Español"
+
+      click_button update_button_text
+
+      visit path
+      expect(page).not_to have_link "Español"
+    end
+
+    scenario 'Change value of a translated field to blank' do
+      possible_blanks = fields.select do |field|
+        translatable.dup.tap { |duplicate| duplicate.send(:"#{field}=", '') }.valid?
+      end
+
+      skip("can't have translatable blank fields") if possible_blanks.empty?
+
+      field = possible_blanks.sample
+
+      visit path
+      expect(page).to have_field(field_for(field, :en), with: text_for(field, :en))
+
+      fill_in field_for(field, :en), with: ''
+      click_button update_button_text
+
+      visit path
+      expect(page).to have_field(field_for(field, :en), with: '')
+    end
+
+    scenario "Add a translation for a locale with non-underscored name", :js do
+      visit path
+      field = fields.sample
+
+      select "Português", from: "translation_locale"
+      fill_in field_for(field, :pt_br), with: text_for(field, :"pt-BR")
+
+      click_button update_button_text
+
+      visit path
+
+      select('Português', from: 'locale-switcher')
+
+      expect(page).to have_field(field_for(field, :pt_br), with: text_for(field, :"pt-BR"))
+    end
+  end
+
+  context "Globalize javascript interface" do
+    scenario "Highlight current locale", :js do
+      visit path
+
+      expect(find("a.js-globalize-locale-link.is-active")).to have_content "English"
+
+      select('Español', from: 'locale-switcher')
+
+      expect(find("a.js-globalize-locale-link.is-active")).to have_content "Español"
+    end
+
+    scenario "Highlight selected locale", :js do
+      visit path
+
+      expect(find("a.js-globalize-locale-link.is-active")).to have_content "English"
+
+      click_link "Español"
+
+      expect(find("a.js-globalize-locale-link.is-active")).to have_content "Español"
+    end
+
+    scenario "Show selected locale form", :js do
+      visit path
+      field = fields.sample
+
+      expect(page).to have_field(field_for(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))
+    end
+
+    scenario "Select a locale and add it to the form", :js do
+      visit path
+
+      select "Français", from: "translation_locale"
+
+      expect(page).to have_link "Français"
+
+      click_link "Français"
+
+      expect(page).to have_field(field_for(fields.sample, :fr))
+    end
+  end
+end
+
+def text_for(field, locale)
+  I18n.with_locale(locale) do
+    "#{translatable_class.human_attribute_name(field)} #{language_texts[locale]}"
+  end
+end
+
+def field_for(field, locale)
+  if translatable_class.name == "I18nContent"
+    "contents_content_#{translatable.key}values_#{field}_#{locale}"
+  else
+    "#{translatable_class.model_name.singular}_#{field}_#{locale}"
+  end
+end
+
+# FIXME: button texts should be consistent. Right now, buttons don't
+# even share the same colour.
+def update_button_text
+  case translatable_class.name
+  when "Budget::Investment::Milestone"
+    "Update milestone"
+  else
+    "Save changes"
+  end
+end

spec/support/common_actions/notifications.rb

@@ -32,20 +32,7 @@ module Notifications
   end
 
   def path_for(resource)
-    nested_path_for(resource) || url_for([resource, only_path: true])
+    polymorphic_hierarchy_path(resource)
-  end
-
-  def nested_path_for(resource)
-    case resource.class.name
-    when "Legislation::Question"
-      legislation_process_question_path(resource.process, resource)
-    when "Legislation::Proposal"
-      legislation_process_proposal_path(resource.process, resource)
-    when "Budget::Investment"
-      budget_investment_path(resource.budget, resource)
-    else
-      false
-    end
   end
 
   def error_message(resource_model = nil)