Merge pull request #3580 from consul/use_find_instead_of_find_by_id

Use find instead of find by

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

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

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

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, except: [:index, :new, :create]
  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_or_id! params[:id]
+ end
+
 end

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_or_id! params[:budget_id]
  end
 
  def load_ballot

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

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_or_id! params[:budget_id]
+ end
+
+ def load_group
+ @group = @budget.groups.find_by_slug_or_id! params[:id]
+ end
  end
 end

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_or_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_or_id! params[:budget_id]
+ end
+
  def set_view
  @view = (params[:view] == "minimal") ? "minimal" : "default"
  end

app/controllers/budgets/results_controller.rb

@@ -14,15 +14,14 @@ def show
  private
 
  def load_budget
- @budget = Budget.find_by(id: params[:budget_id])
+ @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

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

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_or_id! params[:id]
+ end
+
 end

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_or_id! params[:budget_id]
+ end
+
  def load_categories
  @categories = ActsAsTaggableOn::Tag.category.order(:name)
  end

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_or_id! params[:budget_id]
  end
 
  def load_investment

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 >= ?",

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

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

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

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

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