Add latest changes from gitlab-org/gitlab@master

4 files changed, 72 insertions, 61 deletions

diff --git a/app/services/ci/pipeline_schedules/base_save_service.rb b/app/services/ci/pipeline_schedules/base_save_service.rb
new file mode 100644
index 00000000000..45d70e5a65d
--- /dev/null
+++ b/app/services/ci/pipeline_schedules/base_save_service.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+module Ci
+  module PipelineSchedules
+    class BaseSaveService
+      include Gitlab::Utils::StrongMemoize
+
+      def execute
+        schedule.assign_attributes(params)
+
+        return forbidden_to_save unless allowed_to_save?
+        return forbidden_to_save_variables unless allowed_to_save_variables?
+
+        if schedule.save
+          ServiceResponse.success(payload: schedule)
+        else
+          ServiceResponse.error(payload: schedule, message: schedule.errors.full_messages)
+        end
+      end
+
+      private
+
+      attr_reader :project, :user, :params, :schedule
+
+      def allowed_to_save?
+        user.can?(self.class::AUTHORIZE, schedule)
+      end
+
+      def forbidden_to_save
+        # We add the error to the base object too
+        # because model errors are used in the API responses and the `form_errors` helper.
+        schedule.errors.add(:base, authorize_message)
+
+        ServiceResponse.error(payload: schedule, message: [authorize_message], reason: :forbidden)
+      end
+
+      def allowed_to_save_variables?
+        return true if params[:variables_attributes].blank?
+
+        user.can?(:set_pipeline_variables, project)
+      end
+
+      def forbidden_to_save_variables
+        message = _('The current user is not authorized to set pipeline schedule variables')
+
+        # We add the error to the base object too
+        # because model errors are used in the API responses and the `form_errors` helper.
+        schedule.errors.add(:base, message)
+
+        ServiceResponse.error(payload: schedule, message: [message], reason: :forbidden)
+      end
+    end
+  end
+end
diff --git a/app/services/ci/pipeline_schedules/create_service.rb b/app/services/ci/pipeline_schedules/create_service.rb
index c1825865bc0..23775e68399 100644
--- a/app/services/ci/pipeline_schedules/create_service.rb
+++ b/app/services/ci/pipeline_schedules/create_service.rb
@@ -2,46 +2,22 @@
 
 module Ci
   module PipelineSchedules
-    class CreateService
-      def initialize(project, user, params)
-        @project = project
-        @user = user
-        @params = params
+    class CreateService < BaseSaveService
+      AUTHORIZE = :create_pipeline_schedule
 
+      def initialize(project, user, params)
         @schedule = project.pipeline_schedules.new
-      end
-
-      def execute
-        return forbidden unless allowed?
-
-        schedule.assign_attributes(params.merge(owner: user))
-
-        if schedule.save
-          ServiceResponse.success(payload: schedule)
-        else
-          ServiceResponse.error(payload: schedule, message: schedule.errors.full_messages)
-        end
+        @user = user
+        @project = project
+        @params = params.merge(owner: user)
       end
 
       private
 
-      attr_reader :project, :user, :params, :schedule
-
-      def allowed?
-        user.can?(:create_pipeline_schedule, schedule)
-      end
-
-      def forbidden
-        # We add the error to the base object too
-        # because model errors are used in the API responses and the `form_errors` helper.
-        schedule.errors.add(:base, forbidden_message)
-
-        ServiceResponse.error(payload: schedule, message: [forbidden_message], reason: :forbidden)
-      end
-
-      def forbidden_message
+      def authorize_message
         _('The current user is not authorized to create the pipeline schedule')
       end
+      strong_memoize_attr :authorize_message
     end
   end
 end
diff --git a/app/services/ci/pipeline_schedules/update_service.rb b/app/services/ci/pipeline_schedules/update_service.rb
index 28c22e0a868..2fd1173ecce 100644
--- a/app/services/ci/pipeline_schedules/update_service.rb
+++ b/app/services/ci/pipeline_schedules/update_service.rb
@@ -2,44 +2,22 @@
 
 module Ci
   module PipelineSchedules
-    class UpdateService
+    class UpdateService < BaseSaveService
+      AUTHORIZE = :update_pipeline_schedule
+
       def initialize(schedule, user, params)
         @schedule = schedule
         @user = user
+        @project = schedule.project
         @params = params
       end
 
-      def execute
-        return forbidden unless allowed?
-
-        schedule.assign_attributes(params)
-
-        if schedule.save
-          ServiceResponse.success(payload: schedule)
-        else
-          ServiceResponse.error(message: schedule.errors.full_messages)
-        end
-      end
-
       private
 
-      attr_reader :schedule, :user, :params
-
-      def allowed?
-        user.can?(:update_pipeline_schedule, schedule)
-      end
-
-      def forbidden
-        # We add the error to the base object too
-        # because model errors are used in the API responses and the `form_errors` helper.
-        schedule.errors.add(:base, forbidden_message)
-
-        ServiceResponse.error(message: [forbidden_message], reason: :forbidden)
-      end
-
-      def forbidden_message
+      def authorize_message
         _('The current user is not authorized to update the pipeline schedule')
       end
+      strong_memoize_attr :authorize_message
     end
   end
 end
diff --git a/app/services/todos/destroy/group_private_service.rb b/app/services/todos/destroy/group_private_service.rb
index d7ecbb952aa..60599ca9ca4 100644
--- a/app/services/todos/destroy/group_private_service.rb
+++ b/app/services/todos/destroy/group_private_service.rb
@@ -24,7 +24,10 @@ module Todos
 
       override :authorized_users
       def authorized_users
-        group.direct_and_indirect_users.select(:id)
+        User.from_union([
+          group.project_users_with_descendants.select(:id),
+          group.members_with_parents.select(:user_id)
+        ], remove_duplicates: false)
       end
 
       override :todos_to_remove?