Commit 578c7df7 authored by Peter Leitzen's avatar Peter Leitzen

Merge branch 'fj-change-max-file-count-feature-flag' into 'master'

Update snippet file count limit depending on Feature Flag

See merge request gitlab-org/gitlab!32416
parents d2dcd975 e3027502
...@@ -18,7 +18,8 @@ class Snippet < ApplicationRecord ...@@ -18,7 +18,8 @@ class Snippet < ApplicationRecord
include AfterCommitQueue include AfterCommitQueue
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
MAX_FILE_COUNT = 1 MAX_FILE_COUNT = 10
MAX_SINGLE_FILE_COUNT = 1
cache_markdown_field :title, pipeline: :single_line cache_markdown_field :title, pipeline: :single_line
cache_markdown_field :description cache_markdown_field :description
...@@ -169,6 +170,10 @@ class Snippet < ApplicationRecord ...@@ -169,6 +170,10 @@ class Snippet < ApplicationRecord
Snippet.find_by(id: id, project: project) Snippet.find_by(id: id, project: project)
end end
def self.max_file_limit(user)
Feature.enabled?(:snippet_multiple_files, user) ? MAX_FILE_COUNT : MAX_SINGLE_FILE_COUNT
end
def initialize(attributes = {}) def initialize(attributes = {})
# We can't use default_value_for because the database has a default # We can't use default_value_for because the database has a default
# value of 0 for visibility_level. If someone attempts to create a # value of 0 for visibility_level. If someone attempts to create a
......
...@@ -106,7 +106,7 @@ module Gitlab ...@@ -106,7 +106,7 @@ module Gitlab
def check_single_change_access(change) def check_single_change_access(change)
Checks::SnippetCheck.new(change, logger: logger).validate! Checks::SnippetCheck.new(change, logger: logger).validate!
Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet::MAX_FILE_COUNT, logger: logger).validate! Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet.max_file_limit(user), logger: logger).validate!
rescue Checks::TimedLogger::TimeoutError rescue Checks::TimedLogger::TimeoutError
raise TimeoutError, logger.full_message raise TimeoutError, logger.full_message
end end
......
...@@ -8,7 +8,7 @@ describe Gitlab::Checks::PushFileCountCheck do ...@@ -8,7 +8,7 @@ describe Gitlab::Checks::PushFileCountCheck do
let(:timeout) { Gitlab::GitAccess::INTERNAL_TIMEOUT } let(:timeout) { Gitlab::GitAccess::INTERNAL_TIMEOUT }
let(:logger) { Gitlab::Checks::TimedLogger.new(timeout: timeout) } let(:logger) { Gitlab::Checks::TimedLogger.new(timeout: timeout) }
subject { described_class.new(changes, repository: snippet.repository, limit: 1, logger: logger) } subject { described_class.new(changes, repository: snippet.repository, limit: 2, logger: logger) }
describe '#validate!' do describe '#validate!' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
...@@ -31,7 +31,7 @@ describe Gitlab::Checks::PushFileCountCheck do ...@@ -31,7 +31,7 @@ describe Gitlab::Checks::PushFileCountCheck do
where(:old, :new, :valid, :message) do where(:old, :new, :valid, :message) do
'single-file' | 'edit-file' | true | nil 'single-file' | 'edit-file' | true | nil
'single-file' | 'multiple-files' | false | 'The repository can contain at most 1 file(s).' 'single-file' | 'multiple-files' | false | 'The repository can contain at most 2 file(s).'
'single-file' | 'no-files' | false | 'The repository must contain at least 1 file.' 'single-file' | 'no-files' | false | 'The repository must contain at least 1 file.'
'edit-file' | 'rename-and-edit-file' | true | nil 'edit-file' | 'rename-and-edit-file' | true | nil
end end
......
...@@ -278,6 +278,16 @@ describe Gitlab::GitAccessSnippet do ...@@ -278,6 +278,16 @@ describe Gitlab::GitAccessSnippet do
expect { push_access_check }.to raise_forbidden('foo') expect { push_access_check }.to raise_forbidden('foo')
end end
it 'sets the file count limit from Snippet class' do
service = double
expect(service).to receive(:validate!).and_return(nil)
expect(Snippet).to receive(:max_file_limit).with(user).and_return(5)
expect(Gitlab::Checks::PushFileCountCheck).to receive(:new).with(anything, hash_including(limit: 5)).and_return(service)
push_access_check
end
end end
it_behaves_like 'snippet checks' it_behaves_like 'snippet checks'
......
...@@ -734,4 +734,20 @@ describe Snippet do ...@@ -734,4 +734,20 @@ describe Snippet do
it { is_expected.to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "#{snippet.project.full_path}/snippets/#{snippet.id}.git") } it { is_expected.to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "#{snippet.project.full_path}/snippets/#{snippet.id}.git") }
end end
end end
describe '.max_file_limit' do
subject { described_class.max_file_limit(nil) }
it "returns #{Snippet::MAX_FILE_COUNT}" do
expect(subject).to eq Snippet::MAX_FILE_COUNT
end
context 'when feature flag :snippet_multiple_files is disabled' do
it "returns #{described_class::MAX_SINGLE_FILE_COUNT}" do
stub_feature_flags(snippet_multiple_files: false)
expect(subject).to eq described_class::MAX_SINGLE_FILE_COUNT
end
end
end
end end
...@@ -59,7 +59,7 @@ module TestEnv ...@@ -59,7 +59,7 @@ module TestEnv
'merge-commit-analyze-side-branch' => '8a99451', 'merge-commit-analyze-side-branch' => '8a99451',
'merge-commit-analyze-after' => '646ece5', 'merge-commit-analyze-after' => '646ece5',
'snippet/single-file' => '43e4080aaa14fc7d4b77ee1f5c9d067d5a7df10e', 'snippet/single-file' => '43e4080aaa14fc7d4b77ee1f5c9d067d5a7df10e',
'snippet/multiple-files' => 'b80faa8c5b2b62f6489a0d84755580e927e1189b', 'snippet/multiple-files' => '40232f7eb98b3f221886432def6e8bab2432add9',
'snippet/rename-and-edit-file' => '220a1e4b4dff37feea0625a7947a4c60fbe78365', 'snippet/rename-and-edit-file' => '220a1e4b4dff37feea0625a7947a4c60fbe78365',
'snippet/edit-file' => 'c2f074f4f26929c92795a75775af79a6ed6d8430', 'snippet/edit-file' => 'c2f074f4f26929c92795a75775af79a6ed6d8430',
'snippet/no-files' => '671aaa842a4875e5f30082d1ab6feda345fdb94d', 'snippet/no-files' => '671aaa842a4875e5f30082d1ab6feda345fdb94d',
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment