Commit 94cf9e7f authored by Yannis Roussos's avatar Yannis Roussos

Ensure that the lost-and-found group has a unique path

- Add generate_unique_path() to Namespace in
  the UpdateRoutesForLostAndFoundGroupAndOrphanedProjects migration
- Add tests that check for conflicting routes in the spec for
  UpdateRoutesForLostAndFoundGroupAndOrphanedProjects
parent 9f4d5a54
...@@ -83,6 +83,16 @@ class UpdateRoutesForLostAndFoundGroupAndOrphanedProjects < ActiveRecord::Migrat ...@@ -83,6 +83,16 @@ class UpdateRoutesForLostAndFoundGroupAndOrphanedProjects < ActiveRecord::Migrat
) )
end end
end end
def generate_unique_path
# Generate a unique path if there is no route for the namespace
# (an existing route guarantees that the path is already unique)
unless Route.for_source('Namespace', self.id)
self.path = Uniquify.new.string(self.path) do |str|
Route.where(path: str).exists?
end
end
end
end end
class Group < Namespace class Group < Namespace
...@@ -140,6 +150,7 @@ class UpdateRoutesForLostAndFoundGroupAndOrphanedProjects < ActiveRecord::Migrat ...@@ -140,6 +150,7 @@ class UpdateRoutesForLostAndFoundGroupAndOrphanedProjects < ActiveRecord::Migrat
return unless lost_and_found_group return unless lost_and_found_group
# Update the 'lost-and-found' group description to be more self-explanatory # Update the 'lost-and-found' group description to be more self-explanatory
lost_and_found_group.generate_unique_path
lost_and_found_group.description = lost_and_found_group.description =
'Group for storing projects that were not properly deleted. '\ 'Group for storing projects that were not properly deleted. '\
'It should be considered as a system level Group with non-working '\ 'It should be considered as a system level Group with non-working '\
......
...@@ -65,6 +65,79 @@ describe UpdateRoutesForLostAndFoundGroupAndOrphanedProjects, :migration do ...@@ -65,6 +65,79 @@ describe UpdateRoutesForLostAndFoundGroupAndOrphanedProjects, :migration do
created_at: Time.zone.now, created_at: Time.zone.now,
updated_at: Time.zone.now updated_at: Time.zone.now
) )
# Create another user named ghost which is not the Ghost User
# Also create a 'lost-and-found' group for them and add projects to it
# Purpose: test that the routes added for the 'lost-and-found' group and
# its projects are unique
fake_ghost_user = users.create!(
name: 'Ghost User',
username: 'ghost1',
email: 'ghost1@example.com',
user_type: nil,
projects_limit: 100,
state: :active,
bio: 'This is NOT a "Ghost User"'
)
fake_ghost_user_namespace = namespaces.create!(
name: 'Ghost User',
path: 'ghost1',
owner_id: fake_ghost_user.id,
visibility_level: 20
)
routes.create!(
source_id: fake_ghost_user_namespace.id,
source_type: 'Namespace',
path: 'Ghost User',
name: 'ghost1',
created_at: Time.zone.now,
updated_at: Time.zone.now
)
fake_lost_and_found_group = namespaces.create!(
name: 'Lost and Found',
path: described_class::User::LOST_AND_FOUND_GROUP, # same path as the lost-and-found group
type: 'Group',
description: 'Fake lost and found group with the same path as the real one',
visibility_level: 20
)
routes.create!(
source_id: fake_lost_and_found_group.id,
source_type: 'Namespace',
path: described_class::User::LOST_AND_FOUND_GROUP, # same path as the lost-and-found group
name: 'Lost and Found',
created_at: Time.zone.now,
updated_at: Time.zone.now
)
members.create!(
type: 'GroupMember',
source_id: fake_lost_and_found_group.id,
user_id: fake_ghost_user.id,
source_type: 'Namespace',
access_level: described_class::User::ACCESS_LEVEL_OWNER,
notification_level: 3
)
normal_project = projects.create!(
name: 'normal_project',
path: 'normal_project',
visibility_level: 20,
archived: false,
namespace_id: fake_lost_and_found_group.id
)
routes.create!(
source_id: normal_project.id,
source_type: 'Project',
path: "#{described_class::User::LOST_AND_FOUND_GROUP}/normal_project",
name: 'Lost and Found / normal_project',
created_at: Time.zone.now,
updated_at: Time.zone.now
)
end end
it 'creates the route for the ghost user namespace' do it 'creates the route for the ghost user namespace' do
...@@ -75,12 +148,25 @@ describe UpdateRoutesForLostAndFoundGroupAndOrphanedProjects, :migration do ...@@ -75,12 +148,25 @@ describe UpdateRoutesForLostAndFoundGroupAndOrphanedProjects, :migration do
expect(routes.where(path: 'ghost').count).to eq(1) expect(routes.where(path: 'ghost').count).to eq(1)
end end
it 'fixes the path for the lost-and-found group by generating a unique one' do
expect(namespaces.where(path: described_class::User::LOST_AND_FOUND_GROUP).count).to eq(2)
disable_migrations_output { migrate! }
expect(namespaces.where(path: described_class::User::LOST_AND_FOUND_GROUP).count).to eq(1)
lost_and_found_group = namespaces.find_by(name: described_class::User::LOST_AND_FOUND_GROUP)
expect(lost_and_found_group.path).to eq('lost-and-found1')
end
it 'creates the route for the lost-and-found group' do it 'creates the route for the lost-and-found group' do
expect(routes.where(path: described_class::User::LOST_AND_FOUND_GROUP).count).to eq(0) expect(routes.where(path: described_class::User::LOST_AND_FOUND_GROUP).count).to eq(1)
expect(routes.where(path: 'lost-and-found1').count).to eq(0)
disable_migrations_output { migrate! } disable_migrations_output { migrate! }
expect(routes.where(path: described_class::User::LOST_AND_FOUND_GROUP).count).to eq(1) expect(routes.where(path: described_class::User::LOST_AND_FOUND_GROUP).count).to eq(1)
expect(routes.where(path: 'lost-and-found1').count).to eq(1)
end end
it 'updates the route for the orphaned project' do it 'updates the route for the orphaned project' do
...@@ -90,7 +176,7 @@ describe UpdateRoutesForLostAndFoundGroupAndOrphanedProjects, :migration do ...@@ -90,7 +176,7 @@ describe UpdateRoutesForLostAndFoundGroupAndOrphanedProjects, :migration do
disable_migrations_output { migrate! } disable_migrations_output { migrate! }
updated_route = routes.find_by(id: orphaned_project_route.id) updated_route = routes.find_by(id: orphaned_project_route.id)
expect(updated_route.path).to eq("#{described_class::User::LOST_AND_FOUND_GROUP}/orphaned_project") expect(updated_route.path).to eq('lost-and-found1/orphaned_project')
expect(updated_route.name).to eq("#{described_class::User::LOST_AND_FOUND_GROUP} / orphaned_project") expect(updated_route.name).to eq("#{described_class::User::LOST_AND_FOUND_GROUP} / orphaned_project")
end end
end end
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