Make Milestones general, and not specific to Budget Investments

Generalize the Budget::Investment::Milestone model to a
polymorphic Milestone model so it can be used for entities
other than Budget::Investment.

app/controllers/admin/budget_investment_milestones_controller.rb

@@ -2,19 +2,19 @@ class Admin::BudgetInvestmentMilestonesController < Admin::BaseController
   include Translatable
 
   before_action :load_budget_investment, only: [:index, :new, :create, :edit, :update, :destroy]
-  before_action :load_budget_investment_milestone, only: [:edit, :update, :destroy]
+  before_action :load_milestone, only: [:edit, :update, :destroy]
   before_action :load_statuses, only: [:index, :new, :create, :edit, :update]
 
   def index
   end
 
   def new
-    @milestone = Budget::Investment::Milestone.new
+    @milestone = Milestone.new
   end
 
   def create
-    @milestone = Budget::Investment::Milestone.new(milestone_params)
+    @milestone = Milestone.new(milestone_params)
-    @milestone.investment = @investment
+    @milestone.milestoneable = @investment
     if @milestone.save
       redirect_to admin_budget_budget_investment_path(@investment.budget, @investment),
                   notice: t('admin.milestones.create.notice')
@@ -47,22 +47,22 @@ class Admin::BudgetInvestmentMilestonesController < Admin::BaseController
     image_attributes = [:id, :title, :attachment, :cached_attachment, :user_id, :_destroy]
     documents_attributes = [:id, :title, :attachment, :cached_attachment, :user_id, :_destroy]
     attributes = [:publication_date, :budget_investment_id, :status_id,
-                  translation_params(Budget::Investment::Milestone),
+                  translation_params(Milestone),
                   image_attributes: image_attributes, documents_attributes: documents_attributes]
 
-    params.require(:budget_investment_milestone).permit(*attributes)
+    params.require(:milestone).permit(*attributes)
   end
 
   def load_budget_investment
     @investment = Budget::Investment.find(params[:budget_investment_id])
   end
 
-  def load_budget_investment_milestone
+  def load_milestone
     @milestone = get_milestone
   end
 
   def get_milestone
-    Budget::Investment::Milestone.find(params[:id])
+    Milestone.find(params[:id])
   end
 
   def resource

app/models/budget/investment.rb

@@ -24,6 +24,7 @@ class Budget
     include Notifiable
     include Filterable
     include Flaggable
+    include Milestoneable
 
     belongs_to :author, -> { with_hidden }, class_name: 'User', foreign_key: 'author_id'
     belongs_to :heading
@@ -40,8 +41,6 @@ class Budget
     has_many :comments, -> {where(valuation: false)}, as: :commentable, class_name: 'Comment'
     has_many :valuations, -> {where(valuation: true)}, as: :commentable, class_name: 'Comment'
 
-    has_many :milestones
-
     validates :title, presence: true
     validates :author, presence: true
     validates :description, presence: true

app/models/budget/investment/milestone.rb

@@ -1,35 +0,0 @@
-class Budget
-  class Investment
-    class Milestone < ActiveRecord::Base
-      include Imageable
-      include Documentable
-      documentable max_documents_allowed: 3,
-                   max_file_size: 3.megabytes,
-                   accepted_content_types: [ "application/pdf" ]
-
-      translates :title, :description, touch: true
-      include Globalizable
-
-      belongs_to :investment
-      belongs_to :status, class_name: 'Milestone::Status'
-
-      validates :investment, presence: true
-      validates :publication_date, presence: true
-      validate :description_or_status_present?
-
-      scope :order_by_publication_date, -> { order(publication_date: :asc, created_at: :asc) }
-      scope :published,                 -> { where("publication_date <= ?", Date.current) }
-      scope :with_status,               -> { where("status_id IS NOT NULL") }
-
-      def self.title_max_length
-        80
-      end
-
-      def description_or_status_present?
-        unless description.present? || status_id.present?
-          errors.add(:description)
-        end
-      end
-    end
-  end
-end

app/models/concerns/milestoneable.rb

@@ -0,0 +1,7 @@
+module Milestoneable
+  extend ActiveSupport::Concern
+
+  included do
+    has_many :milestones, as: :milestoneable, dependent: :destroy
+  end
+end

app/models/milestone.rb

@@ -0,0 +1,31 @@
+class Milestone < ActiveRecord::Base
+  include Imageable
+  include Documentable
+  documentable max_documents_allowed: 3,
+               max_file_size: 3.megabytes,
+               accepted_content_types: [ "application/pdf" ]
+
+  translates :title, :description, touch: true
+  include Globalizable
+
+  belongs_to :milestoneable, polymorphic: true
+  belongs_to :status
+
+  validates :milestoneable, presence: true
+  validates :publication_date, presence: true
+  validate :description_or_status_present?
+
+  scope :order_by_publication_date, -> { order(publication_date: :asc, created_at: :asc) }
+  scope :published,                 -> { where("publication_date <= ?", Date.current) }
+  scope :with_status,               -> { where("status_id IS NOT NULL") }
+
+  def self.title_max_length
+    80
+  end
+
+  def description_or_status_present?
+    unless description.present? || status_id.present?
+      errors.add(:description)
+    end
+  end
+end

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

@@ -13,7 +13,7 @@
 
   <%= f.translatable_fields do |translations_form| %>
     <%= translations_form.hidden_field :title, value: l(Time.current, format: :datetime),
-                               maxlength: Budget::Investment::Milestone.title_max_length %>
+                               maxlength: Milestone.title_max_length %>
 
     <%= translations_form.text_area :description,
                                     rows: 5,

app/views/admin/budget_investments/_milestones.html.erb

@@ -18,7 +18,7 @@
           <td class="text-center"><%= milestone.id %></td>
           <td>
             <%= link_to milestone.title,
-                        edit_admin_budget_budget_investment_budget_investment_milestone_path(@investment.budget,
+                        edit_admin_budget_budget_investment_milestone_path(@investment.budget,
                                                                                              @investment,
                                                                                              milestone) %>
           </td>
@@ -46,7 +46,7 @@
           </td>
           <td class="small-2">
             <%= link_to t("admin.milestones.index.delete"),
-                        admin_budget_budget_investment_budget_investment_milestone_path(@investment.budget,
+                        admin_budget_budget_investment_milestone_path(@investment.budget,
                                                                                         @investment,
                                                                                         milestone),
                         method: :delete,

app/views/admin/budget_investments/show.html.erb

@@ -63,6 +63,6 @@
 
 <p>
   <%= link_to t("admin.budget_investments.show.new_milestone"),
-              new_admin_budget_budget_investment_budget_investment_milestone_path(@budget, @investment),
+              new_admin_budget_budget_investment_milestone_path(@budget, @investment),
               class: "button hollow" %>
 </p>

config/initializers/routes_hierarchy.rb

@@ -7,8 +7,8 @@ module ActionDispatch::Routing::UrlFor
     case resource.class.name
     when "Budget::Investment"
       [resource.budget, resource]
-    when "Budget::Investment::Milestone"
+    when "Milestone"
-      [resource.investment.budget, resource.investment, resource]
+      [resource.milestoneable.budget, resource.milestoneable, resource]
     when "Legislation::Annotation"
       [resource.draft_version.process, resource.draft_version, resource]
     when "Legislation::Proposal", "Legislation::Question", "Legislation::DraftVersion"

config/locales/en/activerecord.yml

@@ -10,7 +10,7 @@ en:
       budget/investment:
         one: "Investment"
         other: "Investments"
-      budget/investment/milestone:
+      milestone:
         one: "milestone"
         other: "milestones"
       milestone/status:

config/locales/es/activerecord.yml

@@ -10,7 +10,7 @@ es:
       budget/investment:
         one: "Proyecto de gasto"
         other: "Proyectos de gasto"
-      budget/investment/milestone:
+      milestone:
         one: "hito"
         other: "hitos"
       milestone/status:

config/routes/admin.rb

@@ -62,7 +62,7 @@ namespace :admin do
     end
 
     resources :budget_investments, only: [:index, :show, :edit, :update] do
-      resources :budget_investment_milestones
+      resources :milestones, controller: 'budget_investment_milestones'
       member { patch :toggle_selection }
     end
 

db/dev_seeds/budgets.rb

@@ -148,7 +148,7 @@ end
 
 section "Creating investment milestones" do
   Budget::Investment.find_each do |investment|
-    milestone = Budget::Investment::Milestone.new(investment_id: investment.id, publication_date: Date.tomorrow, status_id: Milestone::Status.all.sample)
+    milestone = investment.milestones.build(publication_date: Date.tomorrow, status_id: Milestone::Status.all.sample)
     I18n.available_locales.map do |locale|
       Globalize.with_locale(locale) do
         milestone.description = "Description for locale #{locale}"

db/migrate/20180713124501_make_investment_milestones_polymorphic.rb

@@ -0,0 +1,27 @@
+class MakeInvestmentMilestonesPolymorphic < ActiveRecord::Migration
+  def change
+    create_table :milestones do |t|
+      t.references :milestoneable, polymorphic: true
+      t.string   "title", limit: 80
+      t.text     "description"
+      t.datetime "publication_date"
+
+      t.references :status, index: true
+
+      t.timestamps null: false
+    end
+
+    reversible do |change|
+      change.up do
+        Milestone.create_translation_table!({
+          title: :string,
+          description: :text
+        })
+      end
+
+      change.down do
+        Milestone.drop_translation_table!
+      end
+    end
+  end
+end

db/schema.rb

@@ -791,6 +791,31 @@ ActiveRecord::Schema.define(version: 20181016204729) do
 
   add_index "milestone_statuses", ["hidden_at"], name: "index_milestone_statuses_on_hidden_at", using: :btree
 
+  create_table "milestone_translations", force: :cascade do |t|
+    t.integer  "milestone_id", null: false
+    t.string   "locale",       null: false
+    t.datetime "created_at",   null: false
+    t.datetime "updated_at",   null: false
+    t.string   "title"
+    t.text     "description"
+  end
+
+  add_index "milestone_translations", ["locale"], name: "index_milestone_translations_on_locale", using: :btree
+  add_index "milestone_translations", ["milestone_id"], name: "index_milestone_translations_on_milestone_id", using: :btree
+
+  create_table "milestones", force: :cascade do |t|
+    t.integer  "milestoneable_id"
+    t.string   "milestoneable_type"
+    t.string   "title",              limit: 80
+    t.text     "description"
+    t.datetime "publication_date"
+    t.integer  "status_id"
+    t.datetime "created_at",                    null: false
+    t.datetime "updated_at",                    null: false
+  end
+
+  add_index "milestones", ["status_id"], name: "index_milestones_on_status_id", using: :btree
+
   create_table "moderators", force: :cascade do |t|
     t.integer "user_id"
   end

lib/tasks/globalize.rake

@@ -3,7 +3,7 @@ namespace :globalize do
     [
       AdminNotification,
       Banner,
-      Budget::Investment::Milestone,
+      Milestone,
       I18nContent,
       Legislation::DraftVersion,
       Legislation::Process,

spec/factories/budgets.rb

@@ -198,8 +198,8 @@ FactoryBot.define do
     sequence(:description) { |n| "Milestone status #{n} description" }
   end
 
-  factory :budget_investment_milestone, class: 'Budget::Investment::Milestone' do
+  factory :milestone, class: 'Milestone' do
-    association :investment, factory: :budget_investment
+    association :milestoneable, factory: :budget_investment
     association :status, factory: :milestone_status
     sequence(:title)     { |n| "Budget investment milestone #{n} title" }
     description          'Milestone description'

spec/features/admin/budget_investment_milestones_spec.rb

@@ -10,13 +10,13 @@ feature 'Admin budget investment milestones' do
   end
 
   it_behaves_like "translatable",
-                  "budget_investment_milestone",
+                  "milestone",
-                  "edit_admin_budget_budget_investment_budget_investment_milestone_path",
+                  "edit_admin_budget_budget_investment_milestone_path",
                   %w[description]
 
   context "Index" do
     scenario 'Displaying milestones' do
-      milestone = create(:budget_investment_milestone, investment: @investment)
+      milestone = create(:milestone, milestoneable: @investment)
       create(:image, imageable: milestone)
       document = create(:document, documentable: milestone)
 
@@ -46,9 +46,9 @@ feature 'Admin budget investment milestones' do
 
       click_link 'Create new milestone'
 
-      select status.name, from: 'budget_investment_milestone_status_id'
+      select status.name, from: 'milestone_status_id'
       fill_in 'Description', with: 'New description milestone'
-      fill_in 'budget_investment_milestone_publication_date', with: Date.current
+      fill_in 'milestone_publication_date', with: Date.current
 
       click_button 'Create milestone'
 
@@ -61,7 +61,7 @@ feature 'Admin budget investment milestones' do
       visit admin_budget_budget_investment_path(@investment.budget, @investment)
 
       click_link 'Create new milestone'
-      expect(find("#budget_investment_milestone_status_id").disabled?).to be true
+      expect(find("#milestone_status_id").disabled?).to be true
     end
 
     scenario "Show validation errors on milestone form" do
@@ -73,7 +73,7 @@ feature 'Admin budget investment milestones' do
 
       click_button 'Create milestone'
 
-      within "#new_budget_investment_milestone" do
+      within "#new_milestone" do
         expect(page).to have_content "can't be blank", count: 1
         expect(page).to have_content 'New description milestone'
       end
@@ -82,7 +82,7 @@ feature 'Admin budget investment milestones' do
 
   context "Edit" do
     scenario "Change title, description and document names" do
-      milestone = create(:budget_investment_milestone, investment: @investment)
+      milestone = create(:milestone, milestoneable: @investment)
       create(:image, imageable: milestone)
       document = create(:document, documentable: milestone)
 
@@ -94,8 +94,8 @@ feature 'Admin budget investment milestones' do
       expect(page).to have_css("img[alt='#{milestone.image.title}']")
 
       fill_in 'Description', with: 'Changed description'
-      fill_in 'budget_investment_milestone_publication_date', with: Date.current
+      fill_in 'milestone_publication_date', with: Date.current
-      fill_in 'budget_investment_milestone_documents_attributes_0_title', with: 'New document title'
+      fill_in 'milestone_documents_attributes_0_title', with: 'New document title'
 
       click_button 'Update milestone'
 
@@ -108,7 +108,7 @@ feature 'Admin budget investment milestones' do
 
   context "Delete" do
     scenario "Remove milestone" do
-      milestone = create(:budget_investment_milestone, investment: @investment, title: "Title will it remove")
+      milestone = create(:milestone, milestoneable: @investment, title: "Title will it remove")
 
       visit admin_budget_budget_investment_path(@investment.budget, @investment)
 

spec/features/budgets/executions_spec.rb

@@ -12,7 +12,7 @@ feature 'Executions' do
   let!(:investment3) { create(:budget_investment, :incompatible, heading: heading) }
 
   scenario 'only displays investments with milestones' do
-    create(:budget_investment_milestone, investment: investment1)
+    create(:milestone, milestoneable: investment1)
 
     visit budget_path(budget)
     click_link 'See results'
@@ -28,7 +28,7 @@ feature 'Executions' do
   end
 
   scenario "Do not display headings with no winning investments for selected status" do
-    create(:budget_investment_milestone, investment: investment1)
+    create(:milestone, milestoneable: investment1)
 
     empty_group   = create(:budget_group, budget: budget)
     empty_heading = create(:budget_heading, group: empty_group, price: 1000)
@@ -63,7 +63,7 @@ feature 'Executions' do
   context 'Images' do
 
     scenario 'renders milestone image if available' do
-      milestone1 = create(:budget_investment_milestone, investment: investment1)
+      milestone1 = create(:milestone, milestoneable: investment1)
       create(:image, imageable: milestone1)
 
       visit budget_path(budget)
@@ -76,7 +76,7 @@ feature 'Executions' do
     end
 
     scenario 'renders investment image if no milestone image is available' do
-      create(:budget_investment_milestone, investment: investment2)
+      create(:milestone, milestoneable: investment2)
       create(:image, imageable: investment2)
 
       visit budget_path(budget)
@@ -89,7 +89,7 @@ feature 'Executions' do
     end
 
     scenario 'renders default image if no milestone nor investment images are available' do
-      create(:budget_investment_milestone, investment: investment4)
+      create(:milestone, milestoneable: investment4)
 
       visit budget_path(budget)
 
@@ -101,16 +101,16 @@ feature 'Executions' do
     end
 
     scenario "renders last milestone's image if investment has multiple milestones with images associated" do
-      milestone1 = create(:budget_investment_milestone, investment: investment1,
+      milestone1 = create(:milestone, milestoneable: investment1,
                                       publication_date: Date.yesterday)
 
-      milestone2 = create(:budget_investment_milestone, investment: investment1,
+      milestone2 = create(:milestone, milestoneable: investment1,
                                       publication_date: Date.yesterday)
 
-      milestone3 = create(:budget_investment_milestone, investment: investment1,
+      milestone3 = create(:milestone, milestoneable: investment1,
                                       publication_date: Date.yesterday)
 
-      milestone4 = create(:budget_investment_milestone, investment: investment1,
+      milestone4 = create(:milestone, milestoneable: investment1,
                                       publication_date: Date.yesterday)
 
       create(:image, imageable: milestone2, title: 'Image for first milestone with image')
@@ -133,11 +133,11 @@ feature 'Executions' do
     let!(:status2) { create(:milestone_status, name: I18n.t('seeds.budgets.statuses.bidding')) }
 
     scenario 'Filters select with counter are shown' do
-      create(:budget_investment_milestone, investment: investment1,
+      create(:milestone, milestoneable: investment1,
                          publication_date: Date.yesterday,
                          status: status1)
 
-      create(:budget_investment_milestone, investment: investment2,
+      create(:milestone, milestoneable: investment2,
                          publication_date: Date.yesterday,
                          status: status2)
 
@@ -152,8 +152,8 @@ feature 'Executions' do
     end
 
     scenario 'by milestone status', :js do
-      create(:budget_investment_milestone, investment: investment1, status: status1)
+      create(:milestone, milestoneable: investment1, status: status1)
-      create(:budget_investment_milestone, investment: investment2, status: status2)
+      create(:milestone, milestoneable: investment2, status: status2)
       create(:milestone_status, name: I18n.t('seeds.budgets.statuses.executing_project'))
 
       visit budget_path(budget)
@@ -181,11 +181,11 @@ feature 'Executions' do
     end
 
     scenario 'are based on latest milestone status', :js do
-      create(:budget_investment_milestone, investment: investment1,
+      create(:milestone, milestoneable: investment1,
                          publication_date: 1.month.ago,
                          status: status1)
 
-      create(:budget_investment_milestone, investment: investment1,
+      create(:milestone, milestoneable: investment1,
                          publication_date: Date.yesterday,
                          status: status2)
 
@@ -201,11 +201,11 @@ feature 'Executions' do
     end
 
     scenario 'milestones with future dates are not shown', :js do
-      create(:budget_investment_milestone, investment: investment1,
+      create(:milestone, milestoneable: investment1,
                          publication_date: Date.yesterday,
                          status: status1)
 
-      create(:budget_investment_milestone, investment: investment1,
+      create(:milestone, milestoneable: investment1,
                          publication_date: Date.tomorrow,
                          status: status2)
 
@@ -226,7 +226,7 @@ feature 'Executions' do
     def create_heading_with_investment_with_milestone(group:, name:)
       heading    = create(:budget_heading, group: group, name: name)
       investment = create(:budget_investment, :winner, heading: heading)
-      milestone  = create(:budget_investment_milestone, investment: investment)
+      milestone  = create(:milestone, milestoneable: investment)
       heading
     end
 
@@ -261,7 +261,7 @@ feature 'Executions' do
 
     scenario 'Milestone not yet published' do
       status = create(:milestone_status)
-      unpublished_milestone = create(:budget_investment_milestone, investment: investment1,
+      unpublished_milestone = create(:milestone, milestoneable: investment1,
                                      status: status, publication_date: Date.tomorrow)
 
       visit budget_executions_path(budget, status: status.id)

spec/features/budgets/investments_spec.rb

@@ -1121,11 +1121,11 @@ feature 'Budget Investments' do
   scenario "Show milestones", :js do
     user = create(:user)
     investment = create(:budget_investment)
-    create(:budget_investment_milestone, investment: investment,
+    create(:milestone, milestoneable: investment,
                        description_en: "Last milestone with a link to https://consul.dev",
                        description_es: "Último hito con el link https://consul.dev",
                        publication_date: Date.tomorrow)
-    first_milestone = create(:budget_investment_milestone, investment: investment,
+    first_milestone = create(:milestone, milestoneable: investment,
                                          description: "First milestone",
                                          publication_date: Date.yesterday)
     image = create(:image, imageable: first_milestone)

spec/lib/tasks/globalize_spec.rb

@@ -151,7 +151,7 @@ describe "Globalize tasks" do
       before { I18n.locale = :"pt-BR" }
 
       let!(:milestone) do
-        create(:budget_investment_milestone).tap do |milestone|
+        create(:milestone).tap do |milestone|
           milestone.translations.delete_all
           milestone.update_column(:title, "Português")
           milestone.reload

spec/models/milestone_spec.rb

@@ -1,9 +1,9 @@
 require 'rails_helper'
 
-describe Budget::Investment::Milestone do
+describe Milestone do
 
   describe "Validations" do
-    let(:milestone) { build(:budget_investment_milestone) }
+    let(:milestone) { build(:milestone) }
 
     it "is valid" do
       expect(milestone).to be_valid
@@ -25,8 +25,8 @@ describe Budget::Investment::Milestone do
       expect(milestone).to be_valid
     end
 
-    it "is not valid without an investment" do
+    it "is not valid without a milestoneable" do
-      milestone.investment_id = nil
+      milestone.milestoneable_id = nil
       expect(milestone).not_to be_valid
     end
 
@@ -48,7 +48,7 @@ describe Budget::Investment::Milestone do
   end
 
   describe "#description_or_status_present?" do
-    let(:milestone) { build(:budget_investment_milestone) }
+    let(:milestone) { build(:milestone) }
 
     it "is not valid when status is removed and there's no description" do
       milestone.update(description: nil)
@@ -71,14 +71,14 @@ describe Budget::Investment::Milestone do
 
   describe ".published" do
     it "uses the application's time zone date", :with_different_time_zone do
-      published_in_local_time_zone = create(:budget_investment_milestone,
+      published_in_local_time_zone = create(:milestone,
                                             publication_date: Date.today)
 
-      published_in_application_time_zone = create(:budget_investment_milestone,
+      published_in_application_time_zone = create(:milestone,
                                                   publication_date: Date.current)
 
-      expect(Budget::Investment::Milestone.published).to include(published_in_application_time_zone)
+      expect(Milestone.published).to include(published_in_application_time_zone)
-      expect(Budget::Investment::Milestone.published).not_to include(published_in_local_time_zone)
+      expect(Milestone.published).not_to include(published_in_local_time_zone)
     end
   end
 end

spec/shared/features/translatable.rb

@@ -317,7 +317,7 @@ end
 # even share the same colour.
 def update_button_text
   case translatable_class.name
-  when "Budget::Investment::Milestone"
+  when "Milestone"
     "Update milestone"
   when "AdminNotification"
     "Update notification"