Use find instead of find_by_id

Better raise a 404 HTML NotFound exception than any other unexpected error.
This commit is contained in:
Julian Herrero
2019-01-17 10:47:54 +01:00
committed by Javi Martín
parent 22076dd95c
commit b122302c58
29 changed files with 487 additions and 33 deletions

View File

@@ -46,11 +46,11 @@ class Admin::BudgetGroupsController < Admin::BaseController
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

View File

@@ -47,15 +47,15 @@ class Admin::BudgetHeadingsController < Admin::BaseController
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

View File

@@ -87,7 +87,7 @@ class Admin::BudgetInvestmentsController < Admin::BaseController
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

View File

@@ -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 @@ class Admin::BudgetsController < Admin::BaseController
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

View File

@@ -37,7 +37,7 @@ module Budgets
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

View File

@@ -15,7 +15,7 @@ module Budgets
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

View File

@@ -14,11 +14,11 @@ module Budgets
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

View File

@@ -137,7 +137,7 @@ module Budgets
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 @@ module Budgets
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

View File

@@ -14,16 +14,13 @@ module Budgets
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
if @budget.present?
headings = @budget.headings
@heading = headings.find_by_slug_or_id(params[:heading_id]) || headings.first
end
end

View File

@@ -13,7 +13,7 @@ module Budgets
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

View File

@@ -23,7 +23,7 @@ class BudgetsController < ApplicationController
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

View File

@@ -62,7 +62,7 @@ class Management::Budgets::InvestmentsController < Management::BaseController
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

View File

@@ -65,7 +65,7 @@ class Valuation::BudgetInvestmentsController < Valuation::BaseController
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

View File

@@ -113,7 +113,7 @@ class Budget
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 >= ?",

View File

@@ -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

View File

@@ -32,6 +32,43 @@ describe "Admin budget groups" do
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

View File

@@ -33,6 +33,56 @@ describe "Admin budget headings" do
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

View File

@@ -32,6 +32,34 @@ describe "Admin budget investments" do
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

View File

@@ -24,6 +24,29 @@ describe "Admin budgets" do
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

View File

@@ -8,6 +8,55 @@ describe "Ballots" do
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

View File

@@ -6,6 +6,30 @@ describe "Budgets" do
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

View File

@@ -11,6 +11,22 @@ describe "Executions" do
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)

View File

@@ -2,8 +2,45 @@ require "rails_helper"
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

View File

@@ -26,6 +26,53 @@ describe "Budget Investments" do
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),

View File

@@ -52,6 +52,30 @@ describe "Results" do
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)

View File

@@ -2,14 +2,36 @@ require "rails_helper"
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)

View File

@@ -3,7 +3,7 @@ require "rails_helper"
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 @@ describe "Budget Investments" do
{ "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") }

View File

@@ -11,6 +11,30 @@ describe "Valuation budget investments" do
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{

View File

@@ -382,6 +382,30 @@ describe Budget::Investment do
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