diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6f7a0e2b987..e38e2f765bd 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -17,7 +17,7 @@ stages: # in cases where jobs require Docker-in-Docker, the job # definition must be extended with `.use-docker-in-docker` default: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-11-graphicsmagick-1.3.34" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-11-graphicsmagick-1.3.34" tags: - gitlab-org # All jobs are interruptible by default diff --git a/.gitlab/ci/cng.gitlab-ci.yml b/.gitlab/ci/cng.gitlab-ci.yml index 269996dfd09..af735d3212a 100644 --- a/.gitlab/ci/cng.gitlab-ci.yml +++ b/.gitlab/ci/cng.gitlab-ci.yml @@ -1,6 +1,6 @@ cloud-native-image: extends: .cng:rules - image: ruby:2.7-alpine + image: ${GITLAB_DEPENDENCY_PROXY}ruby:2.7-alpine dependencies: [] stage: post-test variables: diff --git a/.gitlab/ci/docs.gitlab-ci.yml b/.gitlab/ci/docs.gitlab-ci.yml index ce8be9e9976..d486b12ca7b 100644 --- a/.gitlab/ci/docs.gitlab-ci.yml +++ b/.gitlab/ci/docs.gitlab-ci.yml @@ -2,7 +2,7 @@ extends: - .default-retry - .docs:rules:review-docs - image: ruby:2.7-alpine + image: ${GITLAB_DEPENDENCY_PROXY}ruby:2.7-alpine stage: review needs: [] variables: diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index 004742b97ed..1b4b8a12772 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -259,13 +259,13 @@ coverage-frontend: qa-frontend-node:10: extends: .qa-frontend-node - image: node:dubnium + image: ${GITLAB_DEPENDENCY_PROXY}node:dubnium qa-frontend-node:latest: extends: - .qa-frontend-node - .frontend:rules:qa-frontend-node-latest - image: node:latest + image: ${GITLAB_DEPENDENCY_PROXY}node:latest webpack-dev-server: extends: diff --git a/.gitlab/ci/global.gitlab-ci.yml b/.gitlab/ci/global.gitlab-ci.yml index d6484c587c1..f0546f6467c 100644 --- a/.gitlab/ci/global.gitlab-ci.yml +++ b/.gitlab/ci/global.gitlab-ci.yml @@ -79,7 +79,7 @@ policy: pull .use-pg11: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-11-graphicsmagick-1.3.34" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-11-graphicsmagick-1.3.34" services: - name: postgres:11.6 command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"] @@ -88,7 +88,7 @@ POSTGRES_HOST_AUTH_METHOD: trust .use-pg12: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-12-graphicsmagick-1.3.34" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-12-graphicsmagick-1.3.34" services: - name: postgres:12 command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"] @@ -97,7 +97,7 @@ POSTGRES_HOST_AUTH_METHOD: trust .use-pg11-ee: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-11-graphicsmagick-1.3.34" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-11-graphicsmagick-1.3.34" services: - name: postgres:11.6 command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"] @@ -108,7 +108,7 @@ POSTGRES_HOST_AUTH_METHOD: trust .use-pg12-ee: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-12-graphicsmagick-1.3.34" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-12-graphicsmagick-1.3.34" services: - name: postgres:12 command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"] @@ -132,7 +132,7 @@ FOSS_ONLY: '1' .use-docker-in-docker: - image: docker:${DOCKER_VERSION} + image: ${GITLAB_DEPENDENCY_PROXY}docker:${DOCKER_VERSION} services: - docker:${DOCKER_VERSION}-dind variables: diff --git a/.gitlab/ci/notify.gitlab-ci.yml b/.gitlab/ci/notify.gitlab-ci.yml index e18a092bb8f..a8c156c7dba 100644 --- a/.gitlab/ci/notify.gitlab-ci.yml +++ b/.gitlab/ci/notify.gitlab-ci.yml @@ -1,5 +1,5 @@ .notify-slack: - image: alpine + image: ${GITLAB_DEPENDENCY_PROXY}alpine stage: notify dependencies: [] cache: {} diff --git a/.gitlab/ci/qa.gitlab-ci.yml b/.gitlab/ci/qa.gitlab-ci.yml index 1dc403c9d06..788b482f0a6 100644 --- a/.gitlab/ci/qa.gitlab-ci.yml +++ b/.gitlab/ci/qa.gitlab-ci.yml @@ -47,7 +47,7 @@ update-qa-cache: policy: push # We want to rebuild the cache from scratch to ensure stale dependencies are cleaned up. .package-and-qa-base: - image: ruby:2.7-alpine + image: ${GITLAB_DEPENDENCY_PROXY}ruby:2.7-alpine stage: qa retry: 0 script: diff --git a/.gitlab/ci/releases.gitlab-ci.yml b/.gitlab/ci/releases.gitlab-ci.yml index b3f961afe62..77f23814f3c 100644 --- a/.gitlab/ci/releases.gitlab-ci.yml +++ b/.gitlab/ci/releases.gitlab-ci.yml @@ -4,7 +4,7 @@ .merge-train-sync: # We don't need/want any global before/after commands, so we overwrite these # settings. - image: alpine:edge + image: ${GITLAB_DEPENDENCY_PROXY}alpine:edge stage: sync before_script: - apk add --no-cache --update curl bash jq diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index b7d9f18dcb4..c18e898dc12 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -25,7 +25,7 @@ review-build-cng: extends: - .default-retry - .review:rules:review-build-cng - image: ruby:2.7-alpine + image: ${GITLAB_DEPENDENCY_PROXY}ruby:2.7-alpine stage: review-prepare before_script: - source ./scripts/utils.sh @@ -199,7 +199,7 @@ review-performance: parallel-spec-reports: extends: - .review:rules:mr-only-manual - image: ruby:2.7-alpine + image: ${GITLAB_DEPENDENCY_PROXY}ruby:2.7-alpine stage: post-qa dependencies: ["review-qa-all"] variables: diff --git a/.gitlab/ci/setup.gitlab-ci.yml b/.gitlab/ci/setup.gitlab-ci.yml index 74510a0a03a..27b68115edc 100644 --- a/.gitlab/ci/setup.gitlab-ci.yml +++ b/.gitlab/ci/setup.gitlab-ci.yml @@ -26,7 +26,7 @@ cache gems: dont-interrupt-me: extends: .setup:rules:dont-interrupt-me stage: sync - image: alpine:edge + image: ${GITLAB_DEPENDENCY_PROXY}alpine:edge interruptible: false variables: GIT_STRATEGY: none @@ -52,7 +52,7 @@ no_ee_check: verify-tests-yml: extends: - .setup:rules:verify-tests-yml - image: ruby:2.7-alpine + image: ${GITLAB_DEPENDENCY_PROXY}ruby:2.7-alpine stage: test needs: [] script: @@ -61,7 +61,7 @@ verify-tests-yml: - scripts/verify-tff-mapping .detect-test-base: - image: ruby:2.7 + image: ${GITLAB_DEPENDENCY_PROXY}ruby:2.7 needs: [] stage: prepare script: diff --git a/.gitlab/ci/test-metadata.gitlab-ci.yml b/.gitlab/ci/test-metadata.gitlab-ci.yml index aec0a1640f1..b90c02c08ef 100644 --- a/.gitlab/ci/test-metadata.gitlab-ci.yml +++ b/.gitlab/ci/test-metadata.gitlab-ci.yml @@ -1,5 +1,5 @@ .tests-metadata-state: - image: ruby:2.7 + image: ${GITLAB_DEPENDENCY_PROXY}ruby:2.7 before_script: - source scripts/utils.sh artifacts: diff --git a/.gitlab/ci/workhorse.gitlab-ci.yml b/.gitlab/ci/workhorse.gitlab-ci.yml index 29131159876..81016dbfb53 100644 --- a/.gitlab/ci/workhorse.gitlab-ci.yml +++ b/.gitlab/ci/workhorse.gitlab-ci.yml @@ -1,6 +1,6 @@ workhorse: extends: .workhorse:rules:workhorse - image: golang:1.14 + image: ${GITLAB_DEPENDENCY_PROXY}golang:1.14 stage: test needs: [] script: diff --git a/Gemfile b/Gemfile index eadfd3af66c..dddb8b7133c 100644 --- a/Gemfile +++ b/Gemfile @@ -350,7 +350,7 @@ end group :development, :test do gem 'deprecation_toolkit', '~> 1.5.1', require: false - gem 'bullet', '~> 6.1.0' + gem 'bullet', '~> 6.1.3' gem 'gitlab-pry-byebug', platform: :mri, require: ['pry-byebug', 'pry-byebug/pry_remote_ext'] gem 'pry-rails', '~> 0.3.9' gem 'pry-remote' diff --git a/Gemfile.lock b/Gemfile.lock index 9dfa359eefd..909aa25b033 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -147,7 +147,7 @@ GEM brakeman (4.2.1) browser (4.2.0) builder (3.2.4) - bullet (6.1.0) + bullet (6.1.3) activesupport (>= 3.0.0) uniform_notifier (~> 1.11) bundler-audit (0.7.0.1) @@ -1305,7 +1305,7 @@ DEPENDENCIES bootstrap_form (~> 4.2.0) brakeman (~> 4.2) browser (~> 4.2) - bullet (~> 6.1.0) + bullet (~> 6.1.3) bundler-audit (~> 0.7.0.1) capybara (~> 3.34.0) capybara-screenshot (~> 1.0.22) diff --git a/app/assets/javascripts/diffs/i18n.js b/app/assets/javascripts/diffs/i18n.js index d719f4e5352..2a061876937 100644 --- a/app/assets/javascripts/diffs/i18n.js +++ b/app/assets/javascripts/diffs/i18n.js @@ -1,4 +1,4 @@ -import { __ } from '~/locale'; +import { __, s__ } from '~/locale'; export const GENERIC_ERROR = __('Something went wrong on our end. Please try again!'); @@ -9,8 +9,8 @@ export const DIFF_FILE_HEADER = { }; export const DIFF_FILE = { - tooLarge: __('MRDiffFile|Changes are too large to be shown.'), - blobView: __('MRDiffFile|View file @ %{commitSha}'), + tooLarge: s__('MRDiffFile|Changes are too large to be shown.'), + blobView: s__('MRDiffFile|View file @ %{commitSha}'), editInFork: __( "You're not allowed to %{tag_start}edit%{tag_end} files in this project directly. Please fork this project, make your changes there, and submit a merge request.", ), diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index f782e6b5e88..9db29e8491e 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -159,7 +159,12 @@ export default { [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion, diffPositionByLineCode, hash }) { const { latestDiff } = state; - const discussionLineCodes = [discussion.line_code, ...(discussion.line_codes || [])]; + const originalStartLineCode = discussion.original_position?.line_range?.start?.line_code; + const discussionLineCodes = [ + discussion.line_code, + originalStartLineCode, + ...(discussion.line_codes || []), + ]; const fileHash = discussion.diff_file.file_hash; const lineCheck = (line) => discussionLineCodes.some( diff --git a/app/assets/javascripts/integrations/edit/components/integration_form.vue b/app/assets/javascripts/integrations/edit/components/integration_form.vue index 3b054be07bb..e5c4368c32d 100644 --- a/app/assets/javascripts/integrations/edit/components/integration_form.vue +++ b/app/assets/javascripts/integrations/edit/components/integration_form.vue @@ -95,10 +95,7 @@ export default { diff --git a/app/assets/javascripts/jobs/components/log/duration_badge.vue b/app/assets/javascripts/jobs/components/log/duration_badge.vue index 8e5dcdcc902..54b76fd9edd 100644 --- a/app/assets/javascripts/jobs/components/log/duration_badge.vue +++ b/app/assets/javascripts/jobs/components/log/duration_badge.vue @@ -1,5 +1,10 @@ diff --git a/app/assets/javascripts/notes/components/comment_form.vue b/app/assets/javascripts/notes/components/comment_form.vue index 0c52c45738a..bc2b2d6d5d0 100644 --- a/app/assets/javascripts/notes/components/comment_form.vue +++ b/app/assets/javascripts/notes/components/comment_form.vue @@ -143,6 +143,9 @@ export default { trackingLabel() { return slugifyWithUnderscore(`${this.commentButtonTitle} button`); }, + hasCloseAndCommentButton() { + return !this.glFeatures.removeCommentCloseReopen; + }, }, watch: { note(newNote) { @@ -405,7 +408,7 @@ export default { e + Gitlab::ErrorTracking.track_exception(e, project_id: project.id) + { + status: :error, + key: key(base_pipeline, head_pipeline), + status_reason: _('An error occurred while fetching codequality mr diff reports.') + } + end + + def latest?(base_pipeline, head_pipeline, data) + data&.fetch(:key, nil) == key(base_pipeline, head_pipeline) + end + end +end diff --git a/app/services/ci/pipeline_artifacts/create_quality_report_service.rb b/app/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service.rb similarity index 85% rename from app/services/ci/pipeline_artifacts/create_quality_report_service.rb rename to app/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service.rb index c24eb5f8d40..8a4ba039fa3 100644 --- a/app/services/ci/pipeline_artifacts/create_quality_report_service.rb +++ b/app/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service.rb @@ -1,16 +1,16 @@ # frozen_string_literal: true module Ci module PipelineArtifacts - class CreateQualityReportService + class CreateCodeQualityMrDiffReportService def execute(pipeline) return unless pipeline.can_generate_codequality_reports? - return if pipeline.has_codequality_reports? + return if pipeline.has_codequality_mr_diff_report? file = build_carrierwave_file(pipeline) pipeline.pipeline_artifacts.create!( project_id: pipeline.project_id, - file_type: :code_quality, + file_type: :code_quality_mr_diff, file_format: :raw, size: file["tempfile"].size, file: file, @@ -23,7 +23,7 @@ module Ci def build_carrierwave_file(pipeline) CarrierWaveStringFile.new_file( file_content: build_quality_mr_diff_report(pipeline), - filename: Ci::PipelineArtifact::DEFAULT_FILE_NAMES.fetch(:code_quality), + filename: Ci::PipelineArtifact::DEFAULT_FILE_NAMES.fetch(:code_quality_mr_diff), content_type: 'application/json' ) end diff --git a/app/services/groups/import_export/export_service.rb b/app/services/groups/import_export/export_service.rb index 5a1b695dcd6..a436aec1b39 100644 --- a/app/services/groups/import_export/export_service.rb +++ b/app/services/groups/import_export/export_service.rb @@ -126,3 +126,5 @@ module Groups end end end + +Groups::ImportExport::ExportService.prepend_if_ee('EE::Groups::ImportExport::ExportService') diff --git a/app/services/groups/import_export/import_service.rb b/app/services/groups/import_export/import_service.rb index b62d91366e9..bf3f09f22d4 100644 --- a/app/services/groups/import_export/import_service.rb +++ b/app/services/groups/import_export/import_service.rb @@ -123,3 +123,5 @@ module Groups end end end + +Groups::ImportExport::ImportService.prepend_if_ee('EE::Groups::ImportExport::ImportService') diff --git a/app/views/projects/graphs/show.html.haml b/app/views/projects/graphs/show.html.haml index c7508ef4d47..c62d4a35973 100644 --- a/app/views/projects/graphs/show.html.haml +++ b/app/views/projects/graphs/show.html.haml @@ -3,6 +3,6 @@ .sub-header-block.bg-gray-light.gl-p-5 .tree-ref-holder.inline.vertical-align-middle = render 'shared/ref_switcher', destination: 'graphs' - = link_to s_('Commits|History'), project_commits_path(@project, current_ref), class: 'btn gl-button' + = link_to s_('Commits|History'), project_commits_path(@project, current_ref), class: 'btn gl-button btn-default' .js-contributors-graph{ class: container_class, 'data-project-graph-path': project_graph_path(@project, current_ref, format: :json),'data-project-branch': current_ref } diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml index 59b3afa476f..3c99b4c5e68 100644 --- a/app/views/projects/services/_form.html.haml +++ b/app/views/projects/services/_form.html.haml @@ -1,19 +1,14 @@ - if lookup_context.template_exists?('top', "projects/services/#{@service.to_param}", true) = render "projects/services/#{@service.to_param}/top" -.row.gl-mt-3.gl-mb-3 - .col-lg-4 - %h3.page-title.gl-mt-0 - = @service.title - - if @service.operating? - = sprite_icon('check', css_class: 'gl-text-green-500') +%h3.page-title + = @service.title + - if @service.operating? + = sprite_icon('check', css_class: 'gl-text-green-500') - - if @service.respond_to?(:detailed_description) - %p= @service.detailed_description - .col-lg-8 - = form_for(@service, as: :service, url: scoped_integration_path(@service), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => test_project_service_path(@project, @service) } }) do |form| - = render 'shared/service_settings', form: form, integration: @service - %input{ id: 'services_redirect_to', type: 'hidden', name: 'redirect_to', value: request.referrer } += form_for(@service, as: :service, url: scoped_integration_path(@service), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => test_project_service_path(@project, @service) } }) do |form| + = render 'shared/service_settings', form: form, integration: @service + %input{ id: 'services_redirect_to', type: 'hidden', name: 'redirect_to', value: request.referrer } - if lookup_context.template_exists?('show', "projects/services/#{@service.to_param}", true) %hr diff --git a/app/views/shared/_label.html.haml b/app/views/shared/_label.html.haml index 3647abc6403..c70c0572c2b 100644 --- a/app/views/shared/_label.html.haml +++ b/app/views/shared/_label.html.haml @@ -43,10 +43,10 @@ - if current_user %li.inline.label-subscription - if label.can_subscribe_to_label_in_different_levels? - %button.js-unsubscribe-button.label-subscribe-button.btn.btn-default{ class: ('hidden' if status.unsubscribed?), data: { url: toggle_subscription_path, toggle: 'tooltip' }, title: tooltip_title } + %button.js-unsubscribe-button.gl-button.label-subscribe-button.btn.btn-default.gl-ml-3{ class: ('hidden' if status.unsubscribed?), data: { url: toggle_subscription_path, toggle: 'tooltip' }, title: tooltip_title } %span= _('Unsubscribe') .dropdown.dropdown-group-label{ class: ('hidden' unless status.unsubscribed?) } - %button.gl-button.label-subscribe-button.btn.btn-default{ data: { toggle: 'dropdown' } } + %button.gl-button.label-subscribe-button.btn.btn-default.gl-ml-3{ data: { toggle: 'dropdown' } } %span = _('Subscribe') = sprite_icon('chevron-down') @@ -59,7 +59,7 @@ %button.js-subscribe-button.js-group-level.label-subscribe-button.btn.btn-default{ class: ('hidden' unless status.unsubscribed?), data: { status: status, url: toggle_subscription_group_label_path(label.group, label) } } %span= _('Subscribe at group level') - else - %button.gl-button.js-subscribe-button.label-subscribe-button.btn.btn-default{ data: { status: status, url: toggle_subscription_path, toggle: 'tooltip' }, title: tooltip_title } + %button.gl-button.js-subscribe-button.label-subscribe-button.btn.btn-default.gl-ml-3{ data: { status: status, url: toggle_subscription_path, toggle: 'tooltip' }, title: tooltip_title } %span= label_subscription_toggle_button_text(label, @project) = render 'shared/delete_label_modal', label: label diff --git a/app/views/shared/_service_settings.html.haml b/app/views/shared/_service_settings.html.haml index e294608214d..7af356c0820 100644 --- a/app/views/shared/_service_settings.html.haml +++ b/app/views/shared/_service_settings.html.haml @@ -4,7 +4,7 @@ - if @default_integration .js-vue-default-integration-settings{ data: integration_form_data(@default_integration, group: @group) } .js-vue-integration-settings{ data: integration_form_data(integration, group: @group) } - .js-integration-help-html + .js-integration-help-html.gl-display-none -# All content below will be repositioned in Vue - if lookup_context.template_exists?('help', "projects/services/#{integration.to_param}", true) = render "projects/services/#{integration.to_param}/help", subject: integration diff --git a/app/views/shared/integrations/_form.html.haml b/app/views/shared/integrations/_form.html.haml index 11e390a47e2..62f8d986296 100644 --- a/app/views/shared/integrations/_form.html.haml +++ b/app/views/shared/integrations/_form.html.haml @@ -1,10 +1,7 @@ - integration = local_assigns.fetch(:integration) -.row.gl-mt-3 - .col-lg-4 - %h3.page-title.gl-mt-0 - = integration.title +%h3.page-title + = integration.title - .col-lg-8 - = form_for integration, as: :service, url: scoped_integration_path(integration), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => scoped_test_integration_path(integration) } } do |form| - = render 'shared/service_settings', form: form, integration: integration += form_for integration, as: :service, url: scoped_integration_path(integration), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => scoped_test_integration_path(integration) } } do |form| + = render 'shared/service_settings', form: form, integration: integration diff --git a/app/workers/ci/pipeline_artifacts/create_quality_report_worker.rb b/app/workers/ci/pipeline_artifacts/create_quality_report_worker.rb index abdc2ddbedd..810106e8d9c 100644 --- a/app/workers/ci/pipeline_artifacts/create_quality_report_worker.rb +++ b/app/workers/ci/pipeline_artifacts/create_quality_report_worker.rb @@ -12,7 +12,7 @@ module Ci def perform(pipeline_id) Ci::Pipeline.find_by_id(pipeline_id).try do |pipeline| - Ci::PipelineArtifacts::CreateQualityReportService.new.execute(pipeline) + Ci::PipelineArtifacts::CreateCodeQualityMrDiffReportService.new.execute(pipeline) end end end diff --git a/changelogs/unreleased/btn-default-history.yml b/changelogs/unreleased/btn-default-history.yml new file mode 100644 index 00000000000..055da67283a --- /dev/null +++ b/changelogs/unreleased/btn-default-history.yml @@ -0,0 +1,5 @@ +--- +title: Add btn-default class for history button in the contributors page +merge_request: 52861 +author: Yogi (@yo) +type: other diff --git a/changelogs/unreleased/fix-spacing-labels.yml b/changelogs/unreleased/fix-spacing-labels.yml new file mode 100644 index 00000000000..9afd9e3bd25 --- /dev/null +++ b/changelogs/unreleased/fix-spacing-labels.yml @@ -0,0 +1,5 @@ +--- +title: Fix spacing before toggle subscribe button on labels +merge_request: 52459 +author: Yogi (@yo) +type: other diff --git a/changelogs/unreleased/id-bump-bullet.yml b/changelogs/unreleased/id-bump-bullet.yml new file mode 100644 index 00000000000..2c6ce6e49aa --- /dev/null +++ b/changelogs/unreleased/id-bump-bullet.yml @@ -0,0 +1,5 @@ +--- +title: Update bullet gem version to 6.1.3 +merge_request: 53217 +author: +type: other diff --git a/changelogs/unreleased/jivanvl-convert-gl-badge-duration-component.yml b/changelogs/unreleased/jivanvl-convert-gl-badge-duration-component.yml new file mode 100644 index 00000000000..e885cae047a --- /dev/null +++ b/changelogs/unreleased/jivanvl-convert-gl-badge-duration-component.yml @@ -0,0 +1,5 @@ +--- +title: Change the badge design in the jobs page +merge_request: 53168 +author: +type: changed diff --git a/changelogs/unreleased/ph-227161-fixDiscussonsOnCommits.yml b/changelogs/unreleased/ph-227161-fixDiscussonsOnCommits.yml new file mode 100644 index 00000000000..af45d8f1a93 --- /dev/null +++ b/changelogs/unreleased/ph-227161-fixDiscussonsOnCommits.yml @@ -0,0 +1,5 @@ +--- +title: Fixed discussions on merge request commits not showing +merge_request: 53143 +author: +type: fixed diff --git a/changelogs/unreleased/reposition-integration-form-elements.yml b/changelogs/unreleased/reposition-integration-form-elements.yml new file mode 100644 index 00000000000..05880ddcdea --- /dev/null +++ b/changelogs/unreleased/reposition-integration-form-elements.yml @@ -0,0 +1,5 @@ +--- +title: Move integration inheritance override dropdown above grid layout +merge_request: 49325 +author: +type: changed diff --git a/changelogs/unreleased/tor-defect-untranslated-strings.yml b/changelogs/unreleased/tor-defect-untranslated-strings.yml new file mode 100644 index 00000000000..0aebf4d0f23 --- /dev/null +++ b/changelogs/unreleased/tor-defect-untranslated-strings.yml @@ -0,0 +1,5 @@ +--- +title: Switch to correct localization function that strips namespaces +merge_request: 53244 +author: +type: fixed diff --git a/config/feature_flags/development/group_wiki_import_export.yml b/config/feature_flags/development/group_wiki_import_export.yml new file mode 100644 index 00000000000..f9806ef0c28 --- /dev/null +++ b/config/feature_flags/development/group_wiki_import_export.yml @@ -0,0 +1,8 @@ +--- +name: group_wiki_import_export +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51873 +rollout_issue_url: +milestone: '13.9' +type: development +group: group::editor +default_enabled: false diff --git a/config/feature_flags/development/remove_comment_close_reopen.yml b/config/feature_flags/development/remove_comment_close_reopen.yml new file mode 100644 index 00000000000..1d3bf305cae --- /dev/null +++ b/config/feature_flags/development/remove_comment_close_reopen.yml @@ -0,0 +1,8 @@ +--- +name: remove_comment_close_reopen +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49614 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/198562 +milestone: '13.9' +type: development +group: group::code review +default_enabled: false diff --git a/config/object_store_settings.rb b/config/object_store_settings.rb index 5603ea51672..f92bffd45db 100644 --- a/config/object_store_settings.rb +++ b/config/object_store_settings.rb @@ -124,9 +124,12 @@ class ObjectStoreSettings target_config = common_config.merge(overrides.slice(*ALLOWED_OBJECT_STORE_OVERRIDES)) section = settings.try(store_type) - next unless uses_object_storage?(section) + # Admins can selectively disable object storage for a specific + # type as an override in the consolidated settings. + next unless overrides.fetch('enabled', true) + next unless section - if target_config['bucket'].blank? + if section['enabled'] && target_config['bucket'].blank? missing_bucket_for(store_type) next end @@ -146,20 +149,6 @@ class ObjectStoreSettings private - # Admins can selectively disable object storage for a specific type. If - # this hasn't been set, we assume that the consolidated settings - # should be used. - def uses_object_storage?(section) - # Use to_h to avoid https://gitlab.com/gitlab-org/gitlab/-/issues/286873 - section = section.to_h - - enabled_globally = section.fetch('enabled', false) - object_store_settings = section.fetch('object_store', {}) - os_enabled = object_store_settings.fetch('enabled', true) - - enabled_globally && os_enabled - end - # We only can use the common object storage settings if: # 1. The common settings are defined # 2. The legacy settings are not defined diff --git a/config/routes/merge_requests.rb b/config/routes/merge_requests.rb index 41d831f239e..b0bab1717a6 100644 --- a/config/routes/merge_requests.rb +++ b/config/routes/merge_requests.rb @@ -18,6 +18,7 @@ resources :merge_requests, concerns: :awardable, except: [:new, :create, :show], get :coverage_reports get :terraform_reports get :codequality_reports + get :codequality_mr_diff_reports scope constraints: ->(req) { req.format == :json }, as: :json do get :commits diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 9a01ad28081..678b585e6d7 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -160,6 +160,8 @@ - 1 - - group_wikis_git_garbage_collect - 1 +- - groups_schedule_bulk_repository_shard_moves + - 1 - - groups_update_repository_storage - 1 - - hashed_storage diff --git a/doc/administration/pages/index.md b/doc/administration/pages/index.md index fb9ea5a9096..f78dc83da91 100644 --- a/doc/administration/pages/index.md +++ b/doc/administration/pages/index.md @@ -730,6 +730,13 @@ or report an issue. ### Object storage settings +WARNING: +With the following settings, Pages uses both NFS and Object Storage locations when deploying the +site. **Do not remove the existing NFS mount used by Pages** when applying these settings. For more +information, see the epics +[3901](https://gitlab.com/groups/gitlab-org/-/epics/3901#how-to-test-object-storage-integration-in-beta) +and [3910](https://gitlab.com/groups/gitlab-org/-/epics/3910). + The following settings are: - Nested under `pages:` and then `object_store:` on source installations. diff --git a/doc/development/pipelines.md b/doc/development/pipelines.md index 6437b4a5ea8..1bcd502e4b4 100644 --- a/doc/development/pipelines.md +++ b/doc/development/pipelines.md @@ -1,6 +1,6 @@ --- stage: none -group: unassigned +group: Engineering Productivity info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- @@ -44,7 +44,7 @@ The source of truth for these workflow rules is defined in [`.gitlab-ci.yml`](ht In general, pipelines for an MR fall into one or more of the following types, depending on the changes made in the MR: -- [Docs-only MR pipeline](#docs-only-mr-pipeline): This is typically created for an MR that only changes documentation. +- [Documentation only MR pipeline](#documentation-only-mr-pipeline): This is typically created for an MR that only changes documentation. - [Code-only MR pipeline](#code-only-mr-pipeline): This is typically created for an MR that only changes code, either backend or frontend. - [Frontend-only MR pipeline](#frontend-only-mr-pipeline): This is typically created for an MR that only changes frontend code. - [QA-only MR pipeline](#qa-only-mr-pipeline): This is typically created for an MR that only changes end to end tests related code. @@ -53,18 +53,22 @@ We use the [`rules:`](../ci/yaml/README.md#rules) and [`needs:`](../ci/yaml/READ to determine the jobs that need to be run in a pipeline. Note that an MR that includes multiple types of changes would have a pipelines that include jobs from multiple types (e.g. a combination of docs-only and code-only pipelines). -#### Docs-only MR pipeline +#### Documentation only MR pipeline -[Reference pipeline](https://gitlab.com/gitlab-org/gitlab/pipelines/135236627): +[Reference pipeline](https://gitlab.com/gitlab-org/gitlab/-/pipelines/250546928): ```mermaid graph LR subgraph "No needed jobs"; 1-1["danger-review (2.3 minutes)"]; click 1-1 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=8100542&udv=0" - 1-50["docs lint (9 minutes)"]; - click 1-50 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=8356757&udv=0" - end + 1-2["docs-lint markdown (1.5 minutes)"]; + click 1-2 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=10224335&udv=0" + 1-3["docs-lint links (6 minutes)"]; + click 1-3 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=8356757&udv=0" + 1-4["ui-docs-links lint (2.5 minutes)"]; + click 1-4 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=10823717&udv=1020379" + end ``` #### Code-only MR pipeline diff --git a/lib/gitlab/import_export.rb b/lib/gitlab/import_export.rb index 921072a4970..c4867746b0f 100644 --- a/lib/gitlab/import_export.rb +++ b/lib/gitlab/import_export.rb @@ -103,3 +103,7 @@ module Gitlab end Gitlab::ImportExport.prepend_if_ee('EE::Gitlab::ImportExport') + +# The methods in `Gitlab::ImportExport::GroupHelper` should be available as both +# instance and class methods. +Gitlab::ImportExport.extend_if_ee('Gitlab::ImportExport::GroupHelper') diff --git a/lib/gitlab/import_export/wiki_repo_saver.rb b/lib/gitlab/import_export/wiki_repo_saver.rb index b3e390869d3..4b1cf4915e4 100644 --- a/lib/gitlab/import_export/wiki_repo_saver.rb +++ b/lib/gitlab/import_export/wiki_repo_saver.rb @@ -19,3 +19,5 @@ module Gitlab end end end + +Gitlab::ImportExport::WikiRepoSaver.prepend_if_ee('EE::Gitlab::ImportExport::WikiRepoSaver') diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8de52ac1a5d..8a8b65c90e7 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3152,6 +3152,9 @@ msgstr "" msgid "An error occurred while fetching branches. Retry the search." msgstr "" +msgid "An error occurred while fetching codequality mr diff reports." +msgstr "" + msgid "An error occurred while fetching commits. Retry the search." msgstr "" @@ -15907,6 +15910,9 @@ msgstr "" msgid "Integrations|This integration, and inheriting projects were reset." msgstr "" +msgid "Integrations|This requires mirroring your GitHub repository to this project. %{docs_link}" +msgstr "" + msgid "Integrations|To keep this project going, create a new issue." msgstr "" diff --git a/spec/config/object_store_settings_spec.rb b/spec/config/object_store_settings_spec.rb index 8b507acb827..68b37197ca7 100644 --- a/spec/config/object_store_settings_spec.rb +++ b/spec/config/object_store_settings_spec.rb @@ -130,13 +130,14 @@ RSpec.describe ObjectStoreSettings do end end - context 'when object storage is selectively disabled for artifacts' do + context 'when object storage is disabled for artifacts with no bucket' do before do config['artifacts'] = { 'enabled' => true, - 'object_store' => { - 'enabled' => false - } + 'object_store' => {} + } + config['object_store']['objects']['artifacts'] = { + 'enabled' => false } end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index cf8b4c564c4..c53dd1265e6 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -1118,6 +1118,108 @@ RSpec.describe Projects::MergeRequestsController do end end + describe 'GET codequality_mr_diff_reports' do + let_it_be(:merge_request) do + create(:merge_request, + :with_merge_request_pipeline, + target_project: project, + source_project: project) + end + + let(:pipeline) do + create(:ci_pipeline, + :success, + project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha) + end + + before do + allow_any_instance_of(MergeRequest) + .to receive(:find_codequality_mr_diff_reports) + .and_return(report) + + allow_any_instance_of(MergeRequest) + .to receive(:actual_head_pipeline) + .and_return(pipeline) + end + + subject(:get_codequality_mr_diff_reports) do + get :codequality_mr_diff_reports, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid + }, + format: :json + end + + context 'permissions on a public project with private CI/CD' do + let(:project) { create :project, :repository, :public, :builds_private } + let(:report) { { status: :parsed, data: { 'files' => {} } } } + + context 'while signed out' do + before do + sign_out(user) + end + + it 'responds with a 404' do + get_codequality_mr_diff_reports + + expect(response).to have_gitlab_http_status(:not_found) + expect(response.body).to be_blank + end + end + + context 'while signed in as an unrelated user' do + before do + sign_in(create(:user)) + end + + it 'responds with a 404' do + get_codequality_mr_diff_reports + + expect(response).to have_gitlab_http_status(:not_found) + expect(response.body).to be_blank + end + end + end + + context 'when pipeline has jobs with codequality mr diff report' do + before do + allow_any_instance_of(MergeRequest) + .to receive(:has_codequality_mr_diff_report?) + .and_return(true) + end + + context 'when processing codequality mr diff report is in progress' do + let(:report) { { status: :parsing } } + + it 'sends polling interval' do + expect(Gitlab::PollingInterval).to receive(:set_header) + + get_codequality_mr_diff_reports + end + + it 'returns 204 HTTP status' do + get_codequality_mr_diff_reports + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'when processing codequality mr diff report is completed' do + let(:report) { { status: :parsed, data: { 'files' => {} } } } + + it 'returns codequality mr diff report' do + get_codequality_mr_diff_reports + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'files' => {} }) + end + end + end + end + describe 'GET terraform_reports' do let_it_be(:merge_request) do create(:merge_request, diff --git a/spec/factories/ci/pipeline_artifacts.rb b/spec/factories/ci/pipeline_artifacts.rb index ab4a16dbb90..05ff7afed7c 100644 --- a/spec/factories/ci/pipeline_artifacts.rb +++ b/spec/factories/ci/pipeline_artifacts.rb @@ -49,12 +49,12 @@ FactoryBot.define do size { 1.megabyte } end - trait :with_codequality_report do - file_type { :code_quality } + trait :with_codequality_mr_diff_report do + file_type { :code_quality_mr_diff } after(:build) do |artifact, _evaluator| artifact.file = fixture_file_upload( - Rails.root.join('spec/fixtures/pipeline_artifacts/code_quality.json'), 'application/json') + Rails.root.join('spec/fixtures/pipeline_artifacts/code_quality_mr_diff.json'), 'application/json') end size { file.size } diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index 436d309a2ce..530bb0cd25b 100644 --- a/spec/factories/ci/pipelines.rb +++ b/spec/factories/ci/pipelines.rb @@ -155,9 +155,9 @@ FactoryBot.define do end end - trait :with_quality_report_artifact do + trait :with_codequality_mr_diff_report do after(:build) do |pipeline, evaluator| - pipeline.pipeline_artifacts << build(:ci_pipeline_artifact, :with_codequality_report, pipeline: pipeline, project: pipeline.project) + pipeline.pipeline_artifacts << build(:ci_pipeline_artifact, :with_codequality_mr_diff_report, pipeline: pipeline, project: pipeline.project) end end diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index e69743122cc..dba98593a03 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -200,6 +200,18 @@ FactoryBot.define do end end + trait :with_codequality_mr_diff_reports do + after(:build) do |merge_request| + merge_request.head_pipeline = build( + :ci_pipeline, + :success, + :with_codequality_mr_diff_report, + project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha) + end + end + trait :with_terraform_reports do after(:build) do |merge_request| merge_request.head_pipeline = build( diff --git a/spec/features/discussion_comments/issue_spec.rb b/spec/features/discussion_comments/issue_spec.rb index 2ad77a2884c..86743e31fbd 100644 --- a/spec/features/discussion_comments/issue_spec.rb +++ b/spec/features/discussion_comments/issue_spec.rb @@ -8,6 +8,8 @@ RSpec.describe 'Thread Comments Issue', :js do let(:issue) { create(:issue, project: project) } before do + stub_feature_flags(remove_comment_close_reopen: false) + project.add_maintainer(user) sign_in(user) diff --git a/spec/features/discussion_comments/merge_request_spec.rb b/spec/features/discussion_comments/merge_request_spec.rb index 761cc7ae796..82dcdf9f918 100644 --- a/spec/features/discussion_comments/merge_request_spec.rb +++ b/spec/features/discussion_comments/merge_request_spec.rb @@ -9,6 +9,7 @@ RSpec.describe 'Thread Comments Merge Request', :js do before do stub_feature_flags(remove_resolve_note: false) + stub_feature_flags(remove_comment_close_reopen: false) project.add_maintainer(user) sign_in(user) diff --git a/spec/features/issues/issue_state_spec.rb b/spec/features/issues/issue_state_spec.rb index d5a115433aa..409f498798b 100644 --- a/spec/features/issues/issue_state_spec.rb +++ b/spec/features/issues/issue_state_spec.rb @@ -42,9 +42,15 @@ RSpec.describe 'issue state', :js do end describe 'when open', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/297348' do - context 'when clicking the top `Close issue` button', :aggregate_failures do - let(:open_issue) { create(:issue, project: project) } + let(:open_issue) { create(:issue, project: project) } + it_behaves_like 'page with comment and close button', 'Close issue' do + def setup + visit project_issue_path(project, open_issue) + end + end + + context 'when clicking the top `Close issue` button', :aggregate_failures do before do visit project_issue_path(project, open_issue) end @@ -53,9 +59,8 @@ RSpec.describe 'issue state', :js do end context 'when clicking the bottom `Close issue` button', :aggregate_failures do - let(:open_issue) { create(:issue, project: project) } - before do + stub_feature_flags(remove_comment_close_reopen: false) visit project_issue_path(project, open_issue) end @@ -64,9 +69,15 @@ RSpec.describe 'issue state', :js do end describe 'when closed', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/297201' do - context 'when clicking the top `Reopen issue` button', :aggregate_failures do - let(:closed_issue) { create(:issue, project: project, state: 'closed') } + let(:closed_issue) { create(:issue, project: project, state: 'closed') } + it_behaves_like 'page with comment and close button', 'Reopen issue' do + def setup + visit project_issue_path(project, closed_issue) + end + end + + context 'when clicking the top `Reopen issue` button', :aggregate_failures do before do visit project_issue_path(project, closed_issue) end @@ -75,9 +86,8 @@ RSpec.describe 'issue state', :js do end context 'when clicking the bottom `Reopen issue` button', :aggregate_failures do - let(:closed_issue) { create(:issue, project: project, state: 'closed') } - before do + stub_feature_flags(remove_comment_close_reopen: false) visit project_issue_path(project, closed_issue) end diff --git a/spec/features/merge_request/user_closes_reopens_merge_request_state_spec.rb b/spec/features/merge_request/user_closes_reopens_merge_request_state_spec.rb index 70951982c22..ab3ef7c1ac0 100644 --- a/spec/features/merge_request/user_closes_reopens_merge_request_state_spec.rb +++ b/spec/features/merge_request/user_closes_reopens_merge_request_state_spec.rb @@ -12,9 +12,15 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https:// end describe 'when open' do - context 'when clicking the top `Close merge request` link', :aggregate_failures do - let(:open_merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:open_merge_request) { create(:merge_request, source_project: project, target_project: project) } + it_behaves_like 'page with comment and close button', 'Close merge request' do + def setup + visit merge_request_path(open_merge_request) + end + end + + context 'when clicking the top `Close merge request` link', :aggregate_failures do before do visit merge_request_path(open_merge_request) end @@ -34,9 +40,8 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https:// end context 'when clicking the bottom `Close merge request` button', :aggregate_failures do - let(:open_merge_request) { create(:merge_request, source_project: project, target_project: project) } - before do + stub_feature_flags(remove_comment_close_reopen: false) visit merge_request_path(open_merge_request) end @@ -56,9 +61,22 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https:// end describe 'when closed' do - context 'when clicking the top `Reopen merge request` link', :aggregate_failures do - let(:closed_merge_request) { create(:merge_request, source_project: project, target_project: project, state: 'closed') } + let(:closed_merge_request) { create(:merge_request, source_project: project, target_project: project, state: 'closed') } + it_behaves_like 'page with comment and close button', 'Close merge request' do + def setup + visit merge_request_path(closed_merge_request) + + within '.detail-page-header' do + click_button 'Toggle dropdown' + click_link 'Reopen merge request' + end + + wait_for_requests + end + end + + context 'when clicking the top `Reopen merge request` link', :aggregate_failures do before do visit merge_request_path(closed_merge_request) end @@ -78,9 +96,8 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https:// end context 'when clicking the bottom `Reopen merge request` button', :aggregate_failures do - let(:closed_merge_request) { create(:merge_request, source_project: project, target_project: project, state: 'closed') } - before do + stub_feature_flags(remove_comment_close_reopen: false) visit merge_request_path(closed_merge_request) end diff --git a/spec/features/task_lists_spec.rb b/spec/features/task_lists_spec.rb index 0f8daaf8e15..e17521e1d02 100644 --- a/spec/features/task_lists_spec.rb +++ b/spec/features/task_lists_spec.rb @@ -69,7 +69,13 @@ RSpec.describe 'Task Lists', :js do wait_for_requests expect(page).to have_selector(".md .task-list .task-list-item .task-list-item-checkbox") - expect(page).to have_selector('.btn-close') + end + + it_behaves_like 'page with comment and close button', 'Close issue' do + def setup + visit_issue(project, issue) + wait_for_requests + end end it 'is only editable by author' do diff --git a/spec/fixtures/pipeline_artifacts/code_quality.json b/spec/fixtures/pipeline_artifacts/code_quality.json deleted file mode 100644 index 26b6d772a49..00000000000 --- a/spec/fixtures/pipeline_artifacts/code_quality.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "files": { - "demo.rb": [ - { - "line": 5, - "description": "Method `new_array` has 8 arguments (exceeds 4 allowed). Consider refactoring.", - "severity": "major" - }, - { - "line": 5, - "description": "Avoid parameter lists longer than 5 parameters. [8/5]", - "severity": "minor" - } - ] - } -} diff --git a/spec/fixtures/pipeline_artifacts/code_quality_mr_diff.json b/spec/fixtures/pipeline_artifacts/code_quality_mr_diff.json new file mode 100644 index 00000000000..c3ee2bc4cac --- /dev/null +++ b/spec/fixtures/pipeline_artifacts/code_quality_mr_diff.json @@ -0,0 +1,23 @@ +{ + "files": { + "file_a.rb": [ + { + "line": 10, + "description": "Avoid parameter lists longer than 5 parameters. [12/5]", + "severity": "major" + }, + { + "line": 10, + "description": "Method `new_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.", + "severity": "minor" + } + ], + "file_b.rb": [ + { + "line": 10, + "description": "This cop checks for methods with too many parameters.\nThe maximum number of parameters is configurable.\nKeyword arguments can optionally be excluded from the total count.", + "severity": "minor" + } + ] + } +} diff --git a/spec/models/ci/pipeline_artifact_spec.rb b/spec/models/ci/pipeline_artifact_spec.rb index eb46b563a84..1ae85b0a5ba 100644 --- a/spec/models/ci/pipeline_artifact_spec.rb +++ b/spec/models/ci/pipeline_artifact_spec.rb @@ -97,18 +97,18 @@ RSpec.describe Ci::PipelineArtifact, type: :model do end end - context 'when file_type is code_quality' do - let(:file_type) { :code_quality } + context 'when file_type is code_quality_mr_diff' do + let(:file_type) { :code_quality_mr_diff } - context 'when pipeline artifact has a quality report' do - let!(:pipeline_artifact) { create(:ci_pipeline_artifact, :with_codequality_report) } + context 'when pipeline artifact has a codequality mr diff report' do + let!(:pipeline_artifact) { create(:ci_pipeline_artifact, :with_codequality_mr_diff_report) } it 'returns true' do expect(pipeline_artifact).to be_truthy end end - context 'when pipeline artifact does not have a quality report' do + context 'when pipeline artifact does not have a codequality mr diff report' do it 'returns false' do expect(pipeline_artifact).to be_falsey end @@ -145,14 +145,14 @@ RSpec.describe Ci::PipelineArtifact, type: :model do end end - context 'when file_type is code_quality' do - let(:file_type) { :code_quality } + context 'when file_type is code_quality_mr_diff' do + let(:file_type) { :code_quality_mr_diff } context 'when pipeline artifact has a quality report' do - let!(:coverage_report) { create(:ci_pipeline_artifact, :with_codequality_report) } + let!(:coverage_report) { create(:ci_pipeline_artifact, :with_codequality_mr_diff_report) } it 'returns a pipeline artifact with a quality report' do - expect(pipeline_artifact.file_type).to eq('code_quality') + expect(pipeline_artifact.file_type).to eq('code_quality_mr_diff') end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index bad0bda6cf5..6f5f7d2e2fb 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -3510,16 +3510,16 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end - describe '#has_codequality_reports?' do - subject { pipeline.has_codequality_reports? } + describe '#has_codequality_mr_diff_report?' do + subject { pipeline.has_codequality_mr_diff_report? } - context 'when pipeline has a codequality artifact' do - let(:pipeline) { create(:ci_pipeline, :with_quality_report_artifact, :running, project: project) } + context 'when pipeline has a codequality mr diff report' do + let(:pipeline) { create(:ci_pipeline, :with_codequality_mr_diff_report, :running, project: project) } it { expect(subject).to be_truthy } end - context 'when pipeline does not have a codequality artifact' do + context 'when pipeline does not have a codequality mr diff report' do let(:pipeline) { create(:ci_pipeline, :success, project: project) } it { expect(subject).to be_falsey } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index d82f95bb1bb..71584a52c91 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1982,6 +1982,30 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe '#has_codequality_mr_diff_report?' do + subject { merge_request.has_codequality_mr_diff_report? } + + context 'when head pipeline has codequality mr diff report' do + let(:merge_request) { create(:merge_request, :with_codequality_mr_diff_reports) } + + it { is_expected.to be_truthy } + + context 'when feature flag is disabled' do + before do + stub_feature_flags(codequality_mr_diff: false) + end + + it { is_expected.to be_falsey } + end + end + + context 'when head pipeline does not have codeqquality mr diff report' do + let(:merge_request) { create(:merge_request) } + + it { is_expected.to be_falsey } + end + end + describe '#has_codequality_reports?' do subject { merge_request.has_codequality_reports? } @@ -2155,6 +2179,54 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe '#find_codequality_mr_diff_reports' do + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request, :with_codequality_mr_diff_reports, source_project: project) } + let(:pipeline) { merge_request.head_pipeline } + + subject(:mr_diff_report) { merge_request.find_codequality_mr_diff_reports } + + context 'when head pipeline has coverage reports' do + context 'when reactive cache worker is parsing results asynchronously' do + it 'returns status' do + expect(mr_diff_report[:status]).to eq(:parsing) + end + end + + context 'when reactive cache worker is inline' do + before do + synchronous_reactive_cache(merge_request) + end + + it 'returns status and data' do + expect(mr_diff_report[:status]).to eq(:parsed) + end + + context 'when an error occurrs' do + before do + merge_request.update!(head_pipeline: nil) + end + + it 'returns an error message' do + expect(mr_diff_report[:status]).to eq(:error) + end + end + + context 'when cached results is not latest' do + before do + allow_next_instance_of(Ci::GenerateCodequalityMrDiffReportService) do |service| + allow(service).to receive(:latest?).and_return(false) + end + end + + it 'raises and InvalidateReactiveCache error' do + expect { mr_diff_report }.to raise_error(ReactiveCaching::InvalidateReactiveCache) + end + end + end + end + end + describe '#compare_test_reports' do subject { merge_request.compare_test_reports } diff --git a/spec/models/project_services/confluence_service_spec.rb b/spec/models/project_services/confluence_service_spec.rb index 5d153b17070..6c7ba2c9f32 100644 --- a/spec/models/project_services/confluence_service_spec.rb +++ b/spec/models/project_services/confluence_service_spec.rb @@ -43,13 +43,13 @@ RSpec.describe ConfluenceService do end end - describe '#detailed_description' do + describe '#help' do it 'can correctly return a link to the project wiki when active' do project = create(:project) subject.project = project subject.active = true - expect(subject.detailed_description).to include(Gitlab::Routing.url_helpers.project_wikis_url(project)) + expect(subject.help).to include(Gitlab::Routing.url_helpers.project_wikis_url(project)) end context 'when the project wiki is not enabled' do @@ -60,7 +60,7 @@ RSpec.describe ConfluenceService do [true, false].each do |active| subject.active = active - expect(subject.detailed_description).to be_nil + expect(subject.help).to be_nil end end end diff --git a/spec/presenters/ci/pipeline_artifacts/code_quality_mr_diff_presenter_spec.rb b/spec/presenters/ci/pipeline_artifacts/code_quality_mr_diff_presenter_spec.rb new file mode 100644 index 00000000000..06d5422eed3 --- /dev/null +++ b/spec/presenters/ci/pipeline_artifacts/code_quality_mr_diff_presenter_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::PipelineArtifacts::CodeQualityMrDiffPresenter do + let(:pipeline_artifact) { create(:ci_pipeline_artifact, :with_codequality_mr_diff_report) } + + subject(:presenter) { described_class.new(pipeline_artifact) } + + describe '#for_files' do + subject(:quality_data) { presenter.for_files(filenames) } + + context 'when code quality has data' do + context 'when filenames is empty' do + let(:filenames) { %w() } + + it 'returns hash without quality' do + expect(quality_data).to match(files: {}) + end + end + + context 'when filenames do not match code quality data' do + let(:filenames) { %w(demo.rb) } + + it 'returns hash without quality' do + expect(quality_data).to match(files: {}) + end + end + + context 'when filenames matches code quality data' do + context 'when asking for one filename' do + let(:filenames) { %w(file_a.rb) } + + it 'returns quality for the given filename' do + expect(quality_data).to match( + files: { + "file_a.rb" => [ + { line: 10, description: "Avoid parameter lists longer than 5 parameters. [12/5]", severity: "major" }, + { line: 10, description: "Method `new_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.", severity: "minor" } + ] + } + ) + end + end + + context 'when asking for multiple filenames' do + let(:filenames) { %w(file_a.rb file_b.rb) } + + it 'returns quality for the given filenames' do + expect(quality_data).to match( + files: { + "file_a.rb" => [ + { line: 10, description: "Avoid parameter lists longer than 5 parameters. [12/5]", severity: "major" }, + { line: 10, description: "Method `new_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.", severity: "minor" } + ], + "file_b.rb" => [ + { line: 10, description: "This cop checks for methods with too many parameters.\nThe maximum number of parameters is configurable.\nKeyword arguments can optionally be excluded from the total count.", severity: "minor" } + ] + } + ) + end + end + end + end + end +end diff --git a/spec/services/ci/generate_codequality_mr_diff_report_service_spec.rb b/spec/services/ci/generate_codequality_mr_diff_report_service_spec.rb new file mode 100644 index 00000000000..5d747a09f2a --- /dev/null +++ b/spec/services/ci/generate_codequality_mr_diff_report_service_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::GenerateCodequalityMrDiffReportService do + let(:service) { described_class.new(project) } + let(:project) { create(:project, :repository) } + + describe '#execute' do + subject { service.execute(base_pipeline, head_pipeline) } + + context 'when head pipeline has codequality mr diff report' do + let!(:merge_request) { create(:merge_request, :with_codequality_mr_diff_reports, source_project: project) } + let!(:service) { described_class.new(project, nil, id: merge_request.id) } + let!(:head_pipeline) { merge_request.head_pipeline } + let!(:base_pipeline) { nil } + + it 'returns status and data', :aggregate_failures do + expect_any_instance_of(Ci::PipelineArtifact) do |instance| + expect(instance).to receive(:present) + expect(instance).to receive(:for_files).with(merge_request.new_paths).and_call_original + end + + expect(subject[:status]).to eq(:parsed) + expect(subject[:data]).to eq(files: {}) + end + end + + context 'when head pipeline does not have a codequality mr diff report' do + let!(:merge_request) { create(:merge_request, source_project: project) } + let!(:service) { described_class.new(project, nil, id: merge_request.id) } + let!(:head_pipeline) { merge_request.head_pipeline } + let!(:base_pipeline) { nil } + + it 'returns status and error message' do + expect(subject[:status]).to eq(:error) + expect(subject[:status_reason]).to include('An error occurred while fetching codequality mr diff reports.') + end + end + + context 'when head pipeline has codequality mr diff report and no merge request associated' do + let!(:head_pipeline) { create(:ci_pipeline, :with_codequality_mr_diff_report, project: project) } + let!(:base_pipeline) { nil } + + it 'returns status and error message' do + expect(subject[:status]).to eq(:error) + expect(subject[:status_reason]).to include('An error occurred while fetching codequality mr diff reports.') + end + end + end +end diff --git a/spec/services/ci/pipeline_artifacts/create_quality_report_service_spec.rb b/spec/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service_spec.rb similarity index 94% rename from spec/services/ci/pipeline_artifacts/create_quality_report_service_spec.rb rename to spec/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service_spec.rb index 6cb88d1402e..0c48f15d726 100644 --- a/spec/services/ci/pipeline_artifacts/create_quality_report_service_spec.rb +++ b/spec/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ::Ci::PipelineArtifacts::CreateQualityReportService do +RSpec.describe ::Ci::PipelineArtifacts::CreateCodeQualityMrDiffReportService do describe '#execute' do subject(:pipeline_artifact) { described_class.new.execute(pipeline) } @@ -27,7 +27,7 @@ RSpec.describe ::Ci::PipelineArtifacts::CreateQualityReportService do end it 'persists the default file name' do - expect(pipeline_artifact.file.filename).to eq('code_quality.json') + expect(pipeline_artifact.file.filename).to eq('code_quality_mr_diff.json') end it 'sets expire_at to 1 week' do diff --git a/spec/support/shared_examples/features/comment_and_close_button_shared_examples.rb b/spec/support/shared_examples/features/comment_and_close_button_shared_examples.rb new file mode 100644 index 00000000000..4ee2840ed9f --- /dev/null +++ b/spec/support/shared_examples/features/comment_and_close_button_shared_examples.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'page with comment and close button' do |button_text| + context 'when remove_comment_close_reopen feature flag is enabled' do + before do + stub_feature_flags(remove_comment_close_reopen: true) + setup + end + + it "does not show #{button_text} button" do + within '.note-form-actions' do + expect(page).not_to have_button(button_text) + end + end + end + + context 'when remove_comment_close_reopen feature flag is disabled' do + before do + stub_feature_flags(remove_comment_close_reopen: false) + setup + end + + it "shows #{button_text} button" do + within '.note-form-actions' do + expect(page).to have_button(button_text) + end + end + end +end diff --git a/spec/workers/ci/pipeline_artifacts/create_quality_report_worker_spec.rb b/spec/workers/ci/pipeline_artifacts/create_quality_report_worker_spec.rb index 6755c93ae47..be351032b58 100644 --- a/spec/workers/ci/pipeline_artifacts/create_quality_report_worker_spec.rb +++ b/spec/workers/ci/pipeline_artifacts/create_quality_report_worker_spec.rb @@ -11,7 +11,7 @@ RSpec.describe ::Ci::PipelineArtifacts::CreateQualityReportWorker do let(:pipeline_id) { pipeline.id } it 'calls pipeline codequality report service' do - expect_next_instance_of(::Ci::PipelineArtifacts::CreateQualityReportService) do |quality_report_service| + expect_next_instance_of(::Ci::PipelineArtifacts::CreateCodeQualityMrDiffReportService) do |quality_report_service| expect(quality_report_service).to receive(:execute) end @@ -31,7 +31,7 @@ RSpec.describe ::Ci::PipelineArtifacts::CreateQualityReportWorker do let(:pipeline_id) { non_existing_record_id } it 'does not call pipeline codequality report service' do - expect(Ci::PipelineArtifacts::CreateQualityReportService).not_to receive(:execute) + expect(Ci::PipelineArtifacts::CreateCodeQualityMrDiffReportService).not_to receive(:execute) subject end