From dadb7eaa231f416a3d5fa98706bf0246b5435943 Mon Sep 17 00:00:00 2001
From: Lee Machin <lee@leemach.in>
Date: Wed, 15 May 2013 22:45:52 +0200
Subject: [PATCH] fix crash caused by incorrect query in scope

setting all categories to be secured led to a blank screen on all pages

use stabby lambda for consistency in class

make the test a little more concise

- move the local assignments into let blocks for
reusability

- remove calls to `to_a`, which aren't needed

- use 'be_empty' instead of '[]' to be consistent
with the other matchers in the test

add a test for the `secured` scope with multiple
secured categories
---
 app/models/category.rb       |  5 ++---
 spec/models/category_spec.rb | 23 +++++++++++++++++------
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/app/models/category.rb b/app/models/category.rb
index 6ce6d3afc12..2c563416196 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -29,11 +29,10 @@ class Category < ActiveRecord::Base
 
   scope :latest, ->{ order('topic_count desc') }
 
-  scope :secured, lambda { |guardian|
-    ids = nil
+  scope :secured, ->(guardian = nil) {
     ids = guardian.secure_category_ids if guardian
     if ids.present?
-      where("categories.secure ='f' or categories.id = :cats ", cats: ids)
+      where("categories.secure ='f' or categories.id in (:cats)", cats: ids)
     else
       where("categories.secure ='f'")
     end
diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb
index d52e520b577..cb4f937bee8 100644
--- a/spec/models/category_spec.rb
+++ b/spec/models/category_spec.rb
@@ -19,9 +19,12 @@ describe Category do
   it { should have_many :featured_topics }
 
   describe "security" do
-    it "secures categories correctly" do
-      category = Fabricate(:category)
+    let(:category) { Fabricate(:category) }
+    let(:category_2) { Fabricate(:category) }
+    let(:user) { Fabricate(:user) }
+    let(:group) { Fabricate(:group) }
 
+    it "secures categories correctly" do
       category.secure?.should be_false
 
       category.deny(:all)
@@ -30,10 +33,8 @@ describe Category do
       category.allow(:all)
       category.secure?.should be_false
 
-      user = Fabricate(:user)
-      user.secure_categories.to_a.should == []
+      user.secure_categories.should be_empty
 
-      group = Fabricate(:group)
       group.add(user)
       group.save
 
@@ -41,8 +42,18 @@ describe Category do
       category.save
 
       user.reload
-      user.secure_categories.to_a.should == [category]
+      user.secure_categories.should == [category]
+    end
 
+    it "lists all secured categories correctly" do
+      group.add(user)
+      category.allow(group)
+
+      Category.secured.should == [category]
+
+      category_2.allow(group)
+
+      Category.secured.should =~ [category, category_2]
     end
   end