Skip to content

Commit

Permalink
FIX: Category.find_by_slug
Browse files Browse the repository at this point in the history
find_by_slug should ensure that the parent actually exists when its
looking for a parent.
  • Loading branch information
danielwaterworth committed Oct 15, 2019
1 parent e83c248 commit 5f5b232
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 5 deletions.
3 changes: 3 additions & 0 deletions app/controllers/categories_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ def destroy
def find_by_slug
params.require(:category_slug)
@category = Category.find_by_slug(params[:category_slug], params[:parent_category_slug])

raise Discourse::NotFound unless @category.present?

if !guardian.can_see?(@category)
if SiteSetting.detailed_404 && group = @category.access_category_via_group
raise Discourse::InvalidAccess.new(
Expand Down
3 changes: 2 additions & 1 deletion app/models/category.rb
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,8 @@ def update_reviewables

def self.find_by_slug(category_slug, parent_category_slug = nil)
if parent_category_slug
parent_category_id = self.where(slug: parent_category_slug, parent_category_id: nil).pluck(:id).first
parent_category_id = self.where(slug: parent_category_slug, parent_category_id: nil).select(:id)

self.where(slug: category_slug, parent_category_id: parent_category_id).first
else
self.where(slug: category_slug, parent_category_id: nil).first
Expand Down
32 changes: 28 additions & 4 deletions spec/models/category_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -742,12 +742,36 @@
end

describe "find_by_slug" do
it "finds with category and sub category" do
category = Fabricate(:category_with_definition, slug: 'awesome-category')
sub_category = Fabricate(:category_with_definition, parent_category_id: category.id, slug: 'awesome-sub-category')
fab!(:category) do
Fabricate(:category_with_definition, slug: 'awesome-category')
end

fab!(:subcategory) do
Fabricate(
:category_with_definition,
parent_category_id: category.id,
slug: 'awesome-sub-category'
)
end

it "finds a category that exists" do
expect(Category.find_by_slug('awesome-category')).to eq(category)
expect(Category.find_by_slug('awesome-sub-category', 'awesome-category')).to eq(sub_category)
end

it "finds a subcategory that exists" do
expect(Category.find_by_slug('awesome-sub-category', 'awesome-category')).to eq(subcategory)
end

it "produces nil if the parent doesn't exist" do
expect(Category.find_by_slug('awesome-sub-category', 'no-such-category')).to eq(nil)
end

it "produces nil if the parent doesn't exist and the requested category is a root category" do
expect(Category.find_by_slug('awesome-category', 'no-such-category')).to eq(nil)
end

it "produces nil if the subcategory doesn't exist" do
expect(Category.find_by_slug('no-such-category', 'awesome-category')).to eq(nil)
end
end

Expand Down

0 comments on commit 5f5b232

Please sign in to comment.