From b51bebb2001959ce80488181aa9ba721a7269f8b Mon Sep 17 00:00:00 2001 From: Nick Borromeo Date: Sat, 8 Feb 2014 14:10:48 -0800 Subject: [PATCH] Extract queries to keep logic in the Categories Model This creates two methods in the Category model. This moves the model logic to the model and just calls the Category class methods in ListController. This also adds tests for the two methods created in the Category model. The motivation for this refactor is the code climate score of the this class and readability of the code. Please enter the commit message for your changes. Lines starting --- app/controllers/list_controller.rb | 7 +++---- app/models/category.rb | 10 ++++++++++ spec/models/category_spec.rb | 15 +++++++++++++++ 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index 6913ea6b862..a1c4c0ed098 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -214,13 +214,12 @@ class ListController < ApplicationController parent_category_id = nil if parent_slug_or_id.present? - parent_category_id = Category.where(slug: parent_slug_or_id).pluck(:id).first || - Category.where(id: parent_slug_or_id.to_i).pluck(:id).first + parent_category_id = Category.query_parent_category(parent_slug_or_id) raise Discourse::NotFound.new if parent_category_id.blank? end - @category = Category.where(slug: slug_or_id, parent_category_id: parent_category_id).includes(:featured_users).first || - Category.where(id: slug_or_id.to_i, parent_category_id: parent_category_id).includes(:featured_users).first + @category = Category.query_category(slug_or_id, parent_category_id) + guardian.ensure_can_see!(@category) raise Discourse::NotFound.new if @category.blank? diff --git a/app/models/category.rb b/app/models/category.rb index 33f27808f4b..25f6c6fbf9b 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -340,6 +340,16 @@ SQL [read_restricted, mapped] end + def self.query_parent_category(parent_slug) + self.where(slug: parent_slug).pluck(:id).first || + self.where(id: parent_slug.to_i).pluck(:id).first + end + + def self.query_category(slug, parent_category_id) + self.where(slug: slug, parent_category_id: parent_category_id).includes(:featured_users).first || + self.where(id: slug.to_i, parent_category_id: parent_category_id).includes(:featured_users).first + end + def uncategorized? id == SiteSetting.uncategorized_category_id end diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index 11f9e32cab8..2aec9b9a3e8 100644 --- a/spec/models/category_spec.rb +++ b/spec/models/category_spec.rb @@ -372,6 +372,21 @@ describe Category do end + describe ".query_parent_category" do + it "should return the parent category id given a parent slug" do + parent_category.name = "Amazing Category" + parent_category.id.should == Category.query_parent_category(parent_category.slug) + end + end + + describe ".query_category" do + it "should return the category" do + category = Fabricate(:category, name: "Amazing Category", parent_category_id: parent_category.id, user: user) + parent_category.name = "Amazing Parent Category" + category.should == Category.query_category(category.slug, parent_category.id) + end + end + end end