Merge pull request consuldemocracy#2323 from consul/feature/budget_ph…

…ases

Create Budget::Phases backend

CHANGELOG.md

@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Added Capistrano task to automate maintenance mode https://github.com/consul/consul/pull/1932
 - Added actions to edit and delete a budget's headings https://github.com/consul/consul/pull/1917
 - Allow Budget Investments to be Related to other content https://github.com/consul/consul/pull/2311
+- New Budget::Phase model to add dates, enabling and more https://github.com/consul/consul/pull/2323
 
 ### Changed
 - Updated multiple minor & patch gem versions thanks to [Depfu](https://depfu.com)
@@ -28,6 +29,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Design Improvements https://github.com/consul/consul/pull/2327
 
 ### Deprecated
+- Budget's `description_*` columns will be erased from database in next release. Please run rake task `budgets:phases:generate_missing` to migrate them. Details at Warning section of https://github.com/consul/consul/pull/2323
 
 ### Removed
 - Spending Proposals urls from sitemap, that model is getting entirely deprecated soon.

app/controllers/admin/budgets_controller.rb

@@ -54,7 +54,7 @@ def destroy
   private
 
     def budget_params
-      descriptions = Budget::PHASES.map{|p| "description_#{p}"}.map(&:to_sym)
+      descriptions = Budget::Phase::PHASE_KINDS.map{|p| "description_#{p}"}.map(&:to_sym)
       valid_attributes = [:name, :phase, :currency_symbol] + descriptions
       params.require(:budget).permit(*valid_attributes)
     end

app/helpers/budgets_helper.rb

@@ -7,7 +7,7 @@ def csv_params
   end
 
   def budget_phases_select_options
-    Budget::PHASES.map { |ph| [ t("budgets.phase.#{ph}"), ph ] }
+    Budget::Phase::PHASE_KINDS.map { |ph| [ t("budgets.phase.#{ph}"), ph ] }
   end
 
   def budget_currency_symbol_select_options

app/models/budget.rb

@@ -3,24 +3,23 @@ class Budget < ActiveRecord::Base
   include Measurable
   include Sluggable
 
-  PHASES = %w(drafting accepting reviewing selecting valuating publishing_prices
-              balloting reviewing_ballots finished).freeze
-  PUBLISHED_PRICES_PHASES = %w(publishing_prices balloting reviewing_ballots finished).freeze
-
   CURRENCY_SYMBOLS = %w(€ $ £ ¥).freeze
 
   validates :name, presence: true, uniqueness: true
-  validates :phase, inclusion: { in: PHASES }
+  validates :phase, inclusion: { in: Budget::Phase::PHASE_KINDS }
   validates :currency_symbol, presence: true
   validates :slug, presence: true, format: /\A[a-z0-9\-_]+\z/
 
   has_many :investments, dependent: :destroy
   has_many :ballots, dependent: :destroy
   has_many :groups, dependent: :destroy
   has_many :headings, through: :groups
+  has_many :phases, class_name: Budget::Phase
 
   before_validation :sanitize_descriptions
 
+  after_create :generate_phases
+
   scope :drafting, -> { where(phase: "drafting") }
   scope :accepting, -> { where(phase: "accepting") }
   scope :reviewing, -> { where(phase: "reviewing") }
@@ -30,18 +29,27 @@ class Budget < ActiveRecord::Base
   scope :balloting, -> { where(phase: "balloting") }
   scope :reviewing_ballots, -> { where(phase: "reviewing_ballots") }
   scope :finished, -> { where(phase: "finished") }
+
   scope :open, -> { where.not(phase: "finished") }
 
   def self.current
     where.not(phase: "drafting").last
   end
 
+  def current_phase
+    phases.send(phase)
+  end
+
   def description
-    send("description_#{phase}").try(:html_safe)
+    description_for_phase(phase)
   end
 
-  def self.description_max_length
-    2000
+  def description_for_phase(phase)
+    if phases.exists? && phases.send(phase).description.present?
+      phases.send(phase).description
+    else
+      send("description_#{phase}").try(:html_safe)
+    end
   end
 
   def self.title_max_length
@@ -85,7 +93,7 @@ def finished?
   end
 
   def published_prices?
-    PUBLISHED_PRICES_PHASES.include?(phase)
+    Budget::Phase::PUBLISHED_PRICES_PHASES.include?(phase)
   end
 
   def balloting_process?
@@ -144,12 +152,25 @@ def email_unselected
 
   private
 
-    def sanitize_descriptions
-      s = WYSIWYGSanitizer.new
-      PHASES.each do |phase|
-        sanitized = s.sanitize(send("description_#{phase}"))
-        send("description_#{phase}=", sanitized)
-      end
+  def sanitize_descriptions
+    s = WYSIWYGSanitizer.new
+    Budget::Phase::PHASE_KINDS.each do |phase|
+      sanitized = s.sanitize(send("description_#{phase}"))
+      send("description_#{phase}=", sanitized)
+    end
+  end
+
+  def generate_phases
+    Budget::Phase::PHASE_KINDS.each do |phase|
+      Budget::Phase.create(
+        budget: self,
+        kind: phase,
+        prev_phase: phases&.last,
+        starts_at: phases&.last&.ends_at || Date.current,
+        ends_at: (phases&.last&.ends_at || Date.current) + 1.month
+      )
     end
+  end
 end
 
+

app/models/budget/phase.rb

@@ -0,0 +1,85 @@
+class Budget
+  class Phase < ActiveRecord::Base
+    PHASE_KINDS = %w(drafting accepting reviewing selecting valuating publishing_prices balloting
+                reviewing_ballots finished).freeze
+    PUBLISHED_PRICES_PHASES = %w(publishing_prices balloting reviewing_ballots finished).freeze
+    DESCRIPTION_MAX_LENGTH = 2000
+
+    belongs_to :budget
+    belongs_to :next_phase, class_name: 'Budget::Phase', foreign_key: :next_phase_id
+    has_one :prev_phase, class_name: 'Budget::Phase', foreign_key: :next_phase_id
+
+    validates :budget, presence: true
+    validates :kind, presence: true, uniqueness: { scope: :budget }, inclusion: { in: PHASE_KINDS }
+    validates :description, length: { maximum: DESCRIPTION_MAX_LENGTH }
+    validate :invalid_dates_range?
+    validate :prev_phase_dates_valid?
+    validate :next_phase_dates_valid?
+
+    before_validation :sanitize_description
+
+    after_save :adjust_date_ranges
+
+    scope :enabled,           -> { where(enabled: true) }
+    scope :drafting,          -> { find_by_kind('drafting') }
+    scope :accepting,         -> { find_by_kind('accepting')}
+    scope :reviewing,         -> { find_by_kind('reviewing')}
+    scope :selecting,         -> { find_by_kind('selecting')}
+    scope :valuating,         -> { find_by_kind('valuating')}
+    scope :publishing_prices, -> { find_by_kind('publishing_prices')}
+    scope :balloting,         -> { find_by_kind('balloting')}
+    scope :reviewing_ballots, -> { find_by_kind('reviewing_ballots')}
+    scope :finished,          -> { find_by_kind('finished')}
+
+    def next_enabled_phase
+      next_phase&.enabled? ? next_phase : next_phase&.next_enabled_phase
+    end
+
+    def prev_enabled_phase
+      prev_phase&.enabled? ? prev_phase : prev_phase&.prev_enabled_phase
+    end
+
+    def adjust_date_ranges
+      if enabled?
+        next_enabled_phase&.update_column(:starts_at, ends_at)
+        prev_enabled_phase&.update_column(:ends_at, starts_at)
+      elsif enabled_changed?
+        next_enabled_phase&.update_column(:starts_at, starts_at)
+      end
+    end
+
+    def invalid_dates_range?
+      if starts_at.present? && ends_at.present? && starts_at >= ends_at
+        errors.add(:starts_at, I18n.t('budgets.phases.errors.dates_range_invalid'))
+      end
+    end
+
+    private
+
+    def prev_phase_dates_valid?
+      if enabled? && starts_at.present? && prev_enabled_phase.present?
+        prev_enabled_phase.assign_attributes(ends_at: starts_at)
+        if prev_enabled_phase.invalid_dates_range?
+          phase_name = I18n.t("budgets.phase.#{prev_enabled_phase.kind}")
+          error = I18n.t('budgets.phases.errors.prev_phase_dates_invalid', phase_name: phase_name)
+          errors.add(:starts_at, error)
+        end
+      end
+    end
+
+    def next_phase_dates_valid?
+      if enabled? && ends_at.present? && next_enabled_phase.present?
+        next_enabled_phase.assign_attributes(starts_at: ends_at)
+        if next_enabled_phase.invalid_dates_range?
+          phase_name = I18n.t("budgets.phase.#{next_enabled_phase.kind}")
+          error = I18n.t('budgets.phases.errors.next_phase_dates_invalid', phase_name: phase_name)
+          errors.add(:ends_at, error)
+        end
+      end
+    end
+
+    def sanitize_description
+      self.description = WYSIWYGSanitizer.new.sanitize(description)
+    end
+  end
+end

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

@@ -2,9 +2,9 @@
 
   <%= f.text_field :name, maxlength: Budget.title_max_length %>
 
-  <% Budget::PHASES.each do |phase| %>
+  <% Budget::Phase::PHASE_KINDS.each do |phase| %>
     <div class="margin-top">
-      <%= f.cktext_area "description_#{phase}", maxlength: Budget.description_max_length, ckeditor: { language: I18n.locale } %>
+      <%= f.cktext_area "description_#{phase}", maxlength: Budget::Phase::DESCRIPTION_MAX_LENGTH, ckeditor: { language: I18n.locale } %>
     </div>
   <% end %>
 

app/views/budgets/results/show.html.erb

@@ -1,10 +1,10 @@
 <% provide :title, t("budgets.results.page_title", budget: @budget.name) %>
-<% content_for :meta_description do %><%= @budget.description_finished %><% end %>
+<% content_for :meta_description do %><%= @budget.description_for_phase('finished') %><% end %>
 <% provide :social_media_meta_tags do %>
 <%= render "shared/social_media_meta_tags",
             social_url: budget_results_url(@budget),
             social_title: @budget.name,
-            social_description: @budget.description_finished %>
+            social_description: @budget.description_for_phase('finished') %>
 <% end %>
 <% content_for :canonical do %>
   <%= render "shared/canonical", href: budget_results_url(@budget) %>

config/locales/en/budgets.yml

@@ -158,3 +158,8 @@ en:
       accepted: "Accepted spending proposal: "
       discarded: "Discarded spending proposal: "
       incompatibles: Incompatibles
+    phases:
+      errors:
+        dates_range_invalid: "Start date can't be equal or later than End date"
+        prev_phase_dates_invalid: "Start date must be later than the start date of the previous enabled phase (%{phase_name})"
+        next_phase_dates_invalid: "End date must be earlier than the end date of the next enabled phase (%{phase_name})"

config/locales/es/budgets.yml

@@ -158,3 +158,8 @@ es:
       accepted: 'Propuesta de inversión aceptada: '
       discarded: 'Propuesta de inversión descartada: '
       incompatibles: Incompatibles
+    phases:
+      errors:
+        dates_range_invalid: "La fecha de comienzo no puede ser igual o superior a la de finalización"
+        prev_phase_dates_invalid: "La fecha de inicio debe ser posterior a la fecha de inicio de la anterior fase habilitada (%{phase_name})"
+        next_phase_dates_invalid: "La fecha de fin debe ser anterior a la fecha de fin de la siguiente fase habilitada (%{phase_name}) "

db/dev_seeds.rb

@@ -401,8 +401,8 @@ def unique_document_number
 end
 
 section "Creating Budgets" do
-  Budget::PHASES.each_with_index do |phase, i|
-    descriptions = Hash[Budget::PHASES.map do |p|
+  Budget::Phase::PHASE_KINDS.each_with_index do |phase, i|
+    descriptions = Hash[Budget::Phase::PHASE_KINDS.map do |p|
       ["description_#{p}",
        "<p>#{Faker::Lorem.paragraphs(2).join('</p><p>')}</p>"]
     end]

db/migrate/20180112123641_create_budget_phases.rb

@@ -0,0 +1,14 @@
+class CreateBudgetPhases < ActiveRecord::Migration
+  def change
+    create_table :budget_phases do |t|
+      t.references :budget
+      t.references :next_phase, index: true
+      t.string :kind, null: false, index: true
+      t.text :summary
+      t.text :description
+      t.datetime :starts_at, index: true
+      t.datetime :ends_at, index: true
+      t.boolean :enabled, default: true
+    end
+  end
+end

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: 20180109175851) do
+ActiveRecord::Schema.define(version: 20180112123641) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -170,6 +170,22 @@
   add_index "budget_investments", ["heading_id"], name: "index_budget_investments_on_heading_id", using: :btree
   add_index "budget_investments", ["tsv"], name: "index_budget_investments_on_tsv", using: :gin
 
+  create_table "budget_phases", force: :cascade do |t|
+    t.integer  "budget_id"
+    t.integer  "next_phase_id"
+    t.string   "kind",                         null: false
+    t.text     "summary"
+    t.text     "description"
+    t.datetime "starts_at"
+    t.datetime "ends_at"
+    t.boolean  "enabled",       default: true
+  end
+
+  add_index "budget_phases", ["ends_at"], name: "index_budget_phases_on_ends_at", using: :btree
+  add_index "budget_phases", ["kind"], name: "index_budget_phases_on_kind", using: :btree
+  add_index "budget_phases", ["next_phase_id"], name: "index_budget_phases_on_next_phase_id", using: :btree
+  add_index "budget_phases", ["starts_at"], name: "index_budget_phases_on_starts_at", using: :btree
+
   create_table "budget_reclassified_votes", force: :cascade do |t|
     t.integer  "user_id"
     t.integer  "investment_id"

lib/tasks/budgets.rake

@@ -13,4 +13,22 @@ namespace :budgets do
 
   end
 
-end
+  namespace :phases do
+    desc "Generates Phases for existing Budgets without them & migrates description_* attributes"
+    task generate_missing: :environment do
+      Budget.where.not(id: Budget::Phase.all.pluck(:budget_id).uniq.compact).each do |budget|
+        Budget::Phase::PHASE_KINDS.each do |phase|
+          Budget::Phase.create(
+            budget: budget,
+            kind: phase,
+            description: budget.send("description_#{phase}"),
+            prev_phase: phases&.last,
+            starts_at: phases&.last&.ends_at || Date.current,
+            ends_at: (phases&.last&.ends_at || Date.current) + 1.month
+          )
+        end
+      end
+    end
+  end
+
+end

spec/factories.rb

@@ -341,7 +341,16 @@
       feasibility "feasible"
       valuation_finished true
     end
+  end
 
+  factory :budget_phase, class: 'Budget::Phase' do
+    budget
+    kind        :balloting
+    summary     Faker::Lorem.sentence(3)
+    description Faker::Lorem.sentence(10)
+    starts_at   Date.yesterday
+    ends_at     Date.tomorrow
+    enabled     true
   end
 
   factory :image do

spec/features/budgets/investments_spec.rb

@@ -426,7 +426,7 @@ def investments_order
     context "When investment with price is selected" do
 
       scenario "Price & explanation is shown when Budget is on published prices phase" do
-        Budget::PUBLISHED_PRICES_PHASES.each do |phase|
+        Budget::Phase::PUBLISHED_PRICES_PHASES.each do |phase|
           budget.update(phase: phase)
           visit budget_investment_path(budget_id: budget.id, id: investment.id)
 
@@ -440,7 +440,7 @@ def investments_order
       end
 
       scenario "Price & explanation isn't shown when Budget is not on published prices phase" do
-        (Budget::PHASES - Budget::PUBLISHED_PRICES_PHASES).each do |phase|
+        (Budget::Phase::PHASE_KINDS - Budget::Phase::PUBLISHED_PRICES_PHASES).each do |phase|
           budget.update(phase: phase)
           visit budget_investment_path(budget_id: budget.id, id: investment.id)
 
@@ -461,7 +461,7 @@ def investments_order
       end
 
       scenario "Price & explanation isn't shown for any Budget's phase" do
-        Budget::PHASES.each do |phase|
+        Budget::Phase::PHASE_KINDS.each do |phase|
           budget.update(phase: phase)
           visit budget_investment_path(budget_id: budget.id, id: investment.id)