From 5016568b8ad15edca845a0ad6753fb3e08b292d1 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 16:44:31 +0100 Subject: [PATCH 01/16] Correctly indent private function at budget model --- app/models/budget.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/models/budget.rb b/app/models/budget.rb index af1149d66..f3249b5e3 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -145,12 +145,12 @@ class Budget < ActiveRecord::Base 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 + PHASES.each do |phase| + sanitized = s.sanitize(send("description_#{phase}")) + send("description_#{phase}=", sanitized) end + end end From 153b46b46893987732974834e6732ea584ae3490 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 16:48:26 +0100 Subject: [PATCH 02/16] Create description_for_phase helper method at Budget, to make it easier to get non-active-phase description --- app/models/budget.rb | 4 ++++ app/views/budgets/results/show.html.erb | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/budget.rb b/app/models/budget.rb index f3249b5e3..76b0141df 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -34,6 +34,10 @@ class Budget < ActiveRecord::Base scope :current, -> { where.not(phase: "finished") } def description + description_for_phase(phase) + end + + def description_for_phase(phase) send("description_#{phase}").try(:html_safe) end diff --git a/app/views/budgets/results/show.html.erb b/app/views/budgets/results/show.html.erb index c276a4c6f..54b31e7ba 100644 --- a/app/views/budgets/results/show.html.erb +++ b/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) %> From 82d67258e86d218ca94eda5dd4772ef474b4c994 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 16:50:44 +0100 Subject: [PATCH 03/16] Create Budget Phase table at database --- .../20180112123641_create_budget_phases.rb | 14 ++++++++++++++ db/schema.rb | 18 +++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20180112123641_create_budget_phases.rb diff --git a/db/migrate/20180112123641_create_budget_phases.rb b/db/migrate/20180112123641_create_budget_phases.rb new file mode 100644 index 000000000..fbac158f0 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index d2c5d3450..5cdff80f1 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: 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 @@ ActiveRecord::Schema.define(version: 20180109175851) do 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" From 36e74d0ef2a3a3c790e098403d37d7fd7e0293ed Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 19:38:39 +0100 Subject: [PATCH 04/16] Add Budget::Phase model, spec and factory Create a new Budget::Phase model that: * Stablishes a relation with its budget * Stablishes relation with two other Budget::Phases (previous and next) * Validates basic dates range, kind and description rules. * Adds scopes to get the ones enabled as well as each individual phase Create a factory that generates a basic and valid Budget::Phase Create a model spec that checks kind, date range and budget validations. --- app/models/budget/phase.rb | 31 ++++++++++++++++++++++ config/locales/en/budgets.yml | 3 +++ config/locales/es/budgets.yml | 3 +++ spec/factories.rb | 9 +++++++ spec/models/budget/phase_spec.rb | 44 ++++++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+) create mode 100644 app/models/budget/phase.rb create mode 100644 spec/models/budget/phase_spec.rb diff --git a/app/models/budget/phase.rb b/app/models/budget/phase.rb new file mode 100644 index 000000000..52de77e30 --- /dev/null +++ b/app/models/budget/phase.rb @@ -0,0 +1,31 @@ +class Budget + class Phase < ActiveRecord::Base + + 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: Budget::PHASES } + validates :description, length: { maximum: Budget.description_max_length } + validate :dates_range_valid? + + 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 dates_range_valid? + 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 + + end +end diff --git a/config/locales/en/budgets.yml b/config/locales/en/budgets.yml index a0fd28cac..2f28b7e3b 100644 --- a/config/locales/en/budgets.yml +++ b/config/locales/en/budgets.yml @@ -158,3 +158,6 @@ 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" \ No newline at end of file diff --git a/config/locales/es/budgets.yml b/config/locales/es/budgets.yml index 258f394d6..40e7af533 100644 --- a/config/locales/es/budgets.yml +++ b/config/locales/es/budgets.yml @@ -158,3 +158,6 @@ 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" \ No newline at end of file diff --git a/spec/factories.rb b/spec/factories.rb index 7372b5ba9..77bfda68e 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -341,7 +341,16 @@ FactoryBot.define do 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 diff --git a/spec/models/budget/phase_spec.rb b/spec/models/budget/phase_spec.rb new file mode 100644 index 000000000..a02d9bbc1 --- /dev/null +++ b/spec/models/budget/phase_spec.rb @@ -0,0 +1,44 @@ +require 'rails_helper' + +describe Budget::Phase do + + let(:budget) { create(:budget) } + + describe "validates" do + it "is not valid without a budget" do + expect(build(:budget_phase, budget: nil)).not_to be_valid + end + + describe "kind validations" do + it "is not valid without a kind" do + expect(build(:budget_phase, kind: nil)).not_to be_valid + end + + it "is not valid with a kind not in valid budget phases" do + expect(build(:budget_phase, kind: 'invalid_phase_kind')).not_to be_valid + end + + it "is not valid with the same kind as another budget's phase" do + create(:budget_phase, budget: budget) + + expect(build(:budget_phase, budget: budget)).not_to be_valid + end + end + + describe "#dates_range_valid?" do + it "is valid when start & end dates are different & consecutive" do + expect(build(:budget_phase, starts_at: Date.today, ends_at: Date.tomorrow)).to be_valid + end + + it "is not valid when dates are equal" do + expect(build(:budget_phase, starts_at: Date.today, ends_at: Date.today)).not_to be_valid + end + + it "is not valid when start date is later than end date" do + expect(build(:budget_phase, starts_at: Date.tomorrow, ends_at: Date.today)).not_to be_valid + end + end + + end + +end From f2228a908bd4eba25dbde19e559c3648cf59368e Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 19:55:41 +0100 Subject: [PATCH 05/16] Refactor budget's phase max description lenght from Budget to Phase model --- app/models/budget.rb | 4 ---- app/models/budget/phase.rb | 3 ++- app/views/admin/budgets/_form.html.erb | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/app/models/budget.rb b/app/models/budget.rb index 76b0141df..2be6b8144 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -41,10 +41,6 @@ class Budget < ActiveRecord::Base send("description_#{phase}").try(:html_safe) end - def self.description_max_length - 2000 - end - def self.title_max_length 80 end diff --git a/app/models/budget/phase.rb b/app/models/budget/phase.rb index 52de77e30..7c5e82da7 100644 --- a/app/models/budget/phase.rb +++ b/app/models/budget/phase.rb @@ -1,5 +1,6 @@ class Budget class Phase < ActiveRecord::Base + DESCRIPTION_MAX_LENGTH = 2000 belongs_to :budget belongs_to :next_phase, class_name: 'Budget::Phase', foreign_key: :next_phase_id @@ -7,7 +8,7 @@ class Budget validates :budget, presence: true validates :kind, presence: true, uniqueness: { scope: :budget }, inclusion: { in: Budget::PHASES } - validates :description, length: { maximum: Budget.description_max_length } + validates :description, length: { maximum: DESCRIPTION_MAX_LENGTH } validate :dates_range_valid? scope :enabled, -> { where(enabled: true) } diff --git a/app/views/admin/budgets/_form.html.erb b/app/views/admin/budgets/_form.html.erb index 58a56d564..742a7bbc0 100644 --- a/app/views/admin/budgets/_form.html.erb +++ b/app/views/admin/budgets/_form.html.erb @@ -4,7 +4,7 @@ <% Budget::PHASES.each do |phase| %>
- <%= 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 } %>
<% end %> From 66691b644aec3f3691e07c88b80c9c7bd996f4b2 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 20:17:49 +0100 Subject: [PATCH 06/16] Refactor Budget::PHASES constant to Budget::Phase::PHASE_KINDS --- app/controllers/admin/budgets_controller.rb | 2 +- app/helpers/budgets_helper.rb | 2 +- app/models/budget.rb | 6 ++---- app/models/budget/phase.rb | 4 +++- app/views/admin/budgets/_form.html.erb | 2 +- db/dev_seeds.rb | 4 ++-- spec/features/budgets/investments_spec.rb | 4 ++-- spec/features/budgets/results_spec.rb | 2 +- spec/features/tags/budget_investments_spec.rb | 8 ++++---- spec/models/budget/investment_spec.rb | 12 ++++++------ spec/models/budget_spec.rb | 4 ++-- 11 files changed, 25 insertions(+), 25 deletions(-) diff --git a/app/controllers/admin/budgets_controller.rb b/app/controllers/admin/budgets_controller.rb index d2f8706c0..161610f58 100644 --- a/app/controllers/admin/budgets_controller.rb +++ b/app/controllers/admin/budgets_controller.rb @@ -54,7 +54,7 @@ class Admin::BudgetsController < Admin::BaseController 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 diff --git a/app/helpers/budgets_helper.rb b/app/helpers/budgets_helper.rb index d2d6e1f5d..5e1949775 100644 --- a/app/helpers/budgets_helper.rb +++ b/app/helpers/budgets_helper.rb @@ -7,7 +7,7 @@ module BudgetsHelper 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 diff --git a/app/models/budget.rb b/app/models/budget.rb index 2be6b8144..fc65938d1 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -3,14 +3,12 @@ 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/ @@ -147,7 +145,7 @@ class Budget < ActiveRecord::Base def sanitize_descriptions s = WYSIWYGSanitizer.new - PHASES.each do |phase| + Budget::Phase::PHASE_KINDS.each do |phase| sanitized = s.sanitize(send("description_#{phase}")) send("description_#{phase}=", sanitized) end diff --git a/app/models/budget/phase.rb b/app/models/budget/phase.rb index 7c5e82da7..634dbc666 100644 --- a/app/models/budget/phase.rb +++ b/app/models/budget/phase.rb @@ -1,5 +1,7 @@ class Budget class Phase < ActiveRecord::Base + PHASE_KINDS = %w(drafting accepting reviewing selecting valuating publishing_prices balloting + reviewing_ballots finished).freeze DESCRIPTION_MAX_LENGTH = 2000 belongs_to :budget @@ -7,7 +9,7 @@ class Budget 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: Budget::PHASES } + validates :kind, presence: true, uniqueness: { scope: :budget }, inclusion: { in: PHASE_KINDS } validates :description, length: { maximum: DESCRIPTION_MAX_LENGTH } validate :dates_range_valid? diff --git a/app/views/admin/budgets/_form.html.erb b/app/views/admin/budgets/_form.html.erb index 742a7bbc0..48a52079e 100644 --- a/app/views/admin/budgets/_form.html.erb +++ b/app/views/admin/budgets/_form.html.erb @@ -2,7 +2,7 @@ <%= f.text_field :name, maxlength: Budget.title_max_length %> - <% Budget::PHASES.each do |phase| %> + <% Budget::Phase::PHASE_KINDS.each do |phase| %>
<%= f.cktext_area "description_#{phase}", maxlength: Budget::Phase::DESCRIPTION_MAX_LENGTH, ckeditor: { language: I18n.locale } %>
diff --git a/db/dev_seeds.rb b/db/dev_seeds.rb index 8ed80d5cb..cc36569c8 100644 --- a/db/dev_seeds.rb +++ b/db/dev_seeds.rb @@ -401,8 +401,8 @@ section "Creating Valuation Assignments" do 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}", "

#{Faker::Lorem.paragraphs(2).join('

')}

"] end] diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb index d1fdcc474..0e8dc859a 100644 --- a/spec/features/budgets/investments_spec.rb +++ b/spec/features/budgets/investments_spec.rb @@ -440,7 +440,7 @@ feature 'Budget Investments' do 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::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 @@ feature 'Budget Investments' do 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) diff --git a/spec/features/budgets/results_spec.rb b/spec/features/budgets/results_spec.rb index 5e194a393..3eb349df0 100644 --- a/spec/features/budgets/results_spec.rb +++ b/spec/features/budgets/results_spec.rb @@ -65,7 +65,7 @@ feature 'Results' do end scenario "If budget is in a phase different from finished results can't be accessed" do - budget.update(phase: (Budget::PHASES - ['drafting', 'finished']).sample) + budget.update(phase: (Budget::Phase::PHASE_KINDS - ['drafting', 'finished']).sample) visit budget_path(budget) expect(page).not_to have_link "See results" diff --git a/spec/features/tags/budget_investments_spec.rb b/spec/features/tags/budget_investments_spec.rb index f244583a6..04fb05ddb 100644 --- a/spec/features/tags/budget_investments_spec.rb +++ b/spec/features/tags/budget_investments_spec.rb @@ -183,7 +183,7 @@ feature 'Tags' do let!(:investment3) { create(:budget_investment, heading: heading, tag_list: newer_tag) } scenario 'Display user tags' do - Budget::PHASES.each do |phase| + Budget::Phase::PHASE_KINDS.each do |phase| budget.update(phase: phase) login_as(admin) if budget.drafting? @@ -197,7 +197,7 @@ feature 'Tags' do end scenario "Filter by user tags" do - Budget::PHASES.each do |phase| + Budget::Phase::PHASE_KINDS.each do |phase| budget.update(phase: phase) if budget.balloting? @@ -230,7 +230,7 @@ feature 'Tags' do let!(:investment3) { create(:budget_investment, heading: heading, tag_list: tag_economia.name) } scenario 'Display category tags' do - Budget::PHASES.each do |phase| + Budget::Phase::PHASE_KINDS.each do |phase| budget.update(phase: phase) login_as(admin) if budget.drafting? @@ -244,7 +244,7 @@ feature 'Tags' do end scenario "Filter by category tags" do - Budget::PHASES.each do |phase| + Budget::Phase::PHASE_KINDS.each do |phase| budget.update(phase: phase) if budget.balloting? diff --git a/spec/models/budget/investment_spec.rb b/spec/models/budget/investment_spec.rb index 37cfd84ad..4ca0b1d5f 100644 --- a/spec/models/budget/investment_spec.rb +++ b/spec/models/budget/investment_spec.rb @@ -141,7 +141,7 @@ describe Budget::Investment do end it "returns false in any other phase" do - Budget::PHASES.reject {|phase| phase == "selecting"}.each do |phase| + Budget::Phase::PHASE_KINDS.reject {|phase| phase == "selecting"}.each do |phase| budget = create(:budget, phase: phase) investment = create(:budget_investment, budget: budget) @@ -159,7 +159,7 @@ describe Budget::Investment do end it "returns false in any other phase" do - Budget::PHASES.reject {|phase| phase == "valuating"}.each do |phase| + Budget::Phase::PHASE_KINDS.reject {|phase| phase == "valuating"}.each do |phase| budget = create(:budget, phase: phase) investment = create(:budget_investment, budget: budget) @@ -184,7 +184,7 @@ describe Budget::Investment do end it "returns false in any other phase" do - Budget::PHASES.reject {|phase| phase == "balloting"}.each do |phase| + Budget::Phase::PHASE_KINDS.reject {|phase| phase == "balloting"}.each do |phase| budget = create(:budget, phase: phase) investment = create(:budget_investment, :selected, budget: budget) @@ -208,7 +208,7 @@ describe Budget::Investment do end it "returns false in any other phase" do - (Budget::PHASES - Budget::PUBLISHED_PRICES_PHASES).each do |phase| + (Budget::Phase::PHASE_KINDS - Budget::PUBLISHED_PRICES_PHASES).each do |phase| budget.update(phase: phase) expect(investment.should_show_price?).to eq(false) @@ -243,7 +243,7 @@ describe Budget::Investment do end it "returns false in any other phase" do - (Budget::PHASES - Budget::PUBLISHED_PRICES_PHASES).each do |phase| + (Budget::Phase::PHASE_KINDS - Budget::PUBLISHED_PRICES_PHASES).each do |phase| budget.update(phase: phase) expect(investment.should_show_price_explanation?).to eq(false) @@ -785,7 +785,7 @@ describe Budget::Investment do end it "returns false if budget is not balloting phase" do - Budget::PHASES.reject {|phase| phase == "balloting"}.each do |phase| + Budget::Phase::PHASE_KINDS.reject {|phase| phase == "balloting"}.each do |phase| budget.update(phase: phase) investment = create(:budget_investment, budget: budget) diff --git a/spec/models/budget_spec.rb b/spec/models/budget_spec.rb index 70c292962..fe96d8ab7 100644 --- a/spec/models/budget_spec.rb +++ b/spec/models/budget_spec.rb @@ -18,7 +18,7 @@ describe Budget do it "changes depending on the phase" do budget = create(:budget) - Budget::PHASES.each do |phase| + Budget::Phase::PHASE_KINDS.each do |phase| budget.phase = phase expect(budget.description).to eq(budget.send("description_#{phase}")) expect(budget.description).to be_html_safe @@ -30,7 +30,7 @@ describe Budget do let(:budget) { create(:budget) } it "is validated" do - Budget::PHASES.each do |phase| + Budget::Phase::PHASE_KINDS.each do |phase| budget.phase = phase expect(budget).to be_valid end From ca3d759d9f1216d600acb40d2ec3b4e65f8ab187 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 20:20:10 +0100 Subject: [PATCH 07/16] Refactor Budget publishing prices phases constant to Budget::Phase model --- app/models/budget.rb | 4 +--- app/models/budget/phase.rb | 1 + spec/features/budgets/investments_spec.rb | 4 ++-- spec/models/budget/investment_spec.rb | 8 ++++---- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/app/models/budget.rb b/app/models/budget.rb index fc65938d1..bf64c0fd6 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -3,8 +3,6 @@ class Budget < ActiveRecord::Base include Measurable include Sluggable - PUBLISHED_PRICES_PHASES = %w(publishing_prices balloting reviewing_ballots finished).freeze - CURRENCY_SYMBOLS = %w(€ $ £ ¥).freeze validates :name, presence: true, uniqueness: true @@ -80,7 +78,7 @@ class Budget < ActiveRecord::Base end def published_prices? - PUBLISHED_PRICES_PHASES.include?(phase) + Budget::Phase::PUBLISHED_PRICES_PHASES.include?(phase) end def balloting_process? diff --git a/app/models/budget/phase.rb b/app/models/budget/phase.rb index 634dbc666..c55f6a636 100644 --- a/app/models/budget/phase.rb +++ b/app/models/budget/phase.rb @@ -2,6 +2,7 @@ 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 diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb index 0e8dc859a..7569d96bc 100644 --- a/spec/features/budgets/investments_spec.rb +++ b/spec/features/budgets/investments_spec.rb @@ -426,7 +426,7 @@ feature 'Budget Investments' do 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 @@ feature 'Budget Investments' do end scenario "Price & explanation isn't shown when Budget is not on published prices phase" do - (Budget::Phase::PHASE_KINDS - 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) diff --git a/spec/models/budget/investment_spec.rb b/spec/models/budget/investment_spec.rb index 4ca0b1d5f..7fbb46039 100644 --- a/spec/models/budget/investment_spec.rb +++ b/spec/models/budget/investment_spec.rb @@ -200,7 +200,7 @@ describe Budget::Investment do end it "returns true for selected investments which budget's phase is publishing_prices or later" do - Budget::PUBLISHED_PRICES_PHASES.each do |phase| + Budget::Phase::PUBLISHED_PRICES_PHASES.each do |phase| budget.update(phase: phase) expect(investment.should_show_price?).to eq(true) @@ -208,7 +208,7 @@ describe Budget::Investment do end it "returns false in any other phase" do - (Budget::Phase::PHASE_KINDS - Budget::PUBLISHED_PRICES_PHASES).each do |phase| + (Budget::Phase::PHASE_KINDS - Budget::Phase::PUBLISHED_PRICES_PHASES).each do |phase| budget.update(phase: phase) expect(investment.should_show_price?).to eq(false) @@ -235,7 +235,7 @@ describe Budget::Investment do end it "returns true for selected with price_explanation & budget in publishing_prices or later" do - Budget::PUBLISHED_PRICES_PHASES.each do |phase| + Budget::Phase::PUBLISHED_PRICES_PHASES.each do |phase| budget.update(phase: phase) expect(investment.should_show_price_explanation?).to eq(true) @@ -243,7 +243,7 @@ describe Budget::Investment do end it "returns false in any other phase" do - (Budget::Phase::PHASE_KINDS - Budget::PUBLISHED_PRICES_PHASES).each do |phase| + (Budget::Phase::PHASE_KINDS - Budget::Phase::PUBLISHED_PRICES_PHASES).each do |phase| budget.update(phase: phase) expect(investment.should_show_price_explanation?).to eq(false) From 10f5cc0d3bb205abd640e838a6261872f59352d0 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 20:30:45 +0100 Subject: [PATCH 08/16] Add phases relation at Budget model, as well as current_phase helper method --- app/models/budget.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/models/budget.rb b/app/models/budget.rb index bf64c0fd6..56b774a41 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -14,6 +14,7 @@ class Budget < ActiveRecord::Base 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 @@ -26,15 +27,22 @@ class Budget < ActiveRecord::Base scope :balloting, -> { where(phase: "balloting") } scope :reviewing_ballots, -> { where(phase: "reviewing_ballots") } scope :finished, -> { where(phase: "finished") } - scope :current, -> { where.not(phase: "finished") } + def current_phase + phases.send(phase) + end + def description description_for_phase(phase) end def description_for_phase(phase) - send("description_#{phase}").try(:html_safe) + 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 From 59fb0b562c2a436d9dfd914af2f8e9d74feec643 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 20:33:07 +0100 Subject: [PATCH 09/16] Create all Phases after a Budget creation --- app/models/budget.rb | 14 +++++++ spec/models/budget_spec.rb | 76 ++++++++++++++++++++++++++++++++------ 2 files changed, 78 insertions(+), 12 deletions(-) diff --git a/app/models/budget.rb b/app/models/budget.rb index 56b774a41..bf3fc13f5 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -18,6 +18,8 @@ class Budget < ActiveRecord::Base before_validation :sanitize_descriptions + after_create :generate_phases + scope :drafting, -> { where(phase: "drafting") } scope :accepting, -> { where(phase: "accepting") } scope :reviewing, -> { where(phase: "reviewing") } @@ -156,5 +158,17 @@ class Budget < ActiveRecord::Base 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 diff --git a/spec/models/budget_spec.rb b/spec/models/budget_spec.rb index fe96d8ab7..7369d15ac 100644 --- a/spec/models/budget_spec.rb +++ b/spec/models/budget_spec.rb @@ -2,6 +2,8 @@ require 'rails_helper' describe Budget do + let(:budget) { create(:budget) } + it_behaves_like "sluggable" describe "name" do @@ -15,20 +17,38 @@ describe Budget do end describe "description" do - it "changes depending on the phase" do - budget = create(:budget) + describe "Without Budget::Phase associated" do + before do + budget.phases.destroy_all + end - Budget::Phase::PHASE_KINDS.each do |phase| - budget.phase = phase - expect(budget.description).to eq(budget.send("description_#{phase}")) - expect(budget.description).to be_html_safe + it "changes depending on the phase, falling back to budget description attributes" do + Budget::Phase::PHASE_KINDS.each do |phase_kind| + budget.phase = phase_kind + expect(budget.description).to eq(budget.send("description_#{phase_kind}")) + expect(budget.description).to be_html_safe + end + end + end + + describe "With associated Budget::Phases" do + before do + budget.phases.each do |phase| + phase.description = phase.kind.humanize + phase.save + end + end + + it "changes depending on the phase" do + Budget::Phase::PHASE_KINDS.each do |phase_kind| + budget.phase = phase_kind + expect(budget.description).to eq(phase_kind.humanize) + end end end end describe "phase" do - let(:budget) { create(:budget) } - it "is validated" do Budget::Phase::PHASE_KINDS.each do |phase| budget.phase = phase @@ -99,7 +119,6 @@ describe Budget do end describe "heading_price" do - let(:budget) { create(:budget) } let(:group) { create(:budget_group, budget: budget) } it "returns the heading price if the heading provided is part of the budget" do @@ -113,8 +132,6 @@ describe Budget do end describe "investments_orders" do - let(:budget) { create(:budget) } - it "is random when accepting and reviewing" do budget.phase = 'accepting' expect(budget.investments_orders).to eq(['random']) @@ -136,5 +153,40 @@ describe Budget do expect(budget.investments_orders).to eq(['random', 'confidence_score']) end end -end + describe "#generate_phases" do + let(:drafting_phase) { budget.phases.drafting } + let(:accepting_phase) { budget.phases.accepting } + let(:reviewing_phase) { budget.phases.reviewing } + let(:selecting_phase) { budget.phases.selecting } + let(:valuating_phase) { budget.phases.valuating } + let(:publishing_prices_phase) { budget.phases.publishing_prices } + let(:balloting_phase) { budget.phases.balloting } + let(:reviewing_ballots_phase) { budget.phases.reviewing_ballots } + let(:finished_phase) { budget.phases.finished } + + it "generates all phases linked in correct order" do + expect(budget.phases.count).to eq(Budget::Phase::PHASE_KINDS.count) + + expect(drafting_phase.next_phase).to eq(accepting_phase) + expect(accepting_phase.next_phase).to eq(reviewing_phase) + expect(reviewing_phase.next_phase).to eq(selecting_phase) + expect(selecting_phase.next_phase).to eq(valuating_phase) + expect(valuating_phase.next_phase).to eq(publishing_prices_phase) + expect(publishing_prices_phase.next_phase).to eq(balloting_phase) + expect(balloting_phase.next_phase).to eq(reviewing_ballots_phase) + expect(reviewing_ballots_phase.next_phase).to eq(finished_phase) + expect(finished_phase.next_phase).to eq(nil) + + expect(drafting_phase.prev_phase).to eq(nil) + expect(accepting_phase.prev_phase).to eq(drafting_phase) + expect(reviewing_phase.prev_phase).to eq(accepting_phase) + expect(selecting_phase.prev_phase).to eq(reviewing_phase) + expect(valuating_phase.prev_phase).to eq(selecting_phase) + expect(publishing_prices_phase.prev_phase).to eq(valuating_phase) + expect(balloting_phase.prev_phase).to eq(publishing_prices_phase) + expect(reviewing_ballots_phase.prev_phase).to eq(balloting_phase) + expect(finished_phase.prev_phase).to eq(reviewing_ballots_phase) + end + end +end From 21b62106e5a7b45ea6ca297b17904d1b7c06f4e8 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 20:35:52 +0100 Subject: [PATCH 10/16] Add next/prev enabled phase helper functions to Budget::Phase with model specs --- app/models/budget/phase.rb | 8 +++++ spec/models/budget/phase_spec.rb | 50 +++++++++++++++++++++++++++----- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/app/models/budget/phase.rb b/app/models/budget/phase.rb index c55f6a636..a51d82076 100644 --- a/app/models/budget/phase.rb +++ b/app/models/budget/phase.rb @@ -25,6 +25,14 @@ class Budget 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 dates_range_valid? if starts_at.present? && ends_at.present? && starts_at >= ends_at errors.add(:starts_at, I18n.t('budgets.phases.errors.dates_range_invalid')) diff --git a/spec/models/budget/phase_spec.rb b/spec/models/budget/phase_spec.rb index a02d9bbc1..90d1a01b6 100644 --- a/spec/models/budget/phase_spec.rb +++ b/spec/models/budget/phase_spec.rb @@ -2,7 +2,19 @@ require 'rails_helper' describe Budget::Phase do - let(:budget) { create(:budget) } + let(:budget) { create(:budget) } + let(:first_phase) { budget.phases.drafting } + let(:second_phase) { budget.phases.accepting } + let(:third_phase) { budget.phases.reviewing } + let(:fourth_phase) { budget.phases.selecting } + let(:final_phase) { budget.phases.finished} + + before do + first_phase.update_attributes(starts_at: Date.current - 3.days, ends_at: Date.current - 1.day) + second_phase.update_attributes(starts_at: Date.current - 1.days, ends_at: Date.current + 1.day) + third_phase.update_attributes(starts_at: Date.current + 1.days, ends_at: Date.current + 3.day) + fourth_phase.update_attributes(starts_at: Date.current + 3.days, ends_at: Date.current + 5.day) + end describe "validates" do it "is not valid without a budget" do @@ -19,26 +31,50 @@ describe Budget::Phase do end it "is not valid with the same kind as another budget's phase" do - create(:budget_phase, budget: budget) - expect(build(:budget_phase, budget: budget)).not_to be_valid end end describe "#dates_range_valid?" do it "is valid when start & end dates are different & consecutive" do - expect(build(:budget_phase, starts_at: Date.today, ends_at: Date.tomorrow)).to be_valid + first_phase.update_attributes(starts_at: Date.today, ends_at: Date.tomorrow) + + expect(first_phase).to be_valid end it "is not valid when dates are equal" do - expect(build(:budget_phase, starts_at: Date.today, ends_at: Date.today)).not_to be_valid + first_phase.update_attributes(starts_at: Date.today, ends_at: Date.today) + + expect(first_phase).not_to be_valid end it "is not valid when start date is later than end date" do - expect(build(:budget_phase, starts_at: Date.tomorrow, ends_at: Date.today)).not_to be_valid + first_phase.update_attributes(starts_at: Date.tomorrow, ends_at: Date.today) + + expect(first_phase).not_to be_valid + end + end + end + + describe "next & prev enabled phases" do + before do + second_phase.update_attributes(enabled: false) + end + + describe "#next_enabled_phase" do + it "returns the right next enabled phase" do + expect(first_phase.reload.next_enabled_phase).to eq(third_phase) + expect(third_phase.reload.next_enabled_phase).to eq(fourth_phase) + expect(final_phase.reload.next_enabled_phase).to eq(nil) end end + describe "#prev_enabled_phase" do + it "returns the right previous enabled phase" do + expect(first_phase.reload.prev_enabled_phase).to eq(nil) + expect(third_phase.reload.prev_enabled_phase).to eq(first_phase) + expect(fourth_phase.reload.prev_enabled_phase).to eq(third_phase) + end + end end - end From d505cda949bb73dca925533cfa4487d980fa3428 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 20:37:39 +0100 Subject: [PATCH 11/16] Add description sanitization to Budget::Phase with model specs --- app/models/budget/phase.rb | 7 +++++++ spec/models/budget/phase_spec.rb | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/app/models/budget/phase.rb b/app/models/budget/phase.rb index a51d82076..7e59ff6ac 100644 --- a/app/models/budget/phase.rb +++ b/app/models/budget/phase.rb @@ -14,6 +14,9 @@ class Budget validates :description, length: { maximum: DESCRIPTION_MAX_LENGTH } validate :dates_range_valid? + before_validation :sanitize_description + + scope :enabled, -> { where(enabled: true) } scope :drafting, -> { find_by_kind('drafting') } scope :accepting, -> { find_by_kind('accepting')} @@ -39,5 +42,9 @@ class Budget end end + + def sanitize_description + self.description = WYSIWYGSanitizer.new.sanitize(description) + end end end diff --git a/spec/models/budget/phase_spec.rb b/spec/models/budget/phase_spec.rb index 90d1a01b6..d904dc8c7 100644 --- a/spec/models/budget/phase_spec.rb +++ b/spec/models/budget/phase_spec.rb @@ -77,4 +77,12 @@ describe Budget::Phase do end end end + + describe "#sanitize_description" do + it "removes html entities from the description" do + expect{ + first_phase.update_attributes(description: "a

javascript") + }.to change{ first_phase.description }.to('a javascript') + end + end end From 601351d1609191f1e0d2be9b1c174eb939fcd17f Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 20:41:33 +0100 Subject: [PATCH 12/16] Validate next/prev phases before saving a Budget::Phase, with model specs --- app/models/budget/phase.rb | 25 ++++++++++++ config/locales/en/budgets.yml | 4 +- config/locales/es/budgets.yml | 4 +- spec/models/budget/phase_spec.rb | 65 ++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 2 deletions(-) diff --git a/app/models/budget/phase.rb b/app/models/budget/phase.rb index 7e59ff6ac..d5fc42060 100644 --- a/app/models/budget/phase.rb +++ b/app/models/budget/phase.rb @@ -13,6 +13,8 @@ class Budget validates :kind, presence: true, uniqueness: { scope: :budget }, inclusion: { in: PHASE_KINDS } validates :description, length: { maximum: DESCRIPTION_MAX_LENGTH } validate :dates_range_valid? + validate :prev_phase_dates_valid? + validate :next_phase_dates_valid? before_validation :sanitize_description @@ -42,6 +44,29 @@ class Budget 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.dates_range_valid? + 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.dates_range_valid? + 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) diff --git a/config/locales/en/budgets.yml b/config/locales/en/budgets.yml index 2f28b7e3b..a7dd62016 100644 --- a/config/locales/en/budgets.yml +++ b/config/locales/en/budgets.yml @@ -160,4 +160,6 @@ en: incompatibles: Incompatibles phases: errors: - dates_range_invalid: "Start date can't be equal or later than End date" \ No newline at end of file + 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})" \ No newline at end of file diff --git a/config/locales/es/budgets.yml b/config/locales/es/budgets.yml index 40e7af533..0027b1241 100644 --- a/config/locales/es/budgets.yml +++ b/config/locales/es/budgets.yml @@ -160,4 +160,6 @@ es: incompatibles: Incompatibles phases: errors: - dates_range_invalid: "La fecha de comienzo no puede ser igual o superior a la de finalización" \ No newline at end of file + 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}) " \ No newline at end of file diff --git a/spec/models/budget/phase_spec.rb b/spec/models/budget/phase_spec.rb index d904dc8c7..fba482534 100644 --- a/spec/models/budget/phase_spec.rb +++ b/spec/models/budget/phase_spec.rb @@ -54,6 +54,71 @@ describe Budget::Phase do expect(first_phase).not_to be_valid end end + + describe "#prev_phase_dates_valid?" do + let(:error) do + "Start date must be later than the start date of the previous enabled phase"\ + " (Draft (Not visible to the public))" + end + + it "is invalid when start date is same as previous enabled phase start date" do + second_phase.assign_attributes(starts_at: second_phase.prev_enabled_phase.starts_at) + + expect(second_phase).not_to be_valid + expect(second_phase.errors.messages[:starts_at]).to include(error) + end + + it "is invalid when start date is earlier than previous enabled phase start date" do + second_phase.assign_attributes(starts_at: second_phase.prev_enabled_phase.starts_at - 1.day) + + expect(second_phase).not_to be_valid + expect(second_phase.errors.messages[:starts_at]).to include(error) + end + + it "is valid when start date is in between previous enabled phase start & end dates" do + second_phase.assign_attributes(starts_at: second_phase.prev_enabled_phase.starts_at + 1.day) + + expect(second_phase).to be_valid + end + + it "is valid when start date is later than previous enabled phase end date" do + second_phase.assign_attributes(starts_at: second_phase.prev_enabled_phase.ends_at + 1.day) + + expect(second_phase).to be_valid + end + end + + describe "#next_phase_dates_valid?" do + let(:error) do + "End date must be earlier than the end date of the next enabled phase (Reviewing projects)" + end + + it "is invalid when end date is same as next enabled phase end date" do + second_phase.assign_attributes(ends_at: second_phase.next_enabled_phase.ends_at) + + expect(second_phase).not_to be_valid + expect(second_phase.errors.messages[:ends_at]).to include(error) + end + + it "is invalid when end date is later than next enabled phase end date" do + second_phase.assign_attributes(ends_at: second_phase.next_enabled_phase.ends_at + 1.day) + + expect(second_phase).not_to be_valid + expect(second_phase.errors.messages[:ends_at]).to include(error) + end + + it "is valid when end date is in between next enabled phase start & end dates" do + second_phase.assign_attributes(ends_at: second_phase.next_enabled_phase.ends_at - 1.day) + + expect(second_phase).to be_valid + end + + it "is valid when end date is earlier than next enabled phase start date" do + second_phase.assign_attributes(ends_at: second_phase.next_enabled_phase.starts_at - 1.day) + + expect(second_phase).to be_valid + end + end end describe "next & prev enabled phases" do From 313d8d2e119846d95cc9391979a685015367715b Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 20:42:29 +0100 Subject: [PATCH 13/16] Adjust date ranges of prev/next phases when enabling/disabling a Budget::Phase, with model specs --- app/models/budget.rb | 1 + app/models/budget/phase.rb | 10 +++++ spec/models/budget/phase_spec.rb | 77 ++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+) diff --git a/app/models/budget.rb b/app/models/budget.rb index bf3fc13f5..aba7b04ae 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -172,3 +172,4 @@ class Budget < ActiveRecord::Base end end + diff --git a/app/models/budget/phase.rb b/app/models/budget/phase.rb index d5fc42060..462d64ef6 100644 --- a/app/models/budget/phase.rb +++ b/app/models/budget/phase.rb @@ -18,6 +18,7 @@ class Budget before_validation :sanitize_description + after_save :adjust_date_ranges scope :enabled, -> { where(enabled: true) } scope :drafting, -> { find_by_kind('drafting') } @@ -38,6 +39,15 @@ class Budget 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 dates_range_valid? if starts_at.present? && ends_at.present? && starts_at >= ends_at errors.add(:starts_at, I18n.t('budgets.phases.errors.dates_range_invalid')) diff --git a/spec/models/budget/phase_spec.rb b/spec/models/budget/phase_spec.rb index fba482534..5f9ea34c2 100644 --- a/spec/models/budget/phase_spec.rb +++ b/spec/models/budget/phase_spec.rb @@ -121,6 +121,83 @@ describe Budget::Phase do end end + describe "#adjust_date_ranges" do + let(:prev_enabled_phase) { second_phase.prev_enabled_phase } + let(:next_enabled_phase) { second_phase.next_enabled_phase } + + describe "when enabled" do + it "adjusts previous enabled phase end date to its own start date" do + expect(prev_enabled_phase.ends_at).to eq(second_phase.starts_at) + end + + it "adjusts next enabled phase start date to its own end date" do + expect(next_enabled_phase.starts_at).to eq(second_phase.ends_at) + end + end + + describe "when being enabled" do + before do + second_phase.update_attributes(enabled: false, + starts_at: Date.current, + ends_at: Date.current + 2.days) + end + + it "adjusts previous enabled phase end date to its own start date" do + expect{ + second_phase.update_attributes(enabled: true) + }.to change{ + prev_enabled_phase.ends_at.to_date + }.to(Date.current) + end + + it "adjusts next enabled phase start date to its own end date" do + expect{ + second_phase.update_attributes(enabled: true) + }.to change{ + next_enabled_phase.starts_at.to_date + }.to(Date.current + 2.days) + end + end + + describe "when disabled" do + before do + second_phase.update_attributes(enabled: false) + end + + it "doesn't change previous enabled phase end date" do + expect { + second_phase.update_attributes(starts_at: Date.current, + ends_at: Date.current + 2.days) + }.not_to (change{ prev_enabled_phase.ends_at }) + end + + it "doesn't change next enabled phase start date" do + expect{ + second_phase.update_attributes(starts_at: Date.current, + ends_at: Date.current + 2.days) + }.not_to (change{ next_enabled_phase.starts_at }) + end + end + + describe "when being disabled" do + it "doesn't adjust previous enabled phase end date to its own start date" do + expect { + second_phase.update_attributes(enabled: false, + starts_at: Date.current, + ends_at: Date.current + 2.days) + }.not_to (change{ prev_enabled_phase.ends_at }) + end + + it "adjusts next enabled phase start date to its own start date" do + expect { + second_phase.update_attributes(enabled: false, + starts_at: Date.current, + ends_at: Date.current + 2.days) + }.to change{ next_enabled_phase.starts_at.to_date }.to(Date.current) + end + end + end + describe "next & prev enabled phases" do before do second_phase.update_attributes(enabled: false) From 02d596c872eebf5ee9d85a087f1fad5a8ee98dfe Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 20:42:56 +0100 Subject: [PATCH 14/16] Add a rake task to generate missing Budget::Phase's and migrate descricptions --- app/models/budget/phase.rb | 8 ++++---- lib/tasks/budgets.rake | 20 +++++++++++++++++++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/app/models/budget/phase.rb b/app/models/budget/phase.rb index 462d64ef6..903c698aa 100644 --- a/app/models/budget/phase.rb +++ b/app/models/budget/phase.rb @@ -12,7 +12,7 @@ class Budget validates :budget, presence: true validates :kind, presence: true, uniqueness: { scope: :budget }, inclusion: { in: PHASE_KINDS } validates :description, length: { maximum: DESCRIPTION_MAX_LENGTH } - validate :dates_range_valid? + validate :invalid_dates_range? validate :prev_phase_dates_valid? validate :next_phase_dates_valid? @@ -48,7 +48,7 @@ class Budget end end - def dates_range_valid? + 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 @@ -59,7 +59,7 @@ class Budget 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.dates_range_valid? + 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) @@ -70,7 +70,7 @@ class Budget 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.dates_range_valid? + 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) diff --git a/lib/tasks/budgets.rake b/lib/tasks/budgets.rake index f9810ac1c..738649538 100644 --- a/lib/tasks/budgets.rake +++ b/lib/tasks/budgets.rake @@ -13,4 +13,22 @@ namespace :budgets do end -end \ No newline at end of file + 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 From 005d08e71862361dc34d5db67772e12185a25527 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 16 Jan 2018 15:28:28 +0100 Subject: [PATCH 15/16] Update changelog add and deprecated sections for unreleased block --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bf08d329..26b026257 100644 --- a/CHANGELOG.md +++ b/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) @@ -27,6 +28,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Split 'routes.rb' file into multiple small files https://github.com/consul/consul/pull/1908 ### 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. From 8b469c5d989d5049bd214cec5e09d8d8cd6377ab Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 16 Jan 2018 18:44:21 +0100 Subject: [PATCH 16/16] Fix conflicts with merged branch, Budget::PHASES have moved, and described_class usage is a must --- spec/models/budget_spec.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/spec/models/budget_spec.rb b/spec/models/budget_spec.rb index fa0c6ed57..cbc5fd5de 100644 --- a/spec/models/budget_spec.rb +++ b/spec/models/budget_spec.rb @@ -123,13 +123,13 @@ describe Budget do it "returns nil if there is only one budget and it is still in drafting phase" do budget = create(:budget, phase: "drafting") - expect(Budget.current).to eq(nil) + expect(described_class.current).to eq(nil) end it "returns the budget if there is only one and not in drafting phase" do budget = create(:budget, phase: "accepting") - expect(Budget.current).to eq(budget) + expect(described_class.current).to eq(budget) end it "returns the last budget created that is not in drafting phase" do @@ -138,7 +138,7 @@ describe Budget do current_budget = create(:budget, phase: "accepting", created_at: 1.month.ago) next_budget = create(:budget, phase: "drafting", created_at: 1.week.ago) - expect(Budget.current).to eq(current_budget) + expect(described_class.current).to eq(current_budget) end end @@ -146,10 +146,9 @@ describe Budget do describe "#open" do it "returns all budgets that are not in the finished phase" do - phases = Budget::PHASES - ["finished"] - phases.each do |phase| + (Budget::Phase::PHASE_KINDS - ["finished"]).each do |phase| budget = create(:budget, phase: phase) - expect(Budget.open).to include(budget) + expect(described_class.open).to include(budget) end end