Add latest changes from gitlab-org/gitlab@master

18 files changed, 437 insertions, 269 deletions

diff --git a/app/assets/javascripts/registry/explorer/pages/details.vue b/app/assets/javascripts/registry/explorer/pages/details.vue
index 0f4ed1550ce..88e437b16d9 100644
--- a/app/assets/javascripts/registry/explorer/pages/details.vue
+++ b/app/assets/javascripts/registry/explorer/pages/details.vue
@@ -102,7 +102,7 @@ export default {
         return this.tagsPagination.page;
       },
       set(page) {
-        this.requestTagsList({ pagination: { page }, id: this.$route.params.id });
+        this.requestTagsList({ pagination: { page }, params: this.$route.params.id });
       },
     },
   },
diff --git a/app/assets/javascripts/snippets/components/snippet_visibility_edit.vue b/app/assets/javascripts/snippets/components/snippet_visibility_edit.vue
new file mode 100644
index 00000000000..93cd2b58c11
--- /dev/null
+++ b/app/assets/javascripts/snippets/components/snippet_visibility_edit.vue
@@ -0,0 +1,95 @@
+<script>
+import { GlIcon, GlFormGroup, GlFormRadio, GlFormRadioGroup, GlLink } from '@gitlab/ui';
+import { SNIPPET_VISIBILITY } from '~/snippets/constants';
+
+export default {
+  components: {
+    GlIcon,
+    GlFormGroup,
+    GlFormRadio,
+    GlFormRadioGroup,
+    GlLink,
+  },
+  props: {
+    helpLink: {
+      type: String,
+      default: '',
+      required: false,
+    },
+    isProjectSnippet: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    visibilityLevel: {
+      type: String,
+      default: '0',
+      required: false,
+    },
+  },
+  data() {
+    return {
+      selected: this.visibilityLevel,
+    };
+  },
+  computed: {
+    visibilityOptions() {
+      return [
+        {
+          value: '0',
+          icon: 'lock',
+          text: SNIPPET_VISIBILITY.private.label,
+          description: this.isProjectSnippet
+            ? SNIPPET_VISIBILITY.private.description_project
+            : SNIPPET_VISIBILITY.private.description,
+        },
+        {
+          value: '1',
+          icon: 'shield',
+          text: SNIPPET_VISIBILITY.internal.label,
+          description: SNIPPET_VISIBILITY.internal.description,
+        },
+        {
+          value: '2',
+          icon: 'earth',
+          text: SNIPPET_VISIBILITY.public.label,
+          description: SNIPPET_VISIBILITY.public.description,
+        },
+      ];
+    },
+  },
+  methods: {
+    updateSelectedOption(newVal) {
+      if (newVal !== this.selected) {
+        this.selected = newVal;
+      }
+    },
+  },
+};
+</script>
+<template>
+  <div class="form-group">
+    <label>
+      {{ __('Visibility level') }}
+      <gl-link v-if="helpLink" :href="helpLink" target="_blank"
+        ><gl-icon :size="12" name="question"
+      /></gl-link>
+    </label>
+    <gl-form-group id="visibility-level-setting">
+      <gl-form-radio-group :checked="selected" stacked @change="updateSelectedOption">
+        <gl-form-radio
+          v-for="option in visibilityOptions"
+          :key="option.icon"
+          :value="option.value"
+          class="mb-3"
+        >
+          <div class="d-flex align-items-center">
+            <gl-icon :size="16" :name="option.icon" />
+            <span class="font-weight-bold ml-1">{{ option.text }}</span>
+          </div>
+          <template #help>{{ option.description }}</template>
+        </gl-form-radio>
+      </gl-form-radio-group>
+    </gl-form-group>
+  </div>
+</template>
diff --git a/app/assets/javascripts/snippets/constants.js b/app/assets/javascripts/snippets/constants.js
index 87e3fe360a3..ed2f1156292 100644
--- a/app/assets/javascripts/snippets/constants.js
+++ b/app/assets/javascripts/snippets/constants.js
@@ -1,3 +1,21 @@
+import { __ } from '~/locale';
+
 export const SNIPPET_VISIBILITY_PRIVATE = 'private';
 export const SNIPPET_VISIBILITY_INTERNAL = 'internal';
 export const SNIPPET_VISIBILITY_PUBLIC = 'public';
+
+export const SNIPPET_VISIBILITY = {
+  private: {
+    label: __('Private'),
+    description: __('The snippet is visible only to me.'),
+    description_project: __('The snippet is visible only to project members.'),
+  },
+  internal: {
+    label: __('Internal'),
+    description: __('The snippet is visible to any logged in user.'),
+  },
+  public: {
+    label: __('Public'),
+    description: __('The snippet can be accessed without any authentication.'),
+  },
+};
diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb
index 241df8d95d7..48cd42347fc 100644
--- a/app/controllers/projects/snippets_controller.rb
+++ b/app/controllers/projects/snippets_controller.rb
@@ -52,8 +52,15 @@ class Projects::SnippetsController < Projects::ApplicationController
     create_params = snippet_params.merge(spammable_params)
     service_response = Snippets::CreateService.new(project, current_user, create_params).execute
     @snippet = service_response.payload[:snippet]
+    repository_operation_error = service_response.error? && !@snippet.persisted? && @snippet.valid?
 
-    recaptcha_check_with_fallback { render :new }
+    if repository_operation_error
+      flash.now[:alert] = service_response.message
+
+      render :new
+    else
+      recaptcha_check_with_fallback { render :new }
+    end
   end
 
   def update
diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb
index 3f8b13dbcdd..070391c4b51 100644
--- a/app/controllers/snippets_controller.rb
+++ b/app/controllers/snippets_controller.rb
@@ -52,10 +52,17 @@ class SnippetsController < ApplicationController
     create_params = snippet_params.merge(spammable_params)
     service_response = Snippets::CreateService.new(nil, current_user, create_params).execute
     @snippet = service_response.payload[:snippet]
+    repository_operation_error = service_response.error? && !@snippet.persisted? && @snippet.valid?
 
-    move_temporary_files if @snippet.valid? && params[:files]
+    if repository_operation_error
+      flash.now[:alert] = service_response.message
 
-    recaptcha_check_with_fallback { render :new }
+      render :new
+    else
+      move_temporary_files if @snippet.valid? && params[:files]
+
+      recaptcha_check_with_fallback { render :new }
+    end
   end
 
   def update
diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb
index 98b8981754f..3f2106c80bf 100644
--- a/app/models/application_setting_implementation.rb
+++ b/app/models/application_setting_implementation.rb
@@ -219,22 +219,15 @@ module ApplicationSettingImplementation
     self.outbound_local_requests_whitelist.uniq!
   end
 
+  # This method separates out the strings stored in the
+  # application_setting.outbound_local_requests_whitelist array into 2 arrays;
+  # an array of IPAddr objects (`[IPAddr.new('127.0.0.1')]`), and an array of
+  # domain strings (`['www.example.com']`).
   def outbound_local_requests_whitelist_arrays
     strong_memoize(:outbound_local_requests_whitelist_arrays) do
       next [[], []] unless self.outbound_local_requests_whitelist
 
-      ip_whitelist = []
-      domain_whitelist = []
-
-      self.outbound_local_requests_whitelist.each do |str|
-        ip_obj = Gitlab::Utils.string_to_ip_object(str)
-
-        if ip_obj
-          ip_whitelist << ip_obj
-        else
-          domain_whitelist << str
-        end
-      end
+      ip_whitelist, domain_whitelist = separate_whitelists(self.outbound_local_requests_whitelist)
 
       [ip_whitelist, domain_whitelist]
     end
@@ -360,6 +353,20 @@ module ApplicationSettingImplementation
 
   private
 
+  def separate_whitelists(string_array)
+    string_array.reduce([[], []]) do |(ip_whitelist, domain_whitelist), string|
+      ip_obj = Gitlab::Utils.string_to_ip_object(string)
+
+      if ip_obj
+        ip_whitelist << ip_obj
+      else
+        domain_whitelist << string
+      end
+
+      [ip_whitelist, domain_whitelist]
+    end
+  end
+
   def array_to_string(arr)
     arr&.join("\n")
   end
diff --git a/app/models/member.rb b/app/models/member.rb
index a26a0615a6e..99dee67346e 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -374,7 +374,7 @@ class Member < ApplicationRecord
     # always notify when there isn't a user yet
     return true if user.blank?
 
-    NotificationRecipientService.notifiable?(user, type, notifiable_options.merge(opts))
+    NotificationRecipients::BuildService.notifiable?(user, type, notifiable_options.merge(opts))
   end
   # rubocop: enable CodeReuse/ServiceClass
 
diff --git a/app/models/snippet_repository.rb b/app/models/snippet_repository.rb
index 89098971a7d..70f26001b5f 100644
--- a/app/models/snippet_repository.rb
+++ b/app/models/snippet_repository.rb
@@ -4,7 +4,7 @@ class SnippetRepository < ApplicationRecord
   include Shardable
 
   DEFAULT_EMPTY_FILE_NAME = 'snippetfile'
-  EMPTY_FILE_PATTERN = /^#{DEFAULT_EMPTY_FILE_NAME}(\d)\.txt$/.freeze
+  EMPTY_FILE_PATTERN = /^#{DEFAULT_EMPTY_FILE_NAME}(\d+)\.txt$/.freeze
 
   CommitError = Class.new(StandardError)
 
@@ -51,14 +51,14 @@ class SnippetRepository < ApplicationRecord
   end
 
   def transform_file_entries(files)
-    last_index = get_last_empty_file_index
+    next_index = get_last_empty_file_index + 1
 
     files.each do |file_entry|
       file_entry[:action] = infer_action(file_entry) unless file_entry[:action]
 
       if file_entry[:file_path].blank?
-        file_entry[:file_path] = build_empty_file_name(last_index)
-        last_index += 1
+        file_entry[:file_path] = build_empty_file_name(next_index)
+        next_index += 1
       end
     end
   end
@@ -70,12 +70,10 @@ class SnippetRepository < ApplicationRecord
   end
 
   def get_last_empty_file_index
-    last_file = repository.ls_files(nil)
-                          .map! { |file| file.match(EMPTY_FILE_PATTERN) }
-                          .compact
-                          .max_by { |element| element[1] }
-
-    last_file ? (last_file[1].to_i + 1) : 1
+    repository.ls_files(nil).inject(0) do |max, file|
+      idx = file[EMPTY_FILE_PATTERN, 1].to_i
+      [idx, max].max
+    end
   end
 
   def build_empty_file_name(index)
diff --git a/app/services/notification_recipients/build_service.rb b/app/services/notification_recipients/build_service.rb
new file mode 100644
index 00000000000..67f9849aece
--- /dev/null
+++ b/app/services/notification_recipients/build_service.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+#
+# Used by NotificationService to determine who should receive notification
+#
+module NotificationRecipients
+  module BuildService
+    def self.notifiable_users(users, *args)
+      users.compact.map { |u| NotificationRecipient.new(u, *args) }.select(&:notifiable?).map(&:user)
+    end
+
+    def self.notifiable?(user, *args)
+      NotificationRecipient.new(user, *args).notifiable?
+    end
+
+    def self.build_recipients(*args)
+      Builder::Default.new(*args).notification_recipients
+    end
+
+    def self.build_new_note_recipients(*args)
+      Builder::NewNote.new(*args).notification_recipients
+    end
+
+    def self.build_merge_request_unmergeable_recipients(*args)
+      Builder::MergeRequestUnmergeable.new(*args).notification_recipients
+    end
+
+    def self.build_project_maintainers_recipients(*args)
+      Builder::ProjectMaintainers.new(*args).notification_recipients
+    end
+
+    def self.build_new_release_recipients(*args)
+      Builder::NewRelease.new(*args).notification_recipients
+    end
+  end
+end
+
+NotificationRecipients::BuildService.prepend_if_ee('EE::NotificationRecipients::BuildService')
diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipients/builder/base.rb
index 0bdf6a0e6bc..3aa00c09ba2 100644
--- a/app/services/notification_recipient_service.rb
+++ b/app/services/notification_recipients/builder/base.rb
@@ -1,37 +1,6 @@
 # frozen_string_literal: true
 
-#
-# Used by NotificationService to determine who should receive notification
-#
-module NotificationRecipientService
-  def self.notifiable_users(users, *args)
-    users.compact.map { |u| NotificationRecipient.new(u, *args) }.select(&:notifiable?).map(&:user)
-  end
-
-  def self.notifiable?(user, *args)
-    NotificationRecipient.new(user, *args).notifiable?
-  end
-
-  def self.build_recipients(*args)
-    Builder::Default.new(*args).notification_recipients
-  end
-
-  def self.build_new_note_recipients(*args)
-    Builder::NewNote.new(*args).notification_recipients
-  end
-
-  def self.build_merge_request_unmergeable_recipients(*args)
-    Builder::MergeRequestUnmergeable.new(*args).notification_recipients
-  end
-
-  def self.build_project_maintainers_recipients(*args)
-    Builder::ProjectMaintainers.new(*args).notification_recipients
-  end
-
-  def self.build_new_release_recipients(*args)
-    Builder::NewRelease.new(*args).notification_recipients
-  end
-
+module NotificationRecipients
   module Builder
     class Base
       def initialize(*)
@@ -244,186 +213,5 @@ module NotificationRecipientService
         end
       end
     end
-
-    class Default < Base
-      MENTION_TYPE_ACTIONS = [:new_issue, :new_merge_request].freeze
-
-      attr_reader :target
-      attr_reader :current_user
-      attr_reader :action
-      attr_reader :previous_assignees
-      attr_reader :skip_current_user
-
-      def initialize(target, current_user, action:, custom_action: nil, previous_assignees: nil, skip_current_user: true)
-        @target = target
-        @current_user = current_user
-        @action = action
-        @custom_action = custom_action
-        @previous_assignees = previous_assignees
-        @skip_current_user = skip_current_user
-      end
-
-      def add_watchers
-        add_project_watchers
-      end
-
-      def build!
-        add_participants(current_user)
-        add_watchers
-        add_custom_notifications
-
-        # Re-assign is considered as a mention of the new assignee
-        case custom_action
-        when :reassign_merge_request, :reassign_issue
-          add_recipients(previous_assignees, :mention, nil)
-          add_recipients(target.assignees, :mention, NotificationReason::ASSIGNED)
-        end
-
-        add_subscribed_users
-
-        if self.class.mention_type_actions.include?(custom_action)
-          # These will all be participants as well, but adding with the :mention
-          # type ensures that users with the mention notification level will
-          # receive them, too.
-          add_mentions(current_user, target: target)
-
-          # We use the `:participating` notification level in order to match existing legacy behavior as captured
-          # in existing specs (notification_service_spec.rb ~ line 507)
-          if target.is_a?(Issuable)
-            add_recipients(target.assignees, :participating, NotificationReason::ASSIGNED)
-          end
-
-          add_labels_subscribers
-        end
-      end
-
-      def acting_user
-        current_user if skip_current_user
-      end
-
-      # Build event key to search on custom notification level
-      # Check NotificationSetting.email_events
-      def custom_action
-        @custom_action ||= "#{action}_#{target.class.model_name.name.underscore}".to_sym
-      end
-
-      def self.mention_type_actions
-        MENTION_TYPE_ACTIONS.dup
-      end
-    end
-
-    class NewNote < Base
-      attr_reader :note
-      def initialize(note)
-        @note = note
-      end
-
-      def target
-        note.noteable
-      end
-
-      # NOTE: may be nil, in the case of a PersonalSnippet
-      #
-      # (this is okay because NotificationRecipient is written
-      # to handle nil projects)
-      def project
-        note.project
-      end
-
-      def group
-        if note.for_project_noteable?
-          project.group
-        else
-          target.try(:group)
-        end
-      end
-
-      def build!
-        # Add all users participating in the thread (author, assignee, comment authors)
-        add_participants(note.author)
-        add_mentions(note.author, target: note)
-
-        if note.for_project_noteable?
-          # Merge project watchers
-          add_project_watchers
-        else
-          add_group_watchers
-        end
-
-        add_custom_notifications
-        add_subscribed_users
-      end
-
-      def custom_action
-        :new_note
-      end
-
-      def acting_user
-        note.author
-      end
-    end
-
-    class NewRelease < Base
-      attr_reader :target
-
-      def initialize(target)
-        @target = target
-      end
-
-      def build!
-        add_recipients(target.project.authorized_users, :custom, nil)
-      end
-
-      def custom_action
-        :new_release
-      end
-
-      def acting_user
-        target.author
-      end
-    end
-
-    class MergeRequestUnmergeable < Base
-      attr_reader :target
-      def initialize(merge_request)
-        @target = merge_request
-      end
-
-      def build!
-        target.merge_participants.each do |user|
-          add_recipients(user, :participating, nil)
-        end
-      end
-
-      def custom_action
-        :unmergeable_merge_request
-      end
-
-      def acting_user
-        nil
-      end
-    end
-
-    class ProjectMaintainers < Base
-      attr_reader :target
-
-      def initialize(target, action:)
-        @target = target
-        @action = action
-      end
-
-      def build!
-        return [] unless project
-
-        add_recipients(project.team.maintainers, :mention, nil)
-      end
-
-      def acting_user
-        nil
-      end
-    end
   end
 end
-
-NotificationRecipientService::Builder::Default.prepend_if_ee('EE::NotificationRecipientBuilders::Default') # rubocop: disable Cop/InjectEnterpriseEditionModule
-NotificationRecipientService.prepend_if_ee('EE::NotificationRecipientService')
diff --git a/app/services/notification_recipients/builder/default.rb b/app/services/notification_recipients/builder/default.rb
new file mode 100644
index 00000000000..790ce57452c
--- /dev/null
+++ b/app/services/notification_recipients/builder/default.rb
@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+
+module NotificationRecipients
+  module Builder
+    class Default < Base
+      MENTION_TYPE_ACTIONS = [:new_issue, :new_merge_request].freeze
+
+      attr_reader :target
+      attr_reader :current_user
+      attr_reader :action
+      attr_reader :previous_assignees
+      attr_reader :skip_current_user
+
+      def initialize(target, current_user, action:, custom_action: nil, previous_assignees: nil, skip_current_user: true)
+        @target = target
+        @current_user = current_user
+        @action = action
+        @custom_action = custom_action
+        @previous_assignees = previous_assignees
+        @skip_current_user = skip_current_user
+      end
+
+      def add_watchers
+        add_project_watchers
+      end
+
+      def build!
+        add_participants(current_user)
+        add_watchers
+        add_custom_notifications
+
+        # Re-assign is considered as a mention of the new assignee
+        case custom_action
+        when :reassign_merge_request, :reassign_issue
+          add_recipients(previous_assignees, :mention, nil)
+          add_recipients(target.assignees, :mention, NotificationReason::ASSIGNED)
+        end
+
+        add_subscribed_users
+
+        if self.class.mention_type_actions.include?(custom_action)
+          # These will all be participants as well, but adding with the :mention
+          # type ensures that users with the mention notification level will
+          # receive them, too.
+          add_mentions(current_user, target: target)
+
+          # We use the `:participating` notification level in order to match existing legacy behavior as captured
+          # in existing specs (notification_service_spec.rb ~ line 507)
+          if target.is_a?(Issuable)
+            add_recipients(target.assignees, :participating, NotificationReason::ASSIGNED)
+          end
+
+          add_labels_subscribers
+        end
+      end
+
+      def acting_user
+        current_user if skip_current_user
+      end
+
+      # Build event key to search on custom notification level
+      # Check NotificationSetting.email_events
+      def custom_action
+        @custom_action ||= "#{action}_#{target.class.model_name.name.underscore}".to_sym
+      end
+
+      def self.mention_type_actions
+        MENTION_TYPE_ACTIONS.dup
+      end
+    end
+  end
+end
+
+NotificationRecipients::Builder::Default.prepend_if_ee('EE::NotificationRecipients::Builder::Default')
diff --git a/app/services/notification_recipients/builder/merge_request_unmergeable.rb b/app/services/notification_recipients/builder/merge_request_unmergeable.rb
new file mode 100644
index 00000000000..24d96b98002
--- /dev/null
+++ b/app/services/notification_recipients/builder/merge_request_unmergeable.rb
@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+module NotificationRecipients
+  module Builder
+    class MergeRequestUnmergeable < Base
+      attr_reader :target
+      def initialize(merge_request)
+        @target = merge_request
+      end
+
+      def build!
+        target.merge_participants.each do |user|
+          add_recipients(user, :participating, nil)
+        end
+      end
+
+      def custom_action
+        :unmergeable_merge_request
+      end
+
+      def acting_user
+        nil
+      end
+    end
+  end
+end
diff --git a/app/services/notification_recipients/builder/new_note.rb b/app/services/notification_recipients/builder/new_note.rb
new file mode 100644
index 00000000000..27699a0d9cc
--- /dev/null
+++ b/app/services/notification_recipients/builder/new_note.rb
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+module NotificationRecipients
+  module Builder
+    class NewNote < Base
+      attr_reader :note
+      def initialize(note)
+        @note = note
+      end
+
+      def target
+        note.noteable
+      end
+
+      # NOTE: may be nil, in the case of a PersonalSnippet
+      #
+      # (this is okay because NotificationRecipient is written
+      # to handle nil projects)
+      def project
+        note.project
+      end
+
+      def group
+        if note.for_project_noteable?
+          project.group
+        else
+          target.try(:group)
+        end
+      end
+
+      def build!
+        # Add all users participating in the thread (author, assignee, comment authors)
+        add_participants(note.author)
+        add_mentions(note.author, target: note)
+
+        if note.for_project_noteable?
+          # Merge project watchers
+          add_project_watchers
+        else
+          add_group_watchers
+        end
+
+        add_custom_notifications
+        add_subscribed_users
+      end
+
+      def custom_action
+        :new_note
+      end
+
+      def acting_user
+        note.author
+      end
+    end
+  end
+end
diff --git a/app/services/notification_recipients/builder/new_release.rb b/app/services/notification_recipients/builder/new_release.rb
new file mode 100644
index 00000000000..67676b6eec8
--- /dev/null
+++ b/app/services/notification_recipients/builder/new_release.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+module NotificationRecipients
+  module Builder
+    class NewRelease < Base
+      attr_reader :target
+
+      def initialize(target)
+        @target = target
+      end
+
+      def build!
+        add_recipients(target.project.authorized_users, :custom, nil)
+      end
+
+      def custom_action
+        :new_release
+      end
+
+      def acting_user
+        target.author
+      end
+    end
+  end
+end
diff --git a/app/services/notification_recipients/builder/project_maintainers.rb b/app/services/notification_recipients/builder/project_maintainers.rb
new file mode 100644
index 00000000000..e8f22c00a83
--- /dev/null
+++ b/app/services/notification_recipients/builder/project_maintainers.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module NotificationRecipients
+  module Builder
+    class ProjectMaintainers < Base
+      attr_reader :target
+
+      def initialize(target, action:)
+        @target = target
+        @action = action
+      end
+
+      def build!
+        return [] unless project
+
+        add_recipients(project.team.maintainers, :mention, nil)
+      end
+
+      def acting_user
+        nil
+      end
+    end
+  end
+end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 6f2bfa8169b..6b92e5a5625 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -108,7 +108,7 @@ class NotificationService
   #  * users with custom level checked with "reassign issue"
   #
   def reassigned_issue(issue, current_user, previous_assignees = [])
-    recipients = NotificationRecipientService.build_recipients(
+    recipients = NotificationRecipients::BuildService.build_recipients(
       issue,
       current_user,
       action: "reassign",
@@ -161,7 +161,7 @@ class NotificationService
   def push_to_merge_request(merge_request, current_user, new_commits: [], existing_commits: [])
     new_commits = new_commits.map { |c| { short_id: c.short_id, title: c.title } }
     existing_commits = existing_commits.map { |c| { short_id: c.short_id, title: c.title } }
-    recipients = NotificationRecipientService.build_recipients(merge_request, current_user, action: "push_to")
+    recipients = NotificationRecipients::BuildService.build_recipients(merge_request, current_user, action: "push_to")
 
     recipients.each do |recipient|
       mailer.send(:push_to_merge_request_email, recipient.user.id, merge_request.id, current_user.id, recipient.reason, new_commits: new_commits, existing_commits: existing_commits).deliver_later
@@ -197,7 +197,7 @@ class NotificationService
   #  * users with custom level checked with "reassign merge request"
   #
   def reassigned_merge_request(merge_request, current_user, previous_assignees = [])
-    recipients = NotificationRecipientService.build_recipients(
+    recipients = NotificationRecipients::BuildService.build_recipients(
       merge_request,
       current_user,
       action: "reassign",
@@ -260,7 +260,7 @@ class NotificationService
   end
 
   def resolve_all_discussions(merge_request, current_user)
-    recipients = NotificationRecipientService.build_recipients(
+    recipients = NotificationRecipients::BuildService.build_recipients(
       merge_request,
       current_user,
       action: "resolve_all_discussions")
@@ -291,7 +291,7 @@ class NotificationService
   def send_new_note_notifications(note)
     notify_method = "note_#{note.noteable_ability_name}_email".to_sym
 
-    recipients = NotificationRecipientService.build_new_note_recipients(note)
+    recipients = NotificationRecipients::BuildService.build_new_note_recipients(note)
     recipients.each do |recipient|
       mailer.send(notify_method, recipient.user.id, note.id, recipient.reason).deliver_later
     end
@@ -299,7 +299,7 @@ class NotificationService
 
   # Notify users when a new release is created
   def send_new_release_notifications(release)
-    recipients = NotificationRecipientService.build_new_release_recipients(release)
+    recipients = NotificationRecipients::BuildService.build_new_release_recipients(release)
 
     recipients.each do |recipient|
       mailer.new_release_email(recipient.user.id, release, recipient.reason).deliver_later
@@ -413,7 +413,7 @@ class NotificationService
   end
 
   def issue_moved(issue, new_issue, current_user)
-    recipients = NotificationRecipientService.build_recipients(issue, current_user, action: 'moved')
+    recipients = NotificationRecipients::BuildService.build_recipients(issue, current_user, action: 'moved')
 
     recipients.map do |recipient|
       email = mailer.issue_moved_email(recipient.user, issue, new_issue, current_user, recipient.reason)
@@ -490,7 +490,7 @@ class NotificationService
   end
 
   def issue_due(issue)
-    recipients = NotificationRecipientService.build_recipients(
+    recipients = NotificationRecipients::BuildService.build_recipients(
       issue,
       issue.author,
       action: 'due',
@@ -526,7 +526,7 @@ class NotificationService
   protected
 
   def new_resource_email(target, method)
-    recipients = NotificationRecipientService.build_recipients(target, target.author, action: "new")
+    recipients = NotificationRecipients::BuildService.build_recipients(target, target.author, action: "new")
 
     recipients.each do |recipient|
       mailer.send(method, recipient.user.id, target.id, recipient.reason).deliver_later
@@ -534,7 +534,7 @@ class NotificationService
   end
 
   def new_mentions_in_resource_email(target, new_mentioned_users, current_user, method)
-    recipients = NotificationRecipientService.build_recipients(target, current_user, action: "new")
+    recipients = NotificationRecipients::BuildService.build_recipients(target, current_user, action: "new")
     recipients = recipients.select {|r| new_mentioned_users.include?(r.user) }
 
     recipients.each do |recipient|
@@ -545,7 +545,7 @@ class NotificationService
   def close_resource_email(target, current_user, method, skip_current_user: true, closed_via: nil)
     action = method == :merged_merge_request_email ? "merge" : "close"
 
-    recipients = NotificationRecipientService.build_recipients(
+    recipients = NotificationRecipients::BuildService.build_recipients(
       target,
       current_user,
       action: action,
@@ -573,7 +573,7 @@ class NotificationService
   end
 
   def removed_milestone_resource_email(target, current_user, method)
-    recipients = NotificationRecipientService.build_recipients(
+    recipients = NotificationRecipients::BuildService.build_recipients(
       target,
       current_user,
       action: 'removed_milestone'
@@ -585,7 +585,7 @@ class NotificationService
   end
 
   def changed_milestone_resource_email(target, milestone, current_user, method)
-    recipients = NotificationRecipientService.build_recipients(
+    recipients = NotificationRecipients::BuildService.build_recipients(
       target,
       current_user,
       action: 'changed_milestone'
@@ -597,7 +597,7 @@ class NotificationService
   end
 
   def reopen_resource_email(target, current_user, method, status)
-    recipients = NotificationRecipientService.build_recipients(target, current_user, action: "reopen")
+    recipients = NotificationRecipients::BuildService.build_recipients(target, current_user, action: "reopen")
 
     recipients.each do |recipient|
       mailer.send(method, recipient.user.id, target.id, status, current_user.id, recipient.reason).deliver_later
@@ -605,7 +605,7 @@ class NotificationService
   end
 
   def merge_request_unmergeable_email(merge_request)
-    recipients = NotificationRecipientService.build_merge_request_unmergeable_recipients(merge_request)
+    recipients = NotificationRecipients::BuildService.build_merge_request_unmergeable_recipients(merge_request)
 
     recipients.each do |recipient|
       mailer.merge_request_unmergeable_email(recipient.user.id, merge_request.id).deliver_later
@@ -619,15 +619,15 @@ class NotificationService
   private
 
   def project_maintainers_recipients(target, action:)
-    NotificationRecipientService.build_project_maintainers_recipients(target, action: action)
+    NotificationRecipients::BuildService.build_project_maintainers_recipients(target, action: action)
   end
 
   def notifiable?(*args)
-    NotificationRecipientService.notifiable?(*args)
+    NotificationRecipients::BuildService.notifiable?(*args)
   end
 
   def notifiable_users(*args)
-    NotificationRecipientService.notifiable_users(*args)
+    NotificationRecipients::BuildService.notifiable_users(*args)
   end
 
   def deliver_access_request_email(recipient, member)
diff --git a/app/services/snippets/create_service.rb b/app/services/snippets/create_service.rb
index cc645c514b7..2998208f50b 100644
--- a/app/services/snippets/create_service.rb
+++ b/app/services/snippets/create_service.rb
@@ -38,25 +38,30 @@ module Snippets
     private
 
     def save_and_commit(snippet)
-      snippet.with_transaction_returning_status do
+      result = snippet.with_transaction_returning_status do
         (snippet.save && snippet.store_mentions!).tap do |saved|
           break false unless saved
 
           if Feature.enabled?(:version_snippets, current_user)
             create_repository_for(snippet)
-            create_commit(snippet)
           end
         end
-      rescue => e # Rescuing all because we can receive Creation exceptions, GRPC exceptions, Git exceptions, ...
-        snippet.errors.add(:base, e.message)
+      end
 
-        # If the commit action failed we need to remove the repository if exists
-        if snippet.repository_exists?
-          Repositories::DestroyService.new(snippet.repository).execute
-        end
+      create_commit(snippet) if result && snippet.repository_exists?
 
-        false
-      end
+      result
+    rescue => e # Rescuing all because we can receive Creation exceptions, GRPC exceptions, Git exceptions, ...
+      snippet.errors.add(:base, e.message)
+
+      # If the commit action failed we need to remove the repository if exists
+      snippet.repository.remove if snippet.repository_exists?
+
+      # If the snippet was created, we need to remove it as we
+      # would do like if it had had any validation error
+      snippet.delete if snippet.persisted?
+
+      false
     end
 
     def create_repository_for(snippet)
diff --git a/app/views/projects/issues/_discussion.html.haml b/app/views/projects/issues/_discussion.html.haml
index 42b6aaa2634..9c129fa9ecc 100644
--- a/app/views/projects/issues/_discussion.html.haml
+++ b/app/views/projects/issues/_discussion.html.haml
@@ -7,7 +7,7 @@
 
 %section.issuable-discussion.js-vue-notes-event
   #js-vue-notes{ data: { notes_data: notes_data(@issue).to_json,
-    noteable_data: serialize_issuable(@issue),
+    noteable_data: serialize_issuable(@issue, with_blocking_issues: Feature.enabled?(:prevent_closing_blocked_issues, @issue.project)),
     noteable_type: 'Issue',
     target_type: 'issue',
     current_user_data: UserSerializer.new.represent(current_user, {only_path: true}, CurrentUserEntity).to_json } }