FIX: PM should never be allowed to have a category

FIX: TL3 should not be allowed to muck with PM titles

app/controllers/topics_controller.rb

@@ -115,7 +115,8 @@ class TopicsController < ApplicationController
 
     success = false
     Topic.transaction do
-      success = topic.save && topic.change_category_to_id(params[:category_id].to_i)
+      success = topic.save
+      success &= topic.change_category_to_id(params[:category_id].to_i) unless topic.private_message?
     end
 
     # this is used to return the title to the client as it may have been changed by "TextCleaner"

app/models/topic.rb

@@ -513,6 +513,8 @@ class Topic < ActiveRecord::Base
   end
 
   def change_category_to_id(category_id)
+    return false if private_message?
+
     # If the category name is blank, reset the attribute
     if (category_id.nil? || category_id.to_i == 0)
       cat = Category.find_by(id: SiteSetting.uncategorized_category_id)

db/migrate/20140911065449_private_messages_have_no_category_id.rb

@@ -0,0 +1,9 @@
+class PrivateMessageShaveNoCategoryId < ActiveRecord::Migration
+  def up
+    execute "UPDATE topics SET category_id = NULL WHERE category_id IS NOT NULL AND archetype = \'private_message\'"
+    execute "ALTER TABLE topics ADD CONSTRAINT pm_has_no_category CHECK (category_id IS NULL OR archetype <> 'private_message')"
+  end
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/guardian/topic_guardian.rb

@@ -28,7 +28,7 @@ module TopicGuardian
   # Editing Method
   def can_edit_topic?(topic)
     return false if Discourse.static_doc_topic_ids.include?(topic.id) && !is_admin?
-    return true if is_staff? || user.has_trust_level?(TrustLevel[3])
+    return true if is_staff? || (!topic.private_message? && user.has_trust_level?(TrustLevel[3]))
     return false if topic.archived
     is_my_own?(topic)
   end

spec/components/composer_messages_finder_spec.rb

@@ -258,7 +258,7 @@ describe ComposerMessagesFinder do
       end
 
       it "doesn't notify you in a private message" do
-        topic.update_column(:archetype, Archetype.private_message)
+        topic.update_columns(category_id: nil, archetype: Archetype.private_message)
         finder.check_dominating_topic.should be_blank
       end
 

spec/components/guardian_spec.rb

@@ -853,6 +853,13 @@ describe Guardian do
         end
       end
 
+      context 'private message' do
+        it 'returns false at trust level 3' do
+          topic.archetype = 'private_message'
+          Guardian.new(trust_level_3).can_edit?(topic).should eq(false)
+        end
+      end
+
       context 'archived' do
         it 'returns true as a moderator' do
           Guardian.new(moderator).can_edit?(build(:topic, user: user, archived: true)).should == true

spec/fabricators/topic_fabricator.rb

@@ -17,6 +17,7 @@ end
 
 Fabricator(:private_message_topic, from: :topic) do
   user
+  category_id { nil }
   title { sequence(:title) { |i| "This is a private message #{i}" } }
   archetype "private_message"
   topic_allowed_users{|t| [

spec/models/draft_spec.rb

@@ -39,7 +39,7 @@ describe Draft do
     it 'nukes new topic draft after a topic is created' do
       u = Fabricate(:user)
       Draft.set(u, Draft::NEW_TOPIC, 0, 'my draft')
-      t = Fabricate(:topic, user: u)
+      _t = Fabricate(:topic, user: u)
       s = DraftSequence.current(u, Draft::NEW_TOPIC)
       Draft.get(u, Draft::NEW_TOPIC, s).should be_nil
     end
@@ -47,7 +47,7 @@ describe Draft do
     it 'nukes new pm draft after a pm is created' do
       u = Fabricate(:user)
       Draft.set(u, Draft::NEW_PRIVATE_MESSAGE, 0, 'my draft')
-      t = Fabricate(:topic, user: u, archetype: Archetype.private_message)
+      t = Fabricate(:topic, user: u, archetype: Archetype.private_message, category_id: nil)
       s = DraftSequence.current(t.user, Draft::NEW_PRIVATE_MESSAGE)
       Draft.get(u, Draft::NEW_PRIVATE_MESSAGE, s).should be_nil
     end
@@ -55,7 +55,7 @@ describe Draft do
     it 'does not nuke new topic draft after a pm is created' do
       u = Fabricate(:user)
       Draft.set(u, Draft::NEW_TOPIC, 0, 'my draft')
-      t = Fabricate(:topic, user: u, archetype: Archetype.private_message)
+      t = Fabricate(:topic, user: u, archetype: Archetype.private_message, category_id: nil)
       s = DraftSequence.current(t.user, Draft::NEW_TOPIC)
       Draft.get(u, Draft::NEW_TOPIC, s).should == 'my draft'
     end

spec/models/invite_spec.rb

@@ -108,17 +108,18 @@ describe Invite do
   end
 
   context 'an existing user' do
-    let(:topic) { Fabricate(:topic, archetype: Archetype.private_message) }
+    let(:topic) { Fabricate(:topic, category_id: nil, archetype: 'private_message') }
     let(:coding_horror) { Fabricate(:coding_horror) }
     let!(:invite) { topic.invite_by_email(topic.user, coding_horror.email) }
 
-    it "doesn't create an invite" do
+    it "works" do
+      # doesn't create an invite
       invite.should be_blank
-    end
 
-    it "gives the user permission to access the topic" do
+      # gives the user permission to access the topic
       topic.allowed_users.include?(coding_horror).should be_true
     end
+
   end
 
   context '.redeem' do
@@ -254,7 +255,7 @@ describe Invite do
         let!(:user) { invite.redeem }
 
         let(:coding_horror) { User.find_by(username: "CodingHorror") }
-        let(:another_topic) { Fabricate(:topic, archetype: "private_message", user: coding_horror) }
+        let(:another_topic) { Fabricate(:topic, category_id: nil, archetype: "private_message", user: coding_horror) }
 
         it 'adds the user to the topic_users of the first topic' do
           topic.allowed_users.include?(user).should be_true

spec/models/post_alert_observer_spec.rb

@@ -66,7 +66,7 @@ describe PostAlertObserver do
     let(:mention_post) { Fabricate(:post, user: user, raw: 'Hello @eviltrout')}
     let(:topic) do
       topic = mention_post.topic
-      topic.update_column :archetype, Archetype.private_message
+      topic.update_columns archetype: Archetype.private_message, category_id: nil
       topic
     end
 

spec/models/topic_link_spec.rb

@@ -217,7 +217,7 @@ http://b.com/#{'a'*500}
 
   describe 'internal link from pm' do
     it 'works' do
-      pm = Fabricate(:topic, user: user, archetype: 'private_message')
+      pm = Fabricate(:topic, user: user, category_id: nil, archetype: 'private_message')
       pm.posts.create(user: user, raw: "some content")
 
       url = "http://#{test_uri.host}/t/topic-slug/#{topic.id}"

spec/models/topic_participants_summary_spec.rb

@@ -7,7 +7,8 @@ describe TopicParticipantsSummary do
     let(:topic) do
       Fabricate(:topic,
         user: topic_creator,
-        archetype: Archetype::private_message
+        archetype: Archetype::private_message,
+        category_id: nil
       )
     end
 

spec/models/topic_spec.rb

@@ -801,16 +801,22 @@ describe Topic do
     context 'changing category' do
       let(:category) { Fabricate(:category) }
 
-      before do
+      it "creates a new revision" do
         topic.change_category_to_id(category.id)
+        post.revisions.size.should == 1
       end
 
-      it "creates a new revision" do
-        post.revisions.size.should == 1
+      it "does nothing for private messages" do
+        topic.archetype = "private_message"
+        topic.category_id = nil
+
+        topic.change_category_to_id(category.id)
+        topic.category_id.should == nil
       end
 
       context "removing a category" do
         before do
+          topic.change_category_to_id(category.id)
           topic.change_category_to_id(nil)
         end
 

spec/models/user_action_spec.rb

@@ -18,7 +18,7 @@ describe UserAction do
     let(:private_post) { Fabricate(:post) }
     let(:private_topic) do
       topic = private_post.topic
-      topic.update_column(:archetype, Archetype::private_message)
+      topic.update_columns(category_id: nil, archetype: Archetype::private_message)
       topic
     end
 
@@ -136,7 +136,7 @@ describe UserAction do
     context "liking a private message" do
 
       before do
-        post.topic.update_column(:archetype, Archetype::private_message)
+        post.topic.update_columns(category_id: nil, archetype: Archetype::private_message)
       end
 
       it "doesn't add the entry to the stream" do