From 0af5972d6364c3ae37da68143202e93358693b5e Mon Sep 17 00:00:00 2001 From: Julian Herrero Date: Thu, 17 Jan 2019 10:28:22 +0100 Subject: [PATCH 1/4] Use correct param in controller --- app/controllers/budgets/executions_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/budgets/executions_controller.rb b/app/controllers/budgets/executions_controller.rb index 25f27b1ca..d4b9cd740 100644 --- a/app/controllers/budgets/executions_controller.rb +++ b/app/controllers/budgets/executions_controller.rb @@ -25,7 +25,7 @@ module Budgets end def load_budget - @budget = Budget.find_by(slug: params[:id]) || Budget.find_by(id: params[:id]) + @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budgetid]) end def investments_by_heading_ordered_alphabetically From 23b6e38915e25668b804efbe83eefc30db9ccca9 Mon Sep 17 00:00:00 2001 From: Julian Herrero Date: Thu, 17 Jan 2019 10:34:30 +0100 Subject: [PATCH 2/4] Remove unused before_action --- app/controllers/management/budgets/investments_controller.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/app/controllers/management/budgets/investments_controller.rb b/app/controllers/management/budgets/investments_controller.rb index 2e879b6e1..ff1e7ee86 100644 --- a/app/controllers/management/budgets/investments_controller.rb +++ b/app/controllers/management/budgets/investments_controller.rb @@ -4,7 +4,6 @@ class Management::Budgets::InvestmentsController < Management::BaseController load_resource :investment, through: :budget, class: 'Budget::Investment' before_action :only_verified_users, except: :print - before_action :load_heading, only: [:index, :show, :print] def index @investments = @investments.apply_filters_and_search(@budget, params).page(params[:page]) @@ -61,10 +60,6 @@ class Management::Budgets::InvestmentsController < Management::BaseController check_verified_user t("management.budget_investments.alert.unverified_user") end - def load_heading - @heading = @budget.headings.find(params[:heading_id]) if params[:heading_id].present? - end - def load_categories @categories = ActsAsTaggableOn::Tag.category.order(:name) end From 9a233935358a94da2f750b5ee750f0d060985792 Mon Sep 17 00:00:00 2001 From: Julian Herrero Date: Thu, 17 Jan 2019 10:35:53 +0100 Subject: [PATCH 3/4] Use find instead of find_by_id This method will raise an exception if resource is not found when trying to call score_action on nil. Prefer to raise a 404 HTML NotFound error instead. --- app/controllers/related_contents_controller.rb | 2 +- .../related_contents_controller_spec.rb | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 spec/controllers/related_contents_controller_spec.rb diff --git a/app/controllers/related_contents_controller.rb b/app/controllers/related_contents_controller.rb index db6be0018..9dc753fee 100644 --- a/app/controllers/related_contents_controller.rb +++ b/app/controllers/related_contents_controller.rb @@ -31,7 +31,7 @@ class RelatedContentsController < ApplicationController private def score(action) - @related = RelatedContent.find_by(id: params[:id]) + @related = RelatedContent.find params[:id] @related.send("score_#{action}", current_user) render template: 'relationable/_refresh_score_actions' diff --git a/spec/controllers/related_contents_controller_spec.rb b/spec/controllers/related_contents_controller_spec.rb new file mode 100644 index 000000000..c792a267e --- /dev/null +++ b/spec/controllers/related_contents_controller_spec.rb @@ -0,0 +1,15 @@ +require "rails_helper" + +describe RelatedContentsController do + + describe "#score" do + it "raises an error if related content does not exist" do + controller.params[:id] = 0 + + expect do + controller.send(:score, "action") + end.to raise_error ActiveRecord::RecordNotFound + end + end + +end From 3bf2fa1b1713f934144d4923b87e591bc8086fd2 Mon Sep 17 00:00:00 2001 From: Julian Herrero Date: Thu, 17 Jan 2019 10:39:55 +0100 Subject: [PATCH 4/4] Add method find_by_slug_or_id to Sluggable module Make it easier to find by slug or id for sluggable models. Will return nil if resource is not found. --- app/controllers/budgets/executions_controller.rb | 2 +- app/models/concerns/sluggable.rb | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/controllers/budgets/executions_controller.rb b/app/controllers/budgets/executions_controller.rb index d4b9cd740..54decebee 100644 --- a/app/controllers/budgets/executions_controller.rb +++ b/app/controllers/budgets/executions_controller.rb @@ -25,7 +25,7 @@ module Budgets end def load_budget - @budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budgetid]) + @budget = Budget.find_by_slug_or_id params[:budget_id] end def investments_by_heading_ordered_alphabetically diff --git a/app/models/concerns/sluggable.rb b/app/models/concerns/sluggable.rb index 495ffaf77..8fb308d22 100644 --- a/app/models/concerns/sluggable.rb +++ b/app/models/concerns/sluggable.rb @@ -3,6 +3,10 @@ module Sluggable included do before_validation :generate_slug, if: :generate_slug? + + def self.find_by_slug_or_id(slug_or_id) + find_by_slug(slug_or_id) || find_by_id(slug_or_id) + end end def generate_slug