Add latest changes from gitlab-org/gitlab@master

.gitlab/ci/glfm.gitlab-ci.yml

@@ -8,3 +8,9 @@ glfm-verify:
   script:
     - !reference [.base-script, script]
     - bundle exec scripts/glfm/verify-all-generated-files-are-up-to-date.rb
+  artifacts:
+    name: changed-files
+    when: on_failure
+    expire_in: 31d
+    paths:
+      - glfm_specification/

GITALY_SERVER_VERSION

@@ -1 +1 @@
-038b442dc21c71a69ad170a61ce79a12a74fb725
+d4bc56074d6151875943c1b128b89b4f554af68a

app/assets/javascripts/projects/settings/branch_rules/components/view/constants.js

@@ -24,6 +24,11 @@ export const I18N = {
     'BranchRules|Approvals to ensure separation of duties for new merge requests. %{linkStart}Lean more.%{linkEnd}',
   ),
   statusChecksTitle: s__('BranchRules|Status checks'),
+  statusChecksDescription: s__(
+    'BranchRules|Check for a status response in merge requests. Failures do not block merges. %{linkStart}Lean more.%{linkEnd}',
+  ),
+  statusChecksLinkTitle: s__('BranchRules|Manage in Status checks'),
+  statusChecksHeader: s__('BranchRules|Status checks (%{total})'),
   allowedToPushHeader: s__('BranchRules|Allowed to push (%{total})'),
   allowedToMergeHeader: s__('BranchRules|Allowed to merge (%{total})'),
   approvalsHeader: s__('BranchRules|Required approvals (%{total})'),
@@ -40,3 +45,5 @@ export const WILDCARDS_HELP_PATH =
 export const PROTECTED_BRANCHES_HELP_PATH = 'user/project/protected_branches';
 
 export const APPROVALS_HELP_PATH = 'user/project/merge_requests/approvals/index.md';
+
+export const STATUS_CHECKS_HELP_PATH = 'user/project/merge_requests/status_checks.md';

app/assets/javascripts/projects/settings/branch_rules/components/view/index.vue

@@ -12,11 +12,13 @@ import {
   WILDCARDS_HELP_PATH,
   PROTECTED_BRANCHES_HELP_PATH,
   APPROVALS_HELP_PATH,
+  STATUS_CHECKS_HELP_PATH,
 } from './constants';
 
 const wildcardsHelpDocLink = helpPagePath(WILDCARDS_HELP_PATH);
 const protectedBranchesHelpDocLink = helpPagePath(PROTECTED_BRANCHES_HELP_PATH);
 const approvalsHelpDocLink = helpPagePath(APPROVALS_HELP_PATH);
+const statusChecksHelpDocLink = helpPagePath(STATUS_CHECKS_HELP_PATH);
 
 export default {
   name: 'RuleView',
@@ -24,6 +26,7 @@ export default {
   wildcardsHelpDocLink,
   protectedBranchesHelpDocLink,
   approvalsHelpDocLink,
+  statusChecksHelpDocLink,
   components: { Protection, GlSprintf, GlLink, GlLoadingIcon },
   inject: {
     projectPath: {
@@ -35,6 +38,9 @@ export default {
     approvalRulesPath: {
       default: '',
     },
+    statusChecksPath: {
+      default: '',
+    },
   },
   apollo: {
     project: {
@@ -48,6 +54,7 @@ export default {
         const branchRule = branchRules.nodes.find((rule) => rule.name === this.branch);
         this.branchProtection = branchRule?.branchProtection;
         this.approvalRules = branchRule?.approvalRules;
+        this.statusChecks = branchRule?.externalStatusChecks || [];
       },
     },
   },
@@ -56,6 +63,7 @@ export default {
       branch: getParameterByName(BRANCH_PARAM_NAME),
       branchProtection: {},
       approvalRules: {},
+      statusChecks: [],
     };
   },
   computed: {
@@ -91,6 +99,11 @@ export default {
         total,
       });
     },
+    statusChecksHeader() {
+      return sprintf(this.$options.i18n.statusChecksHeader, {
+        total: this.statusChecks.length,
+      });
+    },
     allBranches() {
       return this.branch === ALL_BRANCHES_WILDCARD;
     },
@@ -201,6 +214,21 @@ export default {
     />
 
     <!-- Status checks -->
-    <!-- Follow-up: add status checks section (https://gitlab.com/gitlab-org/gitlab/-/issues/372362) -->
+    <h4 class="gl-mb-1 gl-mt-5">{{ $options.i18n.statusChecksTitle }}</h4>
+    <gl-sprintf :message="$options.i18n.statusChecksDescription">
+      <template #link="{ content }">
+        <gl-link :href="$options.statusChecksHelpDocLink">
+          {{ content }}
+        </gl-link>
+      </template>
+    </gl-sprintf>
+
+    <protection
+      class="gl-mt-3"
+      :header="statusChecksHeader"
+      :header-link-title="$options.i18n.statusChecksLinkTitle"
+      :header-link-href="statusChecksPath"
+      :status-checks="statusChecks"
+    />
   </div>
 </template>

app/assets/javascripts/projects/settings/branch_rules/components/view/protection.vue

@@ -46,6 +46,11 @@ export default {
       required: false,
       default: () => [],
     },
+    statusChecks: {
+      type: Array,
+      required: false,
+      default: () => [],
+    },
   },
   computed: {
     showUsersDivider() {
@@ -95,5 +100,14 @@ export default {
       :users="approval.eligibleApprovers.nodes"
       :approvals-required="approval.approvalsRequired"
     />
+
+    <!-- Status checks -->
+    <protection-row
+      v-for="(statusCheck, index) in statusChecks"
+      :key="statusCheck.id"
+      :show-divider="index !== 0"
+      :title="statusCheck.name"
+      :status-check-url="statusCheck.externalUrl"
+    />
   </gl-card>
 </template>

app/assets/javascripts/projects/settings/branch_rules/components/view/protection_row.vue

@@ -41,6 +41,11 @@ export default {
       required: false,
       default: 0,
     },
+    statusCheckUrl: {
+      type: String,
+      required: false,
+      default: null,
+    },
   },
   computed: {
     avatarBadgeSrOnlyText() {
@@ -67,7 +72,7 @@ export default {
     class="gl-display-flex gl-align-items-center gl-border-gray-100 gl-mb-4 gl-pt-4 gl-border-t-1"
     :class="{ 'gl-border-t-solid': showDivider }"
   >
-    <div class="gl-display-flex gl-w-half gl-justify-content-space-between gl-align-items-center">
+    <div class="gl-display-flex gl-w-full gl-justify-content-space-between gl-align-items-center">
       <div class="gl-mr-7 gl-w-quarter">{{ title }}</div>
 
       <gl-avatars-inline
@@ -94,6 +99,8 @@ export default {
         </template>
       </gl-avatars-inline>
 
+      <div v-if="statusCheckUrl" class="gl-ml-7 gl-flex-grow-1">{{ statusCheckUrl }}</div>
+
       <div
         v-for="(item, index) in accessLevels"
         :key="index"
@@ -104,7 +111,7 @@ export default {
         {{ item.accessLevelDescription }}
       </div>
 
-      <div class="gl-ml-7 gl-w-quarter">{{ approvalsRequiredTitle }}</div>
+      <div class="gl-ml-7 gl-flex-grow-1">{{ approvalsRequiredTitle }}</div>
     </div>
   </div>
 </template>

app/assets/javascripts/projects/settings/branch_rules/mount_branch_rules.js

@@ -14,7 +14,7 @@ export default function mountBranchRules(el) {
     defaultClient: createDefaultClient(),
   });
 
-  const { projectPath, protectedBranchesPath, approvalRulesPath } = el.dataset;
+  const { projectPath, protectedBranchesPath, approvalRulesPath, statusChecksPath } = el.dataset;
 
   return new Vue({
     el,
@@ -23,6 +23,7 @@ export default function mountBranchRules(el) {
       projectPath,
       protectedBranchesPath,
       approvalRulesPath,
+      statusChecksPath,
     },
     render(h) {
       return h(View);

app/models/hooks/system_hook.rb

@@ -10,9 +10,9 @@ class SystemHook < WebHook
     :merge_request_hooks
   ]
 
-  default_value_for :push_events, false
-  default_value_for :repository_update_events, true
-  default_value_for :merge_requests_events, false
+  attribute :push_events, default: false
+  attribute :repository_update_events, default: true
+  attribute :merge_requests_events, default: false
 
   validates :url, system_hook_url: true
 

app/models/label.rb

@@ -14,8 +14,7 @@ class Label < ApplicationRecord
 
   DEFAULT_COLOR = ::Gitlab::Color.of('#6699cc')
 
-  attribute :color, ::Gitlab::Database::Type::Color.new
-  default_value_for :color, DEFAULT_COLOR
+  attribute :color, ::Gitlab::Database::Type::Color.new, default: DEFAULT_COLOR
 
   has_many :lists, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :priorities, class_name: 'LabelPriority'

app/models/members/project_member.rb

@@ -10,7 +10,7 @@ class ProjectMember < Member
   delegate :namespace_id, to: :project
 
   # Make sure project member points only to project as it source
-  default_value_for :source_type, SOURCE_TYPE
+  attribute :source_type, default: SOURCE_TYPE
   validates :source_type, format: { with: SOURCE_TYPE_FORMAT }
   default_scope { where(source_type: SOURCE_TYPE) } # rubocop:disable Cop/DefaultScope
 

app/models/ml/candidate.rb

@@ -13,7 +13,7 @@ module Ml
     has_many :params, class_name: 'Ml::CandidateParam'
     has_many :latest_metrics, -> { latest }, class_name: 'Ml::CandidateMetric', inverse_of: :candidate
 
-    default_value_for(:iid) { SecureRandom.uuid }
+    attribute :iid, default: -> { SecureRandom.uuid }
 
     scope :including_metrics_and_params, -> { includes(:latest_metrics, :params) }
 

app/models/note.rb

@@ -60,7 +60,7 @@ class Note < ApplicationRecord
   # Attribute used to determine whether keep_around_commits will be skipped for diff notes.
   attr_accessor :skip_keep_around_commits
 
-  default_value_for :system, false
+  attribute :system, default: false
 
   attr_mentionable :note, pipeline: :note
   participant :author

app/models/operations/feature_flag.rb

@@ -16,8 +16,8 @@ module Operations
 
     has_internal_id :iid, scope: :project
 
-    default_value_for :active, true
-    default_value_for :version, :new_version_flag
+    attribute :active, default: true
+    attribute :version, default: :new_version_flag
 
     # strategies exists only for the second version
     has_many :strategies, class_name: 'Operations::FeatureFlags::Strategy'

app/models/project_statistics.rb

@@ -7,8 +7,8 @@ class ProjectStatistics < ApplicationRecord
   belongs_to :project
   belongs_to :namespace
 
-  default_value_for :wiki_size, 0
-  default_value_for :snippets_size, 0
+  attribute :wiki_size, default: 0
+  attribute :snippets_size, default: 0
 
   counter_attribute :build_artifacts_size
 
@@ -95,8 +95,7 @@ class ProjectStatistics < ApplicationRecord
   # and the column can be nil.
   # This means that, when the columns were added, all rows had nil
   # values on them.
-  # Therefore, any call to any of those methods will return nil instead
-  # of 0, because `default_value_for` works with new records, not existing ones.
+  # Therefore, any call to any of those methods will return nil instead of 0.
   #
   # These two methods provide consistency and avoid returning nil.
   def wiki_size

app/models/time_tracking/timelog_category.rb

@@ -24,8 +24,7 @@ module TimeTracking
 
     DEFAULT_COLOR = ::Gitlab::Color.of('#6699cc')
 
-    attribute :color, ::Gitlab::Database::Type::Color.new
-    default_value_for :color, DEFAULT_COLOR
+    attribute :color, ::Gitlab::Database::Type::Color.new, default: DEFAULT_COLOR
 
     def self.find_by_name(namespace_id, name)
       where(namespace: namespace_id)

app/services/merge_requests/create_service.rb

@@ -14,11 +14,15 @@ module MergeRequests
     end
 
     def after_create(issuable)
-      issuable.mark_as_preparing
+      current_user_id = current_user.id
 
-      # Add new items to MergeRequests::AfterCreateService if they can
-      # be performed in Sidekiq
-      NewMergeRequestWorker.perform_async(issuable.id, current_user.id)
+      issuable.run_after_commit do
+        # Add new items to MergeRequests::AfterCreateService if they can
+        # be performed in Sidekiq
+        NewMergeRequestWorker.perform_async(issuable.id, current_user_id)
+      end
+
+      issuable.mark_as_preparing
 
       super
     end
@@ -64,4 +68,4 @@ module MergeRequests
   end
 end
 
-MergeRequests::CreateService.include_mod_with('MergeRequests::CreateService')
+MergeRequests::CreateService.prepend_mod_with('MergeRequests::CreateService')

app/views/projects/settings/branch_rules/index.html.haml

@@ -3,4 +3,4 @@
 
 %h3.gl-mb-5= s_('BranchRules|Branch rules details')
 
-#js-branch-rules{ data: { project_path: @project.full_path, protected_branches_path: project_settings_repository_path(@project, anchor: 'js-protected-branches-settings'), approval_rules_path: project_settings_merge_requests_path(@project, anchor: 'js-merge-request-approval-settings') } }
+#js-branch-rules{ data: { project_path: @project.full_path, protected_branches_path: project_settings_repository_path(@project, anchor: 'js-protected-branches-settings'), approval_rules_path: project_settings_merge_requests_path(@project, anchor: 'js-merge-request-approval-settings'), status_checks_path: project_settings_merge_requests_path(@project, anchor: 'js-merge-request-settings') } }

locale/gitlab.pot

@@ -6921,6 +6921,9 @@ msgstr ""
 msgid "BranchRules|Branch rules details"
 msgstr ""
 
+msgid "BranchRules|Check for a status response in merge requests. Failures do not block merges. %{linkStart}Lean more.%{linkEnd}"
+msgstr ""
+
 msgid "BranchRules|Create wildcard: %{searchTerm}"
 msgstr ""
 
@@ -6945,6 +6948,9 @@ msgstr ""
 msgid "BranchRules|Manage in Protected Branches"
 msgstr ""
 
+msgid "BranchRules|Manage in Status checks"
+msgstr ""
+
 msgid "BranchRules|No data to display"
 msgstr ""
 
@@ -6975,6 +6981,9 @@ msgstr ""
 msgid "BranchRules|Status checks"
 msgstr ""
 
+msgid "BranchRules|Status checks (%{total})"
+msgstr ""
+
 msgid "BranchRules|Target Branch"
 msgstr ""
 
@@ -36915,6 +36924,9 @@ msgstr ""
 msgid "See our website for help"
 msgstr ""
 
+msgid "See the Geo troubleshooting documentation to learn more: %{docs_url}"
+msgstr ""
+
 msgid "See the affected projects in the GitLab admin panel"
 msgstr ""
 

scripts/lib/glfm/verify_all_generated_files_are_up_to_date.rb

@@ -41,8 +41,20 @@ module Glfm
 
       return if verify_cmd_output.empty?
 
-      raise "The following files were modified by running GLFM scripts. Please review, verify, and commit " \
-        "the changes:\n#{verify_cmd_output}"
+      warn(
+        "ERROR: The following files were modified by running GLFM scripts. Please review, verify, and commit " \
+        "the changes:\n#{verify_cmd_output}\n"
+      )
+      warn("See the CI artifacts for the modified version of the files.\n")
+
+      warn("This is the output of `git diff`:\n")
+      diff_output = run_external_cmd('git diff')
+      warn(diff_output)
+
+      # Ensure that the diff output is flushed and output before we raise and exit.
+      $stderr.flush
+
+      raise('ERROR: The generated files are not up to date.')
     end
   end
 end

spec/frontend/projects/settings/branch_rules/components/view/index_spec.js

@@ -34,6 +34,7 @@ describe('View branch rules', () => {
   const projectPath = 'test/testing';
   const protectedBranchesPath = 'protected/branches';
   const approvalRulesPath = 'approval/rules';
+  const statusChecksPath = 'status/checks';
   const branchProtectionsMockRequestHandler = jest
     .fn()
     .mockResolvedValue(branchProtectionsMockResponse);
@@ -43,7 +44,7 @@ describe('View branch rules', () => {
 
     wrapper = shallowMountExtended(RuleView, {
       apolloProvider: fakeApollo,
-      provide: { projectPath, protectedBranchesPath, approvalRulesPath },
+      provide: { projectPath, protectedBranchesPath, approvalRulesPath, statusChecksPath },
     });
 
     await waitForPromises();
@@ -59,6 +60,7 @@ describe('View branch rules', () => {
   const findBranchProtections = () => wrapper.findAllComponents(Protection);
   const findForcePushTitle = () => wrapper.findByText(I18N.allowForcePushDescription);
   const findApprovalsTitle = () => wrapper.findByText(I18N.approvalsTitle);
+  const findStatusChecksTitle = () => wrapper.findByText(I18N.statusChecksTitle);
 
   it('gets the branch param from url and renders it in the view', () => {
     expect(util.getParameterByName).toHaveBeenCalledWith('branch');
@@ -111,4 +113,16 @@ describe('View branch rules', () => {
       approvals: approvalRulesMock,
     });
   });
+
+  it('renders a branch protection component for status checks', () => {
+    expect(findStatusChecksTitle().exists()).toBe(true);
+
+    expect(findBranchProtections().at(3).props()).toMatchObject({
+      // status checks BE/FE integration will happen on a follow-up, so we expect data to be empty
+      header: sprintf(I18N.statusChecksHeader, { total: 0 }),
+      headerLinkHref: statusChecksPath,
+      headerLinkTitle: I18N.statusChecksLinkTitle,
+      statusChecks: [],
+    });
+  });
 });

spec/frontend/projects/settings/branch_rules/components/view/mock_data.js

@@ -56,6 +56,11 @@ export const approvalRulesMock = [
   },
 ];
 
+export const statusChecksRulesMock = [
+  { __typename: 'StatusCheckRule', id: '123', name: 'test', externalUrl: 'https://test.test' },
+  { __typename: 'StatusCheckRule', id: '456', name: 'test 2', externalUrl: 'https://test2.test2' },
+];
+
 export const protectionPropsMock = {
   header: 'Test protection',
   headerLinkTitle: 'Test link title',
@@ -64,6 +69,7 @@ export const protectionPropsMock = {
   users: usersMock,
   groups: groupsMock,
   approvals: approvalRulesMock,
+  statusChecks: statusChecksRulesMock,
 };
 
 export const protectionRowPropsMock = {
@@ -71,6 +77,7 @@ export const protectionRowPropsMock = {
   users: usersMock,
   accessLevels: accessLevelsMock,
   approvalsRequired,
+  statusCheckUrl: statusChecksRulesMock[0].externalUrl,
 };
 
 export const accessLevelsMockResponse = [

spec/frontend/projects/settings/branch_rules/components/view/protection_row_spec.js

@@ -27,6 +27,7 @@ describe('Branch rule protection row', () => {
   const findAccessLevels = () => wrapper.findAllByTestId('access-level');
   const findApprovalsRequired = () =>
     wrapper.findByText(`${protectionRowPropsMock.approvalsRequired} approvals required`);
+  const findStatusChecksUrl = () => wrapper.findByText(protectionRowPropsMock.statusCheckUrl);
 
   it('renders a title', () => {
     expect(findTitle().exists()).toBe(true);
@@ -68,4 +69,8 @@ describe('Branch rule protection row', () => {
   it('renders the number of approvals required', () => {
     expect(findApprovalsRequired().exists()).toBe(true);
   });
+
+  it('renders status checks URL', () => {
+    expect(findStatusChecksUrl().exists()).toBe(true);
+  });
 });

spec/frontend/projects/settings/branch_rules/components/view/protection_spec.js

@@ -65,4 +65,15 @@ describe('Branch rule protection', () => {
       approvalsRequired: approval.approvalsRequired,
     });
   });
+
+  it('renders a protection row for status checks', () => {
+    const statusCheck = protectionPropsMock.statusChecks[0];
+    expect(findProtectionRows().at(4).props()).toMatchObject({
+      title: statusCheck.name,
+      showDivider: false,
+      statusCheckUrl: statusCheck.externalUrl,
+    });
+
+    expect(findProtectionRows().at(5).props('showDivider')).toBe(true);
+  });
 });

spec/models/hooks/system_hook_spec.rb

@@ -4,7 +4,7 @@ require "spec_helper"
 
 RSpec.describe SystemHook do
   context 'default attributes' do
-    let(:system_hook) { build(:system_hook) }
+    let(:system_hook) { described_class.new }
 
     it 'sets defined default parameters' do
       attrs = {

spec/models/members/project_member_spec.rb

@@ -13,6 +13,10 @@ RSpec.describe ProjectMember do
     it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) }
   end
 
+  describe 'default values' do
+    it { expect(described_class.new.source_type).to eq('Project') }
+  end
+
   describe 'delegations' do
     it { is_expected.to delegate_method(:namespace_id).to(:project) }
   end

spec/models/ml/candidate_spec.rb

@@ -18,10 +18,8 @@ RSpec.describe Ml::Candidate, factory_default: :keep do
     it { is_expected.to eq("/ml_candidate_#{candidate.iid}/-/") }
   end
 
-  describe '#new' do
-    it 'iid is not null' do
-      expect(candidate.iid).not_to be_nil
-    end
+  describe 'default values' do
+    it { expect(described_class.new.iid).to be_present }
   end
 
   describe '#by_project_id_and_iid' do

spec/models/note_spec.rb

@@ -23,6 +23,10 @@ RSpec.describe Note do
     it { is_expected.to include_module(Sortable) }
   end
 
+  describe 'default values' do
+    it { expect(described_class.new).not_to be_system }
+  end
+
   describe 'validation' do
     it { is_expected.to validate_length_of(:note).is_at_most(1_000_000) }
     it { is_expected.to validate_presence_of(:note) }

spec/models/operations/feature_flag_spec.rb

@@ -16,6 +16,11 @@ RSpec.describe Operations::FeatureFlag do
     it { is_expected.to have_many(:strategies) }
   end
 
+  describe 'default values' do
+    it { expect(described_class.new).to be_active }
+    it { expect(described_class.new.version).to eq('new_version_flag') }
+  end
+
   describe '.reference_pattern' do
     subject { described_class.reference_pattern }
 

spec/models/time_tracking/timelog_category_spec.rb

@@ -7,6 +7,10 @@ RSpec.describe TimeTracking::TimelogCategory, type: :model do
     it { is_expected.to belong_to(:namespace).with_foreign_key('namespace_id') }
   end
 
+  describe 'default values' do
+    it { expect(described_class.new.color).to eq(described_class::DEFAULT_COLOR) }
+  end
+
   describe 'validations' do
     subject { create(:timelog_category) }
 

spec/scripts/lib/glfm/verify_all_generated_files_are_up_to_date_spec.rb

@@ -52,10 +52,13 @@ RSpec.describe Glfm::VerifyAllGeneratedFilesAreUpToDate, '#process' do
       before do
         # Simulate a clean repo, then simulate changes to generated files
         allow(subject).to receive(:run_external_cmd).twice.with(verify_cmd).and_return('', "M #{snapshots_path}")
+        allow(subject).to receive(:run_external_cmd).with('git diff')
+        allow(subject).to receive(:warn).and_call_original
       end
 
       it 'raises an error', :unlimited_max_formatted_output_length do
-        expect { subject.process }.to raise_error(/following files were modified.*#{snapshots_path}/m)
+        expect(subject).to receive(:warn).with(/following files were modified.*#{snapshots_path}/m)
+        expect { subject.process }.to raise_error(/The generated files are not up to date/)
       end
     end
   end

spec/services/merge_requests/create_service_spec.rb

@@ -336,6 +336,12 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
       it_behaves_like 'reviewer_ids filter' do
         let(:execute) { service.execute }
       end
+
+      context 'when called in a transaction' do
+        it 'does not raise an error' do
+          expect { MergeRequest.transaction { described_class.new(project: project, current_user: user, params: opts).execute } }.not_to raise_error
+        end
+      end
     end
 
     it_behaves_like 'issuable record that supports quick actions' do