diff options
81 files changed, 1112 insertions, 302 deletions
diff --git a/app/assets/javascripts/environments/components/container.vue b/app/assets/javascripts/environments/components/container.vue index c6b34fecbb7..9e058af56c4 100644 --- a/app/assets/javascripts/environments/components/container.vue +++ b/app/assets/javascripts/environments/components/container.vue @@ -26,26 +26,6 @@ export default { type: Boolean, required: true, }, - deployBoardsHelpPath: { - type: String, - required: false, - default: '', - }, - helpCanaryDeploymentsPath: { - type: String, - required: false, - default: '', - }, - lockPromotionSvgPath: { - type: String, - required: false, - default: '', - }, - userCalloutsPath: { - type: String, - required: false, - default: '', - }, }, methods: { onChangePage(page) { @@ -62,14 +42,7 @@ export default { <slot name="empty-state"></slot> <div v-if="!isLoading && environments.length > 0" class="table-holder"> - <environment-table - :environments="environments" - :can-read-environment="canReadEnvironment" - :user-callouts-path="userCalloutsPath" - :lock-promotion-svg-path="lockPromotionSvgPath" - :help-canary-deployments-path="helpCanaryDeploymentsPath" - :deploy-boards-help-path="deployBoardsHelpPath" - /> + <environment-table :environments="environments" :can-read-environment="canReadEnvironment" /> <table-pagination v-if="pagination && pagination.totalPages > 1" diff --git a/app/assets/javascripts/environments/components/deploy_board.vue b/app/assets/javascripts/environments/components/deploy_board.vue index 07cb968d8d3..9249ea53a84 100644 --- a/app/assets/javascripts/environments/components/deploy_board.vue +++ b/app/assets/javascripts/environments/components/deploy_board.vue @@ -44,11 +44,6 @@ export default { type: Object, required: true, }, - deployBoardsHelpPath: { - type: String, - required: false, - default: '', - }, isLoading: { type: Boolean, required: true, diff --git a/app/assets/javascripts/environments/components/environments_app.vue b/app/assets/javascripts/environments/components/environments_app.vue index edbcbcf1eb3..1a8a56e892a 100644 --- a/app/assets/javascripts/environments/components/environments_app.vue +++ b/app/assets/javascripts/environments/components/environments_app.vue @@ -51,30 +51,10 @@ export default { type: String, required: true, }, - helpCanaryDeploymentsPath: { - type: String, - required: false, - default: '', - }, helpPagePath: { type: String, required: true, }, - deployBoardsHelpPath: { - type: String, - required: false, - default: '', - }, - lockPromotionSvgPath: { - type: String, - required: false, - default: '', - }, - userCalloutsPath: { - type: String, - required: false, - default: '', - }, }, created() { @@ -195,10 +175,6 @@ export default { :environments="state.environments" :pagination="state.paginationInformation" :can-read-environment="canReadEnvironment" - :user-callouts-path="userCalloutsPath" - :lock-promotion-svg-path="lockPromotionSvgPath" - :help-canary-deployments-path="helpCanaryDeploymentsPath" - :deploy-boards-help-path="deployBoardsHelpPath" @onChangePage="onChangePage" > <template v-if="!isLoading && state.environments.length === 0" #empty-state> diff --git a/app/assets/javascripts/environments/components/environments_table.vue b/app/assets/javascripts/environments/components/environments_table.vue index bbb56ca6f26..c6a7fe1f57d 100644 --- a/app/assets/javascripts/environments/components/environments_table.vue +++ b/app/assets/javascripts/environments/components/environments_table.vue @@ -23,31 +23,11 @@ export default { required: true, default: () => [], }, - deployBoardsHelpPath: { - type: String, - required: false, - default: '', - }, canReadEnvironment: { type: Boolean, required: false, default: false, }, - helpCanaryDeploymentsPath: { - type: String, - required: false, - default: '', - }, - lockPromotionSvgPath: { - type: String, - required: false, - default: '', - }, - userCalloutsPath: { - type: String, - required: false, - default: '', - }, }, data() { return { @@ -189,7 +169,6 @@ export default { <div class="deploy-board-container"> <deploy-board :deploy-board-data="model.deployBoardData" - :deploy-boards-help-path="deployBoardsHelpPath" :is-loading="model.isLoadingDeployBoard" :is-empty="model.isEmptyDeployBoard" :logs-path="model.logs_path" diff --git a/app/assets/javascripts/environments/folder/environments_folder_view.vue b/app/assets/javascripts/environments/folder/environments_folder_view.vue index dbb60fa4622..d6244cbe4d7 100644 --- a/app/assets/javascripts/environments/folder/environments_folder_view.vue +++ b/app/assets/javascripts/environments/folder/environments_folder_view.vue @@ -34,21 +34,6 @@ export default { type: Boolean, required: true, }, - userCalloutsPath: { - type: String, - required: false, - default: '', - }, - lockPromotionSvgPath: { - type: String, - required: false, - default: '', - }, - helpCanaryDeploymentsPath: { - type: String, - required: false, - default: '', - }, }, methods: { successCallback(resp) { @@ -88,9 +73,6 @@ export default { :environments="state.environments" :pagination="state.paginationInformation" :can-read-environment="canReadEnvironment" - :user-callouts-path="userCalloutsPath" - :lock-promotion-svg-path="lockPromotionSvgPath" - :help-canary-deployments-path="helpCanaryDeploymentsPath" @onChangePage="onChangePage" /> </div> diff --git a/app/assets/javascripts/environments/index.js b/app/assets/javascripts/environments/index.js index e5e58f3d63a..68348648e61 100644 --- a/app/assets/javascripts/environments/index.js +++ b/app/assets/javascripts/environments/index.js @@ -30,7 +30,6 @@ export default () => { endpoint: environmentsData.environmentsDataEndpoint, newEnvironmentPath: environmentsData.newEnvironmentPath, helpPagePath: environmentsData.helpPagePath, - deployBoardsHelpPath: environmentsData.deployBoardsHelpPath, canCreateEnvironment: parseBoolean(environmentsData.canCreateEnvironment), canReadEnvironment: parseBoolean(environmentsData.canReadEnvironment), }; @@ -41,7 +40,6 @@ export default () => { endpoint: this.endpoint, newEnvironmentPath: this.newEnvironmentPath, helpPagePath: this.helpPagePath, - deployBoardsHelpPath: this.deployBoardsHelpPath, canCreateEnvironment: this.canCreateEnvironment, canReadEnvironment: this.canReadEnvironment, }, diff --git a/app/assets/javascripts/jira_connect/api.js b/app/assets/javascripts/jira_connect/api.js index 105114e4843..d78aba0a3f7 100644 --- a/app/assets/javascripts/jira_connect/api.js +++ b/app/assets/javascripts/jira_connect/api.js @@ -10,6 +10,10 @@ export const getJwt = () => { export const getLocation = () => { return new Promise((resolve) => { + if (typeof AP.getLocation !== 'function') { + resolve(); + } + AP.getLocation((location) => { resolve(location); }); diff --git a/app/assets/javascripts/jira_connect/index.js b/app/assets/javascripts/jira_connect/index.js index 19a07ed87aa..082c74150c5 100644 --- a/app/assets/javascripts/jira_connect/index.js +++ b/app/assets/javascripts/jira_connect/index.js @@ -1,67 +1,73 @@ import Vue from 'vue'; -import $ from 'jquery'; import setConfigs from '@gitlab/ui/dist/config'; import Translate from '~/vue_shared/translate'; import GlFeatureFlagsPlugin from '~/vue_shared/gl_feature_flags_plugin'; -import { addSubscription, removeSubscription } from '~/jira_connect/api'; +import { addSubscription, removeSubscription, getLocation } from '~/jira_connect/api'; import JiraConnectApp from './components/app.vue'; import createStore from './store'; import { SET_ERROR_MESSAGE } from './store/mutation_types'; const store = createStore(); -/** - * Initialize form handlers for the Jira Connect app - */ -const initJiraFormHandlers = () => { - const reqComplete = () => { - AP.navigator.reload(); - }; - - const reqFailed = (res, fallbackErrorMessage) => { - const { error = fallbackErrorMessage } = res || {}; - - store.commit(SET_ERROR_MESSAGE, error); - }; - - if (typeof AP.getLocation === 'function') { - AP.getLocation((location) => { - $('.js-jira-connect-sign-in').each(function updateSignInLink() { - const updatedLink = `${$(this).attr('href')}?return_to=${location}`; - $(this).attr('href', updatedLink); - }); - }); - } +const reqComplete = () => { + AP.navigator.reload(); +}; - $('#add-subscription-form').on('submit', function onAddSubscriptionForm(e) { - const addPath = $(this).attr('action'); - const namespace = $('#namespace-input').val(); +const reqFailed = (res, fallbackErrorMessage) => { + const { error = fallbackErrorMessage } = res || {}; - e.preventDefault(); + store.commit(SET_ERROR_MESSAGE, error); +}; - addSubscription(addPath, namespace) - .then(reqComplete) - .catch((err) => reqFailed(err.response.data, 'Failed to add namespace. Please try again.')); +const updateSignInLinks = async () => { + const location = await getLocation(); + Array.from(document.querySelectorAll('.js-jira-connect-sign-in')).forEach((el) => { + const updatedLink = `${el.getAttribute('href')}?return_to=${location}`; + el.setAttribute('href', updatedLink); + }); +}; + +const initRemoveSubscriptionButtonHandlers = () => { + Array.from(document.querySelectorAll('.js-jira-connect-remove-subscription')).forEach((el) => { + el.addEventListener('click', function onRemoveSubscriptionClick(e) { + e.preventDefault(); + + const removePath = e.target.getAttribute('href'); + removeSubscription(removePath) + .then(reqComplete) + .catch((err) => + reqFailed(err.response.data, 'Failed to remove namespace. Please try again.'), + ); + }); }); +}; + +const initAddSubscriptionFormHandler = () => { + const formEl = document.querySelector('#add-subscription-form'); + if (!formEl) { + return; + } - $('.remove-subscription').on('click', function onRemoveSubscriptionClick(e) { - const removePath = $(this).attr('href'); + formEl.addEventListener('submit', function onAddSubscriptionForm(e) { e.preventDefault(); - removeSubscription(removePath) + const addPath = e.target.getAttribute('action'); + const namespace = (e.target.querySelector('#namespace-input') || {}).value; + + addSubscription(addPath, namespace) .then(reqComplete) - .catch((err) => - reqFailed(err.response.data, 'Failed to remove namespace. Please try again.'), - ); + .catch((err) => reqFailed(err.response.data, 'Failed to add namespace. Please try again.')); }); }; -function initJiraConnect() { - const el = document.querySelector('.js-jira-connect-app'); +export async function initJiraConnect() { + initAddSubscriptionFormHandler(); + initRemoveSubscriptionButtonHandlers(); - initJiraFormHandlers(); + await updateSignInLinks(); + const el = document.querySelector('.js-jira-connect-app'); if (!el) { return null; } diff --git a/app/assets/javascripts/pipeline_new/components/pipeline_new_form.vue b/app/assets/javascripts/pipeline_new/components/pipeline_new_form.vue index 6b045e6bf1a..69b02463235 100644 --- a/app/assets/javascripts/pipeline_new/components/pipeline_new_form.vue +++ b/app/assets/javascripts/pipeline_new/components/pipeline_new_form.vue @@ -116,6 +116,7 @@ export default { totalWarnings: 0, isWarningDismissed: false, isLoading: false, + submitted: false, }; }, computed: { @@ -294,6 +295,7 @@ export default { }); }, createPipeline() { + this.submitted = true; const filteredVariables = this.variables .filter(({ key, value }) => key !== '' && value !== '') .map(({ variable_type, key, value }) => ({ @@ -313,8 +315,16 @@ export default { redirectTo(`${this.pipelinesPath}/${data.id}`); }) .catch((err) => { - const { errors, warnings, total_warnings: totalWarnings } = err.response.data; + // always re-enable submit button + this.submitted = false; + + const { + errors = [], + warnings = [], + total_warnings: totalWarnings = 0, + } = err?.response?.data; const [error] = errors; + this.error = error; this.warnings = warnings; this.totalWarnings = totalWarnings; @@ -464,6 +474,8 @@ export default { variant="success" class="js-no-auto-disable" data-qa-selector="run_pipeline_button" + data-testid="run_pipeline_button" + :disabled="submitted" >{{ s__('Pipeline|Run Pipeline') }}</gl-button > <gl-button :href="pipelinesPath">{{ __('Cancel') }}</gl-button> diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue index 6102ff3c83d..097dee52800 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue @@ -181,7 +181,7 @@ export default { ); }, shouldRenderSecurityReport() { - return Boolean(window.gon?.features?.coreSecurityMrWidget && this.mr.pipeline.id); + return Boolean(this.mr.pipeline.id); }, mergeError() { let { mergeError } = this.mr; diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index 9ee69c7c07f..79e45bcf929 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -18,7 +18,7 @@ class AutocompleteController < ApplicationController .new(params: params, current_user: current_user, project: project, group: group) .execute - render json: UserSerializer.new(params).represent(users, project: project) + render json: UserSerializer.new(params.merge({ current_user: current_user })).represent(users, project: project) end def user diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 6e4c760b524..6118b20ce5f 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -36,7 +36,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo push_frontend_feature_flag(:drag_comment_selection, @project, default_enabled: true) push_frontend_feature_flag(:unified_diff_components, @project, default_enabled: true) push_frontend_feature_flag(:default_merge_ref_for_diffs, @project) - push_frontend_feature_flag(:core_security_mr_widget, @project, default_enabled: true) push_frontend_feature_flag(:core_security_mr_widget_counts, @project) push_frontend_feature_flag(:remove_resolve_note, @project, default_enabled: true) push_frontend_feature_flag(:diffs_gradual_load, @project, default_enabled: true) diff --git a/app/graphql/mutations/merge_requests/reviewer_rereview.rb b/app/graphql/mutations/merge_requests/reviewer_rereview.rb new file mode 100644 index 00000000000..f6f4881654e --- /dev/null +++ b/app/graphql/mutations/merge_requests/reviewer_rereview.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Mutations + module MergeRequests + class ReviewerRereview < Base + graphql_name 'MergeRequestReviewerRereview' + + argument :user_id, ::Types::GlobalIDType[::User], + loads: Types::UserType, + required: true, + description: <<~DESC + The user ID for the user that has been requested for a new review. + DESC + + def resolve(project_path:, iid:, user:) + merge_request = authorized_find!(project_path: project_path, iid: iid) + + result = ::MergeRequests::RequestReviewService.new(merge_request.project, current_user).execute(merge_request, user) + + { + merge_request: merge_request, + errors: Array(result[:message]) + } + end + end + end +end diff --git a/app/graphql/types/alert_management/alert_type.rb b/app/graphql/types/alert_management/alert_type.rb index dc5b48e9f46..180afd62299 100644 --- a/app/graphql/types/alert_management/alert_type.rb +++ b/app/graphql/types/alert_management/alert_type.rb @@ -112,7 +112,7 @@ module Types field :todos, Types::TodoType.connection_type, null: true, - description: 'Todos of the current user for the alert.', + description: 'To-dos of the current user for the alert.', resolver: Resolvers::TodoResolver field :details_url, diff --git a/app/graphql/types/award_emojis/award_emoji_type.rb b/app/graphql/types/award_emojis/award_emoji_type.rb index 463139a9d0a..9409304e28f 100644 --- a/app/graphql/types/award_emojis/award_emoji_type.rb +++ b/app/graphql/types/award_emojis/award_emoji_type.rb @@ -23,7 +23,7 @@ module Types field :unicode, GraphQL::STRING_TYPE, null: false, - description: 'The emoji in unicode.' + description: 'The emoji in Unicode.' field :emoji, GraphQL::STRING_TYPE, @@ -33,7 +33,7 @@ module Types field :unicode_version, GraphQL::STRING_TYPE, null: false, - description: 'The unicode version for this emoji.' + description: 'The Unicode version for this emoji.' field :user, Types::UserType, diff --git a/app/graphql/types/ci/config/config_type.rb b/app/graphql/types/ci/config/config_type.rb index eaaf6e11144..88caf21c376 100644 --- a/app/graphql/types/ci/config/config_type.rb +++ b/app/graphql/types/ci/config/config_type.rb @@ -10,7 +10,7 @@ module Types field :errors, [GraphQL::STRING_TYPE], null: true, description: 'Linting errors.' field :merged_yaml, GraphQL::STRING_TYPE, null: true, - description: 'Merged CI config YAML.' + description: 'Merged CI configuration YAML.' field :stages, Types::Ci::Config::StageType.connection_type, null: true, description: 'Stages of the pipeline.' field :status, Types::Ci::Config::StatusEnum, null: true, diff --git a/app/graphql/types/ci/detailed_status_type.rb b/app/graphql/types/ci/detailed_status_type.rb index 97e960722cc..0b643a6b676 100644 --- a/app/graphql/types/ci/detailed_status_type.rb +++ b/app/graphql/types/ci/detailed_status_type.rb @@ -18,6 +18,7 @@ module Types description: 'Indicates if the status has further details.', method: :has_details? field :label, GraphQL::STRING_TYPE, null: true, + calls_gitaly: true, description: 'Label of the status.' field :text, GraphQL::STRING_TYPE, null: true, description: 'Text of the status.' @@ -25,8 +26,8 @@ module Types description: 'Tooltip associated with the status.', method: :status_tooltip field :action, Types::Ci::StatusActionType, null: true, - calls_gitaly: true, - description: 'Action information for the status. This includes method, button title, icon, path, and title.' + calls_gitaly: true, + description: 'Action information for the status. This includes method, button title, icon, path, and title.' def action if object.has_action? diff --git a/app/graphql/types/ci/pipeline_type.rb b/app/graphql/types/ci/pipeline_type.rb index 72ef111944c..af7e0fa224f 100644 --- a/app/graphql/types/ci/pipeline_type.rb +++ b/app/graphql/types/ci/pipeline_type.rb @@ -8,6 +8,7 @@ module Types connection_type_class(Types::CountableConnectionType) authorize :read_pipeline + present_using ::Ci::PipelinePresenter expose_permissions Types::PermissionTypes::Ci::Pipeline @@ -30,7 +31,7 @@ module Types description: 'Detailed status of the pipeline.' field :config_source, PipelineConfigSourceEnum, null: true, - description: "Config source of the pipeline (#{::Enums::Ci::Pipeline.config_sources.keys.join(', ').upcase})" + description: "Configuration source of the pipeline (#{::Enums::Ci::Pipeline.config_sources.keys.join(', ').upcase})" field :duration, GraphQL::INT_TYPE, null: true, description: 'Duration of the pipeline in seconds.' diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index c915bce8bbb..166f5617da2 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -51,6 +51,7 @@ module Types mount_mutation Mutations::MergeRequests::SetSubscription mount_mutation Mutations::MergeRequests::SetWip, calls_gitaly: true mount_mutation Mutations::MergeRequests::SetAssignees + mount_mutation Mutations::MergeRequests::ReviewerRereview mount_mutation Mutations::Metrics::Dashboard::Annotations::Create mount_mutation Mutations::Metrics::Dashboard::Annotations::Delete mount_mutation Mutations::Notes::Create::Note, calls_gitaly: true diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index eb377ba4650..20dbbe0987b 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -112,7 +112,7 @@ module Types field :suggestion_commit_message, GraphQL::STRING_TYPE, null: true, description: 'The commit message used to apply merge request suggestions' field :squash_read_only, GraphQL::BOOLEAN_TYPE, null: false, method: :squash_readonly?, - description: 'Indicates if squash readonly is enabled' + description: 'Indicates if `squashReadOnly` is enabled' field :namespace, Types::NamespaceType, null: true, description: 'Namespace of the project' diff --git a/app/graphql/types/snippets/blob_viewer_type.rb b/app/graphql/types/snippets/blob_viewer_type.rb index a2ffa144066..5827e3eeae9 100644 --- a/app/graphql/types/snippets/blob_viewer_type.rb +++ b/app/graphql/types/snippets/blob_viewer_type.rb @@ -11,7 +11,7 @@ module Types null: false field :load_async, GraphQL::BOOLEAN_TYPE, - description: 'Shows whether the blob content is loaded async', + description: 'Shows whether the blob content is loaded asynchronously', null: false field :collapsed, GraphQL::BOOLEAN_TYPE, diff --git a/app/graphql/types/tree/entry_type.rb b/app/graphql/types/tree/entry_type.rb index b40e38ec9d1..5e4cace2e98 100644 --- a/app/graphql/types/tree/entry_type.rb +++ b/app/graphql/types/tree/entry_type.rb @@ -7,7 +7,7 @@ module Types field :id, GraphQL::ID_TYPE, null: false, description: 'ID of the entry' field :sha, GraphQL::STRING_TYPE, null: false, - description: 'Last commit sha for the entry', method: :id + description: 'Last commit SHA for the entry', method: :id field :name, GraphQL::STRING_TYPE, null: false, description: 'Name of the entry' field :type, Tree::TypeEnum, null: false, diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb index 4faa1a11276..494d9875ce4 100644 --- a/app/mailers/emails/merge_requests.rb +++ b/app/mailers/emails/merge_requests.rb @@ -82,6 +82,13 @@ module Emails mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) end + def request_review_merge_request_email(recipient_id, merge_request_id, updated_by_user_id, reason = nil) + setup_merge_request_mail(merge_request_id, recipient_id) + + @updated_by = User.find(updated_by_user_id) + mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) + end + def merge_request_status_email(recipient_id, merge_request_id, status, updated_by_user_id, reason = nil) setup_merge_request_mail(merge_request_id, recipient_id) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index b44aead4296..d880fe10e88 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -367,7 +367,7 @@ module Ci def detailed_status(current_user) Gitlab::Ci::Status::Build::Factory - .new(self, current_user) + .new(self.present, current_user) .fabricate! end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index ffa33bed7e6..c9cd72b73d6 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -960,7 +960,7 @@ module Ci def detailed_status(current_user) Gitlab::Ci::Status::Pipeline::Factory - .new(self, current_user) + .new(self.present, current_user) .fabricate! end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5f9a96b6456..50751fa2642 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1777,6 +1777,10 @@ class MergeRequest < ApplicationRecord true end + def find_reviewer(user) + merge_request_reviewers.find_by(user_id: user.id) + end + private def with_rebase_lock diff --git a/app/models/merge_request_reviewer.rb b/app/models/merge_request_reviewer.rb index c4e5274f832..4a1f31a7f39 100644 --- a/app/models/merge_request_reviewer.rb +++ b/app/models/merge_request_reviewer.rb @@ -1,6 +1,15 @@ # frozen_string_literal: true class MergeRequestReviewer < ApplicationRecord + enum state: { + unreviewed: 0, + reviewed: 1 + } + + validates :state, + presence: true, + inclusion: { in: MergeRequestReviewer.states.keys } + belongs_to :merge_request belongs_to :reviewer, class_name: 'User', foreign_key: :user_id, inverse_of: :merge_request_reviewers end diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb index 03cbb57eb84..51a81158f78 100644 --- a/app/presenters/ci/build_presenter.rb +++ b/app/presenters/ci/build_presenter.rb @@ -50,3 +50,5 @@ module Ci end end end + +Ci::BuildPresenter.prepend_if_ee('EE::Ci::BuildPresenter') diff --git a/app/serializers/merge_request_sidebar_extras_entity.rb b/app/serializers/merge_request_sidebar_extras_entity.rb index 261b6e8e519..b1638ce71e2 100644 --- a/app/serializers/merge_request_sidebar_extras_entity.rb +++ b/app/serializers/merge_request_sidebar_extras_entity.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true class MergeRequestSidebarExtrasEntity < IssuableSidebarExtrasEntity - expose :assignees do |merge_request| - MergeRequestUserEntity.represent(merge_request.assignees, merge_request: merge_request) + expose :assignees do |merge_request, options| + MergeRequestUserEntity.represent(merge_request.assignees, options.merge(merge_request: merge_request)) end - expose :reviewers, if: -> (m) { m.allows_reviewers? } do |merge_request| - MergeRequestUserEntity.represent(merge_request.reviewers, merge_request: merge_request) + expose :reviewers, if: -> (m) { m.allows_reviewers? } do |merge_request, options| + MergeRequestUserEntity.represent(merge_request.reviewers, options.merge(merge_request: merge_request)) end end diff --git a/app/serializers/merge_request_user_entity.rb b/app/serializers/merge_request_user_entity.rb index 82ec4be5895..edb7e10bac5 100644 --- a/app/serializers/merge_request_user_entity.rb +++ b/app/serializers/merge_request_user_entity.rb @@ -2,10 +2,21 @@ class MergeRequestUserEntity < ::API::Entities::UserBasic include UserStatusTooltip + include RequestAwareEntity expose :can_merge do |reviewer, options| options[:merge_request]&.can_be_merged_by?(reviewer) end + + expose :can_update_merge_request do |reviewer, options| + request.current_user&.can?(:update_merge_request, options[:merge_request]) + end + + expose :reviewed, if: -> (_, options) { options[:merge_request] && options[:merge_request].allows_reviewers? } do |reviewer, options| + reviewer = options[:merge_request].find_reviewer(reviewer) + + reviewer&.reviewed? + end end MergeRequestUserEntity.prepend_if_ee('EE::MergeRequestUserEntity') diff --git a/app/services/draft_notes/publish_service.rb b/app/services/draft_notes/publish_service.rb index 316abff4552..82917241347 100644 --- a/app/services/draft_notes/publish_service.rb +++ b/app/services/draft_notes/publish_service.rb @@ -38,6 +38,8 @@ module DraftNotes end draft_notes.delete_all + set_reviewed + notification_service.async.new_review(review) MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(merge_request) end @@ -64,5 +66,9 @@ module DraftNotes discussion.unresolve! end end + + def set_reviewed + ::MergeRequests::MarkReviewerReviewedService.new(project, current_user).execute(merge_request) + end end end diff --git a/app/services/merge_requests/mark_reviewer_reviewed_service.rb b/app/services/merge_requests/mark_reviewer_reviewed_service.rb new file mode 100644 index 00000000000..766a4ca0a49 --- /dev/null +++ b/app/services/merge_requests/mark_reviewer_reviewed_service.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module MergeRequests + class MarkReviewerReviewedService < MergeRequests::BaseService + def execute(merge_request) + return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) + + reviewer = merge_request.find_reviewer(current_user) + + if reviewer + return error("Failed to update reviewer") unless reviewer.update(state: :reviewed) + + success + else + error("Reviewer not found") + end + end + end +end diff --git a/app/services/merge_requests/request_review_service.rb b/app/services/merge_requests/request_review_service.rb new file mode 100644 index 00000000000..b061ed45fee --- /dev/null +++ b/app/services/merge_requests/request_review_service.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module MergeRequests + class RequestReviewService < MergeRequests::BaseService + def execute(merge_request, user) + return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) + + reviewer = merge_request.find_reviewer(user) + + if reviewer + return error("Failed to update reviewer") unless reviewer.update(state: :unreviewed) + + notify_reviewer(merge_request, user) + + success + else + error("Reviewer not found") + end + end + + private + + def notify_reviewer(merge_request, reviewer) + notification_service.async.review_requested_of_merge_request(merge_request, current_user, reviewer) + todo_service.create_request_review_todo(merge_request, current_user, reviewer) + end + end +end diff --git a/app/services/notification_recipients/build_service.rb b/app/services/notification_recipients/build_service.rb index 040ecc29d3a..52070abbad7 100644 --- a/app/services/notification_recipients/build_service.rb +++ b/app/services/notification_recipients/build_service.rb @@ -36,5 +36,9 @@ module NotificationRecipients def self.build_new_review_recipients(*args) ::NotificationRecipients::Builder::NewReview.new(*args).notification_recipients end + + def self.build_requested_review_recipients(*args) + ::NotificationRecipients::Builder::RequestReview.new(*args).notification_recipients + end end end diff --git a/app/services/notification_recipients/builder/request_review.rb b/app/services/notification_recipients/builder/request_review.rb new file mode 100644 index 00000000000..911d89c6a8e --- /dev/null +++ b/app/services/notification_recipients/builder/request_review.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module NotificationRecipients + module Builder + class RequestReview < Base + attr_reader :merge_request, :current_user, :reviewer + + def initialize(merge_request, current_user, reviewer) + @merge_request, @current_user, @reviewer = merge_request, current_user, reviewer + end + + def target + merge_request + end + + def build! + add_recipients(reviewer, :mention, NotificationReason::REVIEW_REQUESTED) + end + end + end +end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 15ba1015dd0..50247532f69 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -265,6 +265,14 @@ class NotificationService end end + def review_requested_of_merge_request(merge_request, current_user, reviewer) + recipients = NotificationRecipients::BuildService.build_requested_review_recipients(merge_request, current_user, reviewer) + + recipients.each do |recipient| + mailer.request_review_merge_request_email(recipient.user.id, merge_request.id, current_user.id, recipient.reason).deliver_later + end + end + # When we add labels to a merge request we should send an email to: # # * watchers of the mr's labels diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 12d26fe890b..dea116c8546 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -212,6 +212,11 @@ class TodoService current_user.update_todos_count_cache end + def create_request_review_todo(target, author, reviewers) + attributes = attributes_for_todo(target.project, target, author, Todo::REVIEW_REQUESTED) + create_todos(reviewers, attributes) + end + private def create_todos(users, attributes) @@ -266,8 +271,7 @@ class TodoService def create_reviewer_todo(target, author, old_reviewers = []) if target.reviewers.any? reviewers = target.reviewers - old_reviewers - attributes = attributes_for_todo(target.project, target, author, Todo::REVIEW_REQUESTED) - create_todos(reviewers, attributes) + create_request_review_todo(target, author, reviewers) end end diff --git a/app/views/admin/hooks/_form.html.haml b/app/views/admin/hooks/_form.html.haml index e6abd8ff85a..ecaf7b9b38c 100644 --- a/app/views/admin/hooks/_form.html.haml +++ b/app/views/admin/hooks/_form.html.haml @@ -2,10 +2,10 @@ .form-group = form.label :url, _('URL'), class: 'label-bold' - = form.text_field :url, class: 'form-control' + = form.text_field :url, class: 'form-control gl-form-input' .form-group = form.label :token, _('Secret Token'), class: 'label-bold' - = form.text_field :token, class: 'form-control' + = form.text_field :token, class: 'form-control gl-form-input' %p.form-text.text-muted= _('Use this token to validate received payloads') .form-group = form.label :url, _('Trigger'), class: 'label-bold' diff --git a/app/views/admin/users/_user_detail.html.haml b/app/views/admin/users/_user_detail.html.haml index 3bafd1cb396..05e387e6479 100644 --- a/app/views/admin/users/_user_detail.html.haml +++ b/app/views/admin/users/_user_detail.html.haml @@ -9,9 +9,10 @@ = render 'admin/users/user_listing_note', user: user - user_badges_in_admin_section(user).each do |badge| - - css_badge = "badge badge-#{badge[:variant]}" if badge[:variant].present? - %span{ class: css_badge } - = badge[:text] + - css_badge = "badge gl-badge sm badge-pill badge-#{badge[:variant]}" if badge[:variant].present? + %span.px-1.py-1 + %span{ class: css_badge } + = badge[:text] .row-second-line.str-truncated-100 = mail_to user.email, user.email, class: 'text-secondary' diff --git a/app/views/jira_connect/subscriptions/index.html.haml b/app/views/jira_connect/subscriptions/index.html.haml index 3f6c8941896..a549ed3540b 100644 --- a/app/views/jira_connect/subscriptions/index.html.haml +++ b/app/views/jira_connect/subscriptions/index.html.haml @@ -45,7 +45,7 @@ %tr %td= subscription.namespace.full_path %td= subscription.created_at - %td= link_to 'Remove', jira_connect_subscription_path(subscription), class: 'remove-subscription' + %td= link_to 'Remove', jira_connect_subscription_path(subscription), class: 'js-jira-connect-remove-subscription' - else %h4.empty-subscriptions No linked namespaces diff --git a/app/views/layouts/_matomo.html.haml b/app/views/layouts/_matomo.html.haml index fcd3156a162..ef7c3a62902 100644 --- a/app/views/layouts/_matomo.html.haml +++ b/app/views/layouts/_matomo.html.haml @@ -1,9 +1,11 @@ <!-- Matomo --> +- matomo_disable_cookies = extra_config.has_key?('matomo_disable_cookies') && extra_config.matomo_disable_cookies = javascript_tag do :plain var _paq = window._paq = window._paq || []; _paq.push(['trackPageView']); _paq.push(['enableLinkTracking']); + #{matomo_disable_cookies ? '_paq.push(["disableCookies"])' : ""}; (function() { var u="//#{extra_config.matomo_url}/"; _paq.push(['setTrackerUrl', u+'matomo.php']); diff --git a/app/views/layouts/jira_connect.html.haml b/app/views/layouts/jira_connect.html.haml index d996b3387a3..da45d84a83b 100644 --- a/app/views/layouts/jira_connect.html.haml +++ b/app/views/layouts/jira_connect.html.haml @@ -9,7 +9,6 @@ = yield :page_specific_styles = javascript_include_tag 'https://connect-cdn.atl-paas.net/all.js' - = javascript_include_tag 'https://unpkg.com/jquery@3.3.1/dist/jquery.min.js' = Gon::Base.render_data(nonce: content_security_policy_nonce) = yield :head %body diff --git a/app/views/notify/request_review_merge_request_email.html.haml b/app/views/notify/request_review_merge_request_email.html.haml new file mode 100644 index 00000000000..d1f72f6529a --- /dev/null +++ b/app/views/notify/request_review_merge_request_email.html.haml @@ -0,0 +1,2 @@ +%p + #{sanitize_name(@updated_by.name)} requested a new review on #{merge_request_reference_link(@merge_request)}. diff --git a/app/views/notify/request_review_merge_request_email.text.erb b/app/views/notify/request_review_merge_request_email.text.erb new file mode 100644 index 00000000000..9ab15332c51 --- /dev/null +++ b/app/views/notify/request_review_merge_request_email.text.erb @@ -0,0 +1 @@ +<%= sanitize_name(@updated_by.name) %> requested a new review on <%= merge_request_reference_link(@merge_request) %>. diff --git a/app/views/projects/environments/index.html.haml b/app/views/projects/environments/index.html.haml index 067c987e721..5da9c25b780 100644 --- a/app/views/projects/environments/index.html.haml +++ b/app/views/projects/environments/index.html.haml @@ -6,5 +6,4 @@ "can-create-environment" => can?(current_user, :create_environment, @project).to_s, "new-environment-path" => new_project_environment_path(@project), "help-page-path" => help_page_path("ci/environments/index.md"), - "deploy-boards-help-path" => help_page_path("user/project/deploy_boards", anchor: "enabling-deploy-boards"), "project-path" => @project.full_path } } diff --git a/changelogs/unreleased/271263-piwik-support-the-disabling-of-cookies.yml b/changelogs/unreleased/271263-piwik-support-the-disabling-of-cookies.yml new file mode 100644 index 00000000000..654b69c2b58 --- /dev/null +++ b/changelogs/unreleased/271263-piwik-support-the-disabling-of-cookies.yml @@ -0,0 +1,5 @@ +--- +title: 'Matomo: Support the disabling of cookies' +merge_request: 52831 +author: 'otheus@gmail.com' +type: added diff --git a/changelogs/unreleased/296856-prevent-creating-duplicate-pipelines-manually.yml b/changelogs/unreleased/296856-prevent-creating-duplicate-pipelines-manually.yml new file mode 100644 index 00000000000..b1baef8036b --- /dev/null +++ b/changelogs/unreleased/296856-prevent-creating-duplicate-pipelines-manually.yml @@ -0,0 +1,5 @@ +--- +title: Prevent creating duplicate pipelines manually +merge_request: 51076 +author: Kev @KevSlashNull +type: changed diff --git a/changelogs/unreleased/feat-bypass-admin-mode-on-git.yml b/changelogs/unreleased/feat-bypass-admin-mode-on-git.yml new file mode 100644 index 00000000000..d2b23d322ce --- /dev/null +++ b/changelogs/unreleased/feat-bypass-admin-mode-on-git.yml @@ -0,0 +1,5 @@ +--- +title: Bypass admin mode for internal api operations (ssh git & http rails) +merge_request: 52697 +author: Diego Louzán +type: changed diff --git a/changelogs/unreleased/fix-merge-trains-cannot-be-retried.yml b/changelogs/unreleased/fix-merge-trains-cannot-be-retried.yml new file mode 100644 index 00000000000..319026a1ad6 --- /dev/null +++ b/changelogs/unreleased/fix-merge-trains-cannot-be-retried.yml @@ -0,0 +1,5 @@ +--- +title: Fix retry option does not work in Merge Trains +merge_request: 52463 +author: +type: fixed diff --git a/changelogs/unreleased/lm-adds-gitaly-call-label.yml b/changelogs/unreleased/lm-adds-gitaly-call-label.yml new file mode 100644 index 00000000000..48e324cfa3d --- /dev/null +++ b/changelogs/unreleased/lm-adds-gitaly-call-label.yml @@ -0,0 +1,6 @@ +--- +title: Increase the complexity score of GraphQL detailedStatus#label as it can call + Gitaly +merge_request: 52708 +author: +type: changed diff --git a/changelogs/unreleased/ph-requestReviewBackend.yml b/changelogs/unreleased/ph-requestReviewBackend.yml new file mode 100644 index 00000000000..42f2c52c6fe --- /dev/null +++ b/changelogs/unreleased/ph-requestReviewBackend.yml @@ -0,0 +1,5 @@ +--- +title: Added ability to re-request a review from a reviewer +merge_request: 52321 +author: +type: added diff --git a/changelogs/unreleased/yo-gl-badge-users-admin.yml b/changelogs/unreleased/yo-gl-badge-users-admin.yml new file mode 100644 index 00000000000..08b371e54e1 --- /dev/null +++ b/changelogs/unreleased/yo-gl-badge-users-admin.yml @@ -0,0 +1,5 @@ +--- +title: Apply new GitLab UI badge for users in the admin page +merge_request: 52289 +author: Yogi (@yo) +type: other diff --git a/changelogs/unreleased/yo-gl-new-ui-admin-hooks.yml b/changelogs/unreleased/yo-gl-new-ui-admin-hooks.yml new file mode 100644 index 00000000000..ce3643057e4 --- /dev/null +++ b/changelogs/unreleased/yo-gl-new-ui-admin-hooks.yml @@ -0,0 +1,5 @@ +--- +title: Apply new GitLab UI for input field in admin/hooks +merge_request: 52412 +author: Yogi (@yo) +type: other diff --git a/config/feature_flags/development/core_security_mr_widget.yml b/config/feature_flags/development/core_security_mr_widget.yml deleted file mode 100644 index dfeb30cd83a..00000000000 --- a/config/feature_flags/development/core_security_mr_widget.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: core_security_mr_widget -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44639 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/249543 -milestone: '13.5' -type: development -group: group::static analysis -default_enabled: true diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index a4609b26e0c..9997772674d 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -1242,6 +1242,7 @@ production: &base ## Matomo analytics. # matomo_url: '_your_matomo_url' # matomo_site_id: '_your_matomo_site_id' + # matomo_disable_cookies: false rack_attack: git_basic_auth: diff --git a/config/initializers/faraday.rb b/config/initializers/faraday.rb new file mode 100644 index 00000000000..2700751e79f --- /dev/null +++ b/config/initializers/faraday.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +::Faraday::Request.register_middleware(gitlab_error_callback: -> { ::Gitlab::Faraday::ErrorCallback }) diff --git a/config/metrics/counts_28d/deployments.yml b/config/metrics/counts_28d/deployments.yml index 85fdc407156..02d620c214e 100644 --- a/config/metrics/counts_28d/deployments.yml +++ b/config/metrics/counts_28d/deployments.yml @@ -1,4 +1,4 @@ -key_path: counts_monthy.deployments +key_path: counts_monthly.deployments description: Total deployments count for recent 28 days value_type: integer stage: release diff --git a/db/migrate/20210122153259_add_state_to_merge_request_reviewers.rb b/db/migrate/20210122153259_add_state_to_merge_request_reviewers.rb new file mode 100644 index 00000000000..dd0c98615f7 --- /dev/null +++ b/db/migrate/20210122153259_add_state_to_merge_request_reviewers.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddStateToMergeRequestReviewers < ActiveRecord::Migration[6.0] + DOWNTIME = false + + REVIEW_DEFAULT_STATE = 0 + + def change + add_column :merge_request_reviewers, :state, :smallint, default: REVIEW_DEFAULT_STATE, null: false + end +end diff --git a/db/schema_migrations/20210122153259 b/db/schema_migrations/20210122153259 new file mode 100644 index 00000000000..887f0ac4a5c --- /dev/null +++ b/db/schema_migrations/20210122153259 @@ -0,0 +1 @@ +4c697cc183a000ee8c18b516e4b1d77d0f8d2d3d7abe11121f2240a60c03216c
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 704ba281d49..37316252119 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14069,7 +14069,8 @@ CREATE TABLE merge_request_reviewers ( id bigint NOT NULL, user_id bigint NOT NULL, merge_request_id bigint NOT NULL, - created_at timestamp with time zone NOT NULL + created_at timestamp with time zone NOT NULL, + state smallint DEFAULT 0 NOT NULL ); CREATE SEQUENCE merge_request_reviewers_id_seq diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index fa7e75352a7..2b01795f8c3 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -350,7 +350,7 @@ type AlertManagementAlert implements Noteable { title: String """ - Todos of the current user for the alert. + To-dos of the current user for the alert. """ todos( """ @@ -1092,12 +1092,12 @@ type AwardEmoji { name: String! """ - The emoji in unicode. + The emoji in Unicode. """ unicode: String! """ - The unicode version for this emoji. + The Unicode version for this emoji. """ unicodeVersion: String! @@ -1287,7 +1287,7 @@ type Blob implements Entry { path: String! """ - Last commit sha for the entry + Last commit SHA for the entry """ sha: String! @@ -2487,7 +2487,7 @@ type CiConfig { errors: [String!] """ - Merged CI config YAML. + Merged CI configuration YAML. """ mergedYaml: String @@ -8180,7 +8180,7 @@ interface Entry { path: String! """ - Last commit sha for the entry + Last commit SHA for the entry """ sha: String! @@ -11804,7 +11804,7 @@ type IncidentManagementOncallRotation { ): OncallParticipantTypeConnection """ - Blocks of time for which a participant is on-call within a given timeframe. Timeframe cannot exceed one month. + Blocks of time for which a participant is on-call within a given time frame. Time frame cannot exceed one month. """ shifts( """ @@ -15107,6 +15107,51 @@ type MergeRequestPermissions { } """ +Autogenerated input type of MergeRequestReviewerRereview +""" +input MergeRequestReviewerRereviewInput { + """ + A unique identifier for the client performing the mutation. + """ + clientMutationId: String + + """ + The IID of the merge request to mutate. + """ + iid: String! + + """ + The project the merge request to mutate is in. + """ + projectPath: ID! + + """ + The user ID for the user that has been requested for a new review. + """ + userId: UserID! +} + +""" +Autogenerated return type of MergeRequestReviewerRereview +""" +type MergeRequestReviewerRereviewPayload { + """ + A unique identifier for the client performing the mutation. + """ + clientMutationId: String + + """ + Errors encountered during execution of the mutation. + """ + errors: [String!]! + + """ + The merge request after mutation. + """ + mergeRequest: MergeRequest +} + +""" Autogenerated input type of MergeRequestSetAssignees """ input MergeRequestSetAssigneesInput { @@ -15945,6 +15990,7 @@ type Mutation { labelCreate(input: LabelCreateInput!): LabelCreatePayload markAsSpamSnippet(input: MarkAsSpamSnippetInput!): MarkAsSpamSnippetPayload mergeRequestCreate(input: MergeRequestCreateInput!): MergeRequestCreatePayload + mergeRequestReviewerRereview(input: MergeRequestReviewerRereviewInput!): MergeRequestReviewerRereviewPayload mergeRequestSetAssignees(input: MergeRequestSetAssigneesInput!): MergeRequestSetAssigneesPayload mergeRequestSetLabels(input: MergeRequestSetLabelsInput!): MergeRequestSetLabelsPayload mergeRequestSetLocked(input: MergeRequestSetLockedInput!): MergeRequestSetLockedPayload @@ -17535,7 +17581,7 @@ type Pipeline { committedAt: Time """ - Config source of the pipeline (UNKNOWN_SOURCE, REPOSITORY_SOURCE, + Configuration source of the pipeline (UNKNOWN_SOURCE, REPOSITORY_SOURCE, AUTO_DEVOPS_SOURCE, WEBIDE_SOURCE, REMOTE_SOURCE, EXTERNAL_PROJECT_SOURCE, BRIDGE_SOURCE, PARAMETER_SOURCE) """ @@ -19605,7 +19651,7 @@ type Project { snippetsEnabled: Boolean """ - Indicates if squash readonly is enabled + Indicates if `squashReadOnly` is enabled """ squashReadOnly: Boolean! @@ -22821,37 +22867,37 @@ Represents summary of a security report """ type SecurityReportSummary { """ - Aggregated counts for the api_fuzzing scan + Aggregated counts for the `api_fuzzing` scan """ apiFuzzing: SecurityReportSummarySection """ - Aggregated counts for the container_scanning scan + Aggregated counts for the `container_scanning` scan """ containerScanning: SecurityReportSummarySection """ - Aggregated counts for the coverage_fuzzing scan + Aggregated counts for the `coverage_fuzzing` scan """ coverageFuzzing: SecurityReportSummarySection """ - Aggregated counts for the dast scan + Aggregated counts for the `dast` scan """ dast: SecurityReportSummarySection """ - Aggregated counts for the dependency_scanning scan + Aggregated counts for the `dependency_scanning` scan """ dependencyScanning: SecurityReportSummarySection """ - Aggregated counts for the sast scan + Aggregated counts for the `sast` scan """ sast: SecurityReportSummarySection """ - Aggregated counts for the secret_detection scan + Aggregated counts for the `secret_detection` scan """ secretDetection: SecurityReportSummarySection } @@ -23824,7 +23870,7 @@ type SnippetBlobViewer { fileType: String! """ - Shows whether the blob content is loaded async + Shows whether the blob content is loaded asynchronously """ loadAsync: Boolean! @@ -24095,7 +24141,7 @@ type Submodule implements Entry { path: String! """ - Last commit sha for the entry + Last commit SHA for the entry """ sha: String! @@ -25135,7 +25181,7 @@ type TreeEntry implements Entry { path: String! """ - Last commit sha for the entry + Last commit SHA for the entry """ sha: String! diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index ba625724ea0..795c7987433 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -953,7 +953,7 @@ }, { "name": "todos", - "description": "Todos of the current user for the alert.", + "description": "To-dos of the current user for the alert.", "args": [ { "name": "action", @@ -2699,7 +2699,7 @@ }, { "name": "unicode", - "description": "The emoji in unicode.", + "description": "The emoji in Unicode.", "args": [ ], @@ -2717,7 +2717,7 @@ }, { "name": "unicodeVersion", - "description": "The unicode version for this emoji.", + "description": "The Unicode version for this emoji.", "args": [ ], @@ -3297,7 +3297,7 @@ }, { "name": "sha", - "description": "Last commit sha for the entry", + "description": "Last commit SHA for the entry", "args": [ ], @@ -6584,7 +6584,7 @@ }, { "name": "mergedYaml", - "description": "Merged CI config YAML.", + "description": "Merged CI configuration YAML.", "args": [ ], @@ -22659,7 +22659,7 @@ }, { "name": "sha", - "description": "Last commit sha for the entry", + "description": "Last commit SHA for the entry", "args": [ ], @@ -32232,7 +32232,7 @@ }, { "name": "shifts", - "description": "Blocks of time for which a participant is on-call within a given timeframe. Timeframe cannot exceed one month.", + "description": "Blocks of time for which a participant is on-call within a given time frame. Time frame cannot exceed one month.", "args": [ { "name": "startTime", @@ -41587,6 +41587,136 @@ }, { "kind": "INPUT_OBJECT", + "name": "MergeRequestReviewerRereviewInput", + "description": "Autogenerated input type of MergeRequestReviewerRereview", + "fields": null, + "inputFields": [ + { + "name": "projectPath", + "description": "The project the merge request to mutate is in.", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "ID", + "ofType": null + } + }, + "defaultValue": null + }, + { + "name": "iid", + "description": "The IID of the merge request to mutate.", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "String", + "ofType": null + } + }, + "defaultValue": null + }, + { + "name": "userId", + "description": "The user ID for the user that has been requested for a new review.\n", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "UserID", + "ofType": null + } + }, + "defaultValue": null + }, + { + "name": "clientMutationId", + "description": "A unique identifier for the client performing the mutation.", + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null + } + ], + "interfaces": null, + "enumValues": null, + "possibleTypes": null + }, + { + "kind": "OBJECT", + "name": "MergeRequestReviewerRereviewPayload", + "description": "Autogenerated return type of MergeRequestReviewerRereview", + "fields": [ + { + "name": "clientMutationId", + "description": "A unique identifier for the client performing the mutation.", + "args": [ + + ], + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "errors", + "description": "Errors encountered during execution of the mutation.", + "args": [ + + ], + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "LIST", + "name": null, + "ofType": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "String", + "ofType": null + } + } + } + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "mergeRequest", + "description": "The merge request after mutation.", + "args": [ + + ], + "type": { + "kind": "OBJECT", + "name": "MergeRequest", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + } + ], + "inputFields": null, + "interfaces": [ + + ], + "enumValues": null, + "possibleTypes": null + }, + { + "kind": "INPUT_OBJECT", "name": "MergeRequestSetAssigneesInput", "description": "Autogenerated input type of MergeRequestSetAssignees", "fields": null, @@ -45875,6 +46005,33 @@ "deprecationReason": null }, { + "name": "mergeRequestReviewerRereview", + "description": null, + "args": [ + { + "name": "input", + "description": null, + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "INPUT_OBJECT", + "name": "MergeRequestReviewerRereviewInput", + "ofType": null + } + }, + "defaultValue": null + } + ], + "type": { + "kind": "OBJECT", + "name": "MergeRequestReviewerRereviewPayload", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { "name": "mergeRequestSetAssignees", "description": null, "args": [ @@ -51696,7 +51853,7 @@ }, { "name": "configSource", - "description": "Config source of the pipeline (UNKNOWN_SOURCE, REPOSITORY_SOURCE, AUTO_DEVOPS_SOURCE, WEBIDE_SOURCE, REMOTE_SOURCE, EXTERNAL_PROJECT_SOURCE, BRIDGE_SOURCE, PARAMETER_SOURCE)", + "description": "Configuration source of the pipeline (UNKNOWN_SOURCE, REPOSITORY_SOURCE, AUTO_DEVOPS_SOURCE, WEBIDE_SOURCE, REMOTE_SOURCE, EXTERNAL_PROJECT_SOURCE, BRIDGE_SOURCE, PARAMETER_SOURCE)", "args": [ ], @@ -56965,7 +57122,7 @@ }, { "name": "squashReadOnly", - "description": "Indicates if squash readonly is enabled", + "description": "Indicates if `squashReadOnly` is enabled", "args": [ ], @@ -65989,7 +66146,7 @@ "fields": [ { "name": "apiFuzzing", - "description": "Aggregated counts for the api_fuzzing scan", + "description": "Aggregated counts for the `api_fuzzing` scan", "args": [ ], @@ -66003,7 +66160,7 @@ }, { "name": "containerScanning", - "description": "Aggregated counts for the container_scanning scan", + "description": "Aggregated counts for the `container_scanning` scan", "args": [ ], @@ -66017,7 +66174,7 @@ }, { "name": "coverageFuzzing", - "description": "Aggregated counts for the coverage_fuzzing scan", + "description": "Aggregated counts for the `coverage_fuzzing` scan", "args": [ ], @@ -66031,7 +66188,7 @@ }, { "name": "dast", - "description": "Aggregated counts for the dast scan", + "description": "Aggregated counts for the `dast` scan", "args": [ ], @@ -66045,7 +66202,7 @@ }, { "name": "dependencyScanning", - "description": "Aggregated counts for the dependency_scanning scan", + "description": "Aggregated counts for the `dependency_scanning` scan", "args": [ ], @@ -66059,7 +66216,7 @@ }, { "name": "sast", - "description": "Aggregated counts for the sast scan", + "description": "Aggregated counts for the `sast` scan", "args": [ ], @@ -66073,7 +66230,7 @@ }, { "name": "secretDetection", - "description": "Aggregated counts for the secret_detection scan", + "description": "Aggregated counts for the `secret_detection` scan", "args": [ ], @@ -69101,7 +69258,7 @@ }, { "name": "loadAsync", - "description": "Shows whether the blob content is loaded async", + "description": "Shows whether the blob content is loaded asynchronously", "args": [ ], @@ -69912,7 +70069,7 @@ }, { "name": "sha", - "description": "Last commit sha for the entry", + "description": "Last commit SHA for the entry", "args": [ ], @@ -73124,7 +73281,7 @@ }, { "name": "sha", - "description": "Last commit sha for the entry", + "description": "Last commit SHA for the entry", "args": [ ], diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 7677fb14c1e..7573c3b64e0 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -102,7 +102,7 @@ Describes an alert from the project's Alert Management. | `startedAt` | Time | Timestamp the alert was raised. | | `status` | AlertManagementStatus | Status of the alert. | | `title` | String | Title of the alert. | -| `todos` | TodoConnection | Todos of the current user for the alert. | +| `todos` | TodoConnection | To-dos of the current user for the alert. | | `updatedAt` | Time | Timestamp the alert was last updated. | ### AlertManagementAlertStatusCountsType @@ -179,8 +179,8 @@ An emoji awarded by a user. | `description` | String! | The emoji description. | | `emoji` | String! | The emoji as an icon. | | `name` | String! | The emoji name. | -| `unicode` | String! | The emoji in unicode. | -| `unicodeVersion` | String! | The unicode version for this emoji. | +| `unicode` | String! | The emoji in Unicode. | +| `unicodeVersion` | String! | The Unicode version for this emoji. | | `user` | User! | The user who awarded the emoji. | ### AwardEmojiAddPayload @@ -231,7 +231,7 @@ Autogenerated return type of AwardEmojiToggle. | `mode` | String | Blob mode in numeric format | | `name` | String! | Name of the entry | | `path` | String! | Path of the entry | -| `sha` | String! | Last commit sha for the entry | +| `sha` | String! | Last commit SHA for the entry | | `type` | EntryType! | Type of tree entry | | `webPath` | String | Web path of the blob | | `webUrl` | String | Web URL of the blob | @@ -397,7 +397,7 @@ Autogenerated return type of CiCdSettingsUpdate. | Field | Type | Description | | ----- | ---- | ----------- | | `errors` | String! => Array | Linting errors. | -| `mergedYaml` | String | Merged CI config YAML. | +| `mergedYaml` | String | Merged CI configuration YAML. | | `stages` | CiConfigStageConnection | Stages of the pipeline. | | `status` | CiConfigStatus | Status of linting, can be either valid or invalid. | @@ -1809,7 +1809,7 @@ Describes an incident management on-call rotation. | `lengthUnit` | OncallRotationUnitEnum | Unit of the on-call rotation length. | | `name` | String! | Name of the on-call rotation. | | `participants` | OncallParticipantTypeConnection | Participants of the on-call rotation. | -| `shifts` | IncidentManagementOncallShiftConnection | Blocks of time for which a participant is on-call within a given timeframe. Timeframe cannot exceed one month. | +| `shifts` | IncidentManagementOncallShiftConnection | Blocks of time for which a participant is on-call within a given time frame. Time frame cannot exceed one month. | | `startsAt` | Time | Start date of the on-call rotation. | ### IncidentManagementOncallSchedule @@ -2287,6 +2287,16 @@ Check permissions for the current user on a merge request. | `revertOnCurrentMergeRequest` | Boolean! | Indicates the user can perform `revert_on_current_merge_request` on this resource | | `updateMergeRequest` | Boolean! | Indicates the user can perform `update_merge_request` on this resource | +### MergeRequestReviewerRereviewPayload + +Autogenerated return type of MergeRequestReviewerRereview. + +| Field | Type | Description | +| ----- | ---- | ----------- | +| `clientMutationId` | String | A unique identifier for the client performing the mutation. | +| `errors` | String! => Array | Errors encountered during execution of the mutation. | +| `mergeRequest` | MergeRequest | The merge request after mutation. | + ### MergeRequestSetAssigneesPayload Autogenerated return type of MergeRequestSetAssignees. @@ -2657,7 +2667,7 @@ Information about pagination in a connection.. | `beforeSha` | String | Base SHA of the source branch. | | `cancelable` | Boolean! | Specifies if a pipeline can be canceled. | | `committedAt` | Time | Timestamp of the pipeline's commit. | -| `configSource` | PipelineConfigSourceEnum | Config source of the pipeline (UNKNOWN_SOURCE, REPOSITORY_SOURCE, AUTO_DEVOPS_SOURCE, WEBIDE_SOURCE, REMOTE_SOURCE, EXTERNAL_PROJECT_SOURCE, BRIDGE_SOURCE, PARAMETER_SOURCE) | +| `configSource` | PipelineConfigSourceEnum | Configuration source of the pipeline (UNKNOWN_SOURCE, REPOSITORY_SOURCE, AUTO_DEVOPS_SOURCE, WEBIDE_SOURCE, REMOTE_SOURCE, EXTERNAL_PROJECT_SOURCE, BRIDGE_SOURCE, PARAMETER_SOURCE) | | `coverage` | Float | Coverage percentage. | | `createdAt` | Time! | Timestamp of the pipeline's creation. | | `detailedStatus` | DetailedStatus! | Detailed status of the pipeline. | @@ -2826,7 +2836,7 @@ Autogenerated return type of PipelineRetry. | `sharedRunnersEnabled` | Boolean | Indicates if shared runners are enabled for the project | | `snippets` | SnippetConnection | Snippets of the project | | `snippetsEnabled` | Boolean | Indicates if Snippets are enabled for the current user | -| `squashReadOnly` | Boolean! | Indicates if squash readonly is enabled | +| `squashReadOnly` | Boolean! | Indicates if `squashReadOnly` is enabled | | `sshUrlToRepo` | String | URL to connect to the project via SSH | | `starCount` | Int! | Number of times the project has been starred | | `statistics` | ProjectStatistics | Statistics of the project | @@ -3283,13 +3293,13 @@ Represents summary of a security report. | Field | Type | Description | | ----- | ---- | ----------- | -| `apiFuzzing` | SecurityReportSummarySection | Aggregated counts for the api_fuzzing scan | -| `containerScanning` | SecurityReportSummarySection | Aggregated counts for the container_scanning scan | -| `coverageFuzzing` | SecurityReportSummarySection | Aggregated counts for the coverage_fuzzing scan | -| `dast` | SecurityReportSummarySection | Aggregated counts for the dast scan | -| `dependencyScanning` | SecurityReportSummarySection | Aggregated counts for the dependency_scanning scan | -| `sast` | SecurityReportSummarySection | Aggregated counts for the sast scan | -| `secretDetection` | SecurityReportSummarySection | Aggregated counts for the secret_detection scan | +| `apiFuzzing` | SecurityReportSummarySection | Aggregated counts for the `api_fuzzing` scan | +| `containerScanning` | SecurityReportSummarySection | Aggregated counts for the `container_scanning` scan | +| `coverageFuzzing` | SecurityReportSummarySection | Aggregated counts for the `coverage_fuzzing` scan | +| `dast` | SecurityReportSummarySection | Aggregated counts for the `dast` scan | +| `dependencyScanning` | SecurityReportSummarySection | Aggregated counts for the `dependency_scanning` scan | +| `sast` | SecurityReportSummarySection | Aggregated counts for the `sast` scan | +| `secretDetection` | SecurityReportSummarySection | Aggregated counts for the `secret_detection` scan | ### SecurityReportSummarySection @@ -3482,7 +3492,7 @@ Represents how the blob content should be displayed. | ----- | ---- | ----------- | | `collapsed` | Boolean! | Shows whether the blob should be displayed collapsed | | `fileType` | String! | Content file type | -| `loadAsync` | Boolean! | Shows whether the blob content is loaded async | +| `loadAsync` | Boolean! | Shows whether the blob content is loaded asynchronously | | `loadingPartialName` | String! | Loading partial name | | `renderError` | String | Error rendering the blob content | | `tooLarge` | Boolean! | Shows whether the blob too large to be displayed | @@ -3532,7 +3542,7 @@ Represents the Geo sync and verification state of a snippet repository. | `id` | ID! | ID of the entry | | `name` | String! | Name of the entry | | `path` | String! | Path of the entry | -| `sha` | String! | Last commit sha for the entry | +| `sha` | String! | Last commit SHA for the entry | | `treeUrl` | String | Tree URL for the sub-module | | `type` | EntryType! | Type of tree entry | | `webUrl` | String | Web URL for the sub-module | @@ -3759,7 +3769,7 @@ Represents a directory. | `id` | ID! | ID of the entry | | `name` | String! | Name of the entry | | `path` | String! | Path of the entry | -| `sha` | String! | Last commit sha for the entry | +| `sha` | String! | Last commit SHA for the entry | | `type` | EntryType! | Type of tree entry | | `webPath` | String | Web path for the tree entry (directory) | | `webUrl` | String | Web URL for the tree entry (directory) | diff --git a/doc/development/cicd/index.md b/doc/development/cicd/index.md index eede1d691a9..196b6461f72 100644 --- a/doc/development/cicd/index.md +++ b/doc/development/cicd/index.md @@ -143,3 +143,25 @@ Finally if the runner can only pick jobs that are tagged, all untagged jobs are At this point we loop through remaining `pending` jobs and we try to assign the first job that the runner "can pick" based on additional policies. For example, runners marked as `protected` can only pick jobs that run against protected branches (such as production deployments). As we increase the number of runners in the pool we also increase the chances of conflicts which would arise if assigning the same job to different runners. To prevent that we gracefully rescue conflict errors and assign the next job in the list. + +## The definition of "Job" in GitLab CI/CD + +"Job" in GitLab CI context refers a task to drive Continuous Integartion, Delivery and Deployment. +Typically, a pipeline contains multiple stages, and a stage contains multiple jobs. + +In Active Record modeling, Job is defined as `CommitStatus` class. +On top of that, we have the following types of jobs: + +- `Ci::Build` ... The job to be executed by runners. +- `Ci::Bridge` ... The job to trigger a downstream pipeline. +- `GenericCommitStatus` ... The job to be executed in an external CI/CD system e.g. Jenkins. + +Please note that, when you use the "Job" terminology in codebase, readers would +assume that the class/object is any type of above. +If you specifically refer `Ci::Build` class, you should not name the object/class +as "job" as this could cause some confusions. In documentation, +we should use "Job" in general, instead of "Build". + +We have a few inconsistencies in our codebase that should be refactored. +For example, `CommitStatus` should be `Ci::Job` and `Ci::JobArtifact` should be `Ci::BuildArtifact`. +Please read [this isse](https://gitlab.com/gitlab-org/gitlab/-/issues/16111) for the full refactoring plan. diff --git a/doc/development/usage_ping/dictionary.md b/doc/development/usage_ping/dictionary.md index f3741c02cd0..4e8bcd4c685 100644 --- a/doc/development/usage_ping/dictionary.md +++ b/doc/development/usage_ping/dictionary.md @@ -63,13 +63,13 @@ Total number of sites in a Geo deployment | `distribution` | ee | | `tier` | premium, ultimate | -## counts_monthy.deployments +## counts_monthly.deployments Total deployments count for recent 28 days | field | value | | --- | --- | -| `key_path` | **counts_monthy.deployments** | +| `key_path` | **counts_monthly.deployments** | | `value_type` | integer | | `stage` | release | | `status` | data_available | diff --git a/doc/user/application_security/index.md b/doc/user/application_security/index.md index 7b58ea0a945..b7ee256c3d6 100644 --- a/doc/user/application_security/index.md +++ b/doc/user/application_security/index.md @@ -128,14 +128,7 @@ with this approach, however, and there is a > - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/4393) in GitLab Free 13.5. > - Made [available in all tiers](https://gitlab.com/gitlab-org/gitlab/-/issues/273205) in 13.6. > - Report download dropdown [added](https://gitlab.com/gitlab-org/gitlab/-/issues/273418) in 13.7. -> - It's [deployed behind a feature flag](../feature_flags.md), enabled by default. -> - It's enabled on GitLab.com. -> - It can be enabled or disabled for a single project. -> - It's recommended for production use. -> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-the-basic-security-widget). **(FREE SELF)** - -WARNING: -This feature might not be available to you. Check the **version history** note above for details. +> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/249550) in GitLab 13.9. Merge requests which have run security scans let you know that the generated reports are available to download. To download a report, click on the @@ -667,31 +660,6 @@ Analyzer results are displayed in the [job logs](../../ci/jobs/index.md#expand-a or [Security Dashboard](security_dashboard/index.md). There is [an open issue](https://gitlab.com/gitlab-org/gitlab/-/issues/235772) in which changes to this behavior are being discussed. -### Enable or disable the basic security widget **(FREE SELF)** - -The basic security widget is under development but ready for production use. -It is deployed behind a feature flag that is **enabled by default**. -[GitLab administrators with access to the GitLab Rails console](../feature_flags.md) -can opt to disable it. - -To enable it: - -```ruby -# For the instance -Feature.enable(:core_security_mr_widget) -# For a single project -Feature.enable(:core_security_mr_widget, Project.find(<project id>)) -``` - -To disable it: - -```ruby -# For the instance -Feature.disable(:core_security_mr_widget) -# For a single project -Feature.disable(:core_security_mr_widget, Project.find(<project id>)) -``` - ### Error: job `is used for configuration only, and its script should not be executed` [Changes made in GitLab 13.4](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41260) diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb index 12bb6e77c3e..6de80c17960 100644 --- a/lib/api/internal/base.rb +++ b/lib/api/internal/base.rb @@ -52,7 +52,9 @@ module API actor.update_last_used_at! check_result = begin - access_check!(actor, params) + Gitlab::Auth::CurrentUserMode.bypass_session!(actor.user&.id) do + access_check!(actor, params) + end rescue Gitlab::GitAccess::ForbiddenError => e # The return code needs to be 401. If we return 403 # the custom message we return won't be shown to the user diff --git a/lib/gitlab/faraday.rb b/lib/gitlab/faraday.rb deleted file mode 100644 index f92392ec1a9..00000000000 --- a/lib/gitlab/faraday.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Faraday - ::Faraday::Request.register_middleware(gitlab_error_callback: -> { ::Gitlab::Faraday::ErrorCallback }) - end -end diff --git a/lib/gitlab/tracking/standard_context.rb b/lib/gitlab/tracking/standard_context.rb index c39f32f71cf..0c4911ba47e 100644 --- a/lib/gitlab/tracking/standard_context.rb +++ b/lib/gitlab/tracking/standard_context.rb @@ -3,7 +3,7 @@ module Gitlab module Tracking class StandardContext - GITLAB_STANDARD_SCHEMA_URL = 'iglu:com.gitlab/gitlab_standard/jsonschema/1-0-2'.freeze + GITLAB_STANDARD_SCHEMA_URL = 'iglu:com.gitlab/gitlab_standard/jsonschema/1-0-3'.freeze GITLAB_RAILS_SOURCE = 'gitlab-rails'.freeze def initialize(namespace: nil, project: nil, user: nil, **data) diff --git a/spec/frontend/jira_connect/index_spec.js b/spec/frontend/jira_connect/index_spec.js new file mode 100644 index 00000000000..eb54fe6476f --- /dev/null +++ b/spec/frontend/jira_connect/index_spec.js @@ -0,0 +1,56 @@ +import waitForPromises from 'helpers/wait_for_promises'; +import { initJiraConnect } from '~/jira_connect'; +import { removeSubscription } from '~/jira_connect/api'; + +jest.mock('~/jira_connect/api', () => ({ + removeSubscription: jest.fn().mockResolvedValue(), + getLocation: jest.fn().mockResolvedValue('test/location'), +})); + +describe('initJiraConnect', () => { + window.AP = { + navigator: { + reload: jest.fn(), + }, + }; + + beforeEach(async () => { + setFixtures(` + <a class="js-jira-connect-sign-in" href="https://gitlab.com">Sign In</a> + <a class="js-jira-connect-sign-in" href="https://gitlab.com">Another Sign In</a> + + <a href="https://gitlab.com/sub1" class="js-jira-connect-remove-subscription">Remove</a> + <a href="https://gitlab.com/sub2" class="js-jira-connect-remove-subscription">Remove</a> + <a href="https://gitlab.com/sub3" class="js-jira-connect-remove-subscription">Remove</a> + `); + + await initJiraConnect(); + }); + + describe('Sign in links', () => { + it('have `return_to` query parameter', () => { + Array.from(document.querySelectorAll('.js-jira-connect-sign-in')).forEach((el) => { + expect(el.href).toContain('return_to=test/location'); + }); + }); + }); + + describe('`remove subscription` buttons', () => { + describe('on click', () => { + it('calls `removeSubscription`', () => { + Array.from(document.querySelectorAll('.js-jira-connect-remove-subscription')).forEach( + (removeSubscriptionButton) => { + removeSubscriptionButton.dispatchEvent(new Event('click')); + + waitForPromises(); + + expect(removeSubscription).toHaveBeenCalledWith(removeSubscriptionButton.href); + expect(removeSubscription).toHaveBeenCalledTimes(1); + + removeSubscription.mockClear(); + }, + ); + }); + }); + }); +}); diff --git a/spec/frontend/pipeline_new/components/pipeline_new_form_spec.js b/spec/frontend/pipeline_new/components/pipeline_new_form_spec.js index cf3745f4156..3f1cf67127e 100644 --- a/spec/frontend/pipeline_new/components/pipeline_new_form_spec.js +++ b/spec/frontend/pipeline_new/components/pipeline_new_form_spec.js @@ -34,6 +34,7 @@ describe('Pipeline New Form', () => { const findForm = () => wrapper.find(GlForm); const findDropdown = () => wrapper.find(GlDropdown); const findDropdownItems = () => wrapper.findAll(GlDropdownItem); + const findSubmitButton = () => wrapper.find('[data-testid="run_pipeline_button"]'); const findVariableRows = () => wrapper.findAll('[data-testid="ci-variable-row"]'); const findRemoveIcons = () => wrapper.findAll('[data-testid="remove-ci-variable-row"]'); const findKeyInputs = () => wrapper.findAll('[data-testid="pipeline-form-ci-variable-key"]'); @@ -155,6 +156,18 @@ describe('Pipeline New Form', () => { await waitForPromises(); }); + + it('disables the submit button immediately after submitting', async () => { + createComponent(); + + expect(findSubmitButton().props('disabled')).toBe(false); + + findForm().vm.$emit('submit', dummySubmitEvent); + await waitForPromises(); + + expect(findSubmitButton().props('disabled')).toBe(true); + }); + it('creates pipeline with full ref and variables', async () => { createComponent(); @@ -167,6 +180,7 @@ describe('Pipeline New Form', () => { expect(getExpectedPostParams().ref).toEqual(wrapper.vm.$data.refValue.fullName); expect(redirectTo).toHaveBeenCalledWith(`${pipelinesPath}/${postResponse.id}`); }); + it('creates a pipeline with short ref and variables', async () => { // query params are used createComponent('', mockParams); @@ -312,31 +326,55 @@ describe('Pipeline New Form', () => { describe('Form errors and warnings', () => { beforeEach(() => { createComponent(); + }); - mock.onPost(pipelinesPath).reply(httpStatusCodes.BAD_REQUEST, mockError); + describe('when the error response can be handled', () => { + beforeEach(async () => { + mock.onPost(pipelinesPath).reply(httpStatusCodes.BAD_REQUEST, mockError); - findForm().vm.$emit('submit', dummySubmitEvent); + findForm().vm.$emit('submit', dummySubmitEvent); - return waitForPromises(); - }); + await waitForPromises(); + }); - it('shows both error and warning', () => { - expect(findErrorAlert().exists()).toBe(true); - expect(findWarningAlert().exists()).toBe(true); - }); + it('shows both error and warning', () => { + expect(findErrorAlert().exists()).toBe(true); + expect(findWarningAlert().exists()).toBe(true); + }); - it('shows the correct error', () => { - expect(findErrorAlert().text()).toBe(mockError.errors[0]); - }); + it('shows the correct error', () => { + expect(findErrorAlert().text()).toBe(mockError.errors[0]); + }); - it('shows the correct warning title', () => { - const { length } = mockError.warnings; + it('shows the correct warning title', () => { + const { length } = mockError.warnings; + + expect(findWarningAlertSummary().attributes('message')).toBe(`${length} warnings found:`); + }); + + it('shows the correct amount of warnings', () => { + expect(findWarnings()).toHaveLength(mockError.warnings.length); + }); - expect(findWarningAlertSummary().attributes('message')).toBe(`${length} warnings found:`); + it('re-enables the submit button', () => { + expect(findSubmitButton().props('disabled')).toBe(false); + }); }); - it('shows the correct amount of warnings', () => { - expect(findWarnings()).toHaveLength(mockError.warnings.length); + describe('when the error response cannot be handled', () => { + beforeEach(async () => { + mock + .onPost(pipelinesPath) + .reply(httpStatusCodes.INTERNAL_SERVER_ERROR, 'something went wrong'); + + findForm().vm.$emit('submit', dummySubmitEvent); + + await waitForPromises(); + }); + + it('re-enables the submit button', () => { + expect(findSubmitButton().props('disabled')).toBe(false); + }); }); }); }); diff --git a/spec/frontend/vue_mr_widget/mr_widget_options_spec.js b/spec/frontend/vue_mr_widget/mr_widget_options_spec.js index 8c642799fb8..872dcd2af7f 100644 --- a/spec/frontend/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/frontend/vue_mr_widget/mr_widget_options_spec.js @@ -825,14 +825,11 @@ describe('MrWidgetOptions', () => { describe('security widget', () => { describe.each` - context | hasPipeline | isFlagEnabled | shouldRender - ${'has pipeline and flag enabled'} | ${true} | ${true} | ${true} - ${'has pipeline and flag disabled'} | ${true} | ${false} | ${false} - ${'no pipeline and flag enabled'} | ${false} | ${true} | ${false} - `('given $context', ({ hasPipeline, isFlagEnabled, shouldRender }) => { + context | hasPipeline | shouldRender + ${'there is a pipeline'} | ${true} | ${true} + ${'no pipeline'} | ${false} | ${false} + `('given $context', ({ hasPipeline, shouldRender }) => { beforeEach(() => { - gon.features.coreSecurityMrWidget = isFlagEnabled; - const mrData = { ...mockData, ...(hasPipeline ? {} : { pipeline: null }), diff --git a/spec/requests/api/graphql/mutations/merge_requests/reviewer_rereview_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/reviewer_rereview_spec.rb new file mode 100644 index 00000000000..2e4f35cbcde --- /dev/null +++ b/spec/requests/api/graphql/mutations/merge_requests/reviewer_rereview_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Setting assignees of a merge request' do + include GraphqlHelpers + + let(:current_user) { create(:user) } + let(:merge_request) { create(:merge_request, reviewers: [user]) } + let(:project) { merge_request.project } + let(:user) { create(:user) } + let(:input) { { user_id: global_id_of(user) } } + + let(:mutation) do + variables = { + project_path: project.full_path, + iid: merge_request.iid.to_s + } + graphql_mutation(:merge_request_reviewer_rereview, variables.merge(input), + <<-QL.strip_heredoc + clientMutationId + errors + QL + ) + end + + def mutation_response + graphql_mutation_response(:merge_request_reviewer_rereview) + end + + def mutation_errors + mutation_response['errors'] + end + + before do + project.add_developer(current_user) + project.add_developer(user) + end + + it 'returns an error if the user is not allowed to update the merge request' do + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).not_to be_empty + end + + describe 'reviewer does not exist' do + let(:input) { { user_id: global_id_of(create(:user)) } } + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_errors).not_to be_empty + end + end + + describe 'reviewer exists' do + it 'does not return an error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_errors).to be_empty + end + end +end diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index e04f63befd0..be4ecd0a734 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -1094,6 +1094,104 @@ RSpec.describe API::Internal::Base do expect(response).to have_gitlab_http_status(:unauthorized) end end + + context 'admin mode' do + shared_examples 'pushes succeed for ssh and http' do + it 'accepts the SSH push' do + push(key, project) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'accepts the HTTP push' do + push(key, project, 'http') + + expect(response).to have_gitlab_http_status(:ok) + end + end + + shared_examples 'pushes fail for ssh and http' do + it 'rejects the SSH push' do + push(key, project) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'rejects the HTTP push' do + push(key, project, 'http') + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'feature flag :user_mode_in_session is enabled' do + context 'with an admin user' do + let(:user) { create(:admin) } + + context 'is member of the project' do + before do + project.add_developer(user) + end + + it_behaves_like 'pushes succeed for ssh and http' + end + + context 'is not member of the project' do + it_behaves_like 'pushes succeed for ssh and http' + end + end + + context 'with a regular user' do + context 'is member of the project' do + before do + project.add_developer(user) + end + + it_behaves_like 'pushes succeed for ssh and http' + end + + context 'is not member of the project' do + it_behaves_like 'pushes fail for ssh and http' + end + end + end + + context 'feature flag :user_mode_in_session is disabled' do + before do + stub_feature_flags(user_mode_in_session: false) + end + + context 'with an admin user' do + let(:user) { create(:admin) } + + context 'is member of the project' do + before do + project.add_developer(user) + end + + it_behaves_like 'pushes succeed for ssh and http' + end + + context 'is not member of the project' do + it_behaves_like 'pushes succeed for ssh and http' + end + end + + context 'with a regular user' do + context 'is member of the project' do + before do + project.add_developer(user) + end + + it_behaves_like 'pushes succeed for ssh and http' + end + + context 'is not member of the project' do + it_behaves_like 'pushes fail for ssh and http' + end + end + end + end end describe 'POST /internal/post_receive', :clean_gitlab_redis_shared_state do diff --git a/spec/serializers/user_serializer_spec.rb b/spec/serializers/user_serializer_spec.rb index d54f33b6a23..f2ef1508098 100644 --- a/spec/serializers/user_serializer_spec.rb +++ b/spec/serializers/user_serializer_spec.rb @@ -9,7 +9,7 @@ RSpec.describe UserSerializer do context 'serializer with merge request context' do let(:merge_request) { create(:merge_request) } let(:project) { merge_request.project } - let(:serializer) { described_class.new(merge_request_iid: merge_request.iid) } + let(:serializer) { described_class.new(current_user: user1, merge_request_iid: merge_request.iid) } before do allow(project).to( diff --git a/spec/services/merge_requests/mark_reviewer_reviewed_service_spec.rb b/spec/services/merge_requests/mark_reviewer_reviewed_service_spec.rb new file mode 100644 index 00000000000..1075f6f9034 --- /dev/null +++ b/spec/services/merge_requests/mark_reviewer_reviewed_service_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::MarkReviewerReviewedService do + let(:current_user) { create(:user) } + let(:merge_request) { create(:merge_request, reviewers: [current_user]) } + let(:reviewer) { merge_request.merge_request_reviewers.find_by(user_id: current_user.id) } + let(:project) { merge_request.project } + let(:service) { described_class.new(project, current_user) } + let(:result) { service.execute(merge_request) } + + before do + project.add_developer(current_user) + end + + describe '#execute' do + describe 'invalid permissions' do + let(:service) { described_class.new(project, create(:user)) } + + it 'returns an error' do + expect(result[:status]).to eq :error + end + end + + describe 'reviewer does not exist' do + let(:service) { described_class.new(project, create(:user)) } + + it 'returns an error' do + expect(result[:status]).to eq :error + end + end + + describe 'reviewer exists' do + it 'returns success' do + expect(result[:status]).to eq :success + end + + it 'updates reviewers state' do + expect(result[:status]).to eq :success + expect(reviewer.state).to eq 'reviewed' + end + end + end +end diff --git a/spec/services/merge_requests/request_review_service_spec.rb b/spec/services/merge_requests/request_review_service_spec.rb new file mode 100644 index 00000000000..5cb4120852a --- /dev/null +++ b/spec/services/merge_requests/request_review_service_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::RequestReviewService do + let(:current_user) { create(:user) } + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request, reviewers: [user]) } + let(:reviewer) { merge_request.find_reviewer(user) } + let(:project) { merge_request.project } + let(:service) { described_class.new(project, current_user) } + let(:result) { service.execute(merge_request, user) } + let(:todo_service) { spy('todo service') } + let(:notification_service) { spy('notification service') } + + before do + allow(NotificationService).to receive(:new) { notification_service } + allow(service).to receive(:todo_service).and_return(todo_service) + allow(service).to receive(:notification_service).and_return(notification_service) + + reviewer.update!(state: MergeRequestReviewer.states[:reviewed]) + + project.add_developer(current_user) + project.add_developer(user) + end + + describe '#execute' do + describe 'invalid permissions' do + let(:service) { described_class.new(project, create(:user)) } + + it 'returns an error' do + expect(result[:status]).to eq :error + end + end + + describe 'reviewer does not exist' do + let(:result) { service.execute(merge_request, create(:user)) } + + it 'returns an error' do + expect(result[:status]).to eq :error + end + end + + describe 'reviewer exists' do + it 'returns success' do + expect(result[:status]).to eq :success + end + + it 'updates reviewers state' do + service.execute(merge_request, user) + reviewer.reload + + expect(reviewer.state).to eq 'unreviewed' + end + + it 'sends email to reviewer' do + expect(notification_service).to receive_message_chain(:async, :review_requested_of_merge_request).with(merge_request, current_user, user) + + service.execute(merge_request, user) + end + + it 'creates a new todo for the reviewer' do + expect(todo_service).to receive(:create_request_review_todo).with(merge_request, current_user, user) + + service.execute(merge_request, user) + end + end + end +end diff --git a/spec/services/notification_recipients/build_service_spec.rb b/spec/services/notification_recipients/build_service_spec.rb index cc08f9fceff..ff54d6ccd2f 100644 --- a/spec/services/notification_recipients/build_service_spec.rb +++ b/spec/services/notification_recipients/build_service_spec.rb @@ -110,4 +110,28 @@ RSpec.describe NotificationRecipients::BuildService do end end end + + describe '#build_requested_review_recipients' do + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + + before do + merge_request.reviewers.push(assignee) + end + + shared_examples 'no N+1 queries' do + it 'avoids N+1 queries', :request_store do + create_user + + service.build_requested_review_recipients(note) + + control_count = ActiveRecord::QueryRecorder.new do + service.build_requested_review_recipients(note) + end + + create_user + + expect { service.build_requested_review_recipients(note) }.not_to exceed_query_limit(control_count) + end + end + end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 85234077b1f..b67c37ba02d 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -2177,6 +2177,46 @@ RSpec.describe NotificationService, :mailer do let(:notification_trigger) { notification.merge_when_pipeline_succeeds(merge_request, @u_disabled) } end end + + describe '#review_requested_of_merge_request' do + let(:merge_request) { create(:merge_request, author: author, source_project: project, reviewers: [reviewer]) } + + let_it_be(:current_user) { create(:user) } + let_it_be(:reviewer) { create(:user) } + + it 'sends email to reviewer', :aggregate_failures do + notification.review_requested_of_merge_request(merge_request, current_user, reviewer) + + merge_request.reviewers.each { |reviewer| should_email(reviewer) } + should_not_email(merge_request.author) + should_not_email(@u_watcher) + should_not_email(@u_participant_mentioned) + should_not_email(@subscriber) + should_not_email(@watcher_and_subscriber) + should_not_email(@u_guest_watcher) + should_not_email(@u_guest_custom) + should_not_email(@u_custom_global) + should_not_email(@unsubscriber) + should_not_email(@u_participating) + should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) + end + + it 'adds "review requested" reason for new reviewer' do + notification.review_requested_of_merge_request(merge_request, current_user, [reviewer]) + + merge_request.reviewers.each do |reviewer| + email = find_email_for(reviewer) + + expect(email).to have_header('X-GitLab-NotificationReason', NotificationReason::REVIEW_REQUESTED) + end + end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.review_requested_of_merge_request(merge_request, current_user, reviewer) } + end + end end describe 'Projects', :deliver_mails_inline do diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 83d233a8112..743dc080b06 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -1193,6 +1193,17 @@ RSpec.describe TodoService do end end + describe '#create_request_review_todo' do + let(:target) { create(:merge_request, author: author, source_project: project) } + let(:reviewer) { create(:user) } + + it 'creates a todo for reviewer' do + service.create_request_review_todo(target, author, reviewer) + + should_create_todo(user: reviewer, target: target, action: Todo::REVIEW_REQUESTED) + end + end + def should_create_todo(attributes = {}) attributes.reverse_merge!( project: project, diff --git a/spec/views/layouts/_head.html.haml_spec.rb b/spec/views/layouts/_head.html.haml_spec.rb index 15fdfaaaa65..0b06309deff 100644 --- a/spec/views/layouts/_head.html.haml_spec.rb +++ b/spec/views/layouts/_head.html.haml_spec.rb @@ -102,6 +102,44 @@ RSpec.describe 'layouts/_head' do expect(rendered).to match(/<script.*>.*var u="\/\/#{matomo_host}\/".*<\/script>/m) expect(rendered).to match(%r(<noscript>.*<img src="//#{matomo_host}/matomo.php.*</noscript>)) end + + context 'matomo_disable_cookies' do + context 'when true' do + before do + stub_config(extra: { matomo_url: matomo_host, matomo_site_id: 12345, matomo_disable_cookies: true }) + end + + it 'disables cookies' do + render + + expect(rendered).to include('_paq.push(["disableCookies"])') + end + end + + context 'when false' do + before do + stub_config(extra: { matomo_url: matomo_host, matomo_site_id: 12345, matomo_disable_cookies: false }) + end + + it 'does not disable cookies' do + render + + expect(rendered).not_to include('_paq.push(["disableCookies"])') + end + end + + context 'when absent' do + before do + stub_config(extra: { matomo_url: matomo_host, matomo_site_id: 12345 }) + end + + it 'does not disable cookies' do + render + + expect(rendered).not_to include('_paq.push(["disableCookies"])') + end + end + end end def stub_helper_with_safe_string(method) |