Skip to content
Snippets Groups Projects
Commit e358cac8 authored by Brett Walker's avatar Brett Walker
Browse files

Allow for Namespace type to be either nil or User

This prepares for supporting records that have both
type == nil and type == User for namespaces.
parent fe535661
No related branches found
No related tags found
1 merge request!68894Ensure code can handle both UserNamespace and type == nil namespaces
......@@ -101,7 +101,7 @@ class Namespace < ApplicationRecord
saved_change_to_name? || saved_change_to_path? || saved_change_to_parent_id?
}
scope :for_user, -> { where(type: nil) }
scope :user_namespaces, -> { where(type: [nil, Namespaces::UserNamespace.sti_name]) }
scope :sort_by_type, -> { order(Gitlab::Database.nulls_first_order(:type)) }
scope :include_route, -> { includes(:route) }
scope :by_parent, -> (parent) { where(parent_id: parent) }
......@@ -143,9 +143,7 @@ def sti_class_for(type_name)
when Namespaces::ProjectNamespace.sti_name
Namespaces::ProjectNamespace
when Namespaces::UserNamespace.sti_name
# TODO: We create a normal Namespace until
# https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68894 is ready
Namespace
Namespaces::UserNamespace
else
Namespace
end
......
......@@ -112,7 +112,7 @@ def update_tracked_fields!(request)
#
# Namespace for personal projects
has_one :namespace, -> { where(type: nil) }, dependent: :destroy, foreign_key: :owner_id, inverse_of: :owner, autosave: true # rubocop:disable Cop/ActiveRecordDependent
has_one :namespace, -> { where(type: [nil, Namespaces::UserNamespace.sti_name]) }, dependent: :destroy, foreign_key: :owner_id, inverse_of: :owner, autosave: true # rubocop:disable Cop/ActiveRecordDependent
# Profile
has_many :keys, -> { regular_keys }, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
......@@ -728,7 +728,7 @@ def find_by_ssh_key_id(key_id)
end
def find_by_full_path(path, follow_redirects: false)
namespace = Namespace.for_user.find_by_full_path(path, follow_redirects: follow_redirects)
namespace = Namespace.user_namespaces.find_by_full_path(path, follow_redirects: follow_redirects)
namespace&.owner
end
......
......@@ -454,14 +454,14 @@ def requires_credit_card?(project)
def namespace_union_for_owned(select = :id)
::Gitlab::SQL::Union.new([
::Namespace.select(select).where(type: nil, owner: self),
::Namespace.select(select).where(type: [nil, ::Namespaces::UserNamespace.sti_name], owner: self),
owned_groups.select(select).where(parent_id: nil)
]).to_sql
end
def namespace_union_for_reporter_developer_maintainer_owned(select = :id)
::Gitlab::SQL::Union.new([
::Namespace.select(select).where(type: nil, owner: self),
::Namespace.select(select).where(type: [nil, ::Namespaces::UserNamespace.sti_name], owner: self),
reporter_developer_maintainer_owned_groups.select(select).where(parent_id: nil)
]).to_sql
end
......
......@@ -5,7 +5,9 @@
RSpec.describe 'Project Insights' do
it_behaves_like 'Insights page' do
let_it_be(:entity) { create(:project) }
let(:route) { url_for([entity.namespace, entity, :insights]) }
let(:path) { project_insights_path(entity) }
let(:route) do
project_insights_url(entity)
end
end
end
......@@ -1336,66 +1336,73 @@
end
context 'paid namespaces' do
let_it_be(:user) { create(:user) }
using RSpec::Parameterized::TableSyntax
let_it_be(:ultimate_group) { create(:group_with_plan, plan: :ultimate_plan) }
let_it_be(:bronze_group) { create(:group_with_plan, plan: :bronze_plan) }
let_it_be(:free_group) { create(:group_with_plan, plan: :free_plan) }
let_it_be(:group_without_plan) { create(:group) }
describe '#has_paid_namespace?' do
context 'when the user has Reporter or higher on at least one paid group' do
it 'returns true' do
ultimate_group.add_reporter(user)
bronze_group.add_guest(user)
where(namespace_type: [:namespace, :user_namespace])
with_them do
let(:user) { create(:user, namespace: create(namespace_type)) }
describe '#has_paid_namespace?' do
context 'when the user has Reporter or higher on at least one paid group' do
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
context 'when the user is only a Guest on paid groups' do
it 'returns false' do
ultimate_group.add_guest(user)
bronze_group.add_guest(user)
free_group.add_owner(user)
context 'when the user is only a Guest on paid groups' do
it 'returns false' do
ultimate_group.add_guest(user)
bronze_group.add_guest(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
context 'when the user is not a member of any groups with plans' do
it 'returns false' do
group_without_plan.add_owner(user)
context 'when the user is not a member of any groups with plans' do
it 'returns false' do
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
describe '#owns_paid_namespace?' do
context 'when the user is an owner of at least one paid group' do
it 'returns true' do
ultimate_group.add_owner(user)
bronze_group.add_owner(user)
describe '#owns_paid_namespace?' do
context 'when the user is an owner of at least one paid group' do
it 'returns true' do
ultimate_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
context 'when the user is only a Maintainer on paid groups' do
it 'returns false' do
ultimate_group.add_maintainer(user)
bronze_group.add_maintainer(user)
free_group.add_owner(user)
context 'when the user is only a Maintainer on paid groups' do
it 'returns false' do
ultimate_group.add_maintainer(user)
bronze_group.add_maintainer(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
context 'when the user is not a member of any groups with plans' do
it 'returns false' do
group_without_plan.add_owner(user)
context 'when the user is not a member of any groups with plans' do
it 'returns false' do
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
......
# frozen_string_literal: true
FactoryBot.define do
factory :user_namespace, class: 'Namespaces::UserNamespace', parent: :namespace do
sequence(:name) { |n| "user_namespace#{n}" }
type { Namespaces::UserNamespace.sti_name }
end
end
......@@ -200,9 +200,7 @@
let(:namespace_type) { user_sti_name }
it 'is valid' do
# TODO: We create a normal Namespace until
# https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68894 is ready
expect(Namespace.find(namespace.id)).to be_a(Namespace)
expect(Namespace.find(namespace.id)).to be_a(Namespaces::UserNamespace)
expect(namespace.kind).to eq('user')
expect(namespace.user_namespace?).to be_truthy
end
......@@ -221,7 +219,7 @@
context 'creating an unknown Namespace type' do
let(:namespace_type) { 'One' }
it 'defaults to a Namespace' do
it 'creates a default Namespace' do
expect(Namespace.find(namespace.id)).to be_a(Namespace)
expect(namespace.kind).to eq('user')
expect(namespace.user_namespace?).to be_truthy
......
# frozen_string_literal: true
require 'spec_helper'
# Main user namespace functionality it still in `Namespace`, so most
# of the specs are in `namespace_spec.rb`.
# UserNamespace specific specs will end up being migrated here.
RSpec.describe Namespaces::UserNamespace, type: :model do
describe 'validations' do
it { is_expected.to validate_presence_of(:owner) }
end
end
......@@ -2542,71 +2542,77 @@
end
describe '.find_by_full_path' do
let!(:user) { create(:user) }
using RSpec::Parameterized::TableSyntax
context 'with a route matching the given path' do
let!(:route) { user.namespace.route }
where(namespace_type: [:namespace, :user_namespace])
it 'returns the user' do
expect(described_class.find_by_full_path(route.path)).to eq(user)
end
with_them do
let!(:user) { create(:user, namespace: create(namespace_type)) }
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.downcase)).to eq(user)
end
end
context 'with a route matching the given path' do
let!(:route) { user.namespace.route }
context 'with a redirect route matching the given path' do
let!(:redirect_route) { user.namespace.redirect_routes.create!(path: 'foo') }
it 'returns the user' do
expect(described_class.find_by_full_path(route.path)).to eq(user)
end
context 'without the follow_redirects option' do
it 'returns nil' do
expect(described_class.find_by_full_path(redirect_route.path)).to eq(nil)
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.downcase)).to eq(user)
end
end
context 'with the follow_redirects option set to true' do
it 'returns the user' do
expect(described_class.find_by_full_path(redirect_route.path, follow_redirects: true)).to eq(user)
context 'with a redirect route matching the given path' do
let!(:redirect_route) { user.namespace.redirect_routes.create!(path: 'foo') }
context 'without the follow_redirects option' do
it 'returns nil' do
expect(described_class.find_by_full_path(redirect_route.path)).to eq(nil)
end
end
it 'is case-insensitive' do
expect(described_class.find_by_full_path(redirect_route.path.upcase, follow_redirects: true)).to eq(user)
expect(described_class.find_by_full_path(redirect_route.path.downcase, follow_redirects: true)).to eq(user)
context 'with the follow_redirects option set to true' do
it 'returns the user' do
expect(described_class.find_by_full_path(redirect_route.path, follow_redirects: true)).to eq(user)
end
it 'is case-insensitive' do
expect(described_class.find_by_full_path(redirect_route.path.upcase, follow_redirects: true)).to eq(user)
expect(described_class.find_by_full_path(redirect_route.path.downcase, follow_redirects: true)).to eq(user)
end
end
end
end
context 'without a route or a redirect route matching the given path' do
context 'without the follow_redirects option' do
it 'returns nil' do
expect(described_class.find_by_full_path('unknown')).to eq(nil)
context 'without a route or a redirect route matching the given path' do
context 'without the follow_redirects option' do
it 'returns nil' do
expect(described_class.find_by_full_path('unknown')).to eq(nil)
end
end
end
context 'with the follow_redirects option set to true' do
it 'returns nil' do
expect(described_class.find_by_full_path('unknown', follow_redirects: true)).to eq(nil)
context 'with the follow_redirects option set to true' do
it 'returns nil' do
expect(described_class.find_by_full_path('unknown', follow_redirects: true)).to eq(nil)
end
end
end
end
context 'with a group route matching the given path' do
let!(:group) { create(:group, path: 'group_path') }
context 'with a group route matching the given path' do
let!(:group) { create(:group, path: 'group_path') }
context 'when the group namespace has an owner_id (legacy data)' do
before do
group.update!(owner_id: user.id)
end
context 'when the group namespace has an owner_id (legacy data)' do
before do
group.update!(owner_id: user.id)
end
it 'returns nil' do
expect(described_class.find_by_full_path('group_path')).to eq(nil)
it 'returns nil' do
expect(described_class.find_by_full_path('group_path')).to eq(nil)
end
end
end
context 'when the group namespace does not have an owner_id' do
it 'returns nil' do
expect(described_class.find_by_full_path('group_path')).to eq(nil)
context 'when the group namespace does not have an owner_id' do
it 'returns nil' do
expect(described_class.find_by_full_path('group_path')).to eq(nil)
end
end
end
end
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment