From 2d9a6f2bd3b4ac55d5e59c8e9eb2013fa46798d9 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Tue, 15 Jan 2019 16:13:28 +0100
Subject: [PATCH] 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.
---
app/models/project.rb | 24 +++++++-----
locale/gitlab.pot | 6 +++
spec/models/project_spec.rb | 78 ++++++++++++++++++++++++++++++++++++-
3 files changed, 96 insertions(+), 12 deletions(-)
diff --git a/app/models/project.rb b/app/models/project.rb
index 27be16720b5..9d67a6ec5e3 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -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 @@ class Project < ActiveRecord::Base
::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
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 7155c39b35d..45f09b031d8 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -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 ""
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 397b4d7c61f..0c5930b7b00 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -209,9 +209,14 @@ describe Project do
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 @@ describe Project do
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
--
2.30.9