Skip to content

Commit

Permalink
Merge pull request opf#16234 from opf/feature/56331-require-explicit-…
Browse files Browse the repository at this point in the history
…type-selection

[#56331] Require explicit type selection on project change
  • Loading branch information
aaron-contreras committed Jul 25, 2024
2 parents 7429fc2 + 58dd83f commit cba2ef8
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 62 deletions.
6 changes: 2 additions & 4 deletions app/services/work_packages/set_attributes_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ def update_project_dependent_attributes
reassign_category
set_parent_to_nil

reassign_type unless work_package.type_id_changed?
assign_default_type unless work_package.type
end
end

Expand Down Expand Up @@ -452,11 +452,9 @@ def reassign_category
end
end

def reassign_type
def assign_default_type
available_types = work_package.project.types.order(:position)

return if available_types.include?(work_package.type) && work_package.type

work_package.type = available_types.first
update_duration
unify_milestone_dates
Expand Down
7 changes: 3 additions & 4 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -589,13 +589,12 @@ en:
unsupported_for_multiple_projects: "Bulk move/copy is not supported for work packages from multiple projects"
current_type_not_available_in_target_project: >
The current type of the work package is not enabled in the target project.
Please enable the type in the target project if you'd like them to remain unchanged.
Otherwise, the work package's type will be automatically re-assigned leading to potential data loss.
Please enable the type in the target project if you'd like it to remain unchanged.
Otherwise, select an available type in the target project from the list.
bulk_current_type_not_available_in_target_project: >
The current types of the work packages aren't enabled in the target project.
Please enable the types in the target project if you'd like them to remain unchanged.
Otherwise, the work packages' types will be automatically re-assigned leading to potential data loss.
Otherwise, select an available type in the target project from the list.
sharing:
missing_workflow_warning:
title: "Workflow missing for work package sharing"
Expand Down
37 changes: 10 additions & 27 deletions spec/features/work_packages/bulk/move_work_package_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,19 +126,22 @@
context "when the target project does not have the type" do
let!(:project2) { create(:project, name: "Target", types: [type2]) }

it "does moves the work package and changes the type", :with_cuprite do
it "does not move the work package", :with_cuprite do
click_on "Move and follow"
wait_for_reload
page.find(".inline-edit--container.subject", text: work_package.subject)
page.find_by_id("projects-menu", text: "Target")

expect(page)
.to have_css(".op-toast.-error",
text: I18n.t(:"work_packages.bulk.none_could_be_saved",
total: 1))

# Should NOT have moved
child_wp.reload
work_package.reload
expect(work_package.project_id).to eq(project2.id)
expect(work_package.type_id).to eq(type2.id)
expect(child_wp.project_id).to eq(project2.id)
expect(child_wp.type_id).to eq(type2.id)
expect(work_package.project_id).to eq(project.id)
expect(work_package.type_id).to eq(type.id)
expect(child_wp.project_id).to eq(project.id)
expect(child_wp.type_id).to eq(type.id)
end
end

Expand Down Expand Up @@ -168,26 +171,6 @@
expect(child_wp.project_id).to eq(project.id)
expect(child_wp.type_id).to eq(type.id)
end

it "does moves the work package when the required field is set" do
select "Risk", from: "Type"
fill_in required_cf.name, with: "1"

# Clicking move and follow might be broken due to the location.href
# in the refresh-on-form-changes component
retry_block do
click_on "Move and follow"
end

expect(page).to have_css(".op-toast.-success")

child_wp.reload
work_package.reload
expect(work_package.project_id).to eq(project2.id)
expect(work_package.type_id).to eq(type2.id)
expect(child_wp.project_id).to eq(project2.id)
expect(child_wp.type_id).to eq(type2.id)
end
end
end

Expand Down
35 changes: 13 additions & 22 deletions spec/services/work_packages/set_attributes_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2068,35 +2068,25 @@
end

context "for type" do
context "when current type exists in new project" do
it "leaves the type" do
subject

expect(work_package.type)
.to eql type
context "when the work package has a type already set" do
let(:work_package) do
build_stubbed(:work_package, project:, type: initial_type)
end
end

context "when a default type exists in new project" do
let(:new_types) { [other_type, default_type] }

it "uses the first type (by position)" do
it "leaves the type" do
subject

expect(work_package.type)
.to eql other_type
.to eql initial_type
end
end

it "adds change to system changes" do
subject

expect(work_package.changed_by_system["type_id"])
.to eql [initial_type.id, other_type.id]
context "when the work package has no type set" do
let(:work_package) do
build_stubbed(:work_package, project:, type: nil)
end
end

context "when no default type exists in new project" do
let(:new_types) { [other_type, yet_another_type] }
let(:new_types) { [other_type] }

it "uses the first type (by position)" do
subject
Expand All @@ -2109,11 +2099,12 @@
subject

expect(work_package.changed_by_system["type_id"])
.to eql [initial_type.id, other_type.id]
.to eql [nil, other_type.id]
end
end

context "when also setting a new type via attributes" do
context "and also setting a new type via attributes" do
let(:new_types) { [yet_another_type] }
let(:expected_attributes) { { project: new_project, type: yet_another_type } }

it "sets the desired type" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,12 +337,9 @@
context "with only non default types" do
let(:target_types) { [other_type] }

it "uses the first type" do
it "is unsuccessful" do
expect(subject)
.to be_success

expect(subject.result.type)
.to eql other_type
.to be_failure
end
end

Expand Down

0 comments on commit cba2ef8

Please sign in to comment.