Commit ae595815 authored by Yorick Peterse's avatar Yorick Peterse
Browse files

Refactor checking personal project limits

This refactors the code used for checking if a user has exceeded the
personal projects limit. As part of this refactor the method has been
renamed from Project#check_limit to "check_personal_projects_limit", as
this name makes it much more clear what the purpose of the method is.
Standalone unit tests have also been added, as before we only had a
single generic validation test that did not cover all cases.

The old implementation of the refactored method also included a `rescue`
statement. This code would only run when a project creator was not set.
The error that would be added wasn't super useful, especially since
there would already be errors for the creator not being present. As none
of the other code in the "check_personal_projects_limit" raises, it has
been removed.
parent 28e3bde6d585
......@@ -331,7 +331,7 @@ class Project < ActiveRecord::Base
ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS },
enforce_user: true }, if: [:external_import?, :import_url_changed?]
validates :star_count, numericality: { greater_than_or_equal_to: 0 }
validate :check_limit, on: :create
validate :check_personal_projects_limit, on: :create
validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? }
validate :visibility_level_allowed_by_group, if: -> { changes.has_key?(:visibility_level) }
validate :visibility_level_allowed_as_fork, if: -> { changes.has_key?(:visibility_level) }
......@@ -809,18 +809,22 @@ def remote_mirror_available?
::Gitlab::CurrentSettings.mirror_available
end
def check_limit
unless creator.can_create_project? || namespace.kind == 'group'
projects_limit = creator.projects_limit
def check_personal_projects_limit
# Since this method is called as validation hook, `creator` might not be
# present. Since the validation for that will fail, we can just return
# early.
return if !creator || creator.can_create_project? ||
namespace.kind == 'group'
if projects_limit == 0
self.errors.add(:limit_reached, "Personal project creation is not allowed. Please contact your administrator with questions")
limit = creator.projects_limit
error =
if limit.zero?
_('Personal project creation is not allowed. Please contact your administrator with questions')
else
self.errors.add(:limit_reached, "Your project limit is #{projects_limit} projects! Please contact your administrator to increase it")
_('Your project limit is %{limit} projects! Please contact your administrator to increase it')
end
end
rescue
self.errors.add(:base, "Can't check your ability to create project")
self.errors.add(:limit_reached, error % { limit: limit })
end
def visibility_level_allowed_by_group
......
......@@ -4907,6 +4907,9 @@ msgstr ""
msgid "Personal Access Token"
msgstr ""
msgid "Personal project creation is not allowed. Please contact your administrator with questions"
msgstr ""
msgid "Pick a name"
msgstr ""
......@@ -8070,6 +8073,9 @@ msgstr ""
msgid "Your name"
msgstr ""
msgid "Your project limit is %{limit} projects! Please contact your administrator to increase it"
msgstr ""
msgid "Your projects"
msgstr ""
......
......@@ -209,9 +209,14 @@
it 'does not allow new projects beyond user limits' do
project2 = build(:project)
allow(project2).to receive(:creator).and_return(double(can_create_project?: false, projects_limit: 0).as_null_object)
allow(project2)
.to receive(:creator)
.and_return(
double(can_create_project?: false, projects_limit: 0).as_null_object
)
expect(project2).not_to be_valid
expect(project2.errors[:limit_reached].first).to match(/Personal project creation is not allowed/)
end
describe 'wiki path conflict' do
......@@ -4431,6 +4436,75 @@ def domain_variable
end
end
describe '#check_personal_projects_limit' do
context 'when creating a project for a group' do
it 'does nothing' do
creator = build(:user)
project = build(:project, namespace: build(:group), creator: creator)
allow(creator)
.to receive(:can_create_project?)
.and_return(false)
project.check_personal_projects_limit
expect(project.errors).to be_empty
end
end
context 'when the user is not allowed to create a personal project' do
let(:user) { build(:user) }
let(:project) { build(:project, creator: user) }
before do
allow(user)
.to receive(:can_create_project?)
.and_return(false)
end
context 'when the project limit is zero' do
it 'adds a validation error' do
allow(user)
.to receive(:projects_limit)
.and_return(0)
project.check_personal_projects_limit
expect(project.errors[:limit_reached].first)
.to match(/Personal project creation is not allowed/)
end
end
context 'when the project limit is greater than zero' do
it 'adds a validation error' do
allow(user)
.to receive(:projects_limit)
.and_return(5)
project.check_personal_projects_limit
expect(project.errors[:limit_reached].first)
.to match(/Your project limit is 5 projects/)
end
end
end
context 'when the user is allowed to create personal projects' do
it 'does nothing' do
user = build(:user)
project = build(:project, creator: user)
allow(user)
.to receive(:can_create_project?)
.and_return(true)
project.check_personal_projects_limit
expect(project.errors).to be_empty
end
end
end
def rugged_config
rugged_repo(project.repository).config
end
......
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