Commit a908d570 authored by Jan Provaznik's avatar Jan Provaznik

Merge branch '337099-cablett-namespace-owner-validation' into 'master'

Refactor namespace owner validation logic

See merge request gitlab-org/gitlab!69201
parents 115b29aa a30da0a1
...@@ -57,7 +57,7 @@ class Namespace < ApplicationRecord ...@@ -57,7 +57,7 @@ class Namespace < ApplicationRecord
has_one :admin_note, inverse_of: :namespace has_one :admin_note, inverse_of: :namespace
accepts_nested_attributes_for :admin_note, update_only: true accepts_nested_attributes_for :admin_note, update_only: true
validates :owner, presence: true, if: ->(n) { n.type.nil? } validates :owner, presence: true, if: ->(n) { n.owner_required? }
validates :name, validates :name,
presence: true, presence: true,
length: { maximum: 255 } length: { maximum: 255 }
...@@ -266,6 +266,10 @@ class Namespace < ApplicationRecord ...@@ -266,6 +266,10 @@ class Namespace < ApplicationRecord
type.nil? || type == Namespaces::UserNamespace.sti_name || !(group? || project?) type.nil? || type == Namespaces::UserNamespace.sti_name || !(group? || project?)
end end
def owner_required?
user?
end
def find_fork_of(project) def find_fork_of(project)
return unless project.fork_network return unless project.fork_network
......
...@@ -4,6 +4,8 @@ module Namespaces ...@@ -4,6 +4,8 @@ module Namespaces
class ProjectNamespace < Namespace class ProjectNamespace < Namespace
has_one :project, foreign_key: :project_namespace_id, inverse_of: :project_namespace has_one :project, foreign_key: :project_namespace_id, inverse_of: :project_namespace
validates :project, presence: true
def self.sti_name def self.sti_name
'Project' 'Project'
end end
......
...@@ -344,6 +344,12 @@ RSpec.describe Namespace do ...@@ -344,6 +344,12 @@ RSpec.describe Namespace do
end end
end end
describe '#owner_required?' do
specify { expect(build(:project_namespace).owner_required?).to be_falsey }
specify { expect(build(:group).owner_required?).to be_falsey }
specify { expect(build(:namespace).owner_required?).to be_truthy }
end
describe '#visibility_level_field' do describe '#visibility_level_field' do
it { expect(namespace.visibility_level_field).to eq(:visibility_level) } it { expect(namespace.visibility_level_field).to eq(:visibility_level) }
end end
......
...@@ -7,6 +7,10 @@ RSpec.describe Namespaces::ProjectNamespace, type: :model do ...@@ -7,6 +7,10 @@ RSpec.describe Namespaces::ProjectNamespace, type: :model do
it { is_expected.to have_one(:project).with_foreign_key(:project_namespace_id).inverse_of(:project_namespace) } it { is_expected.to have_one(:project).with_foreign_key(:project_namespace_id).inverse_of(:project_namespace) }
end end
describe 'validations' do
it { is_expected.not_to validate_presence_of :owner }
end
context 'when deleting project namespace' do context 'when deleting project namespace' do
# using delete rather than destroy due to `delete` skipping AR hooks/callbacks # using delete rather than destroy due to `delete` skipping AR hooks/callbacks
# so it's ensured to work at the DB level. Uses ON DELETE CASCADE on foreign key # so it's ensured to work at the DB level. Uses ON DELETE CASCADE on foreign key
......
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