diff --git a/app/assets/javascripts/vue_merge_request_widget/components/checks/constants.js b/app/assets/javascripts/vue_merge_request_widget/components/checks/constants.js
index 91814a93000504254ff2b24f1e3b19294c3dc88c..e1cffbcbef285ed1dcbc845b445b66f7468b48b4 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/checks/constants.js
+++ b/app/assets/javascripts/vue_merge_request_widget/components/checks/constants.js
@@ -24,4 +24,5 @@ export const FAILURE_REASONS = {
jira_association_missing: __('Either the title or description must reference a Jira issue.'),
requested_changes: __('The change requests must be completed or resolved.'),
approvals_syncing: __('The merge request approvals are currently syncing.'),
+ locked_lfs_files: __('All LFS files must be unlocked.'),
};
diff --git a/app/graphql/types/merge_requests/detailed_merge_status_enum.rb b/app/graphql/types/merge_requests/detailed_merge_status_enum.rb
index c612b297213fe25cba2da45e3188dc194c4e2a79..190417bdaeb2dad7174b584ef5febc9818c36381 100644
--- a/app/graphql/types/merge_requests/detailed_merge_status_enum.rb
+++ b/app/graphql/types/merge_requests/detailed_merge_status_enum.rb
@@ -57,6 +57,9 @@ class DetailedMergeStatusEnum < BaseEnum
value 'APPROVALS_SYNCING',
value: :approvals_syncing,
description: 'Merge request approvals currently syncing.'
+ value 'LOCKED_LFS_FILES',
+ value: :locked_lfs_files,
+ description: 'Merge request includes locked LFS files.'
end
end
end
diff --git a/app/models/lfs_file_lock.rb b/app/models/lfs_file_lock.rb
index 624b1d02e1a61c6cfc8eec829c46443966f8134a..c442760dce10383daf91e64d3d78fe4531fb1262 100644
--- a/app/models/lfs_file_lock.rb
+++ b/app/models/lfs_file_lock.rb
@@ -4,6 +4,9 @@ class LfsFileLock < ApplicationRecord
belongs_to :project
belongs_to :user
+ scope :for_paths, ->(paths) { where(path: paths) }
+ scope :not_for_users, ->(user_ids) { where.not(user_id: user_ids) }
+
validates :project_id, :user_id, :path, presence: true
def can_be_unlocked_by?(current_user, forced = false)
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index ec87b6b36b1bb7d90009721fd9f4446a1f15826b..c4959cd6ee1bb7f1bd500846b1d4c01bc7efbfc1 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -1273,7 +1273,8 @@ def skipped_mergeable_checks(options = {})
skip_discussions_check: merge_when_checks_pass_strat,
skip_external_status_check: merge_when_checks_pass_strat,
skip_requested_changes_check: merge_when_checks_pass_strat,
- skip_jira_check: merge_when_checks_pass_strat
+ skip_jira_check: merge_when_checks_pass_strat,
+ skip_locked_lfs_files_check: merge_when_checks_pass_strat
}
end
@@ -1286,6 +1287,7 @@ def skipped_mergeable_checks(options = {})
# skip_external_status_check
# skip_requested_changes_check
# skip_jira_check
+ # skip_locked_lfs_files_check
def mergeable?(check_mergeability_retry_lease: false, skip_rebase_check: false, **mergeable_state_check_params)
return false unless mergeable_state?(**mergeable_state_check_params)
@@ -1302,7 +1304,8 @@ def self.mergeable_state_checks
::MergeRequests::Mergeability::CheckDraftStatusService,
::MergeRequests::Mergeability::CheckCommitsStatusService,
::MergeRequests::Mergeability::CheckDiscussionsStatusService,
- ::MergeRequests::Mergeability::CheckCiStatusService
+ ::MergeRequests::Mergeability::CheckCiStatusService,
+ ::MergeRequests::Mergeability::CheckLfsFileLocksService
]
end
diff --git a/app/services/merge_requests/mergeability/check_lfs_file_locks_service.rb b/app/services/merge_requests/mergeability/check_lfs_file_locks_service.rb
new file mode 100644
index 0000000000000000000000000000000000000000..f87f26f41c2521cc6c999e01c8d1e34f6dc019b3
--- /dev/null
+++ b/app/services/merge_requests/mergeability/check_lfs_file_locks_service.rb
@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+module MergeRequests
+ module Mergeability
+ class CheckLfsFileLocksService < CheckBaseService
+ include ::Gitlab::Utils::StrongMemoize
+
+ identifier :locked_lfs_files
+ description <<~DESC.chomp
+ Checks whether the merge request contains locked LFS files that are locked by users other than the merge request author
+ DESC
+
+ CACHE_KEY = 'merge_request:%{id}:%{sha}:lfs_file_locks_mergeability:%{epoch}'
+
+ def execute
+ return inactive if check_inactive?
+ return failure if contains_locked_lfs_files?
+
+ success
+ end
+
+ def skip?
+ params[:skip_locked_lfs_files_check].present?
+ end
+
+ def cacheable?
+ true
+ end
+
+ def cache_key
+ # If the feature is disabled we will return inactive so we don't need to
+ # link the cache key to a specific MR.
+ return 'inactive_lfs_file_locks_mergeability_check' if check_inactive?
+
+ # Cache is linked to a specific MR
+ id = merge_request.id
+ # Cache is invalidated when new changes are added
+ sha = merge_request.diff_head_sha
+ # Cache is invalidated when lfs_file_locks are added or removed
+ epoch = project.lfs_file_locks_changed_epoch
+
+ format(CACHE_KEY, id: id, sha: sha, epoch: epoch)
+ end
+
+ private
+
+ delegate :project, :author_id, :changed_paths, to: :merge_request
+
+ def contains_locked_lfs_files?
+ project.lfs_file_locks.for_paths(changed_paths.map(&:path)).not_for_users(author_id).exists?
+ end
+
+ def check_inactive?
+ Feature.disabled?(:locked_lfs_files_mergeability_check, project) || !project.lfs_enabled?
+ end
+ strong_memoize_attr :check_inactive?
+ end
+ end
+end
diff --git a/config/feature_flags/beta/locked_lfs_files_mergeability_check.yml b/config/feature_flags/beta/locked_lfs_files_mergeability_check.yml
new file mode 100644
index 0000000000000000000000000000000000000000..f95e678293b92afe9da53c3495d396f83f6c38b1
--- /dev/null
+++ b/config/feature_flags/beta/locked_lfs_files_mergeability_check.yml
@@ -0,0 +1,9 @@
+---
+name: locked_lfs_files_mergeability_check
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/428247
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/158773
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/474485
+milestone: '17.3'
+group: group::source code
+type: beta
+default_enabled: false
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index ff0dffd0ec690487b409d46d602eb5491b587ac5..1c937aa0b6fbea20fe4e1754ae37d2c927714909 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -35144,6 +35144,7 @@ Detailed representation of whether a GitLab merge request can be merged.
| `DRAFT_STATUS` | Merge request must not be draft before merging. |
| `EXTERNAL_STATUS_CHECKS` | Status checks must pass. |
| `JIRA_ASSOCIATION` | Either the title or description must reference a Jira issue. |
+| `LOCKED_LFS_FILES` | Merge request includes locked LFS files. |
| `MERGEABLE` | Branch can be merged. |
| `NEED_REBASE` | Merge request needs to be rebased. |
| `NOT_APPROVED` | Merge request must be approved before merging. |
@@ -35919,6 +35920,7 @@ Representation of mergeability check identifier.
| `DISCUSSIONS_NOT_RESOLVED` | Checks whether the merge request has open discussions. |
| `DRAFT_STATUS` | Checks whether the merge request is draft. |
| `JIRA_ASSOCIATION_MISSING` | Checks whether the title or description references a Jira issue. |
+| `LOCKED_LFS_FILES` | Checks whether the merge request contains locked LFS files that are locked by users other than the merge request author. |
| `MERGE_REQUEST_BLOCKED` | Checks whether the merge request is blocked. |
| `NEED_REBASE` | Checks whether the merge request needs to be rebased. |
| `NOT_APPROVED` | Checks whether the merge request is approved. |
diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md
index f8086c2a410e100b0ab78a4d16fc5d74775286c5..8e406cd4e9288f78f470b3d7da687fd51cb3acc4 100644
--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
@@ -909,6 +909,7 @@ Use `detailed_merge_status` instead of `merge_status` to account for all potenti
- `not_open`: The merge request must be open before merge.
- `requested_changes`: The merge request has reviewers who have requested changes.
- `unchecked`: Git has not yet tested if a valid merge is possible.
+ - `locked_lfs_files`: LFS files locked by other users must be unlocked before merge.
### Preparation steps
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 68e7692aca37bf5f2c03172c635528c2e4dbfbd5..36fbb507d7c9f2ad169eb0c29755f75240715fe9 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -5383,6 +5383,9 @@ msgstr ""
msgid "All GitLab"
msgstr ""
+msgid "All LFS files must be unlocked."
+msgstr ""
+
msgid "All Members"
msgstr ""
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index e3c86fdd0754f9855c64bf8279ef6d95ae2f49fb..f97a2c20b1eb56a056fa67d5a7cf334a582d3760 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -3947,10 +3947,10 @@ def set_compare(merge_request)
let(:options) { { auto_merge_requested: true, auto_merge_strategy: auto_merge_strategy } }
where(:auto_merge_strategy, :skip_approved_check, :skip_draft_check, :skip_blocked_check,
- :skip_discussions_check, :skip_external_status_check, :skip_requested_changes_check, :skip_jira_check) do
- '' | false | false | false | false | false | false | false
- AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS | false | false | false | false | false | false | false
- AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS | true | true | true | true | true | true | true
+ :skip_discussions_check, :skip_external_status_check, :skip_requested_changes_check, :skip_jira_check, :skip_locked_lfs_files_check) do
+ '' | false | false | false | false | false | false | false | false
+ AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS | false | false | false | false | false | false | false | false
+ AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS | true | true | true | true | true | true | true | true
end
with_them do
diff --git a/spec/services/merge_requests/mergeability/check_lfs_file_locks_service_spec.rb b/spec/services/merge_requests/mergeability/check_lfs_file_locks_service_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..ea25cce4f6c93059292796685e2cd0c924acc938
--- /dev/null
+++ b/spec/services/merge_requests/mergeability/check_lfs_file_locks_service_spec.rb
@@ -0,0 +1,127 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MergeRequests::Mergeability::CheckLfsFileLocksService, feature_category: :code_review_workflow do
+ subject(:check_lfs_file_locks) { described_class.new(merge_request: merge_request, params: params) }
+
+ let_it_be(:project) { create(:project) }
+ let_it_be(:merge_request) { build(:merge_request, source_project: project) }
+ let(:params) { { skip_locked_lfs_files_check: skip_check } }
+ let(:skip_check) { false }
+ let(:feature_flag_enabled) { true }
+ let(:lfs_enabled) { true }
+
+ before do
+ stub_feature_flags(locked_lfs_files_mergeability_check: feature_flag_enabled)
+ allow(project).to receive(:lfs_enabled?).and_return(lfs_enabled)
+ end
+
+ it_behaves_like 'mergeability check service', :locked_lfs_files, <<~DESC.chomp
+ Checks whether the merge request contains locked LFS files that are locked by users other than the merge request author
+ DESC
+
+ describe '#execute' do
+ subject(:execute) { check_lfs_file_locks.execute }
+
+ context 'when lfs is enabled' do
+ let(:only_allow_merge_if_pipeline_succeeds) { true }
+ let(:changed_path) { instance_double('Gitlab::Git::ChangedPath', path: 'README.md') }
+
+ before do
+ allow(merge_request).to receive(:changed_paths).and_return([changed_path])
+ end
+
+ context 'when there are no lfs files locks for this project' do
+ it 'returns a check result with status success' do
+ expect(execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::SUCCESS_STATUS
+ end
+ end
+
+ context 'when there are lfs files locked by the merge request author' do
+ let(:user) { create(:user) }
+
+ before do
+ allow(merge_request).to receive(:author_id).and_return(user.id)
+ create(:lfs_file_lock, project: project, path: changed_path.path, user: user)
+ end
+
+ it 'returns a check result with status success' do
+ expect(execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::SUCCESS_STATUS
+ end
+ end
+
+ context 'when there are lfs files locked by another user' do
+ before do
+ allow(merge_request).to receive(:author_id).and_return(0)
+ create(:lfs_file_lock, project: project, path: changed_path.path)
+ end
+
+ it 'returns a check result with status failure' do
+ expect(execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::FAILED_STATUS
+ end
+ end
+ end
+
+ context 'when lfs is not enabled' do
+ let(:lfs_enabled) { false }
+
+ it 'returns a check result with inactive status' do
+ expect(execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::INACTIVE_STATUS
+ end
+ end
+ end
+
+ describe '#skip?' do
+ subject(:skip) { check_lfs_file_locks.skip? }
+
+ context 'when skip check is true' do
+ let(:skip_check) { true }
+
+ it { expect(skip).to eq(true) }
+ end
+
+ context 'when skip check is false' do
+ let(:skip_check) { false }
+
+ it { expect(skip).to eq(false) }
+ end
+ end
+
+ describe '#cacheable?' do
+ subject(:cacheable) { check_lfs_file_locks.cacheable? }
+
+ it { expect(cacheable).to eq(true) }
+ end
+
+ describe '#cache_key' do
+ subject(:cache_key) { check_lfs_file_locks.cache_key }
+
+ context 'when the feature flag is enabled' do
+ let(:id) { 'id' }
+ let(:sha) { 'sha' }
+ let(:epoch) { 'epoch' }
+ let(:expected_cache_key) { format(described_class::CACHE_KEY, id: id, sha: sha, epoch: epoch) }
+
+ before do
+ allow(merge_request).to receive(:id).and_return(id)
+ allow(merge_request).to receive(:diff_head_sha).and_return(sha)
+ allow(project).to receive(:lfs_file_locks_changed_epoch).and_return(epoch)
+ end
+
+ it { expect(cache_key).to eq(expected_cache_key) }
+ end
+
+ context 'when the feature flag is disabled' do
+ let(:feature_flag_enabled) { false }
+
+ it { expect(cache_key).to eq('inactive_lfs_file_locks_mergeability_check') }
+ end
+
+ context 'when lfs is disabled' do
+ let(:lfs_enabled) { false }
+
+ it { expect(cache_key).to eq('inactive_lfs_file_locks_mergeability_check') }
+ end
+ end
+end