diff options
24 files changed, 119 insertions, 203 deletions
diff --git a/README.md b/README.md index 64eedc9079f..29d5d599972 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,7 @@ Instructions on how to start GitLab and how to run the tests can be found in the GitLab is a Ruby on Rails application that runs on the following software: - Ubuntu/Debian/CentOS/RHEL/OpenSUSE -- Ruby (MRI) 2.7.5 +- Ruby (MRI) 2.7.7 - Git 2.33+ - Redis 5.0+ - PostgreSQL 12+ diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue index ffbea854001..709deaf6bb5 100644 --- a/app/assets/javascripts/diffs/components/tree_list.vue +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -2,6 +2,7 @@ import { GlTooltipDirective, GlIcon } from '@gitlab/ui'; import { mapActions, mapGetters, mapState } from 'vuex'; import micromatch from 'micromatch'; +import { MODIFIER_KEY } from '~/constants'; import { s__, sprintf } from '~/locale'; import FileTree from '~/vue_shared/components/file_tree.vue'; import DiffFileRow from './diff_file_row.vue'; @@ -65,8 +66,8 @@ export default { this.search = ''; }, }, - searchPlaceholder: sprintf(s__('MergeRequest|Search (e.g. *.vue) (%{modifier_key}P)'), { - modifier_key: /Mac/i.test(navigator.userAgent) ? '⌘' : 'Ctrl+', + searchPlaceholder: sprintf(s__('MergeRequest|Search (e.g. *.vue) (%{MODIFIER_KEY}P)'), { + MODIFIER_KEY, }), DiffFileRow, }; diff --git a/app/assets/javascripts/pages/projects/compare/show/index.js b/app/assets/javascripts/pages/projects/compare/show/index.js index b74f7d1cf57..760bf3f7131 100644 --- a/app/assets/javascripts/pages/projects/compare/show/index.js +++ b/app/assets/javascripts/pages/projects/compare/show/index.js @@ -2,6 +2,7 @@ import Diff from '~/diff'; import GpgBadges from '~/gpg_badges'; import { initDiffStatsDropdown } from '~/init_diff_stats_dropdown'; import initCompareSelector from '~/projects/compare'; +import syntaxHighlight from '~/syntax_highlight'; initCompareSelector(); @@ -9,3 +10,5 @@ new Diff(); // eslint-disable-line no-new const paddingTop = 16; initDiffStatsDropdown(document.querySelector('.navbar-gitlab').offsetHeight - paddingTop); GpgBadges.fetch(); + +syntaxHighlight([document.querySelector('.files')]); diff --git a/app/models/commit_collection.rb b/app/models/commit_collection.rb index 7d89ddde0cb..47ecdfa8574 100644 --- a/app/models/commit_collection.rb +++ b/app/models/commit_collection.rb @@ -25,7 +25,7 @@ class CommitCollection end def committers - emails = without_merge_commits.map(&:committer_email).uniq + emails = without_merge_commits.filter_map(&:committer_email).uniq User.by_any_email(emails) end diff --git a/app/models/integrations/flowdock.rb b/app/models/integrations/flowdock.rb deleted file mode 100644 index d7625cfb3d2..00000000000 --- a/app/models/integrations/flowdock.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -# This integration is scheduled for removal. -# All records must be deleted before the class can be removed. -# https://gitlab.com/gitlab-org/gitlab/-/issues/379197 -module Integrations - class Flowdock < Integration - def readonly? - true - end - - def self.to_param - 'flowdock' - end - - def self.supported_events - %w[] - end - end -end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 78c6d983a3d..caedc91ee8f 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -193,6 +193,12 @@ class MergeRequest < ApplicationRecord merge_request.merge_error = nil end + before_transition any => :merged do |merge_request| + if ::Feature.enabled?(:reset_merge_error_on_transition, merge_request.project) + merge_request.merge_error = nil + end + end + after_transition any => :opened do |merge_request| merge_request.run_after_commit do UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id) diff --git a/config/feature_flags/development/ci_refactoring_external_mapper.yml b/config/feature_flags/development/reset_merge_error_on_transition.yml index 22933d253d4..bb0b25d1666 100644 --- a/config/feature_flags/development/ci_refactoring_external_mapper.yml +++ b/config/feature_flags/development/reset_merge_error_on_transition.yml @@ -1,8 +1,8 @@ --- -name: ci_refactoring_external_mapper -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106408 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/385179 -milestone: '15.7' +name: reset_merge_error_on_transition +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106942 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/385859 +milestone: '15.8' type: development -group: group::pipeline authoring +group: group::code review default_enabled: false diff --git a/doc/.vale/gitlab/GitLabFlavoredMarkdown.yml b/doc/.vale/gitlab/GitLabFlavoredMarkdown.yml new file mode 100644 index 00000000000..532f1afd816 --- /dev/null +++ b/doc/.vale/gitlab/GitLabFlavoredMarkdown.yml @@ -0,0 +1,14 @@ +--- +# Warning: gitlab.GitLabFlavoredMarkdown +# +# Checks for unclear use of GLFM or GLM instead of GitLab/GitHub Flavored Markdown +# +# For a list of all options, see https://vale.sh/docs/topics/styles/ +extends: substitution +message: "Use '%s' instead of '%s' when possible." +link: https://docs.gitlab.com/ee/development/documentation/styleguide/word_list.html +level: warning +ignorecase: true +swap: + GLFM: "GitLab Flavored Markdown" + GFM: "GitLab Flavored Markdown' or 'GitHub Flavored Markdown" diff --git a/doc/.vale/gitlab/SubstitutionWarning.yml b/doc/.vale/gitlab/SubstitutionWarning.yml index 383ae38da16..45199c2b3b3 100644 --- a/doc/.vale/gitlab/SubstitutionWarning.yml +++ b/doc/.vale/gitlab/SubstitutionWarning.yml @@ -26,8 +26,6 @@ swap: ex: "for example" file name: "filename" filesystem: "file system" - GLFM: "GitLab Flavored Markdown" - GFM: "GitLab Flavored Markdown' or 'GitHub Flavored Markdown" info: "information" it is recommended: "you should" n/a: "not applicable" diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 5843a10ca59..f5643ba43be 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -652,7 +652,7 @@ Supported attributes: | `latest_build_finished_at` | datetime | Timestamp of when the latest build for the merge request finished. | | `latest_build_started_at` | datetime | Timestamp of when the latest build for the merge request started. | | `merge_commit_sha` | string | SHA of the merge request commit. Returns `null` until merged. | -| `merge_error` | string | Error message due to a merge error. | +| `merge_error` | string | Error message shown when a merge has failed. To check mergeability, use `detailed_merge_status` instead | | `merge_user` | object | The user who merged this merge request, the user who set it to merge when pipeline succeeds, or `null`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/349031) in GitLab 14.7. | | `merge_status` | string | Status of the merge request. Can be `unchecked`, `checking`, `can_be_merged`, `cannot_be_merged`, or `cannot_be_merged_recheck`. Affects the `has_conflicts` property. For important notes on response data, read [Single merge request response notes](#single-merge-request-response-notes). [Deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/3169#note_1162532204) in GitLab 15.6. Use `detailed_merge_status` instead. | | `merge_when_pipeline_succeeds` | boolean | Indicates if the merge has been set to be merged when its pipeline succeeds. | diff --git a/doc/ci/runners/configure_runners.md b/doc/ci/runners/configure_runners.md index 935ee6aace4..2de9156b571 100644 --- a/doc/ci/runners/configure_runners.md +++ b/doc/ci/runners/configure_runners.md @@ -637,13 +637,12 @@ test: - pwd ``` -The `GIT_CLONE_PATH` has to always be within `$CI_BUILDS_DIR`. The directory set in `$CI_BUILDS_DIR` +The `GIT_CLONE_PATH` must always be within `$CI_BUILDS_DIR`. The directory set in `$CI_BUILDS_DIR` is dependent on executor and configuration of [runners.builds_dir](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-runners-section) setting. This can only be used when `custom_build_dir` is enabled in the [runner's configuration](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-runnerscustom_build_dir-section). -This is the default configuration for the `docker` and `kubernetes` executors. #### Handling concurrency diff --git a/doc/development/gitlab_flavored_markdown/index.md b/doc/development/gitlab_flavored_markdown/index.md index 0af31892726..f7f1468003c 100644 --- a/doc/development/gitlab_flavored_markdown/index.md +++ b/doc/development/gitlab_flavored_markdown/index.md @@ -4,6 +4,8 @@ group: Editor info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- +<!-- vale gitlab.GitLabFlavoredMarkdown = NO --> + # GitLab Flavored Markdown (GLFM) developer documentation **(FREE)** This page contains the MVC for the developer documentation for GitLab Flavored Markdown (GLFM). diff --git a/doc/development/gitlab_flavored_markdown/specification_guide/index.md b/doc/development/gitlab_flavored_markdown/specification_guide/index.md index c1c8af52462..e2661145fbc 100644 --- a/doc/development/gitlab_flavored_markdown/specification_guide/index.md +++ b/doc/development/gitlab_flavored_markdown/specification_guide/index.md @@ -4,6 +4,8 @@ group: Editor info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- +<!-- vale gitlab.GitLabFlavoredMarkdown = NO --> + # GitLab Flavored Markdown (GLFM) Specification Guide **(FREE)** ## Summary diff --git a/lib/gitlab/ci/config/external/file/base.rb b/lib/gitlab/ci/config/external/file/base.rb index 65caf4ac47d..7899fe0ff73 100644 --- a/lib/gitlab/ci/config/external/file/base.rb +++ b/lib/gitlab/ci/config/external/file/base.rb @@ -47,7 +47,6 @@ module Gitlab end def validate! - context.check_execution_time! if ::Feature.disabled?(:ci_refactoring_external_mapper, context.project) validate_location! validate_context! if valid? fetch_and_validate_content! if valid? diff --git a/lib/gitlab/ci/config/external/mapper.rb b/lib/gitlab/ci/config/external/mapper.rb index a41bc2b39f2..61b4d1ada10 100644 --- a/lib/gitlab/ci/config/external/mapper.rb +++ b/lib/gitlab/ci/config/external/mapper.rb @@ -7,18 +7,6 @@ module Gitlab class Mapper include Gitlab::Utils::StrongMemoize - # Will be removed with FF ci_refactoring_external_mapper - FILE_CLASSES = [ - External::File::Local, - External::File::Project, - External::File::Remote, - External::File::Template, - External::File::Artifact - ].freeze - - # Will be removed with FF ci_refactoring_external_mapper - FILE_SUBKEYS = FILE_CLASSES.map { |f| f.name.demodulize.downcase }.freeze - Error = Class.new(StandardError) AmbigiousSpecificationError = Class.new(Error) TooManyIncludesError = Class.new(Error) @@ -32,11 +20,7 @@ module Gitlab return [] if @locations.empty? context.logger.instrument(:config_mapper_process) do - if ::Feature.enabled?(:ci_refactoring_external_mapper, context.project) - process_without_instrumentation - else - legacy_process_without_instrumentation - end + process_without_instrumentation end end @@ -57,138 +41,6 @@ module Gitlab files end - - # This and the following methods will be removed with FF ci_refactoring_external_mapper - def legacy_process_without_instrumentation - @locations - .map(&method(:normalize_location)) - .filter_map(&method(:verify_rules)) - .flat_map(&method(:expand_project_files)) - .flat_map(&method(:expand_wildcard_paths)) - .map(&method(:expand_variables)) - .map(&method(:select_first_matching)) - .each(&method(:verify!)) - end - - # convert location if String to canonical form - def normalize_location(location) - if location.is_a?(String) - expanded_location = expand_variables(location) - normalize_location_string(expanded_location) - else - location.deep_symbolize_keys - end - end - - def verify_rules(location) - logger.instrument(:config_mapper_rules) do - verify_rules_without_instrumentation(location) - end - end - - def verify_rules_without_instrumentation(location) - return unless Rules.new(location[:rules]).evaluate(context).pass? - - location - end - - def expand_project_files(location) - return location unless location[:project] - - Array.wrap(location[:file]).map do |file| - location.merge(file: file) - end - end - - def expand_wildcard_paths(location) - logger.instrument(:config_mapper_wildcards) do - expand_wildcard_paths_without_instrumentation(location) - end - end - - def expand_wildcard_paths_without_instrumentation(location) - # We only support local files for wildcard paths - return location unless location[:local] && location[:local].include?('*') - - context.project.repository.search_files_by_wildcard_path(location[:local], context.sha).map do |path| - { local: path } - end - end - - def normalize_location_string(location) - if ::Gitlab::UrlSanitizer.valid?(location) - { remote: location } - else - { local: location } - end - end - - def select_first_matching(location) - logger.instrument(:config_mapper_select) do - select_first_matching_without_instrumentation(location) - end - end - - def select_first_matching_without_instrumentation(location) - matching = FILE_CLASSES.map do |file_class| - file_class.new(location, context) - end.select(&:matching?) - - if matching.one? - matching.first - elsif matching.empty? - raise AmbigiousSpecificationError, "`#{masked_location(location.to_json)}` does not have a valid subkey for include. Valid subkeys are: `#{FILE_SUBKEYS.join('`, `')}`" - else - raise AmbigiousSpecificationError, "Each include must use only one of: `#{FILE_SUBKEYS.join('`, `')}`" - end - end - - def verify!(location_object) - verify_max_includes! - location_object.validate! - expandset.add(location_object) - end - - def verify_max_includes! - if expandset.count >= context.max_includes - raise TooManyIncludesError, "Maximum of #{context.max_includes} nested includes are allowed!" - end - end - - def expand_variables(data) - logger.instrument(:config_mapper_variables) do - expand_variables_without_instrumentation(data) - end - end - - def expand_variables_without_instrumentation(data) - if data.is_a?(String) - expand(data) - else - transform(data) - end - end - - def transform(data) - data.transform_values do |values| - case values - when Array - values.map { |value| expand(value.to_s) } - when String - expand(values) - else - values - end - end - end - - def expand(data) - ExpandVariables.expand(data, -> { context.variables_hash }) - end - - def masked_location(location) - context.mask_variables_from(location) - end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 038af87fbc4..d15cd6b649b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -26235,7 +26235,7 @@ msgstr "" msgid "MergeRequest|No files found" msgstr "" -msgid "MergeRequest|Search (e.g. *.vue) (%{modifier_key}P)" +msgid "MergeRequest|Search (e.g. *.vue) (%{MODIFIER_KEY}P)" msgstr "" msgid "MergeTopics|%{sourceTopic} will be removed" diff --git a/qa/qa/specs/features/api/1_manage/user_inherited_access_spec.rb b/qa/qa/specs/features/api/1_manage/user_inherited_access_spec.rb index 721bf2d2e7a..3df6e988bfa 100644 --- a/qa/qa/specs/features/api/1_manage/user_inherited_access_spec.rb +++ b/qa/qa/specs/features/api/1_manage/user_inherited_access_spec.rb @@ -86,6 +86,12 @@ module QA raise end.not_to raise_error end + + after do + parent_group_user.remove_via_api! + sub_group_project.remove_via_api! + sub_group.remove_via_api! + end end context 'when added to sub-group' do @@ -161,6 +167,12 @@ module QA end.to raise_error(Resource::ApiFabricator::ResourceFabricationFailedError, /403 Forbidden - You are not allowed to push into this branch/) end + + after do + sub_group_user.remove_via_api! + parent_group_project.remove_via_api! + sub_group.remove_via_api! + end end end end diff --git a/qa/qa/specs/features/browser_ui/1_manage/user/user_inherited_access_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/user/user_inherited_access_spec.rb index f7c51319eb3..b7585f00630 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/user/user_inherited_access_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/user/user_inherited_access_spec.rb @@ -51,6 +51,12 @@ module QA expect(file_form).to have_element(:commit_button) end end + + after do + parent_group_user.remove_via_api! + sub_group_project.remove_via_api! + sub_group.remove_via_api! + end end context 'when added to sub-group' do @@ -91,6 +97,12 @@ module QA expect(page).to have_text("You can’t edit files directly in this project.") end + + after do + sub_group_user.remove_via_api! + parent_group_project.remove_via_api! + sub_group.remove_via_api! + end end end end diff --git a/spec/frontend/vue_merge_request_widget/components/widget/widget_spec.js b/spec/frontend/vue_merge_request_widget/components/widget/widget_spec.js index 4c93c88de16..b78945cdde3 100644 --- a/spec/frontend/vue_merge_request_widget/components/widget/widget_spec.js +++ b/spec/frontend/vue_merge_request_widget/components/widget/widget_spec.js @@ -1,6 +1,6 @@ import { nextTick } from 'vue'; import * as Sentry from '@sentry/browser'; -import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import { shallowMountExtended, mountExtended } from 'helpers/vue_test_utils_helper'; import HelpPopover from '~/vue_shared/components/help_popover.vue'; import waitForPromises from 'helpers/wait_for_promises'; import StatusIcon from '~/vue_merge_request_widget/components/extensions/status_icon.vue'; @@ -26,8 +26,8 @@ describe('~/vue_merge_request_widget/components/widget/widget.vue', () => { const findHelpPopover = () => wrapper.findComponent(HelpPopover); const findDynamicScroller = () => wrapper.findByTestId('dynamic-content-scroller'); - const createComponent = ({ propsData, slots } = {}) => { - wrapper = shallowMountExtended(Widget, { + const createComponent = ({ propsData, slots, mountFn = shallowMountExtended } = {}) => { + wrapper = mountFn(Widget, { propsData: { isCollapsible: false, loadingText: 'Loading widget', @@ -425,6 +425,7 @@ describe('~/vue_merge_request_widget/components/widget/widget.vue', () => { beforeEach(() => { createComponent({ + mountFn: mountExtended, propsData: { isCollapsible: true, content, @@ -437,5 +438,11 @@ describe('~/vue_merge_request_widget/components/widget/widget.vue', () => { await waitForPromises(); expect(findDynamicScroller().props('items')).toEqual(content); }); + + it('renders the dynamic content inside the dynamic scroller', async () => { + findToggleButton().vm.$emit('click'); + await waitForPromises(); + expect(wrapper.findByText('Main text for the row').exists()).toBe(true); + }); }); }); diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb index b7e58d4dfa1..9d0e57d4292 100644 --- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' -# This will be removed with FF ci_refactoring_external_mapper and moved to below. +# This will be use with the FF ci_refactoring_external_mapper_verifier in the next MR. +# It can be removed when the FF is removed. RSpec.shared_context 'gitlab_ci_config_external_mapper' do include StubRequests include RepoHelpers @@ -466,12 +467,4 @@ end RSpec.describe Gitlab::Ci::Config::External::Mapper, feature_category: :pipeline_authoring do it_behaves_like 'gitlab_ci_config_external_mapper' - - context 'when the FF ci_refactoring_external_mapper is disabled' do - before do - stub_feature_flags(ci_refactoring_external_mapper: false) - end - - it_behaves_like 'gitlab_ci_config_external_mapper' - end end diff --git a/spec/models/commit_collection_spec.rb b/spec/models/commit_collection_spec.rb index 93c696cae54..6dd34c3e21f 100644 --- a/spec/models/commit_collection_spec.rb +++ b/spec/models/commit_collection_spec.rb @@ -15,26 +15,34 @@ RSpec.describe CommitCollection do end describe '.committers' do + subject(:collection) { described_class.new(project, [commit]) } + it 'returns a relation of users when users are found' do user = create(:user, email: commit.committer_email.upcase) - collection = described_class.new(project, [commit]) expect(collection.committers).to contain_exactly(user) end it 'returns empty array when committers cannot be found' do - collection = described_class.new(project, [commit]) - expect(collection.committers).to be_empty end it 'excludes authors of merge commits' do commit = project.commit("60ecb67744cb56576c30214ff52294f8ce2def98") create(:user, email: commit.committer_email.upcase) - collection = described_class.new(project, [commit]) expect(collection.committers).to be_empty end + + context 'when committer email is nil' do + before do + allow(commit).to receive(:committer_email).and_return(nil) + end + + it 'returns empty array when committers cannot be found' do + expect(collection.committers).to be_empty + end + end end describe '#without_merge_commits' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 05586cbfc64..02b3921ea55 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -4546,6 +4546,34 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe 'transition to merged' do + context 'when reset_merge_error_on_transition feature flag is on' do + before do + stub_feature_flags(reset_merge_error_on_transition: true) + end + + it 'resets the merge error' do + subject.update!(merge_error: 'temp') + + expect { subject.mark_as_merged }.to change { subject.merge_error.present? } + .from(true) + .to(false) + end + end + + context 'when reset_merge_error_on_transition feature flag is off' do + before do + stub_feature_flags(reset_merge_error_on_transition: false) + end + + it 'does not reset the merge error' do + subject.update!(merge_error: 'temp') + + expect { subject.mark_as_merged }.not_to change { subject.merge_error.present? } + end + end + end + describe 'transition to cannot_be_merged' do let(:notification_service) { double(:notification_service) } let(:todo_service) { double(:todo_service) } diff --git a/spec/services/merge_requests/base_service_spec.rb b/spec/services/merge_requests/base_service_spec.rb index 6eeba3029ae..da9c8651b0c 100644 --- a/spec/services/merge_requests/base_service_spec.rb +++ b/spec/services/merge_requests/base_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe MergeRequests::BaseService do +RSpec.describe MergeRequests::BaseService, feature_category: :code_review do include ProjectForksHelper let_it_be(:project) { create(:project, :repository) } diff --git a/spec/workers/merge_requests/create_pipeline_worker_spec.rb b/spec/workers/merge_requests/create_pipeline_worker_spec.rb index 441d7652219..bacf4084e1f 100644 --- a/spec/workers/merge_requests/create_pipeline_worker_spec.rb +++ b/spec/workers/merge_requests/create_pipeline_worker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe MergeRequests::CreatePipelineWorker do +RSpec.describe MergeRequests::CreatePipelineWorker, feature_category: :code_review_workflow do describe '#perform' do let(:user) { create(:user) } let(:project) { create(:project) } |