Commit 0077d524 authored by charlie ablett's avatar charlie ablett

Clean up nil type logic for user namespace

- update tests
parent 444a076a
...@@ -120,12 +120,8 @@ class Namespace < ApplicationRecord ...@@ -120,12 +120,8 @@ class Namespace < ApplicationRecord
saved_change_to_name? || saved_change_to_path? || saved_change_to_parent_id? saved_change_to_name? || saved_change_to_path? || saved_change_to_parent_id?
} }
# TODO: change to `type: Namespaces::UserNamespace.sti_name` when scope :user_namespaces, -> { where(type: Namespaces::UserNamespace.sti_name) }
# working on issue https://gitlab.com/gitlab-org/gitlab/-/issues/341070 scope :without_project_namespaces, -> { where(Namespace.arel_table[:type].not_eq(Namespaces::ProjectNamespace.sti_name)) }
scope :user_namespaces, -> { where(type: [nil, Namespaces::UserNamespace.sti_name]) }
# TODO: this can be simplified with `type != 'Project'` when working on issue
# https://gitlab.com/gitlab-org/gitlab/-/issues/341070
scope :without_project_namespaces, -> { where(Namespace.arel_table[:type].is_distinct_from(Namespaces::ProjectNamespace.sti_name)) }
scope :sort_by_type, -> { order(Gitlab::Database.nulls_first_order(:type)) } scope :sort_by_type, -> { order(Gitlab::Database.nulls_first_order(:type)) }
scope :include_route, -> { includes(:route) } scope :include_route, -> { includes(:route) }
scope :by_parent, -> (parent) { where(parent_id: parent) } scope :by_parent, -> (parent) { where(parent_id: parent) }
......
...@@ -112,10 +112,8 @@ class User < ApplicationRecord ...@@ -112,10 +112,8 @@ class User < ApplicationRecord
# #
# Namespace for personal projects # Namespace for personal projects
# TODO: change to `:namespace, -> { where(type: Namespaces::UserNamespace.sti_name}, class_name: 'Namespaces::UserNamespace'...`
# when working on issue https://gitlab.com/gitlab-org/gitlab/-/issues/341070
has_one :namespace, has_one :namespace,
-> { where(type: [nil, Namespaces::UserNamespace.sti_name]) }, -> { where(type: Namespaces::UserNamespace.sti_name) },
dependent: :destroy, # rubocop:disable Cop/ActiveRecordDependent dependent: :destroy, # rubocop:disable Cop/ActiveRecordDependent
foreign_key: :owner_id, foreign_key: :owner_id,
inverse_of: :owner, inverse_of: :owner,
......
...@@ -474,20 +474,16 @@ module EE ...@@ -474,20 +474,16 @@ module EE
end end
end end
# TODO: change to `type: ::Namespaces::UserNamespace.sti_name` when
# working on issue https://gitlab.com/gitlab-org/gitlab/-/issues/341070
def namespace_union_for_owned(select = :id) def namespace_union_for_owned(select = :id)
::Gitlab::SQL::Union.new([ ::Gitlab::SQL::Union.new([
::Namespace.select(select).where(type: [nil, ::Namespaces::UserNamespace.sti_name], owner: self), ::Namespace.select(select).where(type: ::Namespaces::UserNamespace.sti_name, owner: self),
owned_groups.select(select).where(parent_id: nil) owned_groups.select(select).where(parent_id: nil)
]).to_sql ]).to_sql
end end
# TODO: change to `type: ::Namespaces::UserNamespace.sti_name` when
# working on issue https://gitlab.com/gitlab-org/gitlab/-/issues/341070
def namespace_union_for_reporter_developer_maintainer_owned(select = :id) def namespace_union_for_reporter_developer_maintainer_owned(select = :id)
::Gitlab::SQL::Union.new([ ::Gitlab::SQL::Union.new([
::Namespace.select(select).where(type: [nil, ::Namespaces::UserNamespace.sti_name], owner: self), ::Namespace.select(select).where(type: ::Namespaces::UserNamespace.sti_name, owner: self),
reporter_developer_maintainer_owned_groups.select(select).where(parent_id: nil) reporter_developer_maintainer_owned_groups.select(select).where(parent_id: nil)
]).to_sql ]).to_sql
end end
......
...@@ -1408,68 +1408,62 @@ RSpec.describe User do ...@@ -1408,68 +1408,62 @@ RSpec.describe User do
let_it_be(:free_group) { create(:group_with_plan, plan: :free_plan) } let_it_be(:free_group) { create(:group_with_plan, plan: :free_plan) }
let_it_be(:group_without_plan) { create(:group) } let_it_be(:group_without_plan) { create(:group) }
# TODO: this `where/when` can be removed in issue https://gitlab.com/gitlab-org/gitlab/-/issues/341070 let(:user) { create(:user, namespace: create(:user_namespace)) }
# At that point we only need to check `user_namespace`
where(namespace_type: [:namespace, :user_namespace])
with_them do describe '#has_paid_namespace?' do
let(:user) { create(:user, namespace: create(namespace_type)) } context 'when the user has Reporter or higher on at least one paid group' do
it 'returns true' do
describe '#has_paid_namespace?' do ultimate_group.add_reporter(user)
context 'when the user has Reporter or higher on at least one paid group' do bronze_group.add_guest(user)
it 'returns true' do
ultimate_group.add_reporter(user)
bronze_group.add_guest(user)
expect(user.has_paid_namespace?).to eq(true) expect(user.has_paid_namespace?).to eq(true)
end
end end
end
context 'when the user is only a Guest on paid groups' do context 'when the user is only a Guest on paid groups' do
it 'returns false' do it 'returns false' do
ultimate_group.add_guest(user) ultimate_group.add_guest(user)
bronze_group.add_guest(user) bronze_group.add_guest(user)
free_group.add_owner(user) free_group.add_owner(user)
expect(user.has_paid_namespace?).to eq(false) expect(user.has_paid_namespace?).to eq(false)
end
end end
end
context 'when the user is not a member of any groups with plans' do context 'when the user is not a member of any groups with plans' do
it 'returns false' do it 'returns false' do
group_without_plan.add_owner(user) group_without_plan.add_owner(user)
expect(user.has_paid_namespace?).to eq(false) expect(user.has_paid_namespace?).to eq(false)
end
end end
end end
end
describe '#owns_paid_namespace?', :saas do describe '#owns_paid_namespace?', :saas do
context 'when the user is an owner of at least one paid group' do context 'when the user is an owner of at least one paid group' do
it 'returns true' do it 'returns true' do
ultimate_group.add_owner(user) ultimate_group.add_owner(user)
bronze_group.add_owner(user) bronze_group.add_owner(user)
expect(user.owns_paid_namespace?).to eq(true) expect(user.owns_paid_namespace?).to eq(true)
end
end end
end
context 'when the user is only a Maintainer on paid groups' do context 'when the user is only a Maintainer on paid groups' do
it 'returns false' do it 'returns false' do
ultimate_group.add_maintainer(user) ultimate_group.add_maintainer(user)
bronze_group.add_maintainer(user) bronze_group.add_maintainer(user)
free_group.add_owner(user) free_group.add_owner(user)
expect(user.owns_paid_namespace?).to eq(false) expect(user.owns_paid_namespace?).to eq(false)
end
end end
end
context 'when the user is not a member of any groups with plans' do context 'when the user is not a member of any groups with plans' do
it 'returns false' do it 'returns false' do
group_without_plan.add_owner(user) group_without_plan.add_owner(user)
expect(user.owns_paid_namespace?).to eq(false) expect(user.owns_paid_namespace?).to eq(false)
end
end end
end end
end end
......
...@@ -2604,24 +2604,18 @@ RSpec.describe User do ...@@ -2604,24 +2604,18 @@ RSpec.describe User do
describe '.find_by_full_path' do describe '.find_by_full_path' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
# TODO: this `where/when` can be removed in issue https://gitlab.com/gitlab-org/gitlab/-/issues/341070 let!(:user) { create(:user, namespace: create(:user_namespace)) }
# At that point we only need to check `user_namespace`
where(namespace_type: [:namespace, :user_namespace])
with_them do context 'with a route matching the given path' do
let!(:user) { create(:user, namespace: create(namespace_type)) } let!(:route) { user.namespace.route }
context 'with a route matching the given path' do
let!(:route) { user.namespace.route }
it 'returns the user' do it 'returns the user' do
expect(described_class.find_by_full_path(route.path)).to eq(user) expect(described_class.find_by_full_path(route.path)).to eq(user)
end end
it 'is case-insensitive' do it 'is case-insensitive' do
expect(described_class.find_by_full_path(route.path.upcase)).to eq(user) expect(described_class.find_by_full_path(route.path.upcase)).to eq(user)
expect(described_class.find_by_full_path(route.path.downcase)).to eq(user) expect(described_class.find_by_full_path(route.path.downcase)).to eq(user)
end
end end
context 'with a redirect route matching the given path' do context 'with a redirect route matching the given path' do
......
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