From 1d6c3034cbb812a4527f7f535a7cc84be55d7de8 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 10 Jan 2018 00:58:30 +0100 Subject: [PATCH 01/11] Add test scenarios to check investment price showing rules --- spec/factories.rb | 6 +++ spec/features/budgets/investments_spec.rb | 58 +++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/spec/factories.rb b/spec/factories.rb index 252e40f5c..b4da87586 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -326,6 +326,12 @@ FactoryBot.define do incompatible true end + trait :selected_with_price do + selected + price 1000 + price_explanation 'Because of reasons' + end + trait :unselected do selected false feasibility "feasible" diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb index d4c9a0176..e868c140b 100644 --- a/spec/features/budgets/investments_spec.rb +++ b/spec/features/budgets/investments_spec.rb @@ -418,6 +418,64 @@ feature 'Budget Investments' do end end + context "Show Investment's price & cost explanation" do + + let(:investment) { create(:budget_investment, :selected_with_price, heading: heading) } + + 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.update(phase: phase) + visit budget_investment_path(budget_id: budget.id, id: investment.id) + + expect(page).to have_content(investment.formatted_price) + expect(page).to have_content(investment.price_explanation) + + visit budget_investments_path(budget) + + expect(page).to have_content(investment.formatted_price) + end + 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.update(phase: phase) + visit budget_investment_path(budget_id: budget.id, id: investment.id) + + expect(page).not_to have_content(investment.formatted_price) + expect(page).not_to have_content(investment.price_explanation) + + visit budget_investments_path(budget) + + expect(page).not_to have_content(investment.formatted_price) + end + end + end + + context "When investment with price is unselected" do + + background do + investment.update(selected: false) + end + + scenario "Price & explanation isn't shown for any Budget's phase" do + Budget::PHASES.each do |phase| + budget.update(phase: phase) + visit budget_investment_path(budget_id: budget.id, id: investment.id) + + expect(page).not_to have_content(investment.formatted_price) + expect(page).not_to have_content(investment.price_explanation) + + visit budget_investments_path(budget) + + expect(page).not_to have_content(investment.formatted_price) + end + end + end + + end + scenario 'Can access the community' do Setting['feature.community'] = true From f350b99123d6fc07a507f043b9e2beff6e978ce0 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 10 Jan 2018 00:59:19 +0100 Subject: [PATCH 02/11] Remove deprecated investment test scenario for price explanation --- spec/features/budgets/investments_spec.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb index e868c140b..69ab52df1 100644 --- a/spec/features/budgets/investments_spec.rb +++ b/spec/features/budgets/investments_spec.rb @@ -535,13 +535,6 @@ feature 'Budget Investments' do expect(page).not_to have_content(investment.price_explanation) end - scenario "Budget in balloting phase" do - budget.update(phase: "balloting") - visit budget_investment_path(budget_id: budget.id, id: investment.id) - - expect(page).to have_content("Price explanation") - expect(page).to have_content(investment.price_explanation) - end end scenario "Show (unfeasible budget investment)" do From cb1151f1d619722f673fb4dfdda96bb78bc17342 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 10 Jan 2018 01:02:07 +0100 Subject: [PATCH 03/11] Increase Budget model spec for new publishing_prices phase --- spec/models/budget_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spec/models/budget_spec.rb b/spec/models/budget_spec.rb index b86db7e6e..af42b4bc0 100644 --- a/spec/models/budget_spec.rb +++ b/spec/models/budget_spec.rb @@ -55,6 +55,9 @@ describe Budget do budget.phase = "valuating" expect(budget).to be_valuating + budget.phase = "publishing_prices" + expect(budget).to be_publishing_prices + budget.phase = "balloting" expect(budget).to be_balloting @@ -81,6 +84,9 @@ describe Budget do budget.phase = "valuating" expect(budget).to be_on_hold + budget.phase = "publishing_prices" + expect(budget).to be_on_hold + budget.phase = "balloting" expect(budget).not_to be_on_hold @@ -107,6 +113,9 @@ describe Budget do budget.phase = "valuating" expect(budget).not_to be_balloting_or_later + budget.phase = "publishing_prices" + expect(budget).not_to be_balloting_or_later + budget.phase = "balloting" expect(budget).to be_balloting_or_later @@ -142,6 +151,8 @@ describe Budget do expect(budget.investments_orders).to eq(['random']) end it "is random and price when ballotting and reviewing ballots" do + budget.phase = 'publishing_prices' + expect(budget.investments_orders).to eq(['random', 'price']) budget.phase = 'balloting' expect(budget.investments_orders).to eq(['random', 'price']) budget.phase = 'reviewing_ballots' From d8ceff1a538937f45e0c87cf93702b741fd676a5 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 10 Jan 2018 01:09:13 +0100 Subject: [PATCH 04/11] Refactor Budget's on hold phases to constant, plus alignments --- app/models/budget.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/models/budget.rb b/app/models/budget.rb index ce8bc4dfa..509352372 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -5,6 +5,8 @@ class Budget < ActiveRecord::Base PHASES = %w(drafting accepting reviewing selecting valuating balloting reviewing_ballots finished).freeze + ON_HOLD_PHASES = %w(reviewing valuating reviewing_ballots).freeze + CURRENCY_SYMBOLS = %w(€ $ £ ¥).freeze validates :name, presence: true, uniqueness: true @@ -19,17 +21,17 @@ class Budget < ActiveRecord::Base before_validation :sanitize_descriptions - scope :on_hold, -> { where(phase: %w(reviewing valuating reviewing_ballots")) } - scope :drafting, -> { where(phase: "drafting") } + scope :on_hold, -> { where(phase: ON_HOLD_PHASES) } + scope :drafting, -> { where(phase: "drafting") } scope :accepting, -> { where(phase: "accepting") } scope :reviewing, -> { where(phase: "reviewing") } scope :selecting, -> { where(phase: "selecting") } scope :valuating, -> { where(phase: "valuating") } scope :balloting, -> { where(phase: "balloting") } scope :reviewing_ballots, -> { where(phase: "reviewing_ballots") } - scope :finished, -> { where(phase: "finished") } + scope :finished, -> { where(phase: "finished") } - scope :current, -> { where.not(phase: "finished") } + scope :current, -> { where.not(phase: "finished") } def description send("description_#{phase}").try(:html_safe) @@ -84,7 +86,7 @@ class Budget < ActiveRecord::Base end def on_hold? - reviewing? || valuating? || reviewing_ballots? + ON_HOLD_PHASES.include?(phase) end def current? From 922318b978b28ea4c7bfa9c814579f4b6d030ab4 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 10 Jan 2018 01:10:57 +0100 Subject: [PATCH 05/11] Add publishing prices phase to budget model, plus translation texts --- app/models/budget.rb | 13 +++++++++---- config/locales/en/budgets.yml | 1 + config/locales/es/budgets.yml | 1 + 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/models/budget.rb b/app/models/budget.rb index 509352372..7fa03db9a 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -3,9 +3,9 @@ class Budget < ActiveRecord::Base include Measurable include Sluggable - PHASES = %w(drafting accepting reviewing selecting valuating balloting - reviewing_ballots finished).freeze - ON_HOLD_PHASES = %w(reviewing valuating reviewing_ballots).freeze + PHASES = %w(drafting accepting reviewing selecting valuating publishing_prices + balloting reviewing_ballots finished).freeze + ON_HOLD_PHASES = %w(reviewing valuating publishing_prices reviewing_ballots).freeze CURRENCY_SYMBOLS = %w(€ $ £ ¥).freeze @@ -27,6 +27,7 @@ class Budget < ActiveRecord::Base scope :reviewing, -> { where(phase: "reviewing") } scope :selecting, -> { where(phase: "selecting") } scope :valuating, -> { where(phase: "valuating") } + scope :publishing_prices, -> { where(phase: "publishing_prices") } scope :balloting, -> { where(phase: "balloting") } scope :reviewing_ballots, -> { where(phase: "reviewing_ballots") } scope :finished, -> { where(phase: "finished") } @@ -65,6 +66,10 @@ class Budget < ActiveRecord::Base phase == "valuating" end + def publishing_prices? + phase == "publishing_prices" + end + def balloting? phase == "balloting" end @@ -120,7 +125,7 @@ class Budget < ActiveRecord::Base case phase when 'accepting', 'reviewing' %w{random} - when 'balloting', 'reviewing_ballots' + when 'publishing_prices', 'balloting', 'reviewing_ballots' %w{random price} else %w{random confidence_score} diff --git a/config/locales/en/budgets.yml b/config/locales/en/budgets.yml index 5aba444ab..1209d9206 100644 --- a/config/locales/en/budgets.yml +++ b/config/locales/en/budgets.yml @@ -34,6 +34,7 @@ en: reviewing: Reviewing projects selecting: Selecting projects valuating: Valuating projects + publishing_prices: Publishing projects prices balloting: Balloting projects reviewing_ballots: Reviewing Ballots finished: Finished budget diff --git a/config/locales/es/budgets.yml b/config/locales/es/budgets.yml index b85f0ec47..bbf668f21 100644 --- a/config/locales/es/budgets.yml +++ b/config/locales/es/budgets.yml @@ -34,6 +34,7 @@ es: reviewing: Revisión interna de proyectos selecting: Fase de apoyos valuating: Evaluación de proyectos + publishing_prices: Publicación de precios balloting: Votación final reviewing_ballots: Votación finalizada finished: Resultados From 6772f94c148a28cf41fe65b1deab812c6b11a2dd Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 10 Jan 2018 01:11:13 +0100 Subject: [PATCH 06/11] Add description publishing prices column to budgets table --- .../20180109175851_add_publishing_prices_phase_to_budget.rb | 5 +++++ db/schema.rb | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20180109175851_add_publishing_prices_phase_to_budget.rb diff --git a/db/migrate/20180109175851_add_publishing_prices_phase_to_budget.rb b/db/migrate/20180109175851_add_publishing_prices_phase_to_budget.rb new file mode 100644 index 000000000..e6181f6e1 --- /dev/null +++ b/db/migrate/20180109175851_add_publishing_prices_phase_to_budget.rb @@ -0,0 +1,5 @@ +class AddPublishingPricesPhaseToBudget < ActiveRecord::Migration + def change + add_column :budgets, :description_publishing_prices, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index d599658c1..d2c5d3450 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: 20180108182839) do +ActiveRecord::Schema.define(version: 20180109175851) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -202,6 +202,7 @@ ActiveRecord::Schema.define(version: 20180108182839) do t.text "description_finished" t.string "slug" t.text "description_drafting" + t.text "description_publishing_prices" end create_table "campaigns", force: :cascade do |t| From 3bc683edce61eb00ad0a1a216be8132050f5f68e Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 10 Jan 2018 01:12:11 +0100 Subject: [PATCH 07/11] Increase budget factory with publishing_price trait & description value --- spec/factories.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/factories.rb b/spec/factories.rb index b4da87586..7372b5ba9 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -228,6 +228,7 @@ FactoryBot.define do description_reviewing "This budget is reviewing" description_selecting "This budget is selecting" description_valuating "This budget is valuating" + description_publishing_prices "This budget is publishing prices" description_balloting "This budget is balloting" description_reviewing_ballots "This budget is reviewing ballots" description_finished "This budget is finished" @@ -252,6 +253,10 @@ FactoryBot.define do phase 'valuating' end + trait :publishing_prices do + phase 'publishing_prices' + end + trait :balloting do phase 'balloting' end @@ -313,7 +318,6 @@ FactoryBot.define do selected true feasibility "feasible" valuation_finished true - end trait :winner do From f3a7de55ea62135ff060e4f3740c3be5211b3b66 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 10 Jan 2018 01:13:34 +0100 Subject: [PATCH 08/11] Rename should_show_price_info? to should_show_price_explanation? at Budget::Investment & usage --- app/models/budget/investment.rb | 2 +- app/views/budgets/investments/_investment_show.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index 4e2e5d0b0..b2cc8cba5 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -245,7 +245,7 @@ class Budget (budget.reviewing_ballots? || budget.finished?) end - def should_show_price_info? + def should_show_price_explanation? feasible? && price_explanation.present? && (budget.balloting? || budget.reviewing_ballots? || budget.finished?) diff --git a/app/views/budgets/investments/_investment_show.html.erb b/app/views/budgets/investments/_investment_show.html.erb index 946952bd9..3b0078bf8 100644 --- a/app/views/budgets/investments/_investment_show.html.erb +++ b/app/views/budgets/investments/_investment_show.html.erb @@ -71,7 +71,7 @@

<%= investment.unfeasibility_explanation %>

<% end %> - <% if investment.should_show_price_info? %> + <% if investment.should_show_price_explanation? %>

<%= t('budgets.investments.show.price_explanation') %>

<%= investment.price_explanation %>

<% end %> From 3563b87399da0075c8a067d173bda3bd442ac7ed Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 10 Jan 2018 01:14:23 +0100 Subject: [PATCH 09/11] Increase Budget Investment model spec for should_show_price & explantion methods --- spec/models/budget/investment_spec.rb | 72 ++++++++++++++++++++------- 1 file changed, 54 insertions(+), 18 deletions(-) diff --git a/spec/models/budget/investment_spec.rb b/spec/models/budget/investment_spec.rb index a04134ce4..63065cfbe 100644 --- a/spec/models/budget/investment_spec.rb +++ b/spec/models/budget/investment_spec.rb @@ -193,37 +193,73 @@ describe Budget::Investment do end end - describe "#should_show_price_info?" do - it "returns true for feasibles if phase is balloting or later and price_explanation is present" do - ["balloting", "reviewing_ballots", "finished"].each do |phase| - budget = create(:budget, phase: phase) - investment = create(:budget_investment, :feasible, budget: budget, price_explanation: "price explanation") + describe "#should_show_price?" do + let(:budget) { create(:budget, :publishing_prices) } + let(:investment) do + create(:budget_investment, :selected, budget: budget) + end - expect(investment.should_show_price_info?).to eq(true) + it "returns true for selected investments which budget's phase is publishing_prices or later" do + Budget::PUBLISHED_PRICES_PHASES.each do |phase| + budget.update(phase: phase) + + expect(investment.should_show_price?).to eq(true) end end it "returns false in any other phase" do - (Budget::PHASES - ["balloting", "reviewing_ballots", "finished"]).each do |phase| - budget = create(:budget, phase: phase) - investment = create(:budget_investment, :feasible, budget: budget, price_explanation: "price explanation") + (Budget::PHASES - Budget::PUBLISHED_PRICES_PHASES).each do |phase| + budget.update(phase: phase) - expect(investment.should_show_price_info?).to eq(false) + expect(investment.should_show_price?).to eq(false) end end - it "returns false if investment is unfeasible" do - budget = create(:budget, phase: "balloting") - investment = create(:budget_investment, :unfeasible, budget: budget, price_explanation: "price explanation") + it "returns false if investment is not selected" do + investment.selected = false - expect(investment.should_show_price_info?).to eq(false) + expect(investment.should_show_price?).to eq(false) end - it "returns false if price_explanation is blank" do - budget = create(:budget, phase: "balloting") - investment = create(:budget_investment, :unfeasible, budget: budget, price_explanation: "") + it "returns false if price is not present" do + investment.price = nil - expect(investment.should_show_price_info?).to eq(false) + expect(investment.should_show_price?).to eq(false) + end + end + + describe "#should_show_price_explanation?" do + let(:budget) { create(:budget, :publishing_prices) } + let(:investment) do + create(:budget_investment, :selected, budget: budget, price_explanation: "because of reasons") + end + + it "returns true for selected with price_explanation & budget in publishing_prices or later" do + Budget::PUBLISHED_PRICES_PHASES.each do |phase| + budget.update(phase: phase) + + expect(investment.should_show_price_explanation?).to eq(true) + end + end + + it "returns false in any other phase" do + (Budget::PHASES - Budget::PUBLISHED_PRICES_PHASES).each do |phase| + budget.update(phase: phase) + + expect(investment.should_show_price_explanation?).to eq(false) + end + end + + it "returns false if investment is not selected" do + investment.selected = false + + expect(investment.should_show_price_explanation?).to eq(false) + end + + it "returns false if price_explanation is not present" do + investment.price_explanation = "" + + expect(investment.should_show_price_explanation?).to eq(false) end end From 21d6ce57c5cd0c3a8751b6abfdde93dfc43bd06e Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 10 Jan 2018 01:15:07 +0100 Subject: [PATCH 10/11] Add published_prices? helper method and phases constant at Budget model --- app/models/budget.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/models/budget.rb b/app/models/budget.rb index 7fa03db9a..6111df301 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -6,6 +6,7 @@ class Budget < ActiveRecord::Base PHASES = %w(drafting accepting reviewing selecting valuating publishing_prices balloting reviewing_ballots finished).freeze ON_HOLD_PHASES = %w(reviewing valuating publishing_prices reviewing_ballots).freeze + PUBLISHED_PRICES_PHASES = %w(publishing_prices balloting reviewing_ballots finished).freeze CURRENCY_SYMBOLS = %w(€ $ £ ¥).freeze @@ -82,6 +83,10 @@ class Budget < ActiveRecord::Base phase == "finished" end + def published_prices? + PUBLISHED_PRICES_PHASES.include?(phase) + end + def balloting_process? balloting? || reviewing_ballots? end From 334091710cf737d0c003d044e95e32040ae34246 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 10 Jan 2018 01:16:04 +0100 Subject: [PATCH 11/11] Refactor price & explanation showing logic at Investment model --- app/models/budget/investment.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index b2cc8cba5..579a39525 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -240,15 +240,11 @@ class Budget end def should_show_price? - feasible? && - selected? && - (budget.reviewing_ballots? || budget.finished?) + selected? && price.present? && budget.published_prices? end def should_show_price_explanation? - feasible? && - price_explanation.present? && - (budget.balloting? || budget.reviewing_ballots? || budget.finished?) + should_show_price? && price_explanation.present? end def formatted_price