From 0d761ddfb1f579b531aa4579f9c061f3cd93c7d0 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Thu, 13 Jul 2017 19:29:30 +0200 Subject: [PATCH 1/5] Hotfixing find the only budget for emails --- app/controllers/budgets/results_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/budgets/results_controller.rb b/app/controllers/budgets/results_controller.rb index e8e80a2340c..e44a38cd9ff 100644 --- a/app/controllers/budgets/results_controller.rb +++ b/app/controllers/budgets/results_controller.rb @@ -14,7 +14,7 @@ def show private def load_budget - @budget = Budget.find_by(id: params[:budget_id]) + @budget = Budget.find_by(id: params[:budget_id]) || Budget.first end def load_heading From 998b4d9e39726ecb491a426aa0a5fd71a76b5281 Mon Sep 17 00:00:00 2001 From: rgarcia Date: Sun, 15 Jan 2017 21:36:03 +0100 Subject: [PATCH 2/5] Load budgets using slugs --- app/controllers/admin/budgets_controller.rb | 5 ++ .../budgets/ballot/lines_controller.rb | 2 +- app/controllers/budgets/ballots_controller.rb | 5 ++ app/controllers/budgets/groups_controller.rb | 11 ++++ .../budgets/investments_controller.rb | 7 ++- app/controllers/budgets_controller.rb | 7 +++ .../budgets/investments_controller.rb | 5 ++ .../budget_investments_controller.rb | 2 +- spec/features/budgets/investments_spec.rb | 56 +++++++++---------- 9 files changed, 69 insertions(+), 31 deletions(-) diff --git a/app/controllers/admin/budgets_controller.rb b/app/controllers/admin/budgets_controller.rb index 771a8c91def..0aa86844933 100644 --- a/app/controllers/admin/budgets_controller.rb +++ b/app/controllers/admin/budgets_controller.rb @@ -6,6 +6,7 @@ class Admin::BudgetsController < Admin::BaseController has_filters %w{open finished}, only: :index + before_action :load_budget load_and_authorize_resource def index @@ -66,4 +67,8 @@ def budget_params params.require(:budget).permit(*valid_attributes, *report_attributes, translation_params(Budget)) end + def load_budget + @budget = Budget.find_by(slug: params[:id]) || Budget.find_by(id: params[:id]) + end + end diff --git a/app/controllers/budgets/ballot/lines_controller.rb b/app/controllers/budgets/ballot/lines_controller.rb index c2e617e08f9..67b53535e02 100644 --- a/app/controllers/budgets/ballot/lines_controller.rb +++ b/app/controllers/budgets/ballot/lines_controller.rb @@ -37,7 +37,7 @@ def line_params end def load_budget - @budget = Budget.find(params[:budget_id]) + @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) end def load_ballot diff --git a/app/controllers/budgets/ballots_controller.rb b/app/controllers/budgets/ballots_controller.rb index b5b63b4aaef..501ad7b2480 100644 --- a/app/controllers/budgets/ballots_controller.rb +++ b/app/controllers/budgets/ballots_controller.rb @@ -1,6 +1,7 @@ module Budgets class BallotsController < ApplicationController before_action :authenticate_user! + before_action :load_budget load_and_authorize_resource :budget before_action :load_ballot after_action :store_referer, only: [:show] @@ -13,6 +14,10 @@ def show private + def load_budget + @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) + end + def load_ballot query = Budget::Ballot.where(user: current_user, budget: @budget) @ballot = @budget.balloting? ? query.first_or_create : query.first_or_initialize diff --git a/app/controllers/budgets/groups_controller.rb b/app/controllers/budgets/groups_controller.rb index f298b5a9377..9c15eef28e1 100644 --- a/app/controllers/budgets/groups_controller.rb +++ b/app/controllers/budgets/groups_controller.rb @@ -1,5 +1,7 @@ module Budgets class GroupsController < ApplicationController + before_action :load_budget + before_action :load_group load_and_authorize_resource :budget load_and_authorize_resource :group, class: "Budget::Group" @@ -9,5 +11,14 @@ class GroupsController < ApplicationController def show end + private + + def load_budget + @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) + end + + def load_group + @group = Budget::Group.find_by(slug: params[:id]) || Budget.find_by(id: params[:id]) + end end end \ No newline at end of file diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb index cc762cff501..0280129af44 100644 --- a/app/controllers/budgets/investments_controller.rb +++ b/app/controllers/budgets/investments_controller.rb @@ -10,6 +10,7 @@ class InvestmentsController < ApplicationController PER_PAGE = 10 before_action :authenticate_user!, except: [:index, :show, :json_data] + before_action :load_budget, except: :json_data load_and_authorize_resource :budget, except: :json_data load_and_authorize_resource :investment, through: :budget, class: "Budget::Investment", @@ -136,7 +137,7 @@ def load_ballot def load_heading if params[:heading_id].present? - @heading = @budget.headings.find(params[:heading_id]) + @heading = @budget.headings.find_by(slug: params[:heading_id]) || @budget.headings.find_by(id: params[:heading_id]) @assigned_heading = @ballot.try(:heading_for_group, @heading.try(:group)) load_map end @@ -154,6 +155,10 @@ def tag_cloud TagCloud.new(Budget::Investment, params[:search]) end + def load_budget + @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) + end + def set_view @view = (params[:view] == "minimal") ? "minimal" : "default" end diff --git a/app/controllers/budgets_controller.rb b/app/controllers/budgets_controller.rb index c1f9e03ef9a..3551848975f 100644 --- a/app/controllers/budgets_controller.rb +++ b/app/controllers/budgets_controller.rb @@ -3,6 +3,7 @@ class BudgetsController < ApplicationController include BudgetsHelper feature_flag :budgets + before_action :load_budget, only: :show load_and_authorize_resource before_action :set_default_budget_filter, only: :show has_filters %w[not_unfeasible feasible unfeasible unselected selected winners], only: :show @@ -19,4 +20,10 @@ def index @banners = Banner.in_section("budgets").with_active end + private + + def load_budget + @budget = Budget.find_by(slug: params[:id]) || Budget.find_by(id: params[:id]) + end + end diff --git a/app/controllers/management/budgets/investments_controller.rb b/app/controllers/management/budgets/investments_controller.rb index 0b30820a20a..aba7f072441 100644 --- a/app/controllers/management/budgets/investments_controller.rb +++ b/app/controllers/management/budgets/investments_controller.rb @@ -1,4 +1,5 @@ class Management::Budgets::InvestmentsController < Management::BaseController + before_action :load_budget load_resource :budget load_resource :investment, through: :budget, class: "Budget::Investment" @@ -60,6 +61,10 @@ def only_verified_users check_verified_user t("management.budget_investments.alert.unverified_user") end + def load_budget + @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) + end + def load_categories @categories = ActsAsTaggableOn::Tag.category.order(:name) end diff --git a/app/controllers/valuation/budget_investments_controller.rb b/app/controllers/valuation/budget_investments_controller.rb index aa29120f147..a024c176004 100644 --- a/app/controllers/valuation/budget_investments_controller.rb +++ b/app/controllers/valuation/budget_investments_controller.rb @@ -65,7 +65,7 @@ def resource_name end def load_budget - @budget = Budget.find(params[:budget_id]) + @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) end def load_investment diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb index eece482510b..6d84ed03af4 100644 --- a/spec/features/budgets/investments_spec.rb +++ b/spec/features/budgets/investments_spec.rb @@ -40,7 +40,7 @@ investments.each do |investment| within("#budget-investments") do expect(page).to have_content investment.title - expect(page).to have_css("a[href='#{budget_investment_path(budget_id: budget.id, id: investment.id)}']", text: investment.title) + expect(page).to have_css("a[href='#{budget_investment_path(budget, id: investment.id)}']", text: investment.title) expect(page).not_to have_content(unfeasible_investment.title) end end @@ -476,7 +476,7 @@ investment3 = create(:budget_investment, heading: heading) investment4 = create(:budget_investment, :feasible, heading: heading) - visit budget_investments_path(budget_id: budget.id, heading_id: heading.id, filter: "unfeasible") + visit budget_investments_path(budget, heading_id: heading.id, filter: "unfeasible") within("#budget-investments") do expect(page).to have_css(".budget-investment", count: 1) @@ -810,7 +810,7 @@ def investments_order scenario "Create with invisible_captcha honeypot field" do login_as(author) - visit new_budget_investment_path(budget_id: budget.id) + visit new_budget_investment_path(budget) select heading.name, from: "budget_investment_heading_id" fill_in "budget_investment_title", with: "I am a bot" @@ -822,14 +822,14 @@ def investments_order expect(page.status_code).to eq(200) expect(page.html).to be_empty - expect(page).to have_current_path(budget_investments_path(budget_id: budget.id)) + expect(page).to have_current_path(budget_investments_path(budget)) end scenario "Create budget investment too fast" do allow(InvisibleCaptcha).to receive(:timestamp_threshold).and_return(Float::INFINITY) login_as(author) - visit new_budget_investment_path(budget_id: budget.id) + visit new_budget_investment_path(budget) select heading.name, from: "budget_investment_heading_id" fill_in "budget_investment_title", with: "I am a bot" @@ -839,13 +839,13 @@ def investments_order click_button "Create Investment" expect(page).to have_content "Sorry, that was too quick! Please resubmit" - expect(page).to have_current_path(new_budget_investment_path(budget_id: budget.id)) + expect(page).to have_current_path(new_budget_investment_path(budget)) end scenario "Create" do login_as(author) - visit new_budget_investment_path(budget_id: budget.id) + visit new_budget_investment_path(budget) select heading.name, from: "budget_investment_heading_id" fill_in "budget_investment_title", with: "Build a skyscraper" @@ -872,7 +872,7 @@ def investments_order scenario "Errors on create" do login_as(author) - visit new_budget_investment_path(budget_id: budget.id) + visit new_budget_investment_path(budget) click_button "Create Investment" expect(page).to have_content error_message end @@ -948,7 +948,7 @@ def investments_order login_as(author) - visit new_budget_investment_path(budget_id: budget.id) + visit new_budget_investment_path(budget) select_options = find("#budget_investment_heading_id").all("option").collect(&:text) expect(select_options.first).to eq("") @@ -964,7 +964,7 @@ def investments_order investment = create(:budget_investment, heading: heading) - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) expect(page).to have_content(investment.title) expect(page).to have_content(investment.description) @@ -984,7 +984,7 @@ def investments_order scenario "Price & explanation is shown when Budget is on published prices phase" do Budget::Phase::PUBLISHED_PRICES_PHASES.each do |phase| budget.update(phase: phase) - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) expect(page).to have_content(investment.formatted_price) expect(page).to have_content(investment.price_explanation) @@ -1003,7 +1003,7 @@ def investments_order scenario "Price & explanation isn't shown when Budget is not on published prices phase" do (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) + visit budget_investment_path(budget, id: investment.id) expect(page).not_to have_content(investment.formatted_price) expect(page).not_to have_content(investment.price_explanation) @@ -1025,7 +1025,7 @@ def investments_order scenario "Price & explanation isn't shown for any Budget's phase" do Budget::Phase::PHASE_KINDS.each do |phase| budget.update(phase: phase) - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) expect(page).not_to have_content(investment.formatted_price) expect(page).not_to have_content(investment.price_explanation) @@ -1044,7 +1044,7 @@ def investments_order Setting["feature.community"] = true investment = create(:budget_investment, heading: heading) - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) expect(page).to have_content "Access the community" Setting["feature.community"] = false @@ -1054,14 +1054,14 @@ def investments_order Setting["feature.community"] = false investment = create(:budget_investment, heading: heading) - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) expect(page).not_to have_content "Access the community" end scenario "Don't display flaggable buttons" do investment = create(:budget_investment, heading: heading) - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) expect(page).not_to have_selector ".js-follow" end @@ -1092,7 +1092,7 @@ def investments_order scenario "Budget in selecting phase" do budget.update(phase: "selecting") - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) expect(page).not_to have_content("Unfeasibility explanation") expect(page).not_to have_content("Price explanation") @@ -1120,14 +1120,14 @@ def investments_order heading: heading, unfeasibility_explanation: "The unfeasible explanation") - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) expect(page).not_to have_content("Unfeasibility explanation") expect(page).not_to have_content("Local government is not competent in this") expect(page).not_to have_content("This investment project has been marked as not feasible "\ "and will not go to balloting phase") - visit budget_investment_path(budget_id: budget.id, id: investment_2.id) + visit budget_investment_path(budget, id: investment_2.id) expect(page).to have_content("Unfeasibility explanation") expect(page).to have_content("The unfeasible explanation") @@ -1147,7 +1147,7 @@ def investments_order group: group, heading: heading) - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) expect(page).to have_content("This investment project has been selected for balloting phase") end @@ -1166,13 +1166,13 @@ def investments_order group: group, heading: heading) - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) expect(page).not_to have_content("Winning investment project") budget.update(phase: "finished") - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) expect(page).to have_content("Winning investment project") end @@ -1189,7 +1189,7 @@ def investments_order group: group, heading: heading) - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) expect(page).to have_content("This investment project has not been selected for balloting phase") end @@ -1205,7 +1205,7 @@ def investments_order group: group, heading: heading) - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) within("aside") do expect(page).to have_content("Investment project") @@ -1225,7 +1225,7 @@ def investments_order heading: heading, unfeasibility_explanation: "Local government is not competent in this matter") - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) expect(page).not_to have_content("Unfeasibility explanation") expect(page).not_to have_content("Local government is not competent in this matter") @@ -1243,7 +1243,7 @@ def investments_order heading: heading, unfeasibility_explanation: "Local government is not competent in this matter") - visit budget_investment_path(budget_id: budget.id, id: investment.id) + visit budget_investment_path(budget, id: investment.id) expect(page).not_to have_content("Unfeasibility explanation") expect(page).not_to have_content("Local government is not competent in this matter") @@ -1647,7 +1647,7 @@ def investments_order investment3 = create(:budget_investment, :selected, :feasible, heading: heading, valuation_finished: true) investment4 = create(:budget_investment, :selected, :feasible, heading: heading, valuation_finished: true) - visit budget_investments_path(budget_id: budget.id, heading_id: heading.id, filter: "unselected") + visit budget_investments_path(budget, heading_id: heading.id, filter: "unselected") within("#budget-investments") do expect(page).to have_css(".budget-investment", count: 1) @@ -1691,7 +1691,7 @@ def investments_order scenario "Do not display vote button for unselected investments in index" do investment = create(:budget_investment, :unselected, heading: heading) - visit budget_investments_path(budget_id: budget.id, heading_id: heading.id, filter: "unselected") + visit budget_investments_path(budget, heading_id: heading.id, filter: "unselected") expect(page).to have_content investment.title expect(page).not_to have_link("Vote") From 4bd20eebf4f9e232ef1e55fc85ce1ffb9bcbb90b Mon Sep 17 00:00:00 2001 From: Julian Herrero Date: Thu, 17 Jan 2019 10:28:22 +0100 Subject: [PATCH 3/5] Use correct param in controller There were some custom routes created using the param[:id] but the Rails routes use the param[:budget_id] by default, so the same controller could be asked for different param keys. --- app/controllers/budgets/results_controller.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/budgets/results_controller.rb b/app/controllers/budgets/results_controller.rb index e44a38cd9ff..36868b07c5f 100644 --- a/app/controllers/budgets/results_controller.rb +++ b/app/controllers/budgets/results_controller.rb @@ -14,7 +14,9 @@ def show private def load_budget - @budget = Budget.find_by(id: params[:budget_id]) || Budget.first + @budget = Budget.find_by(slug: params[:budget_id]) + @budget ||= Budget.find_by(id: params[:budget_id]) + @budget ||= Budget.first end def load_heading From 22076dd95c1dfab462119bb028f11e116e9a6b1b Mon Sep 17 00:00:00 2001 From: Julian Herrero Date: Thu, 17 Jan 2019 10:42:15 +0100 Subject: [PATCH 4/5] Add method find_by_slug_or_id! to Sluggable module Make it easier to find by slug or id for sluggable models. It will raise a 404 HTML Not found error if the resource is not found. --- app/models/concerns/sluggable.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models/concerns/sluggable.rb b/app/models/concerns/sluggable.rb index 8fb308d22d5..bdcdff85662 100644 --- a/app/models/concerns/sluggable.rb +++ b/app/models/concerns/sluggable.rb @@ -7,6 +7,10 @@ module Sluggable def self.find_by_slug_or_id(slug_or_id) find_by_slug(slug_or_id) || find_by_id(slug_or_id) end + + def self.find_by_slug_or_id!(slug_or_id) + find_by_slug(slug_or_id) || find(slug_or_id) + end end def generate_slug From b122302c5884bb0b3a73fb624530e06428111ddd Mon Sep 17 00:00:00 2001 From: Julian Herrero Date: Thu, 17 Jan 2019 10:47:54 +0100 Subject: [PATCH 5/5] Use find instead of find_by_id Better raise a 404 HTML NotFound exception than any other unexpected error. --- .../admin/budget_groups_controller.rb | 4 +- .../admin/budget_headings_controller.rb | 6 +-- .../admin/budget_investments_controller.rb | 2 +- app/controllers/admin/budgets_controller.rb | 4 +- .../budgets/ballot/lines_controller.rb | 2 +- app/controllers/budgets/ballots_controller.rb | 2 +- app/controllers/budgets/groups_controller.rb | 4 +- .../budgets/investments_controller.rb | 4 +- app/controllers/budgets/results_controller.rb | 13 ++--- app/controllers/budgets/stats_controller.rb | 2 +- app/controllers/budgets_controller.rb | 2 +- .../budgets/investments_controller.rb | 2 +- .../budget_investments_controller.rb | 2 +- app/models/budget/investment.rb | 2 +- .../budgets/ballots/lines_controller_spec.rb | 25 ++++++++++ spec/features/admin/budget_groups_spec.rb | 37 ++++++++++++++ spec/features/admin/budget_headings_spec.rb | 50 +++++++++++++++++++ .../features/admin/budget_investments_spec.rb | 28 +++++++++++ spec/features/admin/budgets_spec.rb | 23 +++++++++ spec/features/budgets/ballots_spec.rb | 49 ++++++++++++++++++ spec/features/budgets/budgets_spec.rb | 24 +++++++++ spec/features/budgets/executions_spec.rb | 16 ++++++ spec/features/budgets/groups_spec.rb | 41 ++++++++++++++- spec/features/budgets/investments_spec.rb | 47 +++++++++++++++++ spec/features/budgets/results_spec.rb | 24 +++++++++ spec/features/budgets/stats_spec.rb | 28 +++++++++-- .../management/budget_investments_spec.rb | 29 ++++++++++- .../valuation/budget_investments_spec.rb | 24 +++++++++ spec/models/budget/investment_spec.rb | 24 +++++++++ 29 files changed, 487 insertions(+), 33 deletions(-) create mode 100644 spec/controllers/budgets/ballots/lines_controller_spec.rb diff --git a/app/controllers/admin/budget_groups_controller.rb b/app/controllers/admin/budget_groups_controller.rb index aa87e237345..8e44b21013e 100644 --- a/app/controllers/admin/budget_groups_controller.rb +++ b/app/controllers/admin/budget_groups_controller.rb @@ -46,11 +46,11 @@ def destroy private def load_budget - @budget = Budget.includes(:groups).find(params[:budget_id]) + @budget = Budget.find_by_slug_or_id! params[:budget_id] end def load_group - @group = @budget.groups.find(params[:id]) + @group = @budget.groups.find_by_slug_or_id! params[:id] end def groups_index diff --git a/app/controllers/admin/budget_headings_controller.rb b/app/controllers/admin/budget_headings_controller.rb index 46e46bc623a..659bf676085 100644 --- a/app/controllers/admin/budget_headings_controller.rb +++ b/app/controllers/admin/budget_headings_controller.rb @@ -47,15 +47,15 @@ def destroy private def load_budget - @budget = Budget.includes(:groups).find(params[:budget_id]) + @budget = Budget.find_by_slug_or_id! params[:budget_id] end def load_group - @group = @budget.groups.find(params[:group_id]) + @group = @budget.groups.find_by_slug_or_id! params[:group_id] end def load_heading - @heading = @group.headings.find(params[:id]) + @heading = @group.headings.find_by_slug_or_id! params[:id] end def headings_index diff --git a/app/controllers/admin/budget_investments_controller.rb b/app/controllers/admin/budget_investments_controller.rb index 0ab4a4765d3..fa4608bbc6d 100644 --- a/app/controllers/admin/budget_investments_controller.rb +++ b/app/controllers/admin/budget_investments_controller.rb @@ -87,7 +87,7 @@ def budget_investment_params end def load_budget - @budget = Budget.includes(:groups).find(params[:budget_id]) + @budget = Budget.find_by_slug_or_id! params[:budget_id] end def load_investment diff --git a/app/controllers/admin/budgets_controller.rb b/app/controllers/admin/budgets_controller.rb index 0aa86844933..fa9ad67520d 100644 --- a/app/controllers/admin/budgets_controller.rb +++ b/app/controllers/admin/budgets_controller.rb @@ -6,7 +6,7 @@ class Admin::BudgetsController < Admin::BaseController has_filters %w{open finished}, only: :index - before_action :load_budget + before_action :load_budget, except: [:index, :new, :create] load_and_authorize_resource def index @@ -68,7 +68,7 @@ def budget_params end def load_budget - @budget = Budget.find_by(slug: params[:id]) || Budget.find_by(id: params[:id]) + @budget = Budget.find_by_slug_or_id! params[:id] end end diff --git a/app/controllers/budgets/ballot/lines_controller.rb b/app/controllers/budgets/ballot/lines_controller.rb index 67b53535e02..5d882c51bf8 100644 --- a/app/controllers/budgets/ballot/lines_controller.rb +++ b/app/controllers/budgets/ballot/lines_controller.rb @@ -37,7 +37,7 @@ def line_params end def load_budget - @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) + @budget = Budget.find_by_slug_or_id! params[:budget_id] end def load_ballot diff --git a/app/controllers/budgets/ballots_controller.rb b/app/controllers/budgets/ballots_controller.rb index 501ad7b2480..91286e2ec23 100644 --- a/app/controllers/budgets/ballots_controller.rb +++ b/app/controllers/budgets/ballots_controller.rb @@ -15,7 +15,7 @@ def show private def load_budget - @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) + @budget = Budget.find_by_slug_or_id! params[:budget_id] end def load_ballot diff --git a/app/controllers/budgets/groups_controller.rb b/app/controllers/budgets/groups_controller.rb index 9c15eef28e1..e309a3c8c4b 100644 --- a/app/controllers/budgets/groups_controller.rb +++ b/app/controllers/budgets/groups_controller.rb @@ -14,11 +14,11 @@ def show private def load_budget - @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) + @budget = Budget.find_by_slug_or_id! params[:budget_id] end def load_group - @group = Budget::Group.find_by(slug: params[:id]) || Budget.find_by(id: params[:id]) + @group = @budget.groups.find_by_slug_or_id! params[:id] end end end \ No newline at end of file diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb index 0280129af44..1aea0508591 100644 --- a/app/controllers/budgets/investments_controller.rb +++ b/app/controllers/budgets/investments_controller.rb @@ -137,7 +137,7 @@ def load_ballot def load_heading if params[:heading_id].present? - @heading = @budget.headings.find_by(slug: params[:heading_id]) || @budget.headings.find_by(id: params[:heading_id]) + @heading = @budget.headings.find_by_slug_or_id! params[:heading_id] @assigned_heading = @ballot.try(:heading_for_group, @heading.try(:group)) load_map end @@ -156,7 +156,7 @@ def tag_cloud end def load_budget - @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) + @budget = Budget.find_by_slug_or_id! params[:budget_id] end def set_view diff --git a/app/controllers/budgets/results_controller.rb b/app/controllers/budgets/results_controller.rb index 36868b07c5f..f8d54c2a2f9 100644 --- a/app/controllers/budgets/results_controller.rb +++ b/app/controllers/budgets/results_controller.rb @@ -14,17 +14,14 @@ def show private def load_budget - @budget = Budget.find_by(slug: params[:budget_id]) - @budget ||= Budget.find_by(id: params[:budget_id]) - @budget ||= Budget.first + @budget = Budget.find_by_slug_or_id(params[:budget_id]) || Budget.first end def load_heading - @heading = if params[:heading_id].present? - @budget.headings.find(params[:heading_id]) - else - @budget.headings.first - end + if @budget.present? + headings = @budget.headings + @heading = headings.find_by_slug_or_id(params[:heading_id]) || headings.first + end end end diff --git a/app/controllers/budgets/stats_controller.rb b/app/controllers/budgets/stats_controller.rb index d8ac1e2673a..9649741258e 100644 --- a/app/controllers/budgets/stats_controller.rb +++ b/app/controllers/budgets/stats_controller.rb @@ -13,7 +13,7 @@ def show private def load_budget - @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) + @budget = Budget.find_by_slug_or_id! params[:budget_id] end end diff --git a/app/controllers/budgets_controller.rb b/app/controllers/budgets_controller.rb index 3551848975f..19f1e69beba 100644 --- a/app/controllers/budgets_controller.rb +++ b/app/controllers/budgets_controller.rb @@ -23,7 +23,7 @@ def index private def load_budget - @budget = Budget.find_by(slug: params[:id]) || Budget.find_by(id: params[:id]) + @budget = Budget.find_by_slug_or_id! params[:id] end end diff --git a/app/controllers/management/budgets/investments_controller.rb b/app/controllers/management/budgets/investments_controller.rb index aba7f072441..79a65cf15cc 100644 --- a/app/controllers/management/budgets/investments_controller.rb +++ b/app/controllers/management/budgets/investments_controller.rb @@ -62,7 +62,7 @@ def only_verified_users end def load_budget - @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) + @budget = Budget.find_by_slug_or_id! params[:budget_id] end def load_categories diff --git a/app/controllers/valuation/budget_investments_controller.rb b/app/controllers/valuation/budget_investments_controller.rb index a024c176004..afe4ea6b2b8 100644 --- a/app/controllers/valuation/budget_investments_controller.rb +++ b/app/controllers/valuation/budget_investments_controller.rb @@ -65,7 +65,7 @@ def resource_name end def load_budget - @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) + @budget = Budget.find_by_slug_or_id! params[:budget_id] end def load_investment diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index 2eaf801c12d..59b88f8d5e2 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -113,7 +113,7 @@ def self.filter_params(params) end def self.scoped_filter(params, current_filter) - budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) + budget = Budget.find_by_slug_or_id params[:budget_id] results = Investment.by_budget(budget) results = results.where("cached_votes_up + physical_votes >= ?", diff --git a/spec/controllers/budgets/ballots/lines_controller_spec.rb b/spec/controllers/budgets/ballots/lines_controller_spec.rb new file mode 100644 index 00000000000..641a90a958c --- /dev/null +++ b/spec/controllers/budgets/ballots/lines_controller_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +describe Budgets::Ballot::LinesController do + + describe "#load_budget" do + + it "raises an error if budget slug is not found" do + controller.params[:budget_id] = "wrong_budget" + + expect do + controller.send(:load_budget) + end.to raise_error ActiveRecord::RecordNotFound + end + + it "raises an error if budget id is not found" do + controller.params[:budget_id] = 0 + + expect do + controller.send(:load_budget) + end.to raise_error ActiveRecord::RecordNotFound + end + + end + +end diff --git a/spec/features/admin/budget_groups_spec.rb b/spec/features/admin/budget_groups_spec.rb index e0b7e313248..cc0c75d628f 100644 --- a/spec/features/admin/budget_groups_spec.rb +++ b/spec/features/admin/budget_groups_spec.rb @@ -32,6 +32,43 @@ end + context "Load" do + + let!(:budget) { create(:budget, slug: "budget_slug") } + let!(:group) { create(:budget_group, slug: "group_slug", budget: budget) } + + scenario "finds budget and group by slug" do + visit edit_admin_budget_group_path("budget_slug", "group_slug") + expect(page).to have_content(budget.name) + expect(page).to have_field "Group name", with: group.name + end + + scenario "raises an error if budget slug is not found" do + expect do + visit edit_admin_budget_group_path("wrong_budget", group) + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if budget id is not found" do + expect do + visit edit_admin_budget_group_path(0, group) + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if group slug is not found" do + expect do + visit edit_admin_budget_group_path(budget, "wrong_group") + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if group id is not found" do + expect do + visit edit_admin_budget_group_path(budget, 0) + end.to raise_error ActiveRecord::RecordNotFound + end + + end + context "Index" do scenario "Displaying no groups for budget" do diff --git a/spec/features/admin/budget_headings_spec.rb b/spec/features/admin/budget_headings_spec.rb index b4fd06db7b6..6b633058d25 100644 --- a/spec/features/admin/budget_headings_spec.rb +++ b/spec/features/admin/budget_headings_spec.rb @@ -33,6 +33,56 @@ end + context "Load" do + + let!(:budget) { create(:budget, slug: "budget_slug") } + let!(:group) { create(:budget_group, slug: "group_slug", budget: budget) } + let!(:heading) { create(:budget_heading, slug: "heading_slug", group: group) } + + scenario "finds budget, group and heading by slug" do + visit edit_admin_budget_group_heading_path("budget_slug", "group_slug", "heading_slug") + expect(page).to have_content(budget.name) + expect(page).to have_content(group.name) + expect(page).to have_field "Heading name", with: heading.name + end + + scenario "raises an error if budget slug is not found" do + expect do + visit edit_admin_budget_group_heading_path("wrong_budget", group, heading) + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if budget id is not found" do + expect do + visit edit_admin_budget_group_heading_path(0, group, heading) + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if group slug is not found" do + expect do + visit edit_admin_budget_group_heading_path(budget, "wrong_group", heading) + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if group id is not found" do + expect do + visit edit_admin_budget_group_heading_path(budget, 0, heading) + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if heading slug is not found" do + expect do + visit edit_admin_budget_group_heading_path(budget, group, "wrong_heading") + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if heading id is not found" do + expect do + visit edit_admin_budget_group_heading_path(budget, group, 0) + end.to raise_error ActiveRecord::RecordNotFound + end + end + context "Index" do scenario "Displaying no headings for group" do diff --git a/spec/features/admin/budget_investments_spec.rb b/spec/features/admin/budget_investments_spec.rb index 8dd9743d394..dc5b5d6ed8e 100644 --- a/spec/features/admin/budget_investments_spec.rb +++ b/spec/features/admin/budget_investments_spec.rb @@ -32,6 +32,34 @@ end + context "Load" do + + let(:group) { create(:budget_group, budget: budget) } + let(:heading) { create(:budget_heading, group: group) } + let!(:investment) { create(:budget_investment, heading: heading) } + + before { budget.update(slug: "budget_slug") } + + scenario "finds investments using budget slug" do + visit admin_budget_budget_investments_path("budget_slug") + + expect(page).to have_link investment.title + end + + scenario "raises an error if budget slug is not found" do + expect do + visit admin_budget_budget_investments_path("wrong_budget", investment) + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if budget id is not found" do + expect do + visit admin_budget_budget_investments_path(0, investment) + end.to raise_error ActiveRecord::RecordNotFound + end + + end + context "Index" do scenario "Displaying investments" do diff --git a/spec/features/admin/budgets_spec.rb b/spec/features/admin/budgets_spec.rb index e7c99cfb456..912262afcff 100644 --- a/spec/features/admin/budgets_spec.rb +++ b/spec/features/admin/budgets_spec.rb @@ -24,6 +24,29 @@ end + context "Load" do + + let!(:budget) { create(:budget, slug: "budget_slug") } + + scenario "finds budget by slug" do + visit admin_budget_path("budget_slug") + expect(page).to have_content(budget.name) + end + + scenario "raises an error if budget slug is not found" do + expect do + visit admin_budget_path("wrong_budget") + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if budget id is not found" do + expect do + visit admin_budget_path(0) + end.to raise_error ActiveRecord::RecordNotFound + end + + end + context "Index" do scenario "Displaying no open budgets text" do diff --git a/spec/features/budgets/ballots_spec.rb b/spec/features/budgets/ballots_spec.rb index 0f9bb1c0572..707dbf0daf2 100644 --- a/spec/features/budgets/ballots_spec.rb +++ b/spec/features/budgets/ballots_spec.rb @@ -8,6 +8,55 @@ let!(:california) { create(:budget_heading, group: states, name: "California", price: 1000) } let!(:new_york) { create(:budget_heading, group: states, name: "New York", price: 1000000) } + context "Load" do + + let(:ballot) { create(:budget_ballot, user: user, budget: budget) } + + before do + budget.update(slug: "budget_slug") + ballot.investments << create(:budget_investment, :selected, heading: california) + login_as(user) + end + + scenario "finds ballot using budget slug" do + visit budget_ballot_path("budget_slug") + + expect(page).to have_content("You have voted one investment") + end + + scenario "raises an error if budget slug is not found" do + expect do + visit budget_ballot_path("wrong_budget") + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if budget id is not found" do + expect do + visit budget_ballot_path(0) + end.to raise_error ActiveRecord::RecordNotFound + end + + end + + context "Lines Load" do + + let!(:investment) { create(:budget_investment, :selected, heading: california) } + + before do + create(:budget_ballot, user: user, budget: budget) + budget.update(slug: "budget_slug") + login_as(user) + end + + scenario "finds ballot lines using budget slug", :js do + visit budget_investments_path("budget_slug", states, california) + add_to_ballot(investment) + + within("#sidebar") { expect(page).to have_content investment.title } + end + + end + context "Voting" do before do diff --git a/spec/features/budgets/budgets_spec.rb b/spec/features/budgets/budgets_spec.rb index d6fdf13e099..0f2f0a8bdad 100644 --- a/spec/features/budgets/budgets_spec.rb +++ b/spec/features/budgets/budgets_spec.rb @@ -6,6 +6,30 @@ let(:level_two_user) { create(:user, :level_two) } let(:allowed_phase_list) { ["balloting", "reviewing_ballots", "finished"] } + context "Load" do + + before { budget.update(slug: "budget_slug") } + + scenario "finds budget by slug" do + visit budget_path("budget_slug") + + expect(page).to have_content budget.name + end + + scenario "raises an error if budget slug is not found" do + expect do + visit budget_path("wrong_budget") + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if budget id is not found" do + expect do + visit budget_path(0) + end.to raise_error ActiveRecord::RecordNotFound + end + + end + context "Index" do scenario "Show normal index with links" do diff --git a/spec/features/budgets/executions_spec.rb b/spec/features/budgets/executions_spec.rb index dd8966ed906..3083d1ecafa 100644 --- a/spec/features/budgets/executions_spec.rb +++ b/spec/features/budgets/executions_spec.rb @@ -11,6 +11,22 @@ let!(:investment4) { create(:budget_investment, :winner, heading: heading) } let!(:investment3) { create(:budget_investment, :incompatible, heading: heading) } + scenario "finds budget by id or slug" do + budget.update(slug: "budget_slug") + + visit budget_executions_path("budget_slug") + within(".budgets-stats") { expect(page).to have_content budget.name } + + visit budget_executions_path(budget) + within(".budgets-stats") { expect(page).to have_content budget.name } + + visit budget_executions_path("budget_slug") + within(".budgets-stats") { expect(page).to have_content budget.name } + + visit budget_executions_path(budget) + within(".budgets-stats") { expect(page).to have_content budget.name } + end + scenario "only displays investments with milestones" do create(:milestone, milestoneable: investment1) diff --git a/spec/features/budgets/groups_spec.rb b/spec/features/budgets/groups_spec.rb index 808646733d9..a5fad8eeed1 100644 --- a/spec/features/budgets/groups_spec.rb +++ b/spec/features/budgets/groups_spec.rb @@ -2,8 +2,45 @@ describe "Budget Groups" do - let(:budget) { create(:budget) } - let(:group) { create(:budget_group, budget: budget) } + let(:budget) { create(:budget, slug: "budget_slug") } + let!(:group) { create(:budget_group, slug: "group_slug", budget: budget) } + + context "Load" do + + scenario "finds group using budget slug and group slug" do + visit budget_group_path("budget_slug", "group_slug") + expect(page).to have_content "Select an option" + end + + scenario "finds group using budget id and group id" do + visit budget_group_path(budget, group) + expect(page).to have_content "Select an option" + end + + scenario "raises an error if budget slug is not found" do + expect do + visit budget_group_path("wrong_budget", group) + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if budget id is not found" do + expect do + visit budget_group_path(0, group) + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if group slug is not found" do + expect do + visit budget_group_path(budget, "wrong_group") + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if group id is not found" do + expect do + visit budget_group_path(budget, 0) + end.to raise_error ActiveRecord::RecordNotFound + end + end context "Show" do scenario "Headings are sorted by name" do diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb index 6d84ed03af4..50c0e4d079a 100644 --- a/spec/features/budgets/investments_spec.rb +++ b/spec/features/budgets/investments_spec.rb @@ -26,6 +26,53 @@ it_behaves_like "relationable", Budget::Investment end + context "Load" do + + let(:investment) { create(:budget_investment, heading: heading) } + + before do + budget.update(slug: "budget_slug") + heading.update(slug: "heading_slug") + end + + scenario "finds investment using budget slug" do + visit budget_investment_path("budget_slug", investment) + + expect(page).to have_content investment.title + end + + scenario "raises an error if budget slug is not found" do + expect do + visit budget_investment_path("wrong_budget", investment) + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if budget id is not found" do + expect do + visit budget_investment_path(0, investment) + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "finds investment using heading slug" do + visit budget_investment_path(budget, investment, heading_id: "heading_slug") + + expect(page).to have_content investment.title + end + + scenario "raises an error if heading slug is not found" do + expect do + visit budget_investment_path(budget, investment, heading_id: "wrong_heading") + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if heading id is not found" do + expect do + visit budget_investment_path(budget, investment, heading_id: 0) + end.to raise_error ActiveRecord::RecordNotFound + end + + end + scenario "Index" do investments = [create(:budget_investment, heading: heading), create(:budget_investment, heading: heading), diff --git a/spec/features/budgets/results_spec.rb b/spec/features/budgets/results_spec.rb index f8e00bcb31d..e10e9e7b2a9 100644 --- a/spec/features/budgets/results_spec.rb +++ b/spec/features/budgets/results_spec.rb @@ -52,6 +52,30 @@ end end + scenario "Does not raise error if budget (slug or id) is not found" do + visit budget_results_path("wrong budget") + + within(".budgets-stats") do + expect(page).to have_content "Participatory budget results" + end + + visit budget_results_path(0) + + within(".budgets-stats") do + expect(page).to have_content "Participatory budget results" + end + end + + scenario "Loads budget and heading by slug" do + visit budget_results_path(budget.slug, heading.slug) + + expect(page).to have_selector("a.is-active", text: heading.name) + + within("#budget-investments-compatible") do + expect(page).to have_content investment1.title + end + end + scenario "Load first budget heading if not specified" do other_heading = create(:budget_heading, group: group) other_investment = create(:budget_investment, :winner, heading: other_heading) diff --git a/spec/features/budgets/stats_spec.rb b/spec/features/budgets/stats_spec.rb index 5b9305b3329..d6113c7c0a5 100644 --- a/spec/features/budgets/stats_spec.rb +++ b/spec/features/budgets/stats_spec.rb @@ -2,14 +2,36 @@ describe "Stats" do - let(:budget) { create(:budget) } + let(:budget) { create(:budget, :finished) } let(:group) { create(:budget_group, budget: budget) } let(:heading) { create(:budget_heading, group: group, price: 1000) } + context "Load" do + + before { budget.update(slug: "budget_slug") } + + scenario "finds budget by slug" do + visit budget_stats_path("budget_slug") + + expect(page).to have_content budget.name + end + + scenario "raises an error if budget slug is not found" do + expect do + visit budget_stats_path("wrong_budget") + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if budget id is not found" do + expect do + visit budget_stats_path(0) + end.to raise_error ActiveRecord::RecordNotFound + end + + end + describe "Show" do describe "advanced stats" do - let(:budget) { create(:budget, :finished) } - scenario "advanced stats enabled" do budget.update(advanced_stats_enabled: true) diff --git a/spec/features/management/budget_investments_spec.rb b/spec/features/management/budget_investments_spec.rb index efb00d12f87..14297ab4146 100644 --- a/spec/features/management/budget_investments_spec.rb +++ b/spec/features/management/budget_investments_spec.rb @@ -3,7 +3,7 @@ describe "Budget Investments" do let(:manager) { create(:manager) } - let(:budget) { create(:budget, phase: "selecting", name: "2033") } + let(:budget) { create(:budget, phase: "selecting", name: "2033", slug: "budget_slug") } let(:group) { create(:budget_group, budget: budget, name: "Whole city") } let(:heading) { create(:budget_heading, group: group, name: "Health") } @@ -18,6 +18,33 @@ { "budget_id": "budget_id" }, management = true + context "Load" do + + let(:investment) { create(:budget_investment, budget: budget) } + let(:user) { create(:user, :level_two) } + + before { login_managed_user(user) } + + scenario "finds investment using budget slug" do + visit management_budget_investment_path("budget_slug", investment) + + expect(page).to have_content investment.title + end + + scenario "raises an error if budget slug is not found" do + expect do + visit management_budget_investment_path("wrong_budget", investment) + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if budget id is not found" do + expect do + visit management_budget_investment_path(0, investment) + end.to raise_error ActiveRecord::RecordNotFound + end + + end + context "Create" do before { heading.budget.update(phase: "accepting") } diff --git a/spec/features/valuation/budget_investments_spec.rb b/spec/features/valuation/budget_investments_spec.rb index 03b843e03a5..372389f640b 100644 --- a/spec/features/valuation/budget_investments_spec.rb +++ b/spec/features/valuation/budget_investments_spec.rb @@ -11,6 +11,30 @@ login_as(valuator.user) end + context "Load" do + + before { budget.update(slug: "budget_slug") } + + scenario "finds investment using budget slug" do + visit valuation_budget_budget_investments_path("budget_slug") + + expect(page).to have_content budget.name + end + + scenario "raises an error if budget slug is not found" do + expect do + visit valuation_budget_budget_investments_path("wrong_budget") + end.to raise_error ActiveRecord::RecordNotFound + end + + scenario "raises an error if budget id is not found" do + expect do + visit valuation_budget_budget_investments_path(0) + end.to raise_error ActiveRecord::RecordNotFound + end + + end + scenario "Disabled with a feature flag" do Setting["process.budgets"] = nil expect{ diff --git a/spec/models/budget/investment_spec.rb b/spec/models/budget/investment_spec.rb index 044dead9900..21283d1fe7f 100644 --- a/spec/models/budget/investment_spec.rb +++ b/spec/models/budget/investment_spec.rb @@ -382,6 +382,30 @@ end end + describe "scoped_filter" do + + let!(:budget) { create(:budget, slug: "budget_slug") } + let!(:group) { create(:budget_group, budget: budget) } + let!(:heading) { create(:budget_heading, group: group) } + let!(:investment) { create(:budget_investment, :feasible, heading: heading) } + + it "finds budget by id or slug" do + result = described_class.scoped_filter({budget_id: budget.id}, nil) + expect(result.count).to be 1 + expect(result.first.id).to be investment.id + + result = described_class.scoped_filter({budget_id: "budget_slug"}, nil) + expect(result.count).to be 1 + expect(result.first.id).to be investment.id + end + + it "does not raise error if budget is not found" do + result = described_class.scoped_filter({budget_id: "wrong_budget"}, nil) + expect(result).to be_empty + end + + end + describe "scopes" do describe "valuation_open" do it "returns all investments with false valuation_finished" do