From 6a4d92756d1558cac03742a950535cfebc822ed1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ana=20Mar=C3=ADa=20Mart=C3=ADnez=20G=C3=B3mez?=
 <ammartinez@suse.de>
Date: Mon, 19 Dec 2016 14:24:55 +0100
Subject: [PATCH] [api] Delete nobody comments without children

When deleting a comment with children, it is not deleted but belongs to
the nobody user. And then when the child comment was deleted, the nobody
comment used to be kept. Now it is also deleted.

Closes https://github.com/openSUSE/open-build-service/issues/2480
---
 src/api/app/models/comment.rb       |  7 +++++++
 src/api/spec/models/comment_spec.rb | 26 ++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/src/api/app/models/comment.rb b/src/api/app/models/comment.rb
index 449224636f2..de150a4cf02 100644
--- a/src/api/app/models/comment.rb
+++ b/src/api/app/models/comment.rb
@@ -8,6 +8,7 @@ class Comment < ApplicationRecord
   validates :body, :commentable, :user, presence: true
 
   after_create :create_notification
+  after_destroy :delete_parent_if_unused
 
   has_many :children, dependent: :destroy, class_name: 'Comment', foreign_key: 'parent_id'
 
@@ -99,4 +100,10 @@ def blank_or_destroy
   def destroy
     super
   end
+
+  private
+
+  def delete_parent_if_unused
+    parent.destroy if parent && parent.user == User.find_nobody! && parent.children.length.zero?
+  end
 end
diff --git a/src/api/spec/models/comment_spec.rb b/src/api/spec/models/comment_spec.rb
index 29c13ff4672..5d3aa26f681 100644
--- a/src/api/spec/models/comment_spec.rb
+++ b/src/api/spec/models/comment_spec.rb
@@ -3,6 +3,8 @@
 RSpec.describe Comment do
   let(:comment_package) { create(:comment_package) }
   let(:comment_package_with_parent) { create(:comment_package, parent: comment_package) }
+  let(:comment_package_with_parent_2) { create(:comment_package, parent: comment_package) }
+  let(:comment_package_with_grandparent) { create(:comment_package, parent: comment_package_with_parent) }
 
   describe "has a valid Factory" do
     it { expect(comment_package).to be_valid }
@@ -66,6 +68,30 @@
       end
     end
 
+    context "with nobody parent and a brother" do
+      before do
+        comment_package_with_parent
+        comment_package_with_parent_2
+        comment_package.blank_or_destroy
+      end
+
+      it 'should be destroyed' do
+        expect { comment_package_with_parent.blank_or_destroy }.to change { Comment.count }.by(-1)
+      end
+    end
+
+    context "with nobody parent, nobody grandparent and no brother" do
+      before do
+        comment_package_with_grandparent
+        comment_package_with_parent.blank_or_destroy
+        comment_package.blank_or_destroy
+      end
+
+      it 'should be destroyed' do
+        expect { comment_package_with_grandparent.blank_or_destroy }.to change { Comment.count }.by(-3)
+      end
+    end
+
     context "with children" do
       before do
         comment_package_with_parent