Remove todos when project feature visibility changes

6 files changed, 76 insertions, 10 deletions

diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb
index a3c688f8a14..31ab4fbe49e 100644
--- a/app/services/projects/update_service.rb
+++ b/app/services/projects/update_service.rb
@@ -42,9 +42,18 @@ module Projects
     private
 
     def after_update
+      todos_features_changes = %w(
+        issues_access_level
+        merge_requests_access_level
+        repository_access_level
+      )
+      project_changed_feature_keys = project.project_feature.previous_changes.keys
+
       if project.previous_changes.include?(:visibility_level) && project.private?
         # don't enqueue immediately to prevent todos removal in case of a mistake
         TodosDestroyer::ProjectPrivateWorker.perform_in(1.hour, project.id)
+      elsif (project_changed_feature_keys & todos_features_changes).present?
+        TodosDestroyer::PrivateFeaturesWorker.perform_in(1.hour, project.id)
       end
 
       if project.previous_changes.include?('path')
diff --git a/app/services/todos/destroy/base_service.rb b/app/services/todos/destroy/base_service.rb
index 71cd7921f32..dff5e1f30e5 100644
--- a/app/services/todos/destroy/base_service.rb
+++ b/app/services/todos/destroy/base_service.rb
@@ -18,15 +18,15 @@ module Todos
       end
 
       def todos
-        # overridden in subclasses
+        raise NotImplementedError
       end
 
       def project_ids
-        # overridden in subclasses
+        raise NotImplementedError
       end
 
       def todos_to_remove?
-        # overridden in subclasses
+        raise NotImplementedError
       end
     end
   end
diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb
index 129e5505a21..2ff9f94b718 100644
--- a/app/services/todos/destroy/entity_leave_service.rb
+++ b/app/services/todos/destroy/entity_leave_service.rb
@@ -21,24 +21,30 @@ module Todos
         if entity.private?
           Todo.where(project_id: project_ids, user_id: user_id)
         else
-          Todo.where(target_id: confidential_issues.select(:id), target_type: Issue)
+          project_ids.each do |project_id|
+            TodosDestroyer::PrivateFeaturesWorker.perform_async(project_id, user_id)
+          end
+
+          Todo.where(
+            target_id: confidential_issues.select(:id), target_type: Issue, user_id: user_id
+          )
         end
       end
 
       override :project_ids
       def project_ids
-        if entity.is_a?(Project)
-          entity.id
-        else
+        case entity
+        when Project
+          [entity.id]
+        when Namespace
           Project.select(:id).where(namespace_id: entity.self_and_descendants.select(:id))
         end
       end
 
       override :todos_to_remove?
       def todos_to_remove?
-        return unless entity
-
-        entity.private? || confidential_issues.count > 0
+        # if an entity is provided we want to check always at least private features
+        !!entity
       end
 
       def confidential_issues
diff --git a/app/services/todos/destroy/private_features_service.rb b/app/services/todos/destroy/private_features_service.rb
new file mode 100644
index 00000000000..4d8e2877bfb
--- /dev/null
+++ b/app/services/todos/destroy/private_features_service.rb
@@ -0,0 +1,40 @@
+module Todos
+  module Destroy
+    class PrivateFeaturesService < ::Todos::Destroy::BaseService
+      attr_reader :project_ids, :user_id
+
+      def initialize(project_ids, user_id = nil)
+        @project_ids = project_ids
+        @user_id = user_id
+      end
+
+      def execute
+        ProjectFeature.where(project_id: project_ids).each do |project_features|
+          target_types = []
+          target_types << Issue if private?(project_features.issues_access_level)
+          target_types << MergeRequest if private?(project_features.merge_requests_access_level)
+          target_types << Commit if private?(project_features.repository_access_level)
+
+          next if target_types.empty?
+
+          remove_todos(project_features.project_id, target_types)
+        end
+      end
+
+      private
+
+      def private?(feature_level)
+        feature_level == ProjectFeature::PRIVATE
+      end
+
+      def remove_todos(project_id, target_types)
+        items = Todo.where(project_id: project_id)
+        items = items.where(user_id: user_id) if user_id
+
+        items.where('user_id NOT IN (?)', authorized_users)
+          .where(target_type: target_types)
+          .delete_all
+      end
+    end
+  end
+end
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index 4a6bee8af83..f2651cb54ec 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -76,6 +76,7 @@
 - todos_destroyer:todos_destroyer_confidential_issue
 - todos_destroyer:todos_destroyer_entity_leave
 - todos_destroyer:todos_destroyer_project_private
+- todos_destroyer:todos_destroyer_private_features
 
 - default
 - mailers # ActionMailer::DeliveryJob.queue_name
diff --git a/app/workers/todos_destroyer/private_features_worker.rb b/app/workers/todos_destroyer/private_features_worker.rb
new file mode 100644
index 00000000000..f457d5e0471
--- /dev/null
+++ b/app/workers/todos_destroyer/private_features_worker.rb
@@ -0,0 +1,10 @@
+module TodosDestroyer
+  class PrivateFeaturesWorker
+    include ApplicationWorker
+    include TodosDestroyerQueue
+
+    def perform(project_id, user_id = nil)
+      ::Todos::Destroy::PrivateFeaturesService.new(project_id, user_id).execute
+    end
+  end
+end