diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-10 12:08:16 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-10 12:08:16 +0000 |
commit | 1fa79760ad2d4bd67f5c5a27f372a7533b9b7c69 (patch) | |
tree | ffdfbd9113743831ff4f1290959a62cf6567fde5 /app | |
parent | 82fa8a3d1e8466ef36b58604d20fcc145ea12118 (diff) | |
download | gitlab-ce-1fa79760ad2d4bd67f5c5a27f372a7533b9b7c69.tar.gz |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'app')
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 } } |