FIX: Let users delete topics.

FIX: Let users delete topics.

Follow-up to 31053f30dea876395774fc646c50ba3b638acd97.

diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb
index e58b158..d001ab5 100644
--- a/lib/guardian/topic_guardian.rb
+++ b/lib/guardian/topic_guardian.rb
@@ -95,7 +95,7 @@ module TopicGuardian
 
   def can_delete_topic?(topic)
     !topic.trashed? &&
-    (is_staff? || (topic.posts_count <= 1 && topic.created_at && topic.created_at > 24.hours.ago)) &&
+    (is_staff? || (is_my_own?(topic) && topic.posts_count <= 1 && topic.created_at && topic.created_at > 24.hours.ago)) &&
     !topic.is_category_topic? &&
     !Discourse.static_doc_topic_ids.include?(topic.id)
   end
diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb
index 48d0334..9fa878d 100644
--- a/spec/components/guardian_spec.rb
+++ b/spec/components/guardian_spec.rb
@@ -1747,6 +1747,24 @@ describe Guardian do
         SiteSetting.tos_topic_id = tos_topic.id
         expect(Guardian.new(admin).can_delete?(tos_topic)).to be_falsey
       end
+
+      it "returns true for own topics" do
+        topic.update_attribute(:posts_count, 1)
+        topic.update_attribute(:created_at, Time.zone.now)
+        expect(Guardian.new(topic.user).can_delete?(topic)).to be_truthy
+      end
+
+      it "returns false if delete their own topics" do
+        topic.update_attribute(:posts_count, 2)
+        topic.update_attribute(:created_at, Time.zone.now)
+        expect(Guardian.new(topic.user).can_delete?(topic)).to be_falsey
+      end
+
+      it "returns false if delete their own topics" do
+        topic.update_attribute(:posts_count, 1)
+        topic.update_attribute(:created_at, 48.hours.ago)
+        expect(Guardian.new(topic.user).can_delete?(topic)).to be_falsey
+      end
     end
 
     context 'a Post' do
@@ -1774,13 +1792,8 @@ describe Guardian do
         expect(Guardian.new(Fabricate(:user)).can_delete?(post)).to be_falsey
       end
 
-      it "returns true when it's the OP" do
-        post.update!(post_number: 1)
-        expect(Guardian.new(moderator).can_delete?(post)).to be_falsey
-      end
-
       it "returns false when it's the OP, even as a moderator if there are at least two posts" do
-        post.update!(post_number: 1)
+        post.update_attribute(:post_number, 1)
         Fabricate(:post, topic: post.topic)
         expect(Guardian.new(moderator).can_delete?(post)).to be_falsey
       end

GitHub sha: 034b8a7e

I think the description of the specs can be improved here to reflect the conditions where a user can delete their own topic.

Hmm is there a reason we’re using update_attribute instead of update! here? update! raises an error if the record fails to save while update_attribute will return false which we’re not checking for.

1 Like

We don’t have to split it into two update_attribute calls. There is update! and update_attributes! which takes a hash.

topic.update!(
  posts_count: 2,
  created_at: Time.zone.now
)
1 Like

DEV: Improve tests.