From 65908e5b462225e655a317c82b943a5fac0b10e4 Mon Sep 17 00:00:00 2001 From: Olaoluwa Oluro Date: Mon, 10 Mar 2025 14:21:38 +0000 Subject: [PATCH 1/2] Add `organization_id` code logic to `fork_networks` This adds code changes to support the introduction of `organization_id` in `fork_networks` Changelog: added --- app/models/fork_network.rb | 2 + app/services/projects/fork_service.rb | 12 ++++- .../projects/overwrite_project_service.rb | 1 + locale/gitlab.pot | 3 ++ spec/factories/fork_networks.rb | 4 ++ spec/requests/api/projects_spec.rb | 12 +++++ spec/services/projects/fork_service_spec.rb | 1 + .../overwrite_project_service_spec.rb | 45 +++++++++++++++++++ 8 files changed, 79 insertions(+), 1 deletion(-) diff --git a/app/models/fork_network.rb b/app/models/fork_network.rb index 0323a8d222a3b5..d40cba138ba235 100644 --- a/app/models/fork_network.rb +++ b/app/models/fork_network.rb @@ -2,6 +2,8 @@ class ForkNetwork < ApplicationRecord belongs_to :root_project, class_name: 'Project' + belongs_to :organization, class_name: 'Organizations::Organization', optional: true + has_many :fork_network_members has_many :projects, through: :fork_network_members diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index 23028e5a0c8385..4df355be773aed 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -42,6 +42,10 @@ def link_existing_project(fork_to_project) return ServiceResponse.error(message: _('Target project cannot be equal to source project'), reason: :self_fork) end + if fork_to_project.organization_id != fork_network.organization_id + return ServiceResponse.error(message: _('Target project must belong to source project organization'), reason: :fork_organization_mismatch) + end + build_fork_network_member(fork_to_project) if link_fork_network(fork_to_project) @@ -109,7 +113,13 @@ def allowed_fork? end def fork_network - @fork_network ||= @project.fork_network || @project.build_root_of_fork_network + @fork_network ||= @project.fork_network || build_fork_network + end + + def build_fork_network + @project.build_root_of_fork_network.tap do |fork_network| + fork_network.organization = @project.organization + end end def build_fork_network_member(fork_to_project) diff --git a/app/services/projects/overwrite_project_service.rb b/app/services/projects/overwrite_project_service.rb index 3a48e9d18f36bc..31f8c5b07eb1be 100644 --- a/app/services/projects/overwrite_project_service.rb +++ b/app/services/projects/overwrite_project_service.rb @@ -78,6 +78,7 @@ def rename_project(target_project, name, path) def add_source_project_to_fork_network(source_project) return if source_project == @project return unless fork_network + return if fork_network.organization_id != source_project.organization_id # Because they have moved all references in the fork network from the source_project # we won't be able to query the database (only through its cached data), diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 44f0b0f4581ea4..e23b013665b305 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -57533,6 +57533,9 @@ msgstr "" msgid "Target project cannot be equal to source project" msgstr "" +msgid "Target project must belong to source project organization" +msgstr "" + msgid "Target roles" msgstr "" diff --git a/spec/factories/fork_networks.rb b/spec/factories/fork_networks.rb index 12ea688bbce9d8..2d529d9e132bcf 100644 --- a/spec/factories/fork_networks.rb +++ b/spec/factories/fork_networks.rb @@ -3,5 +3,9 @@ FactoryBot.define do factory :fork_network do association :root_project, factory: :project + + before(:create) do |network| + network.organization_id = network.root_project.organization_id + end end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index b4e4b21a57b315..62885b03dde372 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -3553,6 +3553,18 @@ def failure_message(diff) expect(json_response['message']).to eq 'Target project cannot be equal to source project' end end + + context 'when fork target and source project organization are not the same' do + let_it_be(:organization) { create(:organization) } + let_it_be(:project_fork_target_different_organization) { create(:project, organization: organization) } + + it 'returns an error' do + post api("/projects/#{project_fork_target_different_organization.id}/fork/#{project_fork_source.id}", admin, admin_mode: true) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq 'Target project must belong to source project organization' + end + end end end diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 2fd07d3ea95201..6b1cf6029dc622 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -91,6 +91,7 @@ expect(fork_network).not_to be_nil expect(fork_network.root_project).to eq(project) expect(fork_network.projects).to contain_exactly(project, fork_of_project) + expect(fork_network.organization).to eq(project.organization) end it 'imports the repository of the forked project', :sidekiq_might_not_need_inline do diff --git a/spec/services/projects/overwrite_project_service_spec.rb b/spec/services/projects/overwrite_project_service_spec.rb index 99be630d6f6ef6..61ddbeb1ec0f1c 100644 --- a/spec/services/projects/overwrite_project_service_spec.rb +++ b/spec/services/projects/overwrite_project_service_spec.rb @@ -245,5 +245,50 @@ expect(project_from.fork_network_member).to be_nil end end + + context 'fork network membership behavior' do + shared_examples 'does not modify ForkNetworkMember' do + it 'does not add the source project to the fork network' do + expect { subject.execute(project_from) }.not_to change { + ForkNetworkMember.count + } + end + end + + context 'when fork network conditions are met' do + it 'adds the source project to the fork network' do + expect { subject.execute(project_from) }.to change { + ForkNetworkMember.count + }.by(1) + end + end + + context 'when source project is the same as target project' do + it_behaves_like 'does not modify ForkNetworkMember' do + let(:project_from) { project_to } + end + end + + context 'when fork_network does not exist' do + it_behaves_like 'does not modify ForkNetworkMember' do + before do + allow(subject).to receive(:fork_network).and_return(nil) + end + end + end + + context 'when organizations do not match' do + it_behaves_like 'does not modify ForkNetworkMember' do + before do + other_organization = create(:organization) + allow(project_from).to receive(:organization_id).and_return(other_organization.id) + + # we stub this out to ensure the flow continues as the it will get interrupted + # due to project.from now returning a different organization + allow(subject).to receive(:rename_project).and_return({ status: :success }) + end + end + end + end end end -- GitLab From ed3c8a2b0bad09be1da60684c6ae56b5dfbd1d6d Mon Sep 17 00:00:00 2001 From: Olaoluwa Oluro Date: Mon, 17 Mar 2025 16:23:35 +0000 Subject: [PATCH 2/2] Remove validations pre-backfill --- app/services/projects/fork_service.rb | 4 -- .../projects/overwrite_project_service.rb | 1 - locale/gitlab.pot | 3 -- spec/requests/api/projects_spec.rb | 12 ----- .../overwrite_project_service_spec.rb | 45 ------------------- 5 files changed, 65 deletions(-) diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index 4df355be773aed..913e029f481077 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -42,10 +42,6 @@ def link_existing_project(fork_to_project) return ServiceResponse.error(message: _('Target project cannot be equal to source project'), reason: :self_fork) end - if fork_to_project.organization_id != fork_network.organization_id - return ServiceResponse.error(message: _('Target project must belong to source project organization'), reason: :fork_organization_mismatch) - end - build_fork_network_member(fork_to_project) if link_fork_network(fork_to_project) diff --git a/app/services/projects/overwrite_project_service.rb b/app/services/projects/overwrite_project_service.rb index 31f8c5b07eb1be..3a48e9d18f36bc 100644 --- a/app/services/projects/overwrite_project_service.rb +++ b/app/services/projects/overwrite_project_service.rb @@ -78,7 +78,6 @@ def rename_project(target_project, name, path) def add_source_project_to_fork_network(source_project) return if source_project == @project return unless fork_network - return if fork_network.organization_id != source_project.organization_id # Because they have moved all references in the fork network from the source_project # we won't be able to query the database (only through its cached data), diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e23b013665b305..44f0b0f4581ea4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -57533,9 +57533,6 @@ msgstr "" msgid "Target project cannot be equal to source project" msgstr "" -msgid "Target project must belong to source project organization" -msgstr "" - msgid "Target roles" msgstr "" diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 62885b03dde372..b4e4b21a57b315 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -3553,18 +3553,6 @@ def failure_message(diff) expect(json_response['message']).to eq 'Target project cannot be equal to source project' end end - - context 'when fork target and source project organization are not the same' do - let_it_be(:organization) { create(:organization) } - let_it_be(:project_fork_target_different_organization) { create(:project, organization: organization) } - - it 'returns an error' do - post api("/projects/#{project_fork_target_different_organization.id}/fork/#{project_fork_source.id}", admin, admin_mode: true) - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq 'Target project must belong to source project organization' - end - end end end diff --git a/spec/services/projects/overwrite_project_service_spec.rb b/spec/services/projects/overwrite_project_service_spec.rb index 61ddbeb1ec0f1c..99be630d6f6ef6 100644 --- a/spec/services/projects/overwrite_project_service_spec.rb +++ b/spec/services/projects/overwrite_project_service_spec.rb @@ -245,50 +245,5 @@ expect(project_from.fork_network_member).to be_nil end end - - context 'fork network membership behavior' do - shared_examples 'does not modify ForkNetworkMember' do - it 'does not add the source project to the fork network' do - expect { subject.execute(project_from) }.not_to change { - ForkNetworkMember.count - } - end - end - - context 'when fork network conditions are met' do - it 'adds the source project to the fork network' do - expect { subject.execute(project_from) }.to change { - ForkNetworkMember.count - }.by(1) - end - end - - context 'when source project is the same as target project' do - it_behaves_like 'does not modify ForkNetworkMember' do - let(:project_from) { project_to } - end - end - - context 'when fork_network does not exist' do - it_behaves_like 'does not modify ForkNetworkMember' do - before do - allow(subject).to receive(:fork_network).and_return(nil) - end - end - end - - context 'when organizations do not match' do - it_behaves_like 'does not modify ForkNetworkMember' do - before do - other_organization = create(:organization) - allow(project_from).to receive(:organization_id).and_return(other_organization.id) - - # we stub this out to ensure the flow continues as the it will get interrupted - # due to project.from now returning a different organization - allow(subject).to receive(:rename_project).and_return({ status: :success }) - end - end - end - end end end -- GitLab