From 471c9730ccfb428739eeb661ad87b8a5c1852bfe Mon Sep 17 00:00:00 2001 From: iagirre Date: Mon, 16 Apr 2018 17:48:53 +0200 Subject: [PATCH] Refactorings - Cleanup Translatable module (`translation_params` method too large) - Move globalize_helpers partial to admin folder - Use any class for method translation_params - Helpers in `GlobalizeHelpers` make sure all are in use and see if they can be more legible - Review js name clases and methods see if they can be more legible - Refactor milestone views into partials with nice spacing between attributes --- app/assets/javascripts/globalize.js.coffee | 7 ++-- ...budget_investment_milestones_controller.rb | 16 +++++++-- app/controllers/concerns/translatable.rb | 10 +++--- app/helpers/globalize_helper.rb | 22 ++++-------- .../_form.html.erb | 2 +- .../_globalize_locales.html.erb | 4 +-- .../budgets/investments/_milestones.html.erb | 36 +------------------ .../milestones/_milestone.html.erb | 15 +++----- db/schema.rb | 4 +-- 9 files changed, 38 insertions(+), 78 deletions(-) rename app/views/{budgets/investments/milestones => admin/budget_investment_milestones}/_globalize_locales.html.erb (87%) diff --git a/app/assets/javascripts/globalize.js.coffee b/app/assets/javascripts/globalize.js.coffee index 10c50b1c8..e53383ceb 100644 --- a/app/assets/javascripts/globalize.js.coffee +++ b/app/assets/javascripts/globalize.js.coffee @@ -22,14 +22,11 @@ App.Globalize = element.addClass('highlight'); remove_language: (locale) -> - $(".js-globalize-attribute[data-locale=" + locale + "]").val('') - $(".js-globalize-attribute[data-locale=" + locale + "]").hide() + $(".js-globalize-attribute[data-locale=" + locale + "]").val('').hide() $(".js-globalize-locale-link[data-locale=" + locale + "]").hide() - $("#delete-" + locale).hide() next = $(".js-globalize-locale-link:visible").first() App.Globalize.highlight_locale(next) - $(".js-globalize-attribute[data-locale=" + next.data("locale") + "]").show() - $("#delete-" + next.data("locale")).show() + App.Globalize.display_translations(next.data("locale")) $("#delete_translations_" + locale).val(1) initialize: -> diff --git a/app/controllers/admin/budget_investment_milestones_controller.rb b/app/controllers/admin/budget_investment_milestones_controller.rb index 13bfb6460..d2fdf5700 100644 --- a/app/controllers/admin/budget_investment_milestones_controller.rb +++ b/app/controllers/admin/budget_investment_milestones_controller.rb @@ -48,7 +48,7 @@ class Admin::BudgetInvestmentMilestonesController < Admin::BaseController attributes = [:title, :description, :publication_date, :budget_investment_id, image_attributes: image_attributes, documents_attributes: documents_attributes] - params.require(:budget_investment_milestone).permit(*attributes, translation_params) + params.require(:budget_investment_milestone).permit(*attributes, translation_params(params[:budget_investment_milestone])) end def load_budget_investment @@ -56,7 +56,19 @@ class Admin::BudgetInvestmentMilestonesController < Admin::BaseController end def load_budget_investment_milestone - @milestone = Budget::Investment::Milestone.find(params[:id]) + @milestone = get_milestone + end + + def get_milestone + Budget::Investment::Milestone.find(params[:id]) + end + + def resource_model + Budget::Investment::Milestone + end + + def resource + get_milestone end end diff --git a/app/controllers/concerns/translatable.rb b/app/controllers/concerns/translatable.rb index 50cbf2cfe..4b4b14d87 100644 --- a/app/controllers/concerns/translatable.rb +++ b/app/controllers/concerns/translatable.rb @@ -8,9 +8,8 @@ module Translatable private - def translation_params - Budget::Investment::Milestone.globalize_attribute_names. - select { |k, v| params[:budget_investment_milestone].include?(k.to_sym) && params[:budget_investment_milestone][k].present? } + def translation_params(params) + resource_model.globalize_attribute_names.select { |k, v| params.include?(k.to_sym) && params[k].present? } end def set_translation_locale @@ -18,12 +17,11 @@ module Translatable end def delete_translations - locales = Budget::Investment::Milestone.globalize_locales. + locales = resource_model.globalize_locales. select { |k, v| params[:delete_translations].include?(k.to_sym) && params[:delete_translations][k] == "1" } - milestone = Budget::Investment::Milestone.find(params[:id]) locales.each do |l| Globalize.with_locale(l) do - milestone.translation.destroy + resource.translation.destroy end end end diff --git a/app/helpers/globalize_helper.rb b/app/helpers/globalize_helper.rb index 31b73e6ba..97dfeab66 100644 --- a/app/helpers/globalize_helper.rb +++ b/app/helpers/globalize_helper.rb @@ -1,9 +1,5 @@ module GlobalizeHelper - def globalize_locale - params[:globalize_locale] || I18n.locale - end - def options_for_locale_select options_for_select(locale_options, nil) end @@ -15,27 +11,19 @@ module GlobalizeHelper end def display_translation?(locale) - neutral_locale(I18n.locale) == neutral_locale(locale) ? "" : "display: none" + same_locale?(neutral_locale(I18n.locale), neutral_locale(locale)) ? "" : "display: none" end def css_to_display_translation?(resource, locale) resource.translated_locales.include?(neutral_locale(locale)) || locale == I18n.locale ? "" : "display: none" end - def disable_translation?(locale) - locale == "en" ? "" : "disabled" - end - - def css_for_globalize_locale(locale) - globalize_locale == locale ? "highlight" : "" - end - def highlight_current?(locale) - I18n.locale == locale ? 'highlight' : '' + same_locale?(I18n.locale, locale) ? 'highlight' : '' end def show_delete?(locale) - I18n.locale == locale ? '' : 'display: none' + display_translation?(locale) end def neutral_locale(locale) @@ -48,4 +36,8 @@ module GlobalizeHelper end end + def same_locale?(locale1, locale2) + locale1 == locale2 + end + end diff --git a/app/views/admin/budget_investment_milestones/_form.html.erb b/app/views/admin/budget_investment_milestones/_form.html.erb index 28bb80537..d63942cc9 100644 --- a/app/views/admin/budget_investment_milestones/_form.html.erb +++ b/app/views/admin/budget_investment_milestones/_form.html.erb @@ -1,5 +1,5 @@
- <%= render "budgets/investments/milestones/globalize_locales" %> + <%= render "globalize_locales" %>
<%= form_for [:admin, @investment.budget, @investment, @milestone] do |f| %> diff --git a/app/views/budgets/investments/milestones/_globalize_locales.html.erb b/app/views/admin/budget_investment_milestones/_globalize_locales.html.erb similarity index 87% rename from app/views/budgets/investments/milestones/_globalize_locales.html.erb rename to app/views/admin/budget_investment_milestones/_globalize_locales.html.erb index 17aca653d..11bf985dc 100644 --- a/app/views/budgets/investments/milestones/_globalize_locales.html.erb +++ b/app/views/admin/budget_investment_milestones/_globalize_locales.html.erb @@ -6,8 +6,8 @@ data: { locale: neutral_locale(locale) }, remote: true %> <%= link_to t("admin.milestones.form.remove_language"), "#", - id: "delete-#{locale}", - style: show_delete?(neutral_locale(locale)), + id: "delete-#{neutral_locale(locale)}", + style: show_delete?(locale), class: 'float-right delete-language', data: { locale: neutral_locale(locale) } %> diff --git a/app/views/budgets/investments/_milestones.html.erb b/app/views/budgets/investments/_milestones.html.erb index fa912393a..60b52f46e 100644 --- a/app/views/budgets/investments/_milestones.html.erb +++ b/app/views/budgets/investments/_milestones.html.erb @@ -9,41 +9,7 @@
diff --git a/app/views/budgets/investments/milestones/_milestone.html.erb b/app/views/budgets/investments/milestones/_milestone.html.erb index c7d00f415..774819d0c 100644 --- a/app/views/budgets/investments/milestones/_milestone.html.erb +++ b/app/views/budgets/investments/milestones/_milestone.html.erb @@ -10,18 +10,13 @@ <% end %> - <% if milestone.image.present? %> - <%= image_tag(milestone.image_url(:large), - { id: "image_#{milestone.id}", - alt: milestone.image.title, - class: "margin" }) %> - <% end %> + <%= image_tag(milestone.image_url(:large), { id: "image_#{milestone.id}", alt: milestone.image.title, class: "margin" }) if milestone.image.present? %> -

- <% globalize(neutral_locale(locale)) do %> + <% globalize(neutral_locale(locale)) do %> +

<%= text_with_links milestone.description %> - <% end %> -

+

+ <% end %> <% if milestone.documents.present? %>
diff --git a/db/schema.rb b/db/schema.rb index 03a56453a..860378811 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180320104823) do +ActiveRecord::Schema.define(version: 20180323190027) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -174,8 +174,8 @@ ActiveRecord::Schema.define(version: 20180320104823) do t.boolean "winner", default: false t.boolean "incompatible", default: false t.integer "community_id" + t.boolean "visible_to_valuators", default: false t.integer "valuator_group_assignments_count", default: 0 - t.boolean "visible_to_valuators", default: false end add_index "budget_investments", ["administrator_id"], name: "index_budget_investments_on_administrator_id", using: :btree