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

@@ -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: ->

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

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

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

app/views/admin/budget_investment_milestones/_form.html.erb

@@ -1,5 +1,5 @@
 <div>
-  <%= render "budgets/investments/milestones/globalize_locales" %>
+  <%= render "globalize_locales" %>
 </div>
 
 <%= form_for [:admin, @investment.budget, @investment, @milestone] do |f| %>

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) } %>
   </span>

app/views/budgets/investments/_milestones.html.erb

@@ -9,41 +9,7 @@
       <section class="timeline">
         <ul class="no-bullet">
           <% @investment.milestones.order_by_publication_date.each do |milestone| %>
-            <li>
-              <div class="milestone-content">
-                <% if milestone.publication_date.present? %>
-                  <span class="milestone-date">
-                    <strong>
-                      <%= t("budgets.investments.show.milestone_publication_date",
-                                publication_date: l(milestone.publication_date.to_date)) %>
-                    </strong>
-                  </span>
-                <% end %>
-                <%= image_tag(milestone.image_url(:large),
-                             { alt: milestone.image.title.unicode_normalize,
-                               class: "margin",
-                               id: "image_#{milestone.id}" }) if milestone.image.present? %>
-                <p>
-                  <% globalize(neutral_locale(locale)) do %>
-                    <%= text_with_links milestone.description %>
-                  <% end %>
-                </p>
-                <% if milestone.documents.present? %>
-                  <div class="document-link text-center">
-                    <p>
-                      <span class="icon-document"></span>&nbsp;
-                      <strong><%= t("shared.documentation") %></strong>
-                    </p>
-                    <% milestone.documents.each do |document| %>
-                      <%= link_to document.title,
-                              document.attachment.url,
-                              target: "_blank",
-                              rel: "nofollow" %><br>
-                    <% end %>
-                  </div>
-                <% end %>
-              </div>
-            </li>
+            <%= render 'budgets/investments/milestones/milestone', milestone: milestone %>
           <% end %>
         </ul>
       </section>

app/views/budgets/investments/milestones/_milestone.html.erb

@@ -10,18 +10,13 @@
       </span>
     <% 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? %>
 
-    <p>
-      <% globalize(neutral_locale(locale)) do %>
+    <% globalize(neutral_locale(locale)) do %>
+      <p>
         <%= text_with_links milestone.description %>
-      <% end %>
-    </p>
+      </p>
+    <% end %>
 
     <% if milestone.documents.present? %>
       <div class="document-link text-center">

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