Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/contracts/work_packages/copy_project_contract.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ class CopyProjectContract < CopyContract
delegate :to_s,
to: :model

def validate_model? = false

private

def validate_version_is_assignable; end
Expand Down
83 changes: 55 additions & 28 deletions spec/workers/copy_project_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,38 +31,31 @@
require "spec_helper"

RSpec.describe CopyProjectJob, type: :model, with_good_job_batches: [CopyProjectJob, SendCopyProjectStatusEmailJob] do
shared_let(:admin) { create(:admin) }
shared_let(:user_de) { create(:admin, language: :de) }
let(:params) { { name: "Copy", identifier: "copy" } }
let(:user_de) { create(:admin, language: :de) }
let(:mail_double) { double("Mail::Message", deliver: true) } # rubocop:disable RSpec/VerifiedDoubles

before { allow(mail_double).to receive(:deliver_later) }

describe "copy project succeeds with errors" do
let(:admin) { create(:admin) }
let(:source_project) { create(:project, types: [type]) }

let!(:work_package) { create(:work_package, project: source_project, type:) }
let!(:work_package) { create(:work_package, :skip_validations, done_ratio: 101, project: source_project, type:) }

let(:type) { create(:type_bug) }
let(:custom_field) do
create(:work_package_custom_field, name: "required_field", field_format: "text", is_required: true, is_for_all: true)
end

let(:job_args) do
{
target_project_params: params,
associations_to_copy: [:work_packages]
}
end

let(:params) { { name: "Copy", identifier: "copy", type_ids: [type.id], work_package_custom_field_ids: [custom_field.id] } }
let(:params) { { name: "Copy", identifier: "copy", type_ids: [type.id] } }
let(:expected_error_message) do
"#{WorkPackage.model_name.human} '#{work_package.type.name} ##{work_package.id}: #{work_package.subject}': #{custom_field.name} #{I18n.t('errors.messages.blank')}."
end

before do
source_project.work_package_custom_fields << custom_field
type.custom_fields << custom_field
"#{WorkPackage.model_name.human} '#{work_package.type.name} ##{work_package.id}: " \
"#{work_package.subject}': % Complete " \
"#{I18n.t('activerecord.errors.models.work_package.attributes.done_ratio.inclusion')}"
end

it "copies the project", :aggregate_failures do
Expand Down Expand Up @@ -94,13 +87,59 @@
GoodJob.perform_inline
batch.reload

msg = /Arbeitspaket 'Bug #\d+: WorkPackage No. \d+': required_field muss ausgefüllt werden\./
msg = /Arbeitspaket 'Bug #\d+: WorkPackage No. \d+': % abgeschlossen muss zwischen 0 und 100 liegen\./
expect(batch.properties[:errors].first).to match(msg)
end
end

describe "copy project succeeds with invalid custom fields" do
let(:source_project) { create(:project, types: [type]) }

let!(:work_package) { create(:work_package, project: source_project, type:) }

let(:type) { create(:type_bug) }
let(:custom_field) do
create(:work_package_custom_field, name: "required_field", field_format: "text", is_required: true, is_for_all: true)
end

let(:job_args) do
{
target_project_params: params,
associations_to_copy: [:work_packages]
}
end

let(:params) { { name: "Copy", identifier: "copy", type_ids: [type.id], work_package_custom_field_ids: [custom_field.id] } }

before do
source_project.work_package_custom_fields << custom_field
type.custom_fields << custom_field
end

it "copies the project", :aggregate_failures do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a check that work package copying failed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The work package copying should succeed even if the work package is invalid.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The work package, not project

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The work package also.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understood that when copying a project and one of the work packages fails to be copied, it does copy the project, doesn't copy problematic work packages and reports errors

Copy link
Contributor Author

@dombesz dombesz Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is true only in certain cases, ie. if the validation fails on the work package contract, then the error is reported and the problematic work package is not copied.
However the validations on the work package model itself are skipped (due to the def validate_model? = false). Since the custom fields are validated on the model level, they are skipped too, thus the work package copying will succeed.

copy_job = nil
batch = GoodJob::Batch.enqueue(user: admin, source_project:) do
copy_job = described_class.perform_later(**job_args)
end
GoodJob.perform_inline
batch.reload

copied_project = Project.find_by(identifier: params[:identifier])

expect(copied_project).to eq(batch.properties[:target_project])
expect(batch.properties[:errors]).to be_empty

# expect to create a status
expect(copy_job.job_status).to be_present
expect(copy_job.job_status[:status]).to eq "success"
expect(copy_job.job_status[:payload]["redirect"]).to include "/projects/copy"

expected_link = { "href" => "/api/v3/projects/#{copied_project.id}", "title" => copied_project.name }
expect(copy_job.job_status[:payload]["_links"]["project"]).to eq(expected_link)
end
end

describe "project has an invalid repository" do
let(:admin) { create(:admin) }
let(:source_project) do
project = create(:project)

Expand Down Expand Up @@ -136,9 +175,7 @@
end

describe "copy project fails with internal error" do
let(:admin) { create(:admin) }
let(:source_project) { create(:project) }
let(:params) { { name: "Copy", identifier: "copy" } }

before do
allow(User).to receive(:current).and_return(admin)
Expand Down Expand Up @@ -174,9 +211,6 @@
set_factory_default(:project_with_types, source_project)
end

let(:admin) { create(:admin) }
let(:params) { { name: "Copy", identifier: "copy" } }

let_work_packages(<<~TABLE)
hierarchy | work | remaining work | start date | end date | scheduling mode
parent | 1h | 0h | 2024-01-23 | 2024-01-26 | automatic
Expand Down Expand Up @@ -235,7 +269,6 @@
end

describe "subproject" do
let(:params) { { name: "Copy", identifier: "copy" } }
let(:subproject) do
create(:project, parent: project).tap do |p|
create(:member, principal: user, roles: [role], project: p)
Expand Down Expand Up @@ -328,9 +361,6 @@
set_factory_default(:project_with_types, source_project)
end

let(:admin) { create(:admin) }
let(:params) { { name: "Copy", identifier: "copy" } }

let_work_packages(<<~TABLE)
hierarchy | start date | end date | scheduling mode
automatic_parent | 2024-01-23 | 2024-01-26 | automatic
Expand Down Expand Up @@ -362,13 +392,10 @@
# do it.
describe "sending notifications" do
shared_let(:project) { create(:project) }
shared_let(:admin) { create(:admin) }
shared_let(:user) { create(:user) }
shared_let(:roles) { [create(:project_role)] }
shared_let(:member) { create(:member, user:, project:, roles:) }

let(:params) { { name: "Copy", identifier: "copy" } }

def perform_the_job
batch = GoodJob::Batch.enqueue(user: admin, source_project: project) do
described_class.perform_later(target_project_params: params, associations_to_copy: [:members])
Expand Down
Loading