Commit 985092a5 authored by Mike Long's avatar Mike Long Committed by charlie ablett

Improve error messages when adding a child epic and it fails

parent 3c0e2047
...@@ -353,15 +353,15 @@ module EE ...@@ -353,15 +353,15 @@ module EE
preloaded_parent_group_and_descendants ||= parent.group.self_and_descendants preloaded_parent_group_and_descendants ||= parent.group.self_and_descendants
if self == parent if self == parent
errors.add :parent, 'Cannot add an epic as a child of itself' errors.add :parent, "This epic cannot be added. An epic cannot be added to itself."
elsif parent.children.to_a.include?(self) elsif parent.children.to_a.include?(self)
errors.add :parent, "This epic can't be added as it is already assigned to the parent" errors.add :parent, "This epic cannot be added. It is already assigned to the parent epic."
elsif parent.has_ancestor?(self) elsif parent.has_ancestor?(self)
errors.add :parent, "This epic can't be added as it is already assigned to this epic's ancestor" errors.add :parent, "This epic cannot be added. It is already an ancestor of the parent epic."
elsif !preloaded_parent_group_and_descendants.include?(group) elsif !preloaded_parent_group_and_descendants.include?(group)
errors.add :parent, "This epic can't be added because it must belong to the same group as the parent, or subgroup of the parent epic’s group" errors.add :parent, "This epic cannot be added. An epic must belong to the same group or subgroup as its parent epic."
elsif level_depth_exceeded?(parent) elsif level_depth_exceeded?(parent)
errors.add :parent, "This epic can't be added as the maximum depth of nested epics would be exceeded" errors.add :parent, "This epic cannot be added. One or more epics would exceed the maximum depth (#{MAX_HIERARCHY_DEPTH}) from its most distant ancestor."
end end
end end
......
...@@ -8,7 +8,7 @@ module EpicLinks ...@@ -8,7 +8,7 @@ module EpicLinks
end end
if issuable.max_hierarchy_depth_achieved? if issuable.max_hierarchy_depth_achieved?
return error("This epic can't be added because the parent is already at the maximum depth from its most distant ancestor", 409) return error("This epic cannot be added. One or more epics would exceed the maximum depth (#{Epic::MAX_HIERARCHY_DEPTH}) from its most distant ancestor.", 409)
end end
if referenced_issuables.count == 1 if referenced_issuables.count == 1
......
...@@ -29,6 +29,7 @@ RSpec.describe 'Epic Issues', :js do ...@@ -29,6 +29,7 @@ RSpec.describe 'Epic Issues', :js do
before do before do
stub_licensed_features(epics: true, subepics: true) stub_licensed_features(epics: true, subepics: true)
stub_const('Epic', ::Epic, transfer_nested_constants: true)
end end
def visit_epic def visit_epic
...@@ -214,7 +215,7 @@ RSpec.describe 'Epic Issues', :js do ...@@ -214,7 +215,7 @@ RSpec.describe 'Epic Issues', :js do
add_epics(references) add_epics(references)
expect(page).to have_selector('.gl-field-error') expect(page).to have_selector('.gl-field-error')
expect(find('.gl-field-error')).to have_text("This epic can't be added because the parent is already at the maximum depth from its most distant ancestor") expect(find('.gl-field-error')).to have_text("This epic cannot be added. One or more epics would exceed the maximum depth (5) from its most distant ancestor.")
end end
end end
end end
......
...@@ -13,6 +13,10 @@ RSpec.describe EpicLinks::CreateService do ...@@ -13,6 +13,10 @@ RSpec.describe EpicLinks::CreateService do
let(:valid_reference) { epic_to_add.to_reference(full: true) } let(:valid_reference) { epic_to_add.to_reference(full: true) }
before do
stub_const('Epic', ::Epic, transfer_nested_constants: true)
end
shared_examples 'system notes created' do shared_examples 'system notes created' do
it 'creates system notes' do it 'creates system notes' do
expect { subject }.to change { Note.system.count }.from(0).to(2) expect { subject }.to change { Note.system.count }.from(0).to(2)
...@@ -85,7 +89,7 @@ RSpec.describe EpicLinks::CreateService do ...@@ -85,7 +89,7 @@ RSpec.describe EpicLinks::CreateService do
context 'when an epic from another group is given' do context 'when an epic from another group is given' do
let(:other_group) { create(:group) } let(:other_group) { create(:group) }
let(:expected_error) { "This epic can't be added because it must belong to the same group as the parent, or subgroup of the parent epic’s group" } let(:expected_error) { "This epic cannot be added. An epic must belong to the same group or subgroup as its parent epic." }
let(:expected_code) { 409 } let(:expected_code) { 409 }
before do before do
...@@ -97,7 +101,7 @@ RSpec.describe EpicLinks::CreateService do ...@@ -97,7 +101,7 @@ RSpec.describe EpicLinks::CreateService do
context 'when hierarchy is cyclic' do context 'when hierarchy is cyclic' do
context 'when given child epic is the same as given parent' do context 'when given child epic is the same as given parent' do
let(:expected_error) { 'Cannot add an epic as a child of itself' } let(:expected_error) { "This epic cannot be added. An epic cannot be added to itself." }
let(:expected_code) { 409 } let(:expected_code) { 409 }
subject { add_epic([epic.to_reference(full: true)]) } subject { add_epic([epic.to_reference(full: true)]) }
...@@ -106,7 +110,7 @@ RSpec.describe EpicLinks::CreateService do ...@@ -106,7 +110,7 @@ RSpec.describe EpicLinks::CreateService do
end end
context 'when given child epic is parent of the given parent' do context 'when given child epic is parent of the given parent' do
let(:expected_error) { "This epic can't be added as it is already assigned to this epic's ancestor" } let(:expected_error) { "This epic cannot be added. It is already an ancestor of the parent epic." }
let(:expected_code) { 409 } let(:expected_code) { 409 }
before do before do
...@@ -117,7 +121,7 @@ RSpec.describe EpicLinks::CreateService do ...@@ -117,7 +121,7 @@ RSpec.describe EpicLinks::CreateService do
end end
context 'when new child epic is an ancestor of the given parent' do context 'when new child epic is an ancestor of the given parent' do
let(:expected_error) { "This epic can't be added as it is already assigned to this epic's ancestor" } let(:expected_error) { "This epic cannot be added. It is already an ancestor of the parent epic." }
let(:expected_code) { 409 } let(:expected_code) { 409 }
before do before do
...@@ -136,7 +140,7 @@ RSpec.describe EpicLinks::CreateService do ...@@ -136,7 +140,7 @@ RSpec.describe EpicLinks::CreateService do
epic_to_add.update(parent: epic) epic_to_add.update(parent: epic)
end end
let(:expected_error) { "This epic can't be added as it is already assigned to the parent" } let(:expected_error) { "This epic cannot be added. It is already assigned to the parent epic." }
let(:expected_code) { 409 } let(:expected_code) { 409 }
include_examples 'returns an error' include_examples 'returns an error'
...@@ -152,14 +156,14 @@ RSpec.describe EpicLinks::CreateService do ...@@ -152,14 +156,14 @@ RSpec.describe EpicLinks::CreateService do
epic.update(parent: epic4) epic.update(parent: epic4)
end end
let(:expected_error) { "This epic can't be added because the parent is already at the maximum depth from its most distant ancestor" } let(:expected_error) { "This epic cannot be added. One or more epics would exceed the maximum depth (5) from its most distant ancestor." }
let(:expected_code) { 409 } let(:expected_code) { 409 }
include_examples 'returns an error' include_examples 'returns an error'
end end
context 'when total depth after adding would exceed depth limit' do context 'when total depth after adding would exceed depth limit' do
let(:expected_error) { "This epic can't be added as the maximum depth of nested epics would be exceeded" } let(:expected_error) { "This epic cannot be added. One or more epics would exceed the maximum depth (5) from its most distant ancestor." }
let(:expected_code) { 409 } let(:expected_code) { 409 }
before do before do
......
...@@ -237,7 +237,7 @@ RSpec.describe Epics::TreeReorderService do ...@@ -237,7 +237,7 @@ RSpec.describe Epics::TreeReorderService do
epic2.update(parent: epic1) epic2.update(parent: epic1)
end end
it_behaves_like 'error for the tree update', "This epic can't be added because it must belong to the same group as the parent, or subgroup of the parent epic’s group" it_behaves_like 'error for the tree update', "This epic cannot be added. An epic must belong to the same group or subgroup as its parent epic."
end end
context 'when user does not have permissions to admin the new parent' do context 'when user does not have permissions to admin the new parent' do
......
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