Merge branch '341070-cablett-cleanup-user-namespace-code' into 'master'

Clean up nil type logic for user namespace

See merge request gitlab-org/gitlab!73152

app/models/namespace.rb

@@ -120,12 +120,8 @@ class Namespace < ApplicationRecord
       saved_change_to_name? || saved_change_to_path? || saved_change_to_parent_id?
   }
 
-  # TODO: change to `type: Namespaces::UserNamespace.sti_name` when
-  #       working on issue https://gitlab.com/gitlab-org/gitlab/-/issues/341070
-  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 :user_namespaces, -> { where(type: Namespaces::UserNamespace.sti_name) }
+  scope :without_project_namespaces, -> { where(Namespace.arel_table[:type].not_eq(Namespaces::ProjectNamespace.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) }

app/models/user.rb

@@ -112,10 +112,8 @@ class User < ApplicationRecord
   #
 
   # 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,
-          -> { where(type: [nil, Namespaces::UserNamespace.sti_name]) },
+          -> { where(type: Namespaces::UserNamespace.sti_name) },
           dependent: :destroy, # rubocop:disable Cop/ActiveRecordDependent
           foreign_key: :owner_id,
           inverse_of: :owner,

ee/app/models/ee/user.rb

@@ -476,20 +476,16 @@ module EE
       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)
       ::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)
       ]).to_sql
     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)
       ::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)
       ]).to_sql
     end

ee/spec/models/ee/user_spec.rb

@@ -1038,8 +1038,6 @@ RSpec.describe User do
 
     context 'when user is active' do
       context 'when user is internal' do
-        using RSpec::Parameterized::TableSyntax
-
         where(:internal_user_type) do
           described_class::INTERNAL_USER_TYPES
         end
@@ -1409,68 +1407,62 @@ RSpec.describe User do
     let_it_be(:free_group) { create(:group_with_plan, plan: :free_plan) }
     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
-    #       At that point we only need to check `user_namespace`
-    where(namespace_type: [:namespace, :user_namespace])
-
-    with_them do
-      let(:user) { create(:user, namespace: create(namespace_type)) }
+    let(:user) { create(:user, namespace: create(:user_namespace)) }
 
-      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)
+    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)
-          end
+          expect(user.has_paid_namespace?).to eq(true)
         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)
-          end
+          expect(user.has_paid_namespace?).to eq(false)
         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)
-          end
+          expect(user.has_paid_namespace?).to eq(false)
         end
       end
+    end
 
-      describe '#owns_paid_namespace?', :saas 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?', :saas 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)
-          end
+          expect(user.owns_paid_namespace?).to eq(true)
         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)
-          end
+          expect(user.owns_paid_namespace?).to eq(false)
         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)
-          end
+          expect(user.owns_paid_namespace?).to eq(false)
         end
       end
     end

spec/models/user_spec.rb

@@ -2602,26 +2602,18 @@ RSpec.describe User do
   end
 
   describe '.find_by_full_path' do
-    using RSpec::Parameterized::TableSyntax
+    let!(:user) { create(:user, namespace: create(:user_namespace)) }
 
-    # TODO: this `where/when` can be removed in issue https://gitlab.com/gitlab-org/gitlab/-/issues/341070
-    #       At that point we only need to check `user_namespace`
-    where(namespace_type: [:namespace, :user_namespace])
+    context 'with a route matching the given path' do
+      let!(:route) { user.namespace.route }
 
-    with_them do
-      let!(:user) { create(:user, namespace: create(namespace_type)) }
-
-      context 'with a route matching the given path' do
-        let!(:route) { user.namespace.route }
-
-        it 'returns the user' do
-          expect(described_class.find_by_full_path(route.path)).to eq(user)
-        end
+      it 'returns the user' do
+        expect(described_class.find_by_full_path(route.path)).to eq(user)
+      end
 
-        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
+      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
 
       context 'with a redirect route matching the given path' do