Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create Budget::Phases backend #2323

Merged
merged 17 commits into from
Jan 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
5016568
Correctly indent private function at budget model
bertocq Jan 15, 2018
153b46b
Create description_for_phase helper method at Budget, to make it easi…
bertocq Jan 15, 2018
82d6725
Create Budget Phase table at database
bertocq Jan 15, 2018
36e74d0
Add Budget::Phase model, spec and factory
bertocq Jan 15, 2018
f2228a9
Refactor budget's phase max description lenght from Budget to Phase m…
bertocq Jan 15, 2018
66691b6
Refactor Budget::PHASES constant to Budget::Phase::PHASE_KINDS
bertocq Jan 15, 2018
ca3d759
Refactor Budget publishing prices phases constant to Budget::Phase model
bertocq Jan 15, 2018
10f5cc0
Add phases relation at Budget model, as well as current_phase helper …
bertocq Jan 15, 2018
59fb0b5
Create all Phases after a Budget creation
bertocq Jan 15, 2018
21b6210
Add next/prev enabled phase helper functions to Budget::Phase with mo…
bertocq Jan 15, 2018
d505cda
Add description sanitization to Budget::Phase with model specs
bertocq Jan 15, 2018
601351d
Validate next/prev phases before saving a Budget::Phase, with model s…
bertocq Jan 15, 2018
313d8d2
Adjust date ranges of prev/next phases when enabling/disabling a Budg…
bertocq Jan 15, 2018
02d596c
Add a rake task to generate missing Budget::Phase's and migrate descr…
bertocq Jan 15, 2018
005d08e
Update changelog add and deprecated sections for unreleased block
bertocq Jan 16, 2018
d44db9c
Merge branch 'master' into feature/budget_phases
bertocq Jan 16, 2018
8b469c5
Fix conflicts with merged branch, Budget::PHASES have moved, and desc…
bertocq Jan 16, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/admin/budgets_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/budgets_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
51 changes: 36 additions & 15 deletions app/models/budget.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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") }
Expand All @@ -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
Expand Down Expand Up @@ -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?
Expand Down Expand Up @@ -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


85 changes: 85 additions & 0 deletions app/models/budget/phase.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions app/views/admin/budgets/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>

Expand Down
4 changes: 2 additions & 2 deletions app/views/budgets/results/show.html.erb
Original file line number Diff line number Diff line change
@@ -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) %>
Expand Down
5 changes: 5 additions & 0 deletions config/locales/en/budgets.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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})"
5 changes: 5 additions & 0 deletions config/locales/es/budgets.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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}) "
4 changes: 2 additions & 2 deletions db/dev_seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
14 changes: 14 additions & 0 deletions db/migrate/20180112123641_create_budget_phases.rb
Original file line number Diff line number Diff line change
@@ -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
18 changes: 17 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
20 changes: 19 additions & 1 deletion lib/tasks/budgets.rake
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 9 additions & 0 deletions spec/factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions spec/features/budgets/investments_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand All @@ -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)

Expand Down