refactor Category

* move callback bodies to separate methods (easier to test)

app/models/category.rb

@@ -9,23 +9,47 @@ class Category < ActiveRecord::Base
   has_many :category_featured_users
   has_many :featured_users, through: :category_featured_users, source: :user
 
-  validates_presence_of :user_id
+  validates :user_id, presence: true
-  validates_presence_of :name
+  validates :name, presence: true, uniqueness: true
-  validates_uniqueness_of :name
   validate :uncategorized_validator
 
+  before_save :ensure_slug
   after_save :invalidate_site_cache
+  after_create :create_category_definition
   after_destroy :invalidate_site_cache
 
-  scope :popular, lambda { order('topic_count desc') }
+  scope :popular, ->{ order('topic_count desc') }
 
-  def uncategorized_validator
+  delegate :post_template, to: 'self.class'
-    return errors.add(:name, I18n.t(:is_reserved)) if name == SiteSetting.uncategorized_name
+
-    return errors.add(:slug, I18n.t(:is_reserved)) if slug == SiteSetting.uncategorized_name
+  def create_category_definition
+    create_topic(title: I18n.t("category.topic_prefix", category: name), user: user, visible: false)
+    update_column(:topic_id, topic.id)
+    topic.update_column(:category_id, id)
+    topic.posts.create(raw: post_template, user: user)
   end
 
-  # Recalculates `topics_year`, `topics_month`, and `topics_week`
+  def topic_url
-  # for each Category.
+    topic.try(:relative_url)
+  end
+
+  def ensure_slug
+    self.slug = Slug.for(name)
+  end
+
+  # Categories are cached in the site json, so the caches need to be
+  # invalidated whenever the category changes.
+  def invalidate_site_cache
+    Site.invalidate_cache
+  end
+
+  def uncategorized_validator
+    errors.add(:name, I18n.t(:is_reserved)) if name == SiteSetting.uncategorized_name
+    errors.add(:slug, I18n.t(:is_reserved)) if slug == SiteSetting.uncategorized_name
+  end
+
+  # Internal: Update category stats: # of topics in past year, month, week for
+  # all categories.
   def self.update_stats
     topics = Topic
                .select("COUNT(*)")
@@ -41,29 +65,9 @@ class Category < ActiveRecord::Base
                          topics_week = (#{topics_week})")
   end
 
-  def topic_url
+  # Internal: Generate the text of post prompting to enter category
-    topic.try(:relative_url)
+  # description.
-  end
-
-  before_save do
-    self.slug = Slug.for(self.name)
-  end
-
-  after_create do
-    topic = Topic.create!(title: I18n.t("category.topic_prefix", category: name), user: user, visible: false)
-
-    post_contents = I18n.t("category.post_template", replace_paragraph: I18n.t("category.replace_paragraph"))
-    topic.posts.create!(raw: post_contents, user: user)
-    update_column(:topic_id, topic.id)
-    topic.update_column(:category_id, self.id)
-  end
-
   def self.post_template
     I18n.t("category.post_template", replace_paragraph: I18n.t("category.replace_paragraph"))
   end
-
-  # We cache the categories in the site json, so we need to invalidate it when they change
-  def invalidate_site_cache
-    Site.invalidate_cache
-  end
 end

spec/models/category_spec.rb

@@ -3,7 +3,6 @@
 require 'spec_helper'
 
 describe Category do
-
   it { should validate_presence_of :user_id }
   it { should validate_presence_of :name }
 
@@ -20,13 +19,11 @@ describe Category do
   it { should have_many :featured_topics }
 
   describe "uncategorized name" do
-
     let(:category) { Fabricate.build(:category, name: SiteSetting.uncategorized_name) }
 
     it "is invalid to create a category with the reserved name" do
       category.should_not be_valid
     end
-
   end
 
   describe "short name" do
@@ -39,11 +36,9 @@ describe Category do
     it 'has one topic' do
       Topic.where(category_id: category.id).count.should == 1
     end
-
   end
 
   describe 'caching' do
-
     it "invalidates the site cache on creation" do
       Site.expects(:invalidate_cache).once
       Fabricate(:category)
@@ -63,17 +58,14 @@ describe Category do
   end
 
   describe 'non-english characters' do
-
     let(:category) { Fabricate(:category, name: "電車男") }
 
     it "creates a blank slug, this is OK." do
       category.slug.should be_blank
     end
-
   end
 
   describe 'after create' do
-
     before do
       @category = Fabricate(:category)
       @topic = @category.topic
@@ -115,10 +107,7 @@ describe Category do
       @category.topic_url.should be_present
     end
 
-
-
     describe "trying to change the category topic's category" do
-
       before do
         @new_cat = Fabricate(:category, name: '2nd Category', user: @category.user)
         @topic.change_category(@new_cat.name)
@@ -141,7 +130,6 @@ describe Category do
   end
 
   describe 'destroy' do
-
     before do
       @category = Fabricate(:category)
       @category_id = @category.id
@@ -156,17 +144,14 @@ describe Category do
     it 'deletes the forum topic' do
       Topic.exists?(id: @topic_id).should be_false
     end
-
   end
 
   describe 'update_stats' do
-
     before do
       @category = Fabricate(:category)
     end
 
     context 'with regular topics' do
-
       before do
         @category.topics << Fabricate(:topic, user: @category.user)
         Category.update_stats
@@ -184,11 +169,9 @@ describe Category do
       it 'updates topics_year' do
         @category.topics_year.should == 1
       end
-
     end
 
     context 'with deleted topics' do
-
       before do
         @category.topics << Fabricate(:deleted_topic,
                                       user: @category.user)
@@ -207,9 +190,6 @@ describe Category do
       it 'does not count deleted topics for topics_year' do
         @category.topics_year.should == 0
       end
-
     end
-
   end
-
 end