From 5016568b8ad15edca845a0ad6753fb3e08b292d1 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 15 Jan 2018 16:44:31 +0100 Subject: [PATCH 01/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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 From eca971a648e5efd8a69ff39ade7010cb74d3903f Mon Sep 17 00:00:00 2001 From: rgarcia Date: Tue, 16 Jan 2018 19:34:21 +0100 Subject: [PATCH 17/31] Use `current_budget` instead of `Budget.current` --- app/controllers/management/budgets_controller.rb | 2 +- app/controllers/valuation/budgets_controller.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/management/budgets_controller.rb b/app/controllers/management/budgets_controller.rb index 470d1f9fd..b83d843e2 100644 --- a/app/controllers/management/budgets_controller.rb +++ b/app/controllers/management/budgets_controller.rb @@ -19,7 +19,7 @@ class Management::BudgetsController < Management::BaseController end def print_investments - @budget = Budget.current + @budget = current_budget end private diff --git a/app/controllers/valuation/budgets_controller.rb b/app/controllers/valuation/budgets_controller.rb index cf99f95e3..9789ab929 100644 --- a/app/controllers/valuation/budgets_controller.rb +++ b/app/controllers/valuation/budgets_controller.rb @@ -5,7 +5,7 @@ class Valuation::BudgetsController < Valuation::BaseController load_and_authorize_resource def index - @budget = Budget.current + @budget = current_budget if @budget.present? @investments_with_valuation_open = {} @investments_with_valuation_open = @budget.investments From 21cdddcbae55e25be4049ad25c590acb5bed71cc Mon Sep 17 00:00:00 2001 From: rgarcia Date: Tue, 16 Jan 2018 19:44:11 +0100 Subject: [PATCH 18/31] Order budgets by `created_at` instead of by `id` This is a preventive change which will be useful once the rake to migrate from `spending_proposals` to `budget_investments` is complete As after running that migration, old `spending_proposal` budgets will have a newer `id` than the existing budgets. And therefore the last budget will be one of those migrated from the old `spending_proposal` model By ordering by `created_at` and probably updating the `created_at` attribute in the rake that migrates `spending_proposals` to `budget_investments`, we will have a coherent order for budgets --- app/models/budget.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/budget.rb b/app/models/budget.rb index 608599217..418e9aac0 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -33,7 +33,7 @@ class Budget < ActiveRecord::Base scope :open, -> { where.not(phase: "finished") } def self.current - where.not(phase: "drafting").last + where.not(phase: "drafting").order(:created_at).last end def description From a2b950d8dedf21b10b318e39fe90099f1040c46c Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 17 Jan 2018 00:03:23 +0100 Subject: [PATCH 19/31] Fix to_not to not_to on rspec expectations --- spec/features/admin/budgets_spec.rb | 2 +- spec/features/valuation/budgets_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/features/admin/budgets_spec.rb b/spec/features/admin/budgets_spec.rb index 239865317..7b41715fe 100644 --- a/spec/features/admin/budgets_spec.rb +++ b/spec/features/admin/budgets_spec.rb @@ -279,7 +279,7 @@ feature 'Admin budgets' do click_link 'Delete' end - expect(page).to_not have_content 'District 1' + expect(page).not_to have_content 'District 1' end end diff --git a/spec/features/valuation/budgets_spec.rb b/spec/features/valuation/budgets_spec.rb index 4fd821c7e..9057f81d3 100644 --- a/spec/features/valuation/budgets_spec.rb +++ b/spec/features/valuation/budgets_spec.rb @@ -30,8 +30,8 @@ feature 'Valuation budgets' do visit valuation_budgets_path - expect(page).to_not have_content(budget1.name) - expect(page).to_not have_content(budget2.name) + expect(page).not_to have_content(budget1.name) + expect(page).not_to have_content(budget2.name) expect(page).to have_content(budget3.name) end From 2fb1112066007693552c76c6db9a9a972a561c1e Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 17 Jan 2018 00:45:30 +0100 Subject: [PATCH 20/31] Replace existing Time.zone.today.to_date for just Date.current --- spec/factories.rb | 2 +- spec/features/admin/budget_investment_milestones_spec.rb | 8 ++++---- spec/features/admin/poll/shifts_spec.rb | 8 ++++---- spec/features/budgets/investments_spec.rb | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index 77bfda68e..a357a0bb1 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -387,7 +387,7 @@ FactoryBot.define do association :investment, factory: :budget_investment sequence(:title) { |n| "Budget investment milestone #{n} title" } description 'Milestone description' - publication_date Time.zone.today + publication_date Date.current end factory :vote do diff --git a/spec/features/admin/budget_investment_milestones_spec.rb b/spec/features/admin/budget_investment_milestones_spec.rb index 4221d5647..5967d71f5 100644 --- a/spec/features/admin/budget_investment_milestones_spec.rb +++ b/spec/features/admin/budget_investment_milestones_spec.rb @@ -40,12 +40,12 @@ feature 'Admin budget investment milestones' do click_link 'Create new milestone' fill_in 'budget_investment_milestone_description', with: 'New description milestone' - fill_in 'budget_investment_milestone_publication_date', with: Time.zone.today + fill_in 'budget_investment_milestone_publication_date', with: Date.current click_button 'Create milestone' expect(page).to have_content 'New description milestone' - expect(page).to have_content Time.zone.today + expect(page).to have_content Date.current end scenario "Show validation errors on milestone form" do @@ -78,13 +78,13 @@ feature 'Admin budget investment milestones' do expect(page).to have_css("img[alt='#{milestone.image.title}']") fill_in 'budget_investment_milestone_description', with: 'Changed description' - fill_in 'budget_investment_milestone_publication_date', with: Time.zone.today.to_date + fill_in 'budget_investment_milestone_publication_date', with: Date.current fill_in 'budget_investment_milestone_documents_attributes_0_title', with: 'New document title' click_button 'Update milestone' expect(page).to have_content 'Changed description' - expect(page).to have_content Time.zone.today.to_date + expect(page).to have_content Date.current expect(page).to have_link 'Show image' expect(page).to have_link 'New document title' end diff --git a/spec/features/admin/poll/shifts_spec.rb b/spec/features/admin/poll/shifts_spec.rb index bddd1b6b1..8ee8bfe5c 100644 --- a/spec/features/admin/poll/shifts_spec.rb +++ b/spec/features/admin/poll/shifts_spec.rb @@ -14,13 +14,13 @@ feature 'Admin shifts' do booth1 = create(:poll_booth) booth2 = create(:poll_booth) - shift1 = create(:poll_shift, officer: officer, booth: booth1, date: Time.zone.today) + shift1 = create(:poll_shift, officer: officer, booth: booth1, date: Date.current) shift2 = create(:poll_shift, officer: officer, booth: booth2, date: Time.zone.tomorrow) visit new_admin_booth_shift_path(booth1) expect(page).to have_css(".shift", count: 1) - expect(page).to have_content I18n.l(Time.zone.today, format: :long) + expect(page).to have_content I18n.l(Date.current, format: :long) expect(page).to have_content officer.name visit new_admin_booth_shift_path(booth2) @@ -98,11 +98,11 @@ feature 'Admin shifts' do assignment = create(:poll_booth_assignment, poll: poll, booth: booth) officer = create(:poll_officer) - shift1 = create(:poll_shift, :vote_collection_task, officer: officer, booth: booth, date: Time.zone.today) + shift1 = create(:poll_shift, :vote_collection_task, officer: officer, booth: booth, date: Date.current) shift2 = create(:poll_shift, :recount_scrutiny_task, officer: officer, booth: booth, date: Time.zone.tomorrow) vote_collection_dates = (Date.current..poll.ends_at.to_date).to_a - .reject { |date| date == Time.zone.today } + .reject { |date| date == Date.current } .map { |date| I18n.l(date, format: :long) } recount_scrutiny_dates = (poll.ends_at.to_date..poll.ends_at.to_date + 1.week).to_a .reject { |date| date == Time.zone.tomorrow } diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb index 7569d96bc..55abbec7e 100644 --- a/spec/features/budgets/investments_spec.rb +++ b/spec/features/budgets/investments_spec.rb @@ -570,7 +570,7 @@ feature 'Budget Investments' do within("#tab-milestones") do expect(page).to have_content(milestone.description) - expect(page).to have_content(Time.zone.today.to_date) + expect(page).to have_content(Date.current) expect(page.find("#image_#{milestone.id}")['alt']).to have_content image.title expect(page).to have_link document.title end From 952df2947a9df01e35f34636a5eed3dcbb0919ec Mon Sep 17 00:00:00 2001 From: rgarcia Date: Wed, 17 Jan 2018 12:50:17 +0100 Subject: [PATCH 21/31] Duplicate current_budget method in management_base_controller This method is already existent in the application_controller but it seems a little overkill to create a concern just for this method Maybe when we have multiple method it makes sense to create a nice controller. Another option would be to make the management_base_controller extend from the application_controller --- app/controllers/management/base_controller.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/controllers/management/base_controller.rb b/app/controllers/management/base_controller.rb index 66aee5a01..c7610eba9 100644 --- a/app/controllers/management/base_controller.rb +++ b/app/controllers/management/base_controller.rb @@ -5,6 +5,7 @@ class Management::BaseController < ActionController::Base before_action :set_locale helper_method :managed_user + helper_method :current_user private @@ -40,4 +41,7 @@ class Management::BaseController < ActionController::Base I18n.locale = session[:locale] end + def current_budget + Budget.current + end end From 7db0d832d95f96165b12e4e0a9a9bdf029734a01 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 17 Jan 2018 13:02:14 +0100 Subject: [PATCH 22/31] Fix typo at budgets:phases:generate_missing rake task --- lib/tasks/budgets.rake | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/tasks/budgets.rake b/lib/tasks/budgets.rake index 738649538..34805c4d1 100644 --- a/lib/tasks/budgets.rake +++ b/lib/tasks/budgets.rake @@ -22,9 +22,9 @@ namespace :budgets do 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 + prev_phase: budget.phases&.last, + starts_at: budget.phases&.last&.ends_at || Date.current, + ends_at: (budget.phases&.last&.ends_at || Date.current) + 1.month ) end end From 4e5c8f4903a521c6f77e6f622988841590743828 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Mon, 15 Jan 2018 23:19:19 +0100 Subject: [PATCH 23/31] Added js file for alert --- app/assets/javascripts/application.js | 2 ++ app/assets/javascripts/investment_report_alert.js.coffee | 7 +++++++ 2 files changed, 9 insertions(+) create mode 100644 app/assets/javascripts/investment_report_alert.js.coffee diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 611ecc546..561825ccc 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -74,6 +74,7 @@ //= require polls //= require sortable //= require table_sortable +//= require investment_report_alert var initialize_modules = function() { App.Comments.initialize(); @@ -115,6 +116,7 @@ var initialize_modules = function() { App.Polls.initialize(); App.Sortable.initialize(); App.TableSortable.initialize(); + App.InvestmentReportAlert.initialize(); }; $(function(){ diff --git a/app/assets/javascripts/investment_report_alert.js.coffee b/app/assets/javascripts/investment_report_alert.js.coffee new file mode 100644 index 000000000..98b239a55 --- /dev/null +++ b/app/assets/javascripts/investment_report_alert.js.coffee @@ -0,0 +1,7 @@ +App.InvestmentReportAlert = + initialize: -> + $('#js-investment-report-alert').on 'click', -> + if this.checked && $('#budget_investment_feasibility_unfeasible').is(':checked') + confirm(this.dataset.alert + "\n" + this.dataset.notFeasibleAlert); + else if this.checked + confirm(this.dataset.alert); From 14b62601f6c7739f139bb5afd48e3e593bd444b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Mon, 15 Jan 2018 23:19:49 +0100 Subject: [PATCH 24/31] Added alerts data --- app/views/valuation/budget_investments/edit.html.erb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/views/valuation/budget_investments/edit.html.erb b/app/views/valuation/budget_investments/edit.html.erb index 4ecd8748e..7c1d51239 100644 --- a/app/views/valuation/budget_investments/edit.html.erb +++ b/app/views/valuation/budget_investments/edit.html.erb @@ -79,7 +79,11 @@
<%= f.label :valuation_finished do %> - <%= f.check_box :valuation_finished, title: t('valuation.budget_investments.edit.valuation_finished'), label: false %> + <%= f.check_box :valuation_finished, + title: t('valuation.budget_investments.edit.valuation_finished'), + label: false, id: 'js-investment-report-alert', + "data-alert": t("valuation.budget_investments.edit.valuation_finished_alert"), + "data-not-feasible-alert": t("valuation.budget_investments.edit.not_feasible_alert") %> <%= t("valuation.budget_investments.edit.valuation_finished") %> <% end %>
From 4f41dc4d3ac8f25960e6cb76ba002709010878aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Mon, 15 Jan 2018 23:20:55 +0100 Subject: [PATCH 25/31] Added new translations --- config/locales/en/valuation.yml | 4 +++- config/locales/es/valuation.yml | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/config/locales/en/valuation.yml b/config/locales/en/valuation.yml index 761ac2154..d1c267305 100644 --- a/config/locales/en/valuation.yml +++ b/config/locales/en/valuation.yml @@ -68,6 +68,8 @@ en: undefined_feasible: Pending feasible_explanation_html: Feasibility explanation valuation_finished: Valuation finished + valuation_finished_alert: "Are you sure you want to mark this report as completed? If you do it, it can no longer be modified." + not_feasible_alert: "An email will be sent immediately to the author of the project with the report of unfeasibility." duration_html: Time scope internal_comments_html: Internal comments save: Save changes @@ -121,4 +123,4 @@ en: internal_comments_html: Internal comments save: Save changes notice: - valuate: "Dossier updated" \ No newline at end of file + valuate: "Dossier updated" diff --git a/config/locales/es/valuation.yml b/config/locales/es/valuation.yml index 5b4f6efd1..e84b92ad1 100644 --- a/config/locales/es/valuation.yml +++ b/config/locales/es/valuation.yml @@ -68,6 +68,8 @@ es: undefined_feasible: Sin decidir feasible_explanation_html: Informe de inviabilidad (en caso de que lo sea, dato público) valuation_finished: Informe finalizado + valuation_finished_alert: "¿Estás seguro/a de querer marcar este informe como completado? Una vez hecho, no se puede deshacer la acción." + not_feasible_alert: "Un email será enviado inmediatamente al autor del proyecto con el informe de inviabilidad." duration_html: Plazo de ejecución (opcional, dato no público) internal_comments_html: Comentarios y observaciones (para responsables internos, dato no público) save: Guardar Cambios From 4d97d349a1e6c8a5c710d77b920d9ed666265f1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Mon, 15 Jan 2018 23:21:04 +0100 Subject: [PATCH 26/31] Added tests for alert --- .../features/admin/budget_investments_spec.rb | 44 +++++++++++++++++++ spec/features/emails_spec.rb | 2 +- .../valuation/budget_investments_spec.rb | 2 +- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/spec/features/admin/budget_investments_spec.rb b/spec/features/admin/budget_investments_spec.rb index e20ec2b32..627824045 100644 --- a/spec/features/admin/budget_investments_spec.rb +++ b/spec/features/admin/budget_investments_spec.rb @@ -508,6 +508,50 @@ feature 'Admin budget investments' do expect(page).not_to have_content "Refugees, Solidarity" end + scenario "Shows alert when 'Valuation finished' is checked", :js do + budget_investment = create(:budget_investment) + + visit admin_budget_budget_investment_path(budget_investment.budget, budget_investment) + click_link 'Edit dossier' + + expect(page).to have_content 'Valuation finished' + + find_field('budget_investment[valuation_finished]').click + + page.accept_confirm("Are you sure you want to mark this report as completed? If you do it, it can no longer be modified.") + + expect(page).to have_field('budget_investment[valuation_finished]', checked: true) + end + + scenario "Shows alert with unfeasible status when 'Valuation finished' is checked", :js do + budget_investment = create(:budget_investment) + + visit admin_budget_budget_investment_path(budget_investment.budget, budget_investment) + click_link 'Edit dossier' + + expect(page).to have_content 'Valuation finished' + + find_field('budget_investment_feasibility_unfeasible').click + find_field('budget_investment[valuation_finished]').click + + page.accept_confirm("Are you sure you want to mark this report as completed? If you do it, it can no longer be modified.\nAn email will be sent immediately to the author of the project with the report of unfeasibility.") + + expect(page).to have_field('budget_investment[valuation_finished]', checked: true) + end + + scenario "Undoes check in 'Valuation finished' if user clicks 'cancel' on alert", :js do + budget_investment = create(:budget_investment) + + visit admin_budget_budget_investment_path(budget_investment.budget, budget_investment) + click_link 'Edit dossier' + + dismiss_confirm do + find_field('budget_investment[valuation_finished]').click + end + + expect(page).to have_field('budget_investment[valuation_finished]', checked: false) + end + scenario "Errors on update" do budget_investment = create(:budget_investment) diff --git a/spec/features/emails_spec.rb b/spec/features/emails_spec.rb index f4bd7e08e..16be1cbf4 100644 --- a/spec/features/emails_spec.rb +++ b/spec/features/emails_spec.rb @@ -396,7 +396,7 @@ feature 'Emails' do choose 'budget_investment_feasibility_unfeasible' fill_in 'budget_investment_unfeasibility_explanation', with: 'This is not legal as stated in Article 34.9' - check 'budget_investment_valuation_finished' + find_field('budget_investment[valuation_finished]').click click_button 'Save changes' expect(page).to have_content "Dossier updated" diff --git a/spec/features/valuation/budget_investments_spec.rb b/spec/features/valuation/budget_investments_spec.rb index 7bd955e0e..33ab47b8e 100644 --- a/spec/features/valuation/budget_investments_spec.rb +++ b/spec/features/valuation/budget_investments_spec.rb @@ -345,7 +345,7 @@ feature 'Valuation budget investments' do visit valuation_budget_budget_investment_path(@budget, @investment) click_link 'Edit dossier' - check 'budget_investment_valuation_finished' + find_field('budget_investment[valuation_finished]').click click_button 'Save changes' visit valuation_budget_budget_investments_path(@budget) From a6b06ca7307cb7ca233380768f88d8337f20a582 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 17 Jan 2018 14:44:33 +0100 Subject: [PATCH 27/31] Improve Budget's homepage * Add budget's phases number status * Add budget's headings and groups list * Placeholders for future improvements --- app/controllers/budgets_controller.rb | 1 + app/models/budget/phase.rb | 1 + app/views/budgets/index.html.erb | 149 ++++++++++++++++++---- config/locales/en/budgets.yml | 8 ++ config/locales/es/budgets.yml | 8 ++ spec/features/budgets/budgets_spec.rb | 37 +++++- spec/features/budgets/investments_spec.rb | 3 +- 7 files changed, 181 insertions(+), 26 deletions(-) diff --git a/app/controllers/budgets_controller.rb b/app/controllers/budgets_controller.rb index 4f215c46f..c0cb86565 100644 --- a/app/controllers/budgets_controller.rb +++ b/app/controllers/budgets_controller.rb @@ -15,6 +15,7 @@ class BudgetsController < ApplicationController def index @budgets = @budgets.order(:created_at) + @budget = current_budget end end diff --git a/app/models/budget/phase.rb b/app/models/budget/phase.rb index 903c698aa..5475d97be 100644 --- a/app/models/budget/phase.rb +++ b/app/models/budget/phase.rb @@ -21,6 +21,7 @@ class Budget after_save :adjust_date_ranges scope :enabled, -> { where(enabled: true) } + scope :published, -> { enabled.where.not(kind: 'drafting') } scope :drafting, -> { find_by_kind('drafting') } scope :accepting, -> { find_by_kind('accepting')} scope :reviewing, -> { find_by_kind('reviewing')} diff --git a/app/views/budgets/index.html.erb b/app/views/budgets/index.html.erb index 0ccc8bcef..fbda297cc 100644 --- a/app/views/budgets/index.html.erb +++ b/app/views/budgets/index.html.erb @@ -3,32 +3,135 @@ <%= render "shared/canonical", href: budgets_url %> <% end %> -<%= render "shared/section_header", i18n_namespace: "budgets.index.section_header", image: "budgets" %> +
+
+
-
-
- - - - - - - - - <% @budgets.each do |budget| %> - <% if budget_published?(budget) %> - - - - +

<%= @budget.name %>

+ + <%= safe_html_with_links @budget.description %> + <%= link_to t("budgets.index.section_header.help"), "#section_help" %> + +
+

+ <% published_phases = @budget.phases.published %> + <% current_phase_number = published_phases.index(@budget.current_phase) + 1 || 0 %> + <% phases_progress_numbers = "(#{current_phase_number}/#{published_phases.count})" %> + <%= t('budgets.show.phase') %> <%= phases_progress_numbers %> +

+

<%= t("budgets.phase.#{@budget.phase}") %>

+ + <%= link_to t("budgets.index.section_header.all_phases"), "#all_phases" %> + + <% if @budget.accepting? %> + <% if current_user %> + <% if current_user.level_two_or_three_verified? %> + <%= link_to t("budgets.investments.index.sidebar.create"), + new_budget_investment_path(@budget), + class: "button margin-top expanded" %> + <% else %> +
+ <%= t("budgets.investments.index.sidebar.verified_only", + verify: link_to(t("budgets.investments.index.sidebar.verify_account"), + verification_path)).html_safe %> +
+ <% end %> + <% else %> +
+ <%= t("budgets.investments.index.sidebar.not_logged_in", + sign_in: link_to(t("budgets.investments.index.sidebar.sign_in"), + new_user_session_path), + sign_up: link_to(t("budgets.investments.index.sidebar.sign_up"), + new_user_registration_path)).html_safe %> +
<% end %> <% end %> -
-
<%= Budget.human_attribute_name(:name) %><%= Budget.human_attribute_name(:phase) %>
- <%= link_to budget.name, budget %> - - <%= budget.translated_phase %> -
+ + + <% if @budget.finished? || (@budget.balloting? && can?(:read_results, @budget)) %> + <%= link_to t("budgets.show.see_results"), + budget_results_path(@budget, heading_id: @budget.headings.first), + class: "button margin-top expanded" %> + <% end %> +
+
+
+ +
+
+ +
+ <% @budget.groups.each do |group| %> +

<%= group.name %>

+
+ <% group.headings.each do |heading| %> +
+ <%= link_to heading.name, + budget_investments_path(@budget.id, heading_id: heading.id) %> + <%= @budget.formatted_heading_price(heading) %> +
+ <% end %> +
+ <% end %> +
+ +
+

<%= t("budgets.index.map") %>

+
+ +
+ <%= link_to t("budgets.index.investment_proyects"), + budget_investments_path(@budget.id) %>
+ <%= link_to t("budgets.index.unfeasible_investment_proyects"), + budget_investments_path(budget_id: @budget.id, filter: 'unfeasible') %>
+ <%= link_to t("budgets.index.not_selected_investment_proyects"), + budget_investments_path(budget_id: @budget.id, filter: 'unselected') %> +
+ +
+

<%= t("budgets.index.all_phases") %>

+ <% @budget.phases.published.pluck(:kind) do |phase_kind| %> +
+
+

<%= t("budgets.phase.#{phase_kind}") %>

+ <%= @budget.description_for_phase(phase_kind) %> +
+
+ <% end %> +
+ + +
+
+
+
    +
  • + <%= link_to "#other_budgets" do %> +

    + <%= t("budgets.index.finished_budgets") %> +

    + <% end %> +
    • +
+
+
+ +
+ <% @budgets.each do |b| %> + <% if budget_published?(b) %> +
+
+ <%= b.name %> + <%= b.description %> +
+
+ <%= link_to t("budgets.index.see_results"), budget_results_path(b.id) %> +
+
+ <% end %> + <% end %> +
+

diff --git a/config/locales/en/budgets.yml b/config/locales/en/budgets.yml index 7f25d243f..ea5730bfa 100644 --- a/config/locales/en/budgets.yml +++ b/config/locales/en/budgets.yml @@ -44,6 +44,14 @@ en: icon_alt: Participatory budgets icon title: Participatory budgets help: Help about participatory budgets + all_phases: See all phases + all_phases: Budget investment's phases + map: Budget investments' proposals located geographically + investment_proyects: List of all investment projects + unfeasible_investment_proyects: List of all unfeasible investment projects + not_selected_investment_proyects: List of all investment projects not selected for balloting + finished_budgets: Finished participatory budgets + see_results: See results section_footer: title: Help about participatory budgets description: With the participatory budgets the citizens decide to which projects presented by the neighbors is destined a part of the municipal budget. diff --git a/config/locales/es/budgets.yml b/config/locales/es/budgets.yml index c7f84a3a9..326ddabbe 100644 --- a/config/locales/es/budgets.yml +++ b/config/locales/es/budgets.yml @@ -44,6 +44,14 @@ es: icon_alt: Icono de Presupuestos participativos title: Presupuestos participativos help: Ayuda sobre presupuestos participativos + all_phases: Ver todas las fases + all_phases: Fases de los presupuestos participativos + map: Propuestas de los presupuestos participativos localizables geográficamente + investment_proyects: Ver lista completa de proyectos de inversión + unfeasible_investment_proyects: Ver lista de proyectos de inversión inviables + not_selected_investment_proyects: Ver lista de proyectos de inversión no seleccionados para la votación final + finished_budgets: Presupuestos participativos terminados + see_results: Ver resultados section_footer: title: Ayuda sobre presupuestos participativos description: Con los presupuestos participativos la ciudadanía decide a qué proyectos presentados por los vecinos y vecinas va destinada una parte del presupuesto municipal. diff --git a/spec/features/budgets/budgets_spec.rb b/spec/features/budgets/budgets_spec.rb index ee537a854..bb7ab2e91 100644 --- a/spec/features/budgets/budgets_spec.rb +++ b/spec/features/budgets/budgets_spec.rb @@ -7,8 +7,42 @@ feature 'Budgets' do scenario 'Index' do budgets = create_list(:budget, 3) + last_budget = budgets.last + group1 = create(:budget_group, budget: last_budget) + group2 = create(:budget_group, budget: last_budget) + + heading1 = create(:budget_heading, group: group1) + heading2 = create(:budget_heading, group: group2) + visit budgets_path - budgets.each {|budget| expect(page).to have_link(budget.name)} + + within("#budget_heading") do + expect(page).to have_content(last_budget.name) + expect(page).to have_content(last_budget.description) + expect(page).to have_content("Actual phase (1/8)") + expect(page).to have_content("Accepting projects") + expect(page).to have_link 'Help about participatory budgets' + expect(page).to have_link 'See all phases' + end + + last_budget.update_attributes(phase: 'publishing_prices') + visit budgets_path + + within("#budget_heading") do + expect(page).to have_content("Actual phase (5/8)") + end + + within('#budget_info') do + expect(page).to have_content group1.name + expect(page).to have_content group2.name + expect(page).to have_content heading1.name + expect(page).to have_content last_budget.formatted_heading_price(heading1) + expect(page).to have_content heading2.name + expect(page).to have_content last_budget.formatted_heading_price(heading2) + + expect(page).to have_content budgets.first.name + expect(page).to have_content budgets[2].name + end end context 'Show' do @@ -70,6 +104,7 @@ feature 'Budgets' do background do logout budget.update(phase: 'drafting') + create(:budget) end context "Listed" do diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb index 55abbec7e..00f10e111 100644 --- a/spec/features/budgets/investments_spec.rb +++ b/spec/features/budgets/investments_spec.rb @@ -778,8 +778,7 @@ feature 'Budget Investments' do visit root_path first(:link, "Participatory budgeting").click - click_link budget.name - click_link "Health" + click_link "More hospitals" within("#budget_investment_#{sp1.id}") do expect(page).to have_content sp1.title From 8486c183df776ba1695746c6e65db74cd06ace4a Mon Sep 17 00:00:00 2001 From: decabeza Date: Wed, 17 Jan 2018 18:36:39 +0100 Subject: [PATCH 28/31] improves name on each method --- app/views/budgets/index.html.erb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/views/budgets/index.html.erb b/app/views/budgets/index.html.erb index fbda297cc..da259a202 100644 --- a/app/views/budgets/index.html.erb +++ b/app/views/budgets/index.html.erb @@ -117,15 +117,15 @@

- <% @budgets.each do |b| %> - <% if budget_published?(b) %> + <% @budgets.each do |budget| %> + <% if budget_published?(budget) %>
- <%= b.name %> - <%= b.description %> + <%= budget.name %> + <%= budget.description %>
- <%= link_to t("budgets.index.see_results"), budget_results_path(b.id) %> + <%= link_to t("budgets.index.see_results"), budget_results_path(budget.id) %>
<% end %> From 7cbb1dd3a7281fd7a9afb3a9c6026ef0eb6f87ae Mon Sep 17 00:00:00 2001 From: decabeza Date: Wed, 17 Jan 2018 18:36:58 +0100 Subject: [PATCH 29/31] adds comments to pending content --- app/views/budgets/index.html.erb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/views/budgets/index.html.erb b/app/views/budgets/index.html.erb index da259a202..366e33e2c 100644 --- a/app/views/budgets/index.html.erb +++ b/app/views/budgets/index.html.erb @@ -77,6 +77,10 @@

<%= t("budgets.index.map") %>

+
@@ -90,14 +94,10 @@

<%= t("budgets.index.all_phases") %>

- <% @budget.phases.published.pluck(:kind) do |phase_kind| %> -
-
-

<%= t("budgets.phase.#{phase_kind}") %>

- <%= @budget.description_for_phase(phase_kind) %> -
-
- <% end %> +
From 9cfd60afc24920b3f8b9e459606c1e6560fee092 Mon Sep 17 00:00:00 2001 From: decabeza Date: Wed, 17 Jan 2018 20:08:02 +0100 Subject: [PATCH 30/31] adds styles to budgets homepage --- app/assets/stylesheets/participation.scss | 82 ++++++++--- app/views/budgets/index.html.erb | 161 ++++++++++++---------- 2 files changed, 148 insertions(+), 95 deletions(-) diff --git a/app/assets/stylesheets/participation.scss b/app/assets/stylesheets/participation.scss index 606de72f3..a8a593dab 100644 --- a/app/assets/stylesheets/participation.scss +++ b/app/assets/stylesheets/participation.scss @@ -5,7 +5,7 @@ // 03. Show participation // 04. List participation // 05. Featured -// 06. Budget +// 06. Budgets // 07. Proposals successful // 08. Polls // 09. Polls results and stats @@ -1096,31 +1096,39 @@ } } -// 06. Budget -// ---------- +// 06. Budgets +// ----------- -.expanded.budget { - background: $budget; +.expanded { - h1, - h2, - p, - .back, - .icon-angle-left { - color: #fff; - } + &.budget { + background: $budget; - .button { - background: #fff; - color: $budget; - } + h1, + h2, + p, + a, + .back, + .icon-angle-left { + color: #fff; + } - .info { - background: #6a2a72; + a { + text-decoration: underline; + } - p { - margin-bottom: 0; - text-transform: uppercase; + .button { + background: #fff; + color: $budget; + } + + .info { + background: #6a2a72; + + p { + margin-bottom: 0; + text-transform: uppercase; + } } } } @@ -1182,7 +1190,7 @@ a { text-decoration: underline; - } + } .button { background: #fff; @@ -1212,6 +1220,36 @@ } } +.groups-and-headings { + + .heading { + border: 1px solid $border; + border-radius: rem-calc(3); + display: inline-block; + margin-bottom: $line-height / 2; + + &:hover { + background: $highlight; + text-decoration: none; + } + + a { + display: block; + padding: $line-height / 2; + + &:hover { + text-decoration: none; + } + } + + span { + color: $text; + display: block; + font-size: $small-font-size; + } + } +} + .progress-votes { position: relative; diff --git a/app/views/budgets/index.html.erb b/app/views/budgets/index.html.erb index 366e33e2c..793ded774 100644 --- a/app/views/budgets/index.html.erb +++ b/app/views/budgets/index.html.erb @@ -57,92 +57,107 @@
-
-
+
+
+
-
- <% @budget.groups.each do |group| %> -

<%= group.name %>

-
- <% group.headings.each do |heading| %> -
- <%= link_to heading.name, - budget_investments_path(@budget.id, heading_id: heading.id) %> - <%= @budget.formatted_heading_price(heading) %> -
- <% end %> -
- <% end %> -
- -
-

<%= t("budgets.index.map") %>

- -
- -
- <%= link_to t("budgets.index.investment_proyects"), - budget_investments_path(@budget.id) %>
- <%= link_to t("budgets.index.unfeasible_investment_proyects"), - budget_investments_path(budget_id: @budget.id, filter: 'unfeasible') %>
- <%= link_to t("budgets.index.not_selected_investment_proyects"), - budget_investments_path(budget_id: @budget.id, filter: 'unselected') %> -
- -
-

<%= t("budgets.index.all_phases") %>

- -
- - -
-
-
-
    -
  • - <%= link_to "#other_budgets" do %> -

    - <%= t("budgets.index.finished_budgets") %> -

    - <% end %> -
    • +
    + <% @budget.groups.each do |group| %> +

    <%= group.name %>

    +
      + <% group.headings.each do |heading| %> +
    • + <%= link_to budget_investments_path(@budget.id, heading_id: heading.id) do %> + <%= heading.name %> + <%= @budget.formatted_heading_price(heading) %> + <% end %> +
      • + <% end %>
    -
    + <% end %>
-
+
+

<%= t("budgets.index.map") %>

+ +
+ +

+ <%= link_to budget_investments_path(@budget.id) do %> + <%= t("budgets.index.investment_proyects") %> + <% end %>
+ <%= link_to budget_investments_path(budget_id: @budget.id, filter: 'unfeasible') do %> + <%= t("budgets.index.unfeasible_investment_proyects") %> + <% end %>
+ <%= link_to budget_investments_path(budget_id: @budget.id, filter: 'unselected') do %> + <%= t("budgets.index.not_selected_investment_proyects") %> + <% end %> +

+ +
+

<%= t("budgets.index.all_phases") %>

+ +
+
+
+ +
+
+
    +
  • + <%= link_to "#other_budgets", class: "active" do %> +

    + <%= t("budgets.index.finished_budgets") %> +

    + <% end %> +
    • +
+ +
<% @budgets.each do |budget| %> <% if budget_published?(budget) %> -
-
- <%= budget.name %> - <%= budget.description %> -
-
- <%= link_to t("budgets.index.see_results"), budget_results_path(budget.id) %> +
+
+
+
+

<%= budget.name %>

+ <%= budget.description %> +
+
+
+ <%= link_to t("budgets.index.see_results"), + budget_results_path(budget.id), + class: "button expanded" %> +
+
+
<% end %> <% end %>
+
-
-

- <%= t("budgets.index.section_footer.title") %> -

-

<%= t("budgets.index.section_footer.description") %>

-

<%= t("budgets.index.section_footer.help_text_1") %>

-

<%= t("budgets.index.section_footer.help_text_2") %>

-

<%= t("budgets.index.section_footer.help_text_3", - org: link_to(setting['org_name'], new_user_registration_path)).html_safe %>

-

<%= t("budgets.index.section_footer.help_text_4") %>

+
+
+
+

+ <%= t("budgets.index.section_footer.title") %> +

+

<%= t("budgets.index.section_footer.description") %>

+

<%= t("budgets.index.section_footer.help_text_1") %>

+

<%= t("budgets.index.section_footer.help_text_2") %>

+

<%= t("budgets.index.section_footer.help_text_3", + org: link_to(setting['org_name'], new_user_registration_path)).html_safe %>

+

<%= t("budgets.index.section_footer.help_text_4") %>

+
From 0950a98ddd73486673592d8bb827cd695a1b3648 Mon Sep 17 00:00:00 2001 From: decabeza Date: Wed, 17 Jan 2018 20:08:21 +0100 Subject: [PATCH 31/31] fixes css class name --- app/assets/stylesheets/layout.scss | 2 +- app/views/polls/_poll_group.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index 854d6b1e6..09902f649 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -368,7 +368,7 @@ a { vertical-align: top; } -.aling-middle { +.align-middle { vertical-align: middle; } diff --git a/app/views/polls/_poll_group.html.erb b/app/views/polls/_poll_group.html.erb index 033aff23e..3e14b3bf9 100644 --- a/app/views/polls/_poll_group.html.erb +++ b/app/views/polls/_poll_group.html.erb @@ -57,7 +57,7 @@
-
+
<%= link_to poll, class: "button hollow expanded" do %> <% if poll.expired? %> <%= t("polls.index.participate_button_expired") %>