diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-25 12:10:52 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-25 12:10:52 +0300 |
commit | 326b4d3216d107b40142ee847c06f2c41a1ef220 (patch) | |
tree | f5190e9c606a92c0ed4ce606a52b49366055065e | |
parent | 92e4789eb069d5fe43025601c6b7839fddda3ad1 (diff) |
Add latest changes from gitlab-org/gitlab@master
54 files changed, 709 insertions, 426 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 11fe5bfe9bb..c7fba969292 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -bb2e3f4a916f031f38c9fb1c4fc955f50f0e4275 +bdbed7a8a30246ee83d325615bb43213bf7a1d69 diff --git a/app/assets/javascripts/incidents/list.js b/app/assets/javascripts/incidents/list.js index 8644ff3a249..6e6461cd7a9 100644 --- a/app/assets/javascripts/incidents/list.js +++ b/app/assets/javascripts/incidents/list.js @@ -24,7 +24,7 @@ export default () => { } = domEl.dataset; const apolloProvider = new VueApollo({ - defaultClient: createDefaultClient(), + defaultClient: createDefaultClient({}, { assumeImmutableResults: true }), }); return new Vue({ diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue index b7e24a8b17e..2c9a512acdb 100644 --- a/app/assets/javascripts/issue_show/components/app.vue +++ b/app/assets/javascripts/issue_show/components/app.vue @@ -1,5 +1,5 @@ <script> -import { GlIcon, GlIntersectionObserver } from '@gitlab/ui'; +import { GlIcon, GlIntersectionObserver, GlTooltipDirective } from '@gitlab/ui'; import Visibility from 'visibilityjs'; import createFlash from '~/flash'; import Poll from '~/lib/utils/poll'; @@ -32,6 +32,9 @@ export default { formComponent, PinnedLinks, }, + directives: { + GlTooltip: GlTooltipDirective, + }, props: { endpoint: { required: true, @@ -183,6 +186,11 @@ export default { required: false, default: true, }, + isHidden: { + type: Boolean, + required: false, + default: false, + }, }, data() { const store = new Store({ @@ -508,6 +516,15 @@ export default { <span v-if="isConfidential" data-testid="confidential" class="issuable-warning-icon"> <gl-icon name="eye-slash" :aria-label="__('Confidential')" /> </span> + <span + v-if="isHidden" + v-gl-tooltip + :title="__('This issue is hidden because its author has been banned')" + data-testid="hidden" + class="issuable-warning-icon" + > + <gl-icon name="spam" /> + </span> <p class="gl-font-weight-bold gl-overflow-hidden gl-white-space-nowrap gl-text-overflow-ellipsis gl-my-0" :title="state.titleText" diff --git a/app/assets/javascripts/runner/components/runner_update_form.vue b/app/assets/javascripts/runner/components/runner_update_form.vue index a5bc1680852..9a6fc07f6dd 100644 --- a/app/assets/javascripts/runner/components/runner_update_form.vue +++ b/app/assets/javascripts/runner/components/runner_update_form.vue @@ -135,9 +135,9 @@ export default { </gl-form-checkbox> <gl-form-checkbox + v-if="canBeLockedToProject" v-model="model.locked" data-testid="runner-field-locked" - :disabled="!canBeLockedToProject" > {{ __('Lock to current projects') }} <template #help> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue index f33f4d3fda0..59010b773f0 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue @@ -28,6 +28,7 @@ import { CONFIRM, WARNING, MT_MERGE_STRATEGY, + PIPELINE_FAILED_STATE, } from '../../constants'; import eventHub from '../../event_hub'; import mergeRequestQueryVariablesMixin from '../../mixins/merge_request_query_variables'; @@ -39,7 +40,6 @@ import CommitsHeader from './commits_header.vue'; import SquashBeforeMerge from './squash_before_merge.vue'; const PIPELINE_RUNNING_STATE = 'running'; -const PIPELINE_FAILED_STATE = 'failed'; const PIPELINE_PENDING_STATE = 'pending'; const PIPELINE_SUCCESS_STATE = 'success'; @@ -105,6 +105,10 @@ export default { import( 'ee_component/vue_merge_request_widget/components/merge_immediately_confirmation_dialog.vue' ), + MergeTrainFailedPipelineConfirmationDialog: () => + import( + 'ee_component/vue_merge_request_widget/components/merge_train_failed_pipeline_confirmation_dialog.vue' + ), }, directives: { GlTooltip: GlTooltipDirective, @@ -125,6 +129,7 @@ export default { squashBeforeMerge: this.mr.squashIsSelected, isSquashReadOnly: this.mr.squashIsReadonly, squashCommitMessage: this.mr.squashCommitMessage, + isPipelineFailedModalVisible: false, }; }, computed: { @@ -327,7 +332,12 @@ export default { : this.mr.commitMessageWithDescription; this.commitMessage = includeDescription ? commitMessageWithDescription : commitMessage; }, - handleMergeButtonClick(useAutoMerge, mergeImmediately = false) { + handleMergeButtonClick(useAutoMerge, mergeImmediately = false, confirmationClicked = false) { + if (this.showFailedPipelineModal && !confirmationClicked) { + this.isPipelineFailedModalVisible = true; + return; + } + if (mergeImmediately) { this.isMergingImmediately = true; } @@ -522,6 +532,11 @@ export default { @mergeImmediately="onMergeImmediatelyConfirmation" /> </gl-dropdown> + <merge-train-failed-pipeline-confirmation-dialog + :visible="isPipelineFailedModalVisible" + @startMergeTrain="onStartMergeTrainConfirmation" + @cancel="isPipelineFailedModalVisible = false" + /> </gl-button-group> <div v-if="shouldShowMergeControls" diff --git a/app/assets/javascripts/vue_merge_request_widget/constants.js b/app/assets/javascripts/vue_merge_request_widget/constants.js index d067e531fad..f5710f46b7e 100644 --- a/app/assets/javascripts/vue_merge_request_widget/constants.js +++ b/app/assets/javascripts/vue_merge_request_widget/constants.js @@ -10,6 +10,8 @@ export const MWPS_MERGE_STRATEGY = 'merge_when_pipeline_succeeds'; export const MTWPS_MERGE_STRATEGY = 'add_to_merge_train_when_pipeline_succeeds'; export const MT_MERGE_STRATEGY = 'merge_train'; +export const PIPELINE_FAILED_STATE = 'failed'; + export const AUTO_MERGE_STRATEGIES = [MWPS_MERGE_STRATEGY, MTWPS_MERGE_STRATEGY, MT_MERGE_STRATEGY]; // SP - "Suggest Pipelines" diff --git a/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js b/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js index 23215982e6e..9d8e5d12d58 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js +++ b/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js @@ -38,5 +38,13 @@ export default { pipelineId() { return this.pipeline.id; }, + showFailedPipelineModal() { + return false; + }, + }, + methods: { + onStartMergeTrainConfirmation() { + return false; + }, }, }; diff --git a/app/assets/javascripts/vue_shared/components/issuable/init_issuable_header_warning.js b/app/assets/javascripts/vue_shared/components/issuable/init_issuable_header_warning.js index 18bfcc268dc..28aa93d6680 100644 --- a/app/assets/javascripts/vue_shared/components/issuable/init_issuable_header_warning.js +++ b/app/assets/javascripts/vue_shared/components/issuable/init_issuable_header_warning.js @@ -1,10 +1,20 @@ import Vue from 'vue'; +import { parseBoolean } from '~/lib/utils/common_utils'; import IssuableHeaderWarnings from './issuable_header_warnings.vue'; export default function issuableHeaderWarnings(store) { + const el = document.getElementById('js-issuable-header-warnings'); + + if (!el) { + return false; + } + + const { hidden } = el.dataset; + return new Vue({ - el: document.getElementById('js-issuable-header-warnings'), + el, store, + provide: { hidden: parseBoolean(hidden) }, render(createElement) { return createElement(IssuableHeaderWarnings); }, diff --git a/app/assets/javascripts/vue_shared/components/issuable/issuable_header_warnings.vue b/app/assets/javascripts/vue_shared/components/issuable/issuable_header_warnings.vue index 56adbe8c606..82223ab9ef4 100644 --- a/app/assets/javascripts/vue_shared/components/issuable/issuable_header_warnings.vue +++ b/app/assets/javascripts/vue_shared/components/issuable/issuable_header_warnings.vue @@ -1,11 +1,16 @@ <script> -import { GlIcon } from '@gitlab/ui'; +import { GlIcon, GlTooltipDirective } from '@gitlab/ui'; import { mapGetters } from 'vuex'; +import { __ } from '~/locale'; export default { components: { GlIcon, }, + directives: { + GlTooltip: GlTooltipDirective, + }, + inject: ['hidden'], computed: { ...mapGetters(['getNoteableData']), isLocked() { @@ -26,6 +31,12 @@ export default { visible: this.isConfidential, dataTestId: 'confidential', }, + { + iconName: 'spam', + visible: this.hidden, + dataTestId: 'hidden', + tooltip: __('This issue is hidden because its author has been banned'), + }, ]; }, }, @@ -35,8 +46,15 @@ export default { <template> <div class="gl-display-inline-block"> <template v-for="meta in warningIconsMeta"> - <div v-if="meta.visible" :key="meta.iconName" class="issuable-warning-icon inline"> - <gl-icon :name="meta.iconName" :data-testid="meta.dataTestId" class="icon" /> + <div + v-if="meta.visible" + :key="meta.iconName" + v-gl-tooltip + :data-testid="meta.dataTestId" + :title="meta.tooltip || null" + class="issuable-warning-icon inline" + > + <gl-icon :name="meta.iconName" class="icon" /> </div> </template> </div> diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index ee97e8af296..4e05775fca6 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -7,6 +7,7 @@ text-align: center; margin-right: $issuable-warning-icon-margin; line-height: $gl-line-height-24; + flex: 0 0 auto; } .limit-container-width { diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 7f5750d2011..4242f918ea0 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -77,7 +77,12 @@ class InvitesController < ApplicationController def track_invite_join_click return unless member && initial_invite_email? - experiment(:invite_email_preview_text, actor: member).track(:join_clicked) if params[:experiment_name] == 'invite_email_preview_text' + if params[:experiment_name] == 'invite_email_preview_text' + experiment(:invite_email_preview_text, actor: member).track(:join_clicked) + elsif params[:experiment_name] == 'invite_email_from' + experiment(:invite_email_from, actor: member).track(:join_clicked) + end + Gitlab::Tracking.event(self.class.name, 'join_clicked', label: 'invite_email', property: member.id.to_s) end diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index cc985e84542..3dffbbf3108 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -201,6 +201,7 @@ class RegistrationsController < Devise::RegistrationsController experiment_name = session.delete(:invite_email_experiment_name) experiment(:invite_email_preview_text, actor: member).track(:accepted) if experiment_name == 'invite_email_preview_text' + experiment(:invite_email_from, actor: member).track(:accepted) if experiment_name == 'invite_email_from' Gitlab::Tracking.event(self.class.name, 'accepted', label: 'invite_email', property: member.id.to_s) end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index d8ba530f3f6..62a25164a47 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -256,7 +256,8 @@ module IssuablesHelper issueType: issuable.issue_type, zoomMeetingUrl: ZoomMeeting.canonical_meeting_url(issuable), sentryIssueIdentifier: SentryIssue.find_by(issue: issuable)&.sentry_issue_identifier, # rubocop:disable CodeReuse/ActiveRecord - iid: issuable.iid.to_s + iid: issuable.iid.to_s, + isHidden: issue_hidden?(issuable) } end diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 152d813daea..8d2f04d0fab 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -60,8 +60,16 @@ module IssuesHelper sprite_icon('eye-slash', css_class: 'gl-vertical-align-text-bottom') if issue.confidential? end + def issue_hidden?(issue) + Feature.enabled?(:ban_user_feature_flag) && issue.hidden? + end + def hidden_issue_icon(issue) - sprite_icon('spam', css_class: 'gl-vertical-align-text-bottom') if issue.hidden? + return unless issue_hidden?(issue) + + content_tag(:span, class: 'has-tooltip', title: _('This issue is hidden because its author has been banned')) do + sprite_icon('spam', css_class: 'gl-vertical-align-text-bottom') + end end def award_user_list(awards, current_user, limit: 10) diff --git a/app/helpers/notify_helper.rb b/app/helpers/notify_helper.rb index da42740f993..ed96f3cef4f 100644 --- a/app/helpers/notify_helper.rb +++ b/app/helpers/notify_helper.rb @@ -24,7 +24,14 @@ module NotifyHelper def invited_join_url(token, member) additional_params = { invite_type: Emails::Members::INITIAL_INVITE } - if experiment(:invite_email_preview_text, actor: member).enabled? + # order important below to our scheduled testing of these + # `from` experiment will be after the `text` on, but we may not cleanup + # from the `text` one by the time we run the `from` experiment, + # therefore we want to support `text` being fully enabled + # but if `from` is also enabled, then we only care about `from` + if experiment(:invite_email_from, actor: member).enabled? + additional_params[:experiment_name] = 'invite_email_from' + elsif experiment(:invite_email_preview_text, actor: member).enabled? additional_params[:experiment_name] = 'invite_email_preview_text' end diff --git a/app/mailers/emails/members.rb b/app/mailers/emails/members.rb index fe2d2891547..53b38cc5972 100644 --- a/app/mailers/emails/members.rb +++ b/app/mailers/emails/members.rb @@ -57,7 +57,7 @@ module Emails Gitlab::Tracking.event(self.class.name, 'invite_email_sent', label: 'invite_email', property: member_id.to_s) - mail(to: member.invite_email, subject: invite_email_subject, **invite_email_headers) do |format| + mail(to: member.invite_email, subject: invite_email_subject, **invite_email_headers.merge(additional_invite_settings)) do |format| format.html { render layout: 'unknown_user_mailer' } format.text { render layout: 'unknown_user_mailer' } end @@ -147,7 +147,17 @@ module Emails def invite_email_subject if member.created_by - subject(s_("MemberInviteEmail|%{member_name} invited you to join GitLab") % { member_name: member.created_by.name }) + experiment(:invite_email_from, actor: member) do |experiment_instance| + experiment_instance.use do + subject(s_("MemberInviteEmail|%{member_name} invited you to join GitLab") % { member_name: member.created_by.name }) + end + + experiment_instance.candidate do + subject(s_("MemberInviteEmail|I've invited you to join me in GitLab")) + end + + experiment_instance.run + end else subject(s_("MemberInviteEmail|Invitation to join the %{project_or_group} %{project_or_group_name}") % { project_or_group: member_source.human_name, project_or_group_name: member_source.model_name.singular }) end @@ -164,6 +174,21 @@ module Emails end end + def additional_invite_settings + return {} unless member.created_by + + experiment(:invite_email_from, actor: member) do |experiment_instance| + experiment_instance.use { {} } + experiment_instance.candidate do + { + from: "#{member.created_by.name} <#{member.created_by.email}>" + } + end + + experiment_instance.run + end + end + def member_exists? Gitlab::AppLogger.info("Tried to send an email invitation for a deleted group. Member id: #{@member_id}") if member.blank? member.present? diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb index 74bed6b6c4e..575e532c615 100644 --- a/app/policies/issue_policy.rb +++ b/app/policies/issue_policy.rb @@ -69,6 +69,14 @@ class IssuePolicy < IssuablePolicy rule { persisted & can?(:admin_issue) }.policy do enable :set_issue_metadata end + + rule { can?(:set_issue_metadata) }.policy do + enable :set_confidentiality + end + + rule { ~persisted & can?(:create_issue) }.policy do + enable :set_confidentiality + end end IssuePolicy.prepend_mod_with('IssuePolicy') diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 0984238517e..59e521853de 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -51,9 +51,12 @@ class IssuableBaseService < ::BaseProjectService params.delete(:canonical_issue_id) params.delete(:project) params.delete(:discussion_locked) - params.delete(:confidential) end + # confidential attribute is a special type of metadata and needs to be allowed to be set + # by non-members on issues in public projects so that security issues can be reported as confidential. + params.delete(:confidential) unless can?(current_user, :set_confidentiality, issuable) + filter_assignees(issuable) filter_milestone filter_labels diff --git a/app/views/admin/runners/show.html.haml b/app/views/admin/runners/show.html.haml index 6f3c16f7abf..ffbf89585d0 100644 --- a/app/views/admin/runners/show.html.haml +++ b/app/views/admin/runners/show.html.haml @@ -4,15 +4,7 @@ - page_title "##{@runner.id} (#{@runner.short_sha})" - add_to_breadcrumbs _('Runners'), admin_runners_path -- if Feature.enabled?(:runner_detailed_view_vue_ui, current_user, default_enabled: :yaml) - #js-runner-details{ data: {runner_id: @runner.id} } -- else - %h2.page-title - = s_('Runners|Runner #%{runner_id}' % { runner_id: @runner.id }) - = render 'shared/runners/runner_type_badge', runner: @runner - = render 'shared/runners/runner_type_alert', runner: @runner - .gl-mb-6 - = render 'shared/runners/form', runner: @runner, runner_form_url: admin_runner_path(@runner), in_gitlab_com_admin_context: Gitlab.com? +#js-runner-details{ data: {runner_id: @runner.id} } .row .col-md-6 diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml index ee3aaee6dbb..2de2c2cba6c 100644 --- a/app/views/projects/issues/_issue.html.haml +++ b/app/views/projects/issues/_issue.html.haml @@ -12,9 +12,7 @@ - if issue.confidential? %span.has-tooltip{ title: _('Confidential') } = confidential_icon(issue) - - if Feature.enabled?(:ban_user_feature_flag) && issue.hidden? - %span.has-tooltip{ title: _('This issue is hidden because its author has been banned') } - = hidden_issue_icon(issue) + = hidden_issue_icon(issue) = link_to issue.title, issue_path(issue) = render_if_exists 'projects/issues/subepic_flag', issue: issue - if issue.tasks? diff --git a/app/views/shared/issuable/form/_metadata.html.haml b/app/views/shared/issuable/form/_metadata.html.haml index 1043eb49752..2f05d272ca3 100644 --- a/app/views/shared/issuable/form/_metadata.html.haml +++ b/app/views/shared/issuable/form/_metadata.html.haml @@ -1,13 +1,10 @@ - project = local_assigns.fetch(:project) - issuable = local_assigns.fetch(:issuable) - presenter = local_assigns.fetch(:presenter) - -- return unless can?(current_user, :"set_#{issuable.to_ability_name}_metadata", issuable) - - has_due_date = issuable.has_attribute?(:due_date) - form = local_assigns.fetch(:form) -- if issuable.respond_to?(:confidential) +- if issuable.respond_to?(:confidential) && can?(current_user, :set_confidentiality, issuable) .form-group.row .offset-sm-2.col-sm-10 .form-check @@ -15,39 +12,40 @@ = form.label :confidential, class: 'form-check-label' do This issue is confidential and should only be visible to team members with at least Reporter access. -%hr -.row - %div{ class: (has_due_date ? "col-lg-6" : "col-12") } - .form-group.row.merge-request-assignee - = render "shared/issuable/form/metadata_issuable_assignee", issuable: issuable, form: form, has_due_date: has_due_date - - - if issuable.allows_reviewers? - .form-group.row.merge-request-reviewer - = render "shared/issuable/form/metadata_issuable_reviewer", issuable: issuable, form: form, has_due_date: has_due_date, presenter: presenter +- if can?(current_user, :"set_#{issuable.to_ability_name}_metadata", issuable) + %hr + .row + %div{ class: (has_due_date ? "col-lg-6" : "col-12") } + .form-group.row.merge-request-assignee + = render "shared/issuable/form/metadata_issuable_assignee", issuable: issuable, form: form, has_due_date: has_due_date - = render_if_exists "shared/issuable/form/epic", issuable: issuable, form: form, project: project - - - if issuable.supports_milestone? - .form-group.row.issue-milestone - = form.label :milestone_id, "Milestone", class: "col-form-label #{has_due_date ? "col-md-2 col-lg-4" : "col-sm-2"}" - .col-sm-10{ class: ("col-md-8" if has_due_date) } - .issuable-form-select-holder - = render "shared/issuable/milestone_dropdown", selected: issuable.milestone, name: "#{issuable.class.model_name.param_key}[milestone_id]", show_any: false, show_upcoming: false, show_started: false, extra_class: "qa-issuable-milestone-dropdown js-issuable-form-dropdown js-dropdown-keep-input", dropdown_title: "Select milestone" + - if issuable.allows_reviewers? + .form-group.row.merge-request-reviewer + = render "shared/issuable/form/metadata_issuable_reviewer", issuable: issuable, form: form, has_due_date: has_due_date, presenter: presenter - .form-group.row - = form.label :label_ids, "Labels", class: "col-form-label #{has_due_date ? "col-md-2 col-lg-4" : "col-sm-2"}" - = form.hidden_field :label_ids, multiple: true, value: '' - .col-sm-10{ class: "#{"col-md-8" if has_due_date}" } - .issuable-form-select-holder - = render "shared/issuable/label_dropdown", classes: ["js-issuable-form-dropdown"], selected: issuable.labels, data_options: { field_name: "#{issuable.class.model_name.param_key}[label_ids][]", show_any: false }, dropdown_title: "Select label" + = render_if_exists "shared/issuable/form/epic", issuable: issuable, form: form, project: project - = render_if_exists "shared/issuable/form/merge_request_blocks", issuable: issuable, form: form + - if issuable.supports_milestone? + .form-group.row.issue-milestone + = form.label :milestone_id, "Milestone", class: "col-form-label #{has_due_date ? "col-md-2 col-lg-4" : "col-sm-2"}" + .col-sm-10{ class: ("col-md-8" if has_due_date) } + .issuable-form-select-holder + = render "shared/issuable/milestone_dropdown", selected: issuable.milestone, name: "#{issuable.class.model_name.param_key}[milestone_id]", show_any: false, show_upcoming: false, show_started: false, extra_class: "qa-issuable-milestone-dropdown js-issuable-form-dropdown js-dropdown-keep-input", dropdown_title: "Select milestone" - - if has_due_date - .col-lg-6 - = render_if_exists "shared/issuable/form/weight", issuable: issuable, form: form .form-group.row - = form.label :due_date, "Due date", class: "col-form-label col-md-2 col-lg-4" - .col-8 + = form.label :label_ids, "Labels", class: "col-form-label #{has_due_date ? "col-md-2 col-lg-4" : "col-sm-2"}" + = form.hidden_field :label_ids, multiple: true, value: '' + .col-sm-10{ class: "#{"col-md-8" if has_due_date}" } .issuable-form-select-holder - = form.text_field :due_date, id: "issuable-due-date", class: "datepicker form-control", placeholder: "Select due date", autocomplete: 'off' + = render "shared/issuable/label_dropdown", classes: ["js-issuable-form-dropdown"], selected: issuable.labels, data_options: { field_name: "#{issuable.class.model_name.param_key}[label_ids][]", show_any: false }, dropdown_title: "Select label" + + = render_if_exists "shared/issuable/form/merge_request_blocks", issuable: issuable, form: form + + - if has_due_date + .col-lg-6 + = render_if_exists "shared/issuable/form/weight", issuable: issuable, form: form + .form-group.row + = form.label :due_date, "Due date", class: "col-form-label col-md-2 col-lg-4" + .col-8 + .issuable-form-select-holder + = form.text_field :due_date, id: "issuable-due-date", class: "datepicker form-control", placeholder: "Select due date", autocomplete: 'off' diff --git a/app/views/shared/issue_type/_details_header.html.haml b/app/views/shared/issue_type/_details_header.html.haml index a25e35cdcd4..eca61819cca 100644 --- a/app/views/shared/issue_type/_details_header.html.haml +++ b/app/views/shared/issue_type/_details_header.html.haml @@ -15,7 +15,7 @@ = _('Open') .issuable-meta - #js-issuable-header-warnings + #js-issuable-header-warnings{ data: { hidden: issue_hidden?(issuable).to_s } } = issuable_meta(issuable, @project) %a.btn.gl-button.btn-default.btn-icon.float-right.gl-display-block.d-sm-none.gutter-toggle.issuable-gutter-toggle.js-sidebar-toggle{ href: "#" } diff --git a/config/feature_flags/development/dast_profile_disable_joins.yml b/config/feature_flags/development/dast_profile_disable_joins.yml deleted file mode 100644 index 469e930dd7c..00000000000 --- a/config/feature_flags/development/dast_profile_disable_joins.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: dast_profile_disable_joins -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66709 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/336944 -milestone: '14.2' -type: development -group: group::dynamic analysis -default_enabled: true diff --git a/config/feature_flags/development/dast_scanner_profile_disable_joins.yml b/config/feature_flags/development/dast_scanner_profile_disable_joins.yml deleted file mode 100644 index 8c27633b785..00000000000 --- a/config/feature_flags/development/dast_scanner_profile_disable_joins.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: dast_scanner_profile_disable_joins -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66709 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/336946 -milestone: '14.2' -type: development -group: group::dynamic analysis -default_enabled: true diff --git a/config/feature_flags/development/dast_site_profile_disable_joins.yml b/config/feature_flags/development/dast_site_profile_disable_joins.yml deleted file mode 100644 index 08327a4abf1..00000000000 --- a/config/feature_flags/development/dast_site_profile_disable_joins.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: dast_site_profile_disable_joins -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66709 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/336945 -milestone: '14.2' -type: development -group: group::dynamic analysis -default_enabled: true diff --git a/config/feature_flags/development/runner_detailed_view_vue_ui.yml b/config/feature_flags/development/runner_detailed_view_vue_ui.yml deleted file mode 100644 index 78a09910e6c..00000000000 --- a/config/feature_flags/development/runner_detailed_view_vue_ui.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: runner_detailed_view_vue_ui -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57256 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/325737 -milestone: '13.11' -type: development -group: group::runner -default_enabled: true diff --git a/config/feature_flags/development/set_full_path.yml b/config/feature_flags/development/set_full_path.yml deleted file mode 100644 index a2f249b60fd..00000000000 --- a/config/feature_flags/development/set_full_path.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: set_full_path -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66929 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/337002 -milestone: '14.2' -type: development -group: group::gitaly -default_enabled: false diff --git a/config/feature_flags/experiment/invite_email_from.yml b/config/feature_flags/experiment/invite_email_from.yml new file mode 100644 index 00000000000..59baf249341 --- /dev/null +++ b/config/feature_flags/experiment/invite_email_from.yml @@ -0,0 +1,8 @@ +--- +name: invite_email_from +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68376 +rollout_issue_url: https://gitlab.com/gitlab-org/growth/team-tasks/-/issues/429 +milestone: '14.3' +type: experiment +group: group::expansion +default_enabled: false diff --git a/doc/ci/migration/circleci.md b/doc/ci/migration/circleci.md index 106f5b3d819..d85c1137f6a 100644 --- a/doc/ci/migration/circleci.md +++ b/doc/ci/migration/circleci.md @@ -121,11 +121,11 @@ stages: - test - deploy -job 1: +job1: stage: build script: make build dependencies -job 2: +job2: stage: build script: make build artifacts @@ -216,12 +216,12 @@ jobs: Example of the same workflow using `rules` in GitLab CI/CD: ```yaml -deploy_prod: +deploy: stage: deploy script: - - echo "Deploy to production server" + - echo "Deploy job" rules: - - if: '$CI_COMMIT_BRANCH == "main"' + - if: '$CI_COMMIT_BRANCH == "main" || $CI_COMMIT_BRANCH =~ /^rc-/' ``` ### Caching diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 5d8985455ad..10dc51556b9 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -336,7 +336,7 @@ module API lines = Gitlab::Diff::Parser.new.parse(diff.diff.each_line) lines.each do |line| - next unless line.new_pos == params[:line] && line.type == params[:line_type] + next unless line.line == params[:line] && line.type == params[:line_type] break opts[:line_code] = Gitlab::Git.diff_line_code(diff.new_path, line.new_pos, line.old_pos) end diff --git a/lib/api/entities/commit_note.rb b/lib/api/entities/commit_note.rb index d08b6fc8225..fe91712b48d 100644 --- a/lib/api/entities/commit_note.rb +++ b/lib/api/entities/commit_note.rb @@ -5,7 +5,7 @@ module API class CommitNote < Grape::Entity expose :note expose(:path) { |note| note.diff_file.try(:file_path) if note.diff_note? } - expose(:line) { |note| note.diff_line.try(:new_line) if note.diff_note? } + expose(:line) { |note| note.diff_line.try(:line) if note.diff_note? } expose(:line_type) { |note| note.diff_line.try(:type) if note.diff_note? } expose :author, using: Entities::UserBasic expose :created_at diff --git a/lib/gitlab/database/load_balancing/host_list.rb b/lib/gitlab/database/load_balancing/host_list.rb index aa731521732..fb3175c7d5d 100644 --- a/lib/gitlab/database/load_balancing/host_list.rb +++ b/lib/gitlab/database/load_balancing/host_list.rb @@ -35,12 +35,6 @@ module Gitlab def hosts=(hosts) @mutex.synchronize do - ::Gitlab::Database::LoadBalancing::Logger.info( - event: :host_list_update, - message: "Updating the host list for service discovery", - host_list_length: hosts.length, - old_host_list_length: @hosts.length - ) @hosts = hosts unsafe_shuffle end diff --git a/lib/gitlab/database/load_balancing/service_discovery.rb b/lib/gitlab/database/load_balancing/service_discovery.rb index 251961c8246..d22134379da 100644 --- a/lib/gitlab/database/load_balancing/service_discovery.rb +++ b/lib/gitlab/database/load_balancing/service_discovery.rb @@ -13,11 +13,17 @@ module Gitlab # balancer with said hosts. Requests may continue to use the old hosts # until they complete. class ServiceDiscovery + EmptyDnsResponse = Class.new(StandardError) + attr_reader :interval, :record, :record_type, :disconnect_timeout, :load_balancer MAX_SLEEP_ADJUSTMENT = 10 + MAX_DISCOVERY_RETRIES = 3 + + RETRY_DELAY_RANGE = (0.1..0.2).freeze + RECORD_TYPES = { 'A' => Net::DNS::A, 'SRV' => Net::DNS::SRV @@ -76,15 +82,21 @@ module Gitlab end def perform_service_discovery - refresh_if_necessary - rescue StandardError => error - # Any exceptions that might occur should be reported to - # Sentry, instead of silently terminating this thread. - Gitlab::ErrorTracking.track_exception(error) - - Gitlab::AppLogger.error( - "Service discovery encountered an error: #{error.message}" - ) + MAX_DISCOVERY_RETRIES.times do + return refresh_if_necessary + rescue StandardError => error + # Any exceptions that might occur should be reported to + # Sentry, instead of silently terminating this thread. + Gitlab::ErrorTracking.track_exception(error) + + Gitlab::AppLogger.error( + "Service discovery encountered an error: #{error.message}" + ) + + # Slightly randomize the retry delay so that, in the case of a total + # dns outage, all starting services do not pressure the dns server at the same time. + sleep(rand(RETRY_DELAY_RANGE)) + end interval end @@ -99,7 +111,22 @@ module Gitlab current = addresses_from_load_balancer - replace_hosts(from_dns) if from_dns != current + if from_dns != current + ::Gitlab::Database::LoadBalancing::Logger.info( + event: :host_list_update, + message: "Updating the host list for service discovery", + host_list_length: from_dns.length, + old_host_list_length: current.length + ) + replace_hosts(from_dns) + else + ::Gitlab::Database::LoadBalancing::Logger.info( + event: :host_list_unchanged, + message: "Unchanged host list for service discovery", + host_list_length: from_dns.length, + old_host_list_length: current.length + ) + end interval end @@ -141,6 +168,8 @@ module Gitlab addresses_from_srv_record(response) end + raise EmptyDnsResponse if addresses.empty? + # Addresses are sorted so we can directly compare the old and new # addresses, without having to use any additional data structures. [new_wait_time_for(resources), addresses.sort] diff --git a/lib/gitlab/database/transaction/context.rb b/lib/gitlab/database/transaction/context.rb index 7c520f356fa..e0193474535 100644 --- a/lib/gitlab/database/transaction/context.rb +++ b/lib/gitlab/database/transaction/context.rb @@ -6,9 +6,8 @@ module Gitlab class Context attr_reader :context - LOG_DEPTH_THRESHOLD = 4 # 3 nested subtransactions + 1 real transaction - LOG_SAVEPOINTS_THRESHOLD = 5 # 5 `SAVEPOINTS` created in sequence or nested - LOG_DURATION_S_THRESHOLD = 120 # 2 minutes long transaction + LOG_SAVEPOINTS_THRESHOLD = 1 # 1 `SAVEPOINT` created in a transaction + LOG_DURATION_S_THRESHOLD = 120 # transaction that is running for 2 minutes or longer LOG_THROTTLE_DURATION = 1 def initialize @@ -19,6 +18,10 @@ module Gitlab @context[:start_time] = current_timestamp end + def set_depth(depth) + @context[:depth] = [@context[:depth].to_i, depth].max + end + def increment_savepoints @context[:savepoints] = @context[:savepoints].to_i + 1 end @@ -31,10 +34,6 @@ module Gitlab @context[:releases] = @context[:releases].to_i + 1 end - def set_depth(depth) - @context[:depth] = [@context[:depth].to_i, depth].max - end - def track_sql(sql) (@context[:queries] ||= []).push(sql) end @@ -45,10 +44,6 @@ module Gitlab current_timestamp - @context[:start_time] end - def depth_threshold_exceeded? - @context[:depth].to_i >= LOG_DEPTH_THRESHOLD - end - def savepoints_threshold_exceeded? @context[:savepoints].to_i >= LOG_SAVEPOINTS_THRESHOLD end @@ -57,16 +52,10 @@ module Gitlab duration.to_i >= LOG_DURATION_S_THRESHOLD end - def log_savepoints? - depth_threshold_exceeded? || savepoints_threshold_exceeded? - end - - def log_duration? - duration_threshold_exceeded? - end - def should_log? - !logged_already? && (log_savepoints? || log_duration?) + return false if logged_already? + + savepoints_threshold_exceeded? || duration_threshold_exceeded? end def commit diff --git a/lib/gitlab/database/transaction/observer.rb b/lib/gitlab/database/transaction/observer.rb index 7888f0916e3..2c685574802 100644 --- a/lib/gitlab/database/transaction/observer.rb +++ b/lib/gitlab/database/transaction/observer.rb @@ -21,7 +21,7 @@ module Gitlab context.set_start_time context.set_depth(0) context.track_sql(event.payload[:sql]) - elsif cmd.start_with?('SAVEPOINT ') + elsif cmd.start_with?('SAVEPOINT', 'EXCEPTION') context.set_depth(manager.open_transactions) context.increment_savepoints elsif cmd.start_with?('ROLLBACK TO SAVEPOINT') diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 32f31c0a964..d447e73be5c 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -898,17 +898,7 @@ module Gitlab # This guard avoids Gitaly log/error spam raise NoRepository, 'repository does not exist' unless exists? - if Feature.enabled?(:set_full_path) - gitaly_repository_client.set_full_path(full_path) - else - set_config('gitlab.fullpath' => full_path) - end - end - - def set_config(entries) - wrapped_gitaly_errors do - gitaly_repository_client.set_config(entries) - end + gitaly_repository_client.set_full_path(full_path) end def disconnect_alternates diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index eda3f5db144..7e7d543d0a5 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -264,25 +264,6 @@ module Gitlab nil end - def set_config(entries) - return if entries.empty? - - request = Gitaly::SetConfigRequest.new(repository: @gitaly_repo) - entries.each do |key, value| - request.entries << build_set_config_entry(key, value) - end - - GitalyClient.call( - @storage, - :repository_service, - :set_config, - request, - timeout: GitalyClient.fast_timeout - ) - - nil - end - def license_short_name request = Gitaly::FindLicenseRequest.new(repository: @gitaly_repo) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 33ee854e7a9..ba6e7738206 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4379,6 +4379,9 @@ msgstr "" msgid "Are you sure you want to %{action} %{name}?" msgstr "" +msgid "Are you sure you want to attempt to merge?" +msgstr "" + msgid "Are you sure you want to cancel editing this comment?" msgstr "" @@ -20784,6 +20787,9 @@ msgstr "" msgid "MemberInviteEmail|%{member_name} invited you to join GitLab" msgstr "" +msgid "MemberInviteEmail|I've invited you to join me in GitLab" +msgstr "" + msgid "MemberInviteEmail|Invitation to join the %{project_or_group} %{project_or_group_name}" msgstr "" @@ -33334,6 +33340,9 @@ msgstr "" msgid "The latest pipeline for this merge request did not complete successfully." msgstr "" +msgid "The latest pipeline for this merge request has failed." +msgstr "" + msgid "The license key is invalid. Make sure it is exactly as you received it from GitLab Inc." msgstr "" diff --git a/spec/controllers/admin/runners_controller_spec.rb b/spec/controllers/admin/runners_controller_spec.rb index 8e57b4f03a7..996964fdcf0 100644 --- a/spec/controllers/admin/runners_controller_spec.rb +++ b/spec/controllers/admin/runners_controller_spec.rb @@ -23,10 +23,6 @@ RSpec.describe Admin::RunnersController do describe '#show' do render_views - before do - stub_feature_flags(runner_detailed_view_vue_ui: false) - end - let_it_be(:project) { create(:project) } let_it_be(:project_two) { create(:project) } @@ -61,30 +57,6 @@ RSpec.describe Admin::RunnersController do expect(response).to have_gitlab_http_status(:ok) end - - describe 'Cost factors values' do - context 'when it is Gitlab.com' do - before do - expect(Gitlab).to receive(:com?).at_least(:once) { true } - end - - it 'renders cost factors fields' do - get :show, params: { id: runner.id } - - expect(response.body).to match /Private projects Minutes cost factor/ - expect(response.body).to match /Public projects Minutes cost factor/ - end - end - - context 'when it is not Gitlab.com' do - it 'does not show cost factor fields' do - get :show, params: { id: runner.id } - - expect(response.body).not_to match /Private projects Minutes cost factor/ - expect(response.body).not_to match /Public projects Minutes cost factor/ - end - end - end end describe '#update' do diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index dc1fb0454df..d4091461062 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -120,6 +120,29 @@ RSpec.describe InvitesController do end end + context 'when it is part of the invite_email_from experiment' do + let(:extra_params) { { invite_type: 'initial_email', experiment_name: 'invite_email_from' } } + + it 'tracks the initial join click from email' do + experiment = double(track: true) + allow(controller).to receive(:experiment).with(:invite_email_from, actor: member).and_return(experiment) + + request + + expect(experiment).to have_received(:track).with(:join_clicked) + end + + context 'when member does not exist' do + let(:raw_invite_token) { '_bogus_token_' } + + it 'does not track the experiment' do + expect(controller).not_to receive(:experiment).with(:invite_email_from, actor: member) + + request + end + end + end + context 'when member does not exist' do let(:raw_invite_token) { '_bogus_token_' } @@ -147,8 +170,9 @@ RSpec.describe InvitesController do end context 'when it is not part of our invite email experiment' do - it 'does not track via experiment' do + it 'does not track via experiment', :aggregate_failures do expect(controller).not_to receive(:experiment).with(:invite_email_preview_text, actor: member) + expect(controller).not_to receive(:experiment).with(:invite_email_from, actor: member) request end diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 301c60e89c8..a5a0f16f2b1 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -227,6 +227,40 @@ RSpec.describe RegistrationsController do end end end + + context 'with the invite_email_preview_text experiment', :experiment do + let(:extra_session_params) { { invite_email_experiment_name: 'invite_email_from' } } + + context 'when member and invite_email_experiment_name exists from the session key value' do + it 'tracks the invite acceptance' do + expect(experiment(:invite_email_from)).to track(:accepted) + .with_context(actor: member) + .on_next_instance + + subject + end + end + + context 'when member does not exist from the session key value' do + let(:originating_member_id) { -1 } + + it 'does not track invite acceptance' do + expect(experiment(:invite_email_from)).not_to track(:accepted) + + subject + end + end + + context 'when invite_email_experiment_name does not exist from the session key value' do + let(:extra_session_params) { {} } + + it 'does not track invite acceptance' do + expect(experiment(:invite_email_from)).not_to track(:accepted) + + subject + end + end + end end context 'when invite email matches email used on registration' do diff --git a/spec/features/invites_spec.rb b/spec/features/invites_spec.rb index d56bedd4852..583daba37f1 100644 --- a/spec/features/invites_spec.rb +++ b/spec/features/invites_spec.rb @@ -216,6 +216,20 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do end end + context 'with invite email acceptance for the invite_email_from experiment', :experiment do + let(:extra_params) do + { invite_type: Emails::Members::INITIAL_INVITE, experiment_name: 'invite_email_from' } + end + + it 'tracks the accepted invite' do + expect(experiment(:invite_email_from)).to track(:accepted) + .with_context(actor: group_invite) + .on_next_instance + + fill_in_sign_up_form(new_user) + end + end + it 'signs up and redirects to the group activity page with all the project/groups invitation automatically accepted' do fill_in_sign_up_form(new_user) fill_in_welcome_form diff --git a/spec/frontend/issue_show/components/app_spec.js b/spec/frontend/issue_show/components/app_spec.js index babe3a66578..bd05cb1ac5a 100644 --- a/spec/frontend/issue_show/components/app_spec.js +++ b/spec/frontend/issue_show/components/app_spec.js @@ -1,7 +1,8 @@ import { GlIntersectionObserver } from '@gitlab/ui'; -import { mount } from '@vue/test-utils'; import MockAdapter from 'axios-mock-adapter'; import { nextTick } from 'vue'; +import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; import '~/behaviors/markdown/render_gfm'; import IssuableApp from '~/issue_show/components/app.vue'; import DescriptionComponent from '~/issue_show/components/description.vue'; @@ -33,13 +34,17 @@ describe('Issuable output', () => { let realtimeRequestCount = 0; let wrapper; - const findStickyHeader = () => wrapper.find('[data-testid="issue-sticky-header"]'); - const findLockedBadge = () => wrapper.find('[data-testid="locked"]'); - const findConfidentialBadge = () => wrapper.find('[data-testid="confidential"]'); + const findStickyHeader = () => wrapper.findByTestId('issue-sticky-header'); + const findLockedBadge = () => wrapper.findByTestId('locked'); + const findConfidentialBadge = () => wrapper.findByTestId('confidential'); + const findHiddenBadge = () => wrapper.findByTestId('hidden'); const findAlert = () => wrapper.find('.alert'); const mountComponent = (props = {}, options = {}, data = {}) => { - wrapper = mount(IssuableApp, { + wrapper = mountExtended(IssuableApp, { + directives: { + GlTooltip: createMockDirective(), + }, propsData: { ...appProps, ...props }, provide: { fullPath: 'gitlab-org/incidents', @@ -539,8 +544,8 @@ describe('Issuable output', () => { it.each` title | isConfidential - ${'does not show confidential badge when issue is not confidential'} | ${true} - ${'shows confidential badge when issue is confidential'} | ${false} + ${'does not show confidential badge when issue is not confidential'} | ${false} + ${'shows confidential badge when issue is confidential'} | ${true} `('$title', async ({ isConfidential }) => { wrapper.setProps({ isConfidential }); @@ -551,8 +556,8 @@ describe('Issuable output', () => { it.each` title | isLocked - ${'does not show locked badge when issue is not locked'} | ${true} - ${'shows locked badge when issue is locked'} | ${false} + ${'does not show locked badge when issue is not locked'} | ${false} + ${'shows locked badge when issue is locked'} | ${true} `('$title', async ({ isLocked }) => { wrapper.setProps({ isLocked }); @@ -560,6 +565,27 @@ describe('Issuable output', () => { expect(findLockedBadge().exists()).toBe(isLocked); }); + + it.each` + title | isHidden + ${'does not show hidden badge when issue is not hidden'} | ${false} + ${'shows hidden badge when issue is hidden'} | ${true} + `('$title', async ({ isHidden }) => { + wrapper.setProps({ isHidden }); + + await nextTick(); + + const hiddenBadge = findHiddenBadge(); + + expect(hiddenBadge.exists()).toBe(isHidden); + + if (isHidden) { + expect(hiddenBadge.attributes('title')).toBe( + 'This issue is hidden because its author has been banned', + ); + expect(getBinding(hiddenBadge.element, 'gl-tooltip')).not.toBeUndefined(); + } + }); }); }); diff --git a/spec/frontend/runner/components/runner_update_form_spec.js b/spec/frontend/runner/components/runner_update_form_spec.js index 15029d7a911..0e0844a785b 100644 --- a/spec/frontend/runner/components/runner_update_form_spec.js +++ b/spec/frontend/runner/components/runner_update_form_spec.js @@ -54,7 +54,7 @@ describe('RunnerUpdateForm', () => { ? ACCESS_LEVEL_REF_PROTECTED : ACCESS_LEVEL_NOT_PROTECTED, runUntagged: findRunUntaggedCheckbox().element.checked, - locked: findLockedCheckbox().element.checked, + locked: findLockedCheckbox().element?.checked || false, ipAddress: findIpInput().element.value, maximumTimeout: findMaxJobTimeoutInput().element.value || null, tagList: findTagsInput().element.value.split(',').filter(Boolean), @@ -153,15 +153,15 @@ describe('RunnerUpdateForm', () => { }); it.each` - runnerType | attrDisabled | outcome - ${INSTANCE_TYPE} | ${'disabled'} | ${'disabled'} - ${GROUP_TYPE} | ${'disabled'} | ${'disabled'} - ${PROJECT_TYPE} | ${undefined} | ${'enabled'} - `(`When runner is $runnerType, locked field is $outcome`, ({ runnerType, attrDisabled }) => { + runnerType | exists | outcome + ${INSTANCE_TYPE} | ${false} | ${'hidden'} + ${GROUP_TYPE} | ${false} | ${'hidden'} + ${PROJECT_TYPE} | ${true} | ${'shown'} + `(`When runner is $runnerType, locked field is $outcome`, ({ runnerType, exists }) => { const runner = { ...mockRunner, runnerType }; createComponent({ props: { runner } }); - expect(findLockedCheckbox().attributes('disabled')).toBe(attrDisabled); + expect(findLockedCheckbox().exists()).toBe(exists); }); describe('On submit, runner gets updated', () => { diff --git a/spec/frontend/vue_shared/components/issuable/issuable_header_warnings_spec.js b/spec/frontend/vue_shared/components/issuable/issuable_header_warnings_spec.js index 573501233b9..ad8331afcff 100644 --- a/spec/frontend/vue_shared/components/issuable/issuable_header_warnings_spec.js +++ b/spec/frontend/vue_shared/components/issuable/issuable_header_warnings_spec.js @@ -1,5 +1,7 @@ -import { shallowMount, createLocalVue } from '@vue/test-utils'; +import { createLocalVue } from '@vue/test-utils'; import Vuex from 'vuex'; +import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import { createStore as createMrStore } from '~/mr_notes/stores'; import createIssueStore from '~/notes/stores'; import IssuableHeaderWarnings from '~/vue_shared/components/issuable/issuable_header_warnings.vue'; @@ -12,52 +14,53 @@ localVue.use(Vuex); describe('IssuableHeaderWarnings', () => { let wrapper; - let store; - const findConfidentialIcon = () => wrapper.find('[data-testid="confidential"]'); - const findLockedIcon = () => wrapper.find('[data-testid="locked"]'); + const findConfidentialIcon = () => wrapper.findByTestId('confidential'); + const findLockedIcon = () => wrapper.findByTestId('locked'); + const findHiddenIcon = () => wrapper.findByTestId('hidden'); const renderTestMessage = (renders) => (renders ? 'renders' : 'does not render'); - const setLock = (locked) => { - store.getters.getNoteableData.discussion_locked = locked; - }; - - const setConfidential = (confidential) => { - store.getters.getNoteableData.confidential = confidential; - }; - - const createComponent = () => { - wrapper = shallowMount(IssuableHeaderWarnings, { store, localVue }); + const createComponent = ({ store, provide }) => { + wrapper = shallowMountExtended(IssuableHeaderWarnings, { + store, + localVue, + provide, + directives: { + GlTooltip: createMockDirective(), + }, + }); }; afterEach(() => { wrapper.destroy(); wrapper = null; - store = null; }); describe.each` issuableType ${ISSUABLE_TYPE_ISSUE} | ${ISSUABLE_TYPE_MR} `(`when issuableType=$issuableType`, ({ issuableType }) => { - beforeEach(() => { - store = issuableType === ISSUABLE_TYPE_ISSUE ? createIssueStore() : createMrStore(); - createComponent(); - }); - describe.each` - lockStatus | confidentialStatus - ${true} | ${true} - ${true} | ${false} - ${false} | ${true} - ${false} | ${false} + lockStatus | confidentialStatus | hiddenStatus + ${true} | ${true} | ${false} + ${true} | ${false} | ${false} + ${false} | ${true} | ${false} + ${false} | ${false} | ${false} + ${true} | ${true} | ${true} + ${true} | ${false} | ${true} + ${false} | ${true} | ${true} + ${false} | ${false} | ${true} `( - `when locked=$lockStatus and confidential=$confidentialStatus`, - ({ lockStatus, confidentialStatus }) => { + `when locked=$lockStatus, confidential=$confidentialStatus, and hidden=$hiddenStatus`, + ({ lockStatus, confidentialStatus, hiddenStatus }) => { + const store = issuableType === ISSUABLE_TYPE_ISSUE ? createIssueStore() : createMrStore(); + beforeEach(() => { - setLock(lockStatus); - setConfidential(confidentialStatus); + store.getters.getNoteableData.confidential = confidentialStatus; + store.getters.getNoteableData.discussion_locked = lockStatus; + + createComponent({ store, provide: { hidden: hiddenStatus } }); }); it(`${renderTestMessage(lockStatus)} the locked icon`, () => { @@ -67,6 +70,19 @@ describe('IssuableHeaderWarnings', () => { it(`${renderTestMessage(confidentialStatus)} the confidential icon`, () => { expect(findConfidentialIcon().exists()).toBe(confidentialStatus); }); + + it(`${renderTestMessage(confidentialStatus)} the hidden icon`, () => { + const hiddenIcon = findHiddenIcon(); + + expect(hiddenIcon.exists()).toBe(hiddenStatus); + + if (hiddenStatus) { + expect(hiddenIcon.attributes('title')).toBe( + 'This issue is hidden because its author has been banned', + ); + expect(getBinding(hiddenIcon.element, 'gl-tooltip')).not.toBeUndefined(); + } + }); }, ); }); diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index ecaee03eeea..679871b6672 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -285,7 +285,8 @@ RSpec.describe IssuablesHelper do initialDescriptionText: 'issue text', initialTaskStatus: '0 of 0 tasks completed', issueType: 'issue', - iid: issue.iid.to_s + iid: issue.iid.to_s, + isHidden: false } expect(helper.issuable_initial_data(issue)).to match(hash_including(expected_data)) end diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index 4cb795b4eab..53c3e845e10 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -410,4 +410,55 @@ RSpec.describe IssuesHelper do end end end + + describe '#issue_hidden?' do + context 'when issue is hidden' do + let_it_be(:banned_user) { build(:user, :banned) } + let_it_be(:hidden_issue) { build(:issue, author: banned_user) } + + context 'when `ban_user_feature_flag` feature flag is enabled' do + it 'returns `true`' do + expect(helper.issue_hidden?(hidden_issue)).to eq(true) + end + end + + context 'when `ban_user_feature_flag` feature flag is disabled' do + before do + stub_feature_flags(ban_user_feature_flag: false) + end + + it 'returns `false`' do + expect(helper.issue_hidden?(hidden_issue)).to eq(false) + end + end + end + + context 'when issue is not hidden' do + it 'returns `false`' do + expect(helper.issue_hidden?(issue)).to eq(false) + end + end + end + + describe '#hidden_issue_icon' do + let_it_be(:banned_user) { build(:user, :banned) } + let_it_be(:hidden_issue) { build(:issue, author: banned_user) } + let_it_be(:mock_svg) { '<svg></svg>'.html_safe } + + before do + allow(helper).to receive(:sprite_icon).and_return(mock_svg) + end + + context 'when issue is hidden' do + it 'returns icon with tooltip' do + expect(helper.hidden_issue_icon(hidden_issue)).to eq("<span class=\"has-tooltip\" title=\"This issue is hidden because its author has been banned\">#{mock_svg}</span>") + end + end + + context 'when issue is not hidden' do + it 'returns `nil`' do + expect(helper.hidden_issue_icon(issue)).to be_nil + end + end + end end diff --git a/spec/helpers/notify_helper_spec.rb b/spec/helpers/notify_helper_spec.rb index 633a4b65139..a4193444528 100644 --- a/spec/helpers/notify_helper_spec.rb +++ b/spec/helpers/notify_helper_spec.rb @@ -70,6 +70,28 @@ RSpec.describe NotifyHelper do expect(helper.invited_join_url(token, member)) .to eq("http://test.host/-/invites/#{token}?experiment_name=invite_email_preview_text&invite_type=initial_email") end + + context 'when invite_email_from is enabled' do + before do + stub_experiments(invite_email_from: :control) + end + + it 'has correct params' do + expect(helper.invited_join_url(token, member)) + .to eq("http://test.host/-/invites/#{token}?experiment_name=invite_email_from&invite_type=initial_email") + end + end + end + + context 'when invite_email_from is enabled' do + before do + stub_experiments(invite_email_from: :control) + end + + it 'has correct params' do + expect(helper.invited_join_url(token, member)) + .to eq("http://test.host/-/invites/#{token}?experiment_name=invite_email_from&invite_type=initial_email") + end end context 'when invite_email_preview_text is disabled' do diff --git a/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb b/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb index a27341a3324..c1a8a612254 100644 --- a/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb @@ -69,18 +69,69 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery do end describe '#perform_service_discovery' do - it 'reports exceptions to Sentry' do - error = StandardError.new + context 'without any failures' do + it 'runs once' do + expect(service) + .to receive(:refresh_if_necessary).once - expect(service) - .to receive(:refresh_if_necessary) - .and_raise(error) + expect(service).not_to receive(:sleep) - expect(Gitlab::ErrorTracking) - .to receive(:track_exception) - .with(error) + expect(Gitlab::ErrorTracking).not_to receive(:track_exception) - service.perform_service_discovery + service.perform_service_discovery + end + end + context 'with failures' do + before do + allow(Gitlab::ErrorTracking).to receive(:track_exception) + allow(service).to receive(:sleep) + end + + let(:valid_retry_sleep_duration) { satisfy { |val| described_class::RETRY_DELAY_RANGE.include?(val) } } + + it 'retries service discovery when under the retry limit' do + error = StandardError.new + + expect(service) + .to receive(:refresh_if_necessary) + .and_raise(error).exactly(described_class::MAX_DISCOVERY_RETRIES - 1).times.ordered + + expect(service) + .to receive(:sleep).with(valid_retry_sleep_duration) + .exactly(described_class::MAX_DISCOVERY_RETRIES - 1).times + + expect(service).to receive(:refresh_if_necessary).and_return(45).ordered + + expect(service.perform_service_discovery).to eq(45) + end + + it 'does not retry service discovery after exceeding the limit' do + error = StandardError.new + + expect(service) + .to receive(:refresh_if_necessary) + .and_raise(error).exactly(described_class::MAX_DISCOVERY_RETRIES).times + + expect(service) + .to receive(:sleep).with(valid_retry_sleep_duration) + .exactly(described_class::MAX_DISCOVERY_RETRIES).times + + service.perform_service_discovery + end + + it 'reports exceptions to Sentry' do + error = StandardError.new + + expect(service) + .to receive(:refresh_if_necessary) + .and_raise(error).exactly(described_class::MAX_DISCOVERY_RETRIES).times + + expect(Gitlab::ErrorTracking) + .to receive(:track_exception) + .with(error).exactly(described_class::MAX_DISCOVERY_RETRIES).times + + service.perform_service_discovery + end end end @@ -224,6 +275,16 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery do expect(service.addresses_from_dns).to eq([90, addresses]) end end + + context 'when the resolver returns an empty response' do + let(:packet) { double(:packet, answer: []) } + + let(:record_type) { 'A' } + + it 'raises EmptyDnsResponse' do + expect { service.addresses_from_dns }.to raise_error(Gitlab::Database::LoadBalancing::ServiceDiscovery::EmptyDnsResponse) + end + end end describe '#new_wait_time_for' do diff --git a/spec/lib/gitlab/database/transaction/context_spec.rb b/spec/lib/gitlab/database/transaction/context_spec.rb index 65d52b4d099..3c2c5649784 100644 --- a/spec/lib/gitlab/database/transaction/context_spec.rb +++ b/spec/lib/gitlab/database/transaction/context_spec.rb @@ -70,24 +70,6 @@ RSpec.describe Gitlab::Database::Transaction::Context do it { expect(subject.duration).to be >= 0 } end - context 'when depth is low' do - it 'does not log data upon COMMIT' do - expect(subject).not_to receive(:application_info) - - subject.commit - end - - it 'does not log data upon ROLLBACK' do - expect(subject).not_to receive(:application_info) - - subject.rollback - end - - it '#should_log? returns false' do - expect(subject.should_log?).to be false - end - end - shared_examples 'logs transaction data' do it 'logs once upon COMMIT' do expect(subject).to receive(:application_info).and_call_original @@ -116,17 +98,9 @@ RSpec.describe Gitlab::Database::Transaction::Context do end end - context 'when depth exceeds threshold' do - before do - subject.set_depth(described_class::LOG_DEPTH_THRESHOLD + 1) - end - - it_behaves_like 'logs transaction data' - end - context 'when savepoints count exceeds threshold' do before do - data[:savepoints] = described_class::LOG_SAVEPOINTS_THRESHOLD + 1 + data[:savepoints] = 1 end it_behaves_like 'logs transaction data' diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 926883022b0..1dcf12b1049 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1710,83 +1710,42 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do end describe '#set_full_path' do - shared_examples '#set_full_path' do - before do - repository_rugged.config["gitlab.fullpath"] = repository_path - end - - context 'is given a path' do - it 'writes it to disk' do - repository.set_full_path(full_path: "not-the/real-path.git") - - config = File.read(File.join(repository_path, "config")) - - expect(config).to include("[gitlab]") - expect(config).to include("fullpath = not-the/real-path.git") - end - end - - context 'it is given an empty path' do - it 'does not write it to disk' do - repository.set_full_path(full_path: "") - - config = File.read(File.join(repository_path, "config")) - - expect(config).to include("[gitlab]") - expect(config).to include("fullpath = #{repository_path}") - end - end + before do + repository_rugged.config["gitlab.fullpath"] = repository_path + end - context 'repository does not exist' do - it 'raises NoRepository and does not call Gitaly WriteConfig' do - repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '', 'group/project') + context 'is given a path' do + it 'writes it to disk' do + repository.set_full_path(full_path: "not-the/real-path.git") - expect(repository.gitaly_repository_client).not_to receive(:set_full_path) + config = File.read(File.join(repository_path, "config")) - expect do - repository.set_full_path(full_path: 'foo/bar.git') - end.to raise_error(Gitlab::Git::Repository::NoRepository) - end + expect(config).to include("[gitlab]") + expect(config).to include("fullpath = not-the/real-path.git") end end - context 'with :set_full_path enabled' do - before do - stub_feature_flags(set_full_path: true) - end + context 'it is given an empty path' do + it 'does not write it to disk' do + repository.set_full_path(full_path: "") - it_behaves_like '#set_full_path' - end + config = File.read(File.join(repository_path, "config")) - context 'with :set_full_path disabled' do - before do - stub_feature_flags(set_full_path: false) + expect(config).to include("[gitlab]") + expect(config).to include("fullpath = #{repository_path}") end - - it_behaves_like '#set_full_path' - end - end - - describe '#set_config' do - let(:repository) { mutable_repository } - let(:entries) do - { - 'test.foo1' => 'bla bla', - 'test.foo2' => 1234, - 'test.foo3' => true - } end - it 'can set config settings' do - expect(repository.set_config(entries)).to be_nil + context 'repository does not exist' do + it 'raises NoRepository and does not call Gitaly WriteConfig' do + repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '', 'group/project') - expect(repository_rugged.config['test.foo1']).to eq('bla bla') - expect(repository_rugged.config['test.foo2']).to eq('1234') - expect(repository_rugged.config['test.foo3']).to eq('true') - end + expect(repository.gitaly_repository_client).not_to receive(:set_full_path) - after do - entries.keys.each { |k| repository_rugged.config.delete(k) } + expect do + repository.set_full_path(full_path: 'foo/bar.git') + end.to raise_error(Gitlab::Git::Repository::NoRepository) + end end end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 5d2b136043e..ecff5c15816 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -834,6 +834,35 @@ RSpec.describe Notify do invite_type: Emails::Members::INITIAL_INVITE, experiment_name: 'invite_email_preview_text')) end + + it 'tracks the sent invite' do + expect(experiment(:invite_email_preview_text)).to track(:assignment) + .with_context(actor: project_member) + .on_next_instance + + invite_email.deliver_now + end + end + + context 'with invite_email_from enabled', :experiment do + before do + stub_experiments(invite_email_from: :control) + end + + it 'has the correct invite_url with params' do + is_expected.to have_link('Join now', + href: invite_url(project_member.invite_token, + invite_type: Emails::Members::INITIAL_INVITE, + experiment_name: 'invite_email_from')) + end + + it 'tracks the sent invite' do + expect(experiment(:invite_email_from)).to track(:assignment) + .with_context(actor: project_member) + .on_next_instance + + invite_email.deliver_now + end end context 'when invite email sent is tracked', :snowplow do diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb index d62271eedf6..3805976b3e7 100644 --- a/spec/policies/issue_policy_spec.rb +++ b/spec/policies/issue_policy_spec.rb @@ -27,17 +27,17 @@ RSpec.describe IssuePolicy do end it 'allows support_bot to read issues, create and set metadata on new issues' do - expect(permissions(support_bot, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(support_bot, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(support_bot, new_issue)).to be_allowed(:create_issue, :set_issue_metadata) + expect(permissions(support_bot, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(support_bot, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(support_bot, new_issue)).to be_allowed(:create_issue, :set_issue_metadata, :set_confidentiality) end end shared_examples 'support bot with service desk disabled' do - it 'allows support_bot to read issues, create and set metadata on new issues' do - expect(permissions(support_bot, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(support_bot, issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(support_bot, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata) + it 'does not allow support_bot to read issues, create and set metadata on new issues' do + expect(permissions(support_bot, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(support_bot, issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(support_bot, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata, :set_confidentiality) end end @@ -60,50 +60,50 @@ RSpec.describe IssuePolicy do it 'allows guests to read issues' do expect(permissions(guest, issue)).to be_allowed(:read_issue, :read_issue_iid) - expect(permissions(guest, issue)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(guest, issue)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(guest, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) - expect(permissions(guest, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(guest, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) - expect(permissions(guest, new_issue)).to be_allowed(:create_issue, :set_issue_metadata) + expect(permissions(guest, new_issue)).to be_allowed(:create_issue, :set_issue_metadata, :set_confidentiality) end it 'allows reporters to read, update, and admin issues' do - expect(permissions(reporter, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(reporter, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(reporter, new_issue)).to be_allowed(:create_issue, :set_issue_metadata) + expect(permissions(reporter, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(reporter, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(reporter, new_issue)).to be_allowed(:create_issue, :set_issue_metadata, :set_confidentiality) end it 'allows reporters from group links to read, update, and admin issues' do - expect(permissions(reporter_from_group_link, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(reporter_from_group_link, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(reporter_from_group_link, new_issue)).to be_allowed(:create_issue, :set_issue_metadata) + expect(permissions(reporter_from_group_link, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(reporter_from_group_link, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(reporter_from_group_link, new_issue)).to be_allowed(:create_issue, :set_issue_metadata, :set_confidentiality) end it 'allows issue authors to read and update their issues' do expect(permissions(author, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) - expect(permissions(author, issue)).to be_disallowed(:admin_issue, :set_issue_metadata) + expect(permissions(author, issue)).to be_disallowed(:admin_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(author, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) - expect(permissions(author, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(author, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) - expect(permissions(author, new_issue)).to be_allowed(:create_issue, :set_issue_metadata) + expect(permissions(author, new_issue)).to be_allowed(:create_issue, :set_issue_metadata, :set_confidentiality) end it 'allows issue assignees to read and update their issues' do expect(permissions(assignee, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) - expect(permissions(assignee, issue)).to be_disallowed(:admin_issue, :set_issue_metadata) + expect(permissions(assignee, issue)).to be_disallowed(:admin_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(assignee, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) - expect(permissions(assignee, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(assignee, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) - expect(permissions(assignee, new_issue)).to be_allowed(:create_issue, :set_issue_metadata) + expect(permissions(assignee, new_issue)).to be_allowed(:create_issue, :set_issue_metadata, :set_confidentiality) end it 'does not allow non-members to read, update or create issues' do - expect(permissions(non_member, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(non_member, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata) + expect(permissions(non_member, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(non_member, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata, :set_confidentiality) end it_behaves_like 'support bot with service desk disabled' @@ -115,49 +115,49 @@ RSpec.describe IssuePolicy do it 'does not allow non-members to read confidential issues' do expect(permissions(non_member, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) - expect(permissions(non_member, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(non_member, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) end it 'does not allow guests to read confidential issues' do expect(permissions(guest, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) - expect(permissions(guest, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(guest, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) end it 'allows reporters to read, update, and admin confidential issues' do - expect(permissions(reporter, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(reporter, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(reporter, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(reporter, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) end it 'allows reporters from group links to read, update, and admin confidential issues' do - expect(permissions(reporter_from_group_link, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(reporter_from_group_link, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(reporter_from_group_link, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(reporter_from_group_link, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) end it 'allows issue authors to read and update their confidential issues' do expect(permissions(author, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) - expect(permissions(author, confidential_issue)).to be_disallowed(:admin_issue, :set_issue_metadata) + expect(permissions(author, confidential_issue)).to be_disallowed(:admin_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(author, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) - expect(permissions(author, confidential_issue_no_assignee)).to be_disallowed(:admin_issue, :set_issue_metadata) + expect(permissions(author, confidential_issue_no_assignee)).to be_disallowed(:admin_issue, :set_issue_metadata, :set_confidentiality) end it 'does not allow issue author to read or update confidential issue moved to an private project' do confidential_issue.project = create(:project, :private) - expect(permissions(author, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :set_issue_metadata) + expect(permissions(author, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :set_issue_metadata, :set_confidentiality) end it 'allows issue assignees to read and update their confidential issues' do expect(permissions(assignee, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) - expect(permissions(assignee, confidential_issue)).to be_disallowed(:admin_issue, :set_issue_metadata) + expect(permissions(assignee, confidential_issue)).to be_disallowed(:admin_issue, :set_issue_metadata, :set_confidentiality) - expect(permissions(assignee, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(assignee, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) end it 'does not allow issue assignees to read or update confidential issue moved to an private project' do confidential_issue.project = create(:project, :private) - expect(permissions(assignee, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :set_issue_metadata) + expect(permissions(assignee, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :set_issue_metadata, :set_confidentiality) end end end @@ -180,48 +180,48 @@ RSpec.describe IssuePolicy do it 'does not allow anonymous user to create todos' do expect(permissions(nil, issue)).to be_allowed(:read_issue) - expect(permissions(nil, issue)).to be_disallowed(:create_todo, :update_subscription, :set_issue_metadata) - expect(permissions(nil, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata) + expect(permissions(nil, issue)).to be_disallowed(:create_todo, :update_subscription, :set_issue_metadata, :set_confidentiality) + expect(permissions(nil, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata, :set_confidentiality) end it 'allows guests to read issues' do expect(permissions(guest, issue)).to be_allowed(:read_issue, :read_issue_iid, :create_todo, :update_subscription) - expect(permissions(guest, issue)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue, :set_issue_metadata) + expect(permissions(guest, issue)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(guest, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) - expect(permissions(guest, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue, :set_issue_metadata) + expect(permissions(guest, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(guest, issue_locked)).to be_allowed(:read_issue, :read_issue_iid) - expect(permissions(guest, issue_locked)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue, :set_issue_metadata) + expect(permissions(guest, issue_locked)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality) - expect(permissions(guest, new_issue)).to be_allowed(:create_issue, :set_issue_metadata) + expect(permissions(guest, new_issue)).to be_allowed(:create_issue, :set_issue_metadata, :set_confidentiality) end it 'allows reporters to read, update, reopen, and admin issues' do - expect(permissions(reporter, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue, :set_issue_metadata) - expect(permissions(reporter, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue, :set_issue_metadata) - expect(permissions(reporter, issue_locked)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(reporter, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(reporter, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(reporter, issue_locked)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(reporter, issue_locked)).to be_disallowed(:reopen_issue) - expect(permissions(reporter, new_issue)).to be_allowed(:create_issue, :set_issue_metadata) + expect(permissions(reporter, new_issue)).to be_allowed(:create_issue, :set_issue_metadata, :set_confidentiality) end it 'allows reporters from group links to read, update, reopen and admin issues' do - expect(permissions(reporter_from_group_link, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue, :set_issue_metadata) - expect(permissions(reporter_from_group_link, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue, :set_issue_metadata) - expect(permissions(reporter_from_group_link, issue_locked)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(reporter_from_group_link, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(reporter_from_group_link, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(reporter_from_group_link, issue_locked)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(reporter_from_group_link, issue_locked)).to be_disallowed(:reopen_issue) - expect(permissions(reporter, new_issue)).to be_allowed(:create_issue, :set_issue_metadata) + expect(permissions(reporter, new_issue)).to be_allowed(:create_issue, :set_issue_metadata, :set_confidentiality) end it 'allows issue authors to read, reopen and update their issues' do expect(permissions(author, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :reopen_issue) - expect(permissions(author, issue)).to be_disallowed(:admin_issue, :set_issue_metadata) + expect(permissions(author, issue)).to be_disallowed(:admin_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(author, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) - expect(permissions(author, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue, :set_issue_metadata) + expect(permissions(author, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(author, issue_locked)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) - expect(permissions(author, issue_locked)).to be_disallowed(:admin_issue, :reopen_issue, :set_issue_metadata) + expect(permissions(author, issue_locked)).to be_disallowed(:admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(author, new_issue)).to be_allowed(:create_issue) expect(permissions(author, new_issue)).to be_disallowed(:set_issue_metadata) @@ -229,13 +229,13 @@ RSpec.describe IssuePolicy do it 'allows issue assignees to read, reopen and update their issues' do expect(permissions(assignee, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :reopen_issue) - expect(permissions(assignee, issue)).to be_disallowed(:admin_issue, :set_issue_metadata) + expect(permissions(assignee, issue)).to be_disallowed(:admin_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(assignee, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) - expect(permissions(assignee, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue, :set_issue_metadata) + expect(permissions(assignee, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(assignee, issue_locked)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) - expect(permissions(assignee, issue_locked)).to be_disallowed(:admin_issue, :reopen_issue, :set_issue_metadata) + expect(permissions(assignee, issue_locked)).to be_disallowed(:admin_issue, :reopen_issue, :set_issue_metadata, :set_confidentiality) end it 'allows non-members to read and create issues' do @@ -249,22 +249,25 @@ RSpec.describe IssuePolicy do expect(permissions(non_member, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) end - it 'does not allow non-members to update, admin or set metadata' do - expect(permissions(non_member, issue)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) + it 'does not allow non-members to update, admin or set metadata except for set confidential flag' do + expect(permissions(non_member, issue)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(non_member, new_issue)).to be_disallowed(:set_issue_metadata) + # this is allowed for non-members in a public project, as we want to let users report security issues + # see https://gitlab.com/gitlab-org/gitlab/-/issues/337665 + expect(permissions(non_member, new_issue)).to be_allowed(:set_confidentiality) end it 'allows support_bot to read issues' do # support_bot is still allowed read access in public projects through :public_access permission, # see project_policy public_access rules policy (rule { can?(:public_access) }.policy {...}) expect(permissions(support_bot, issue)).to be_allowed(:read_issue, :read_issue_iid) - expect(permissions(support_bot, issue)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(support_bot, issue)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) expect(permissions(support_bot, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) - expect(permissions(support_bot, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(support_bot, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) - expect(permissions(support_bot, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata) + expect(permissions(support_bot, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata, :set_confidentiality) end it_behaves_like 'support bot with service desk enabled' @@ -318,9 +321,9 @@ RSpec.describe IssuePolicy do end it 'does not allow non-members to update or create issues' do - expect(permissions(non_member, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata) - expect(permissions(non_member, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata) + expect(permissions(non_member, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) + expect(permissions(non_member, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata, :set_confidentiality) end it_behaves_like 'support bot with service desk disabled' @@ -333,31 +336,31 @@ RSpec.describe IssuePolicy do it 'does not allow guests to read confidential issues' do expect(permissions(guest, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) - expect(permissions(guest, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(guest, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) end it 'allows reporters to read, update, and admin confidential issues' do expect(permissions(reporter, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) - expect(permissions(reporter, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(reporter, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) end it 'allows reporter from group links to read, update, and admin confidential issues' do expect(permissions(reporter_from_group_link, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) - expect(permissions(reporter_from_group_link, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(reporter_from_group_link, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) end it 'allows issue authors to read and update their confidential issues' do expect(permissions(author, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) - expect(permissions(author, confidential_issue)).to be_disallowed(:admin_issue, :set_issue_metadata) + expect(permissions(author, confidential_issue)).to be_disallowed(:admin_issue, :set_issue_metadata, :set_confidentiality) - expect(permissions(author, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(author, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) end it 'allows issue assignees to read and update their confidential issues' do expect(permissions(assignee, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) - expect(permissions(assignee, confidential_issue)).to be_disallowed(:admin_issue, :set_issue_metadata) + expect(permissions(assignee, confidential_issue)).to be_disallowed(:admin_issue, :set_issue_metadata, :set_confidentiality) - expect(permissions(assignee, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata) + expect(permissions(assignee, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata, :set_confidentiality) end end diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 1162ae76d15..1d76c281dee 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -1879,6 +1879,26 @@ RSpec.describe API::Commits do expect(json_response['line_type']).to eq('new') end + it 'correctly adds a note for the "old" line type' do + commit = project.repository.commit("markdown") + commit_id = commit.id + route = "/projects/#{project_id}/repository/commits/#{commit_id}/comments" + + post api(route, current_user), params: { + note: 'My comment', + path: commit.raw_diffs.first.old_path, + line: 4, + line_type: 'old' + } + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/commit_note') + expect(json_response['note']).to eq('My comment') + expect(json_response['path']).to eq(commit.raw_diffs.first.old_path) + expect(json_response['line']).to eq(4) + expect(json_response['line_type']).to eq('old') + end + context 'when ref does not exist' do let(:commit_id) { 'unknown' } |