diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 13315eada..557a9c6f4 100644 --- a/app/controllers/topics_controller.rb +++ b/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" diff --git a/app/models/topic.rb b/app/models/topic.rb index 9b35c05de..1ec59f198 100644 --- a/app/models/topic.rb +++ b/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) diff --git a/db/migrate/20140911065449_private_messages_have_no_category_id.rb b/db/migrate/20140911065449_private_messages_have_no_category_id.rb new file mode 100644 index 000000000..8c8c24d25 --- /dev/null +++ b/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 diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index bbec93ee4..f2829fb83 100644 --- a/lib/guardian/topic_guardian.rb +++ b/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 diff --git a/spec/components/composer_messages_finder_spec.rb b/spec/components/composer_messages_finder_spec.rb index cab37bdbb..1321013a9 100644 --- a/spec/components/composer_messages_finder_spec.rb +++ b/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 diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index 1597cc5d3..5ff55a9f4 100644 --- a/spec/components/guardian_spec.rb +++ b/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 diff --git a/spec/fabricators/topic_fabricator.rb b/spec/fabricators/topic_fabricator.rb index 3e2632cda..c11b05e4b 100644 --- a/spec/fabricators/topic_fabricator.rb +++ b/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| [ diff --git a/spec/models/draft_spec.rb b/spec/models/draft_spec.rb index 77cf977e9..38943cfa4 100644 --- a/spec/models/draft_spec.rb +++ b/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 diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index e05a8ed5e..d9e45e62a 100644 --- a/spec/models/invite_spec.rb +++ b/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 diff --git a/spec/models/post_alert_observer_spec.rb b/spec/models/post_alert_observer_spec.rb index a57830c88..3ff4802b3 100644 --- a/spec/models/post_alert_observer_spec.rb +++ b/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 diff --git a/spec/models/topic_link_spec.rb b/spec/models/topic_link_spec.rb index 871ad6430..5e78b51de 100644 --- a/spec/models/topic_link_spec.rb +++ b/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}" diff --git a/spec/models/topic_participants_summary_spec.rb b/spec/models/topic_participants_summary_spec.rb index 58863e614..b70475249 100644 --- a/spec/models/topic_participants_summary_spec.rb +++ b/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 diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 2222d2570..74dbc6dd4 100644 --- a/spec/models/topic_spec.rb +++ b/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 diff --git a/spec/models/user_action_spec.rb b/spec/models/user_action_spec.rb index 07877bf6d..bcfd11f82 100644 --- a/spec/models/user_action_spec.rb +++ b/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