Commit 18b1cae4 authored by Jarka Košanová's avatar Jarka Košanová

Check for adjacent_reference type

- and improve specs
parent 046f4323
...@@ -10,6 +10,7 @@ class EpicIssue < ApplicationRecord ...@@ -10,6 +10,7 @@ class EpicIssue < ApplicationRecord
belongs_to :issue belongs_to :issue
alias_attribute :parent_ids, :epic_id alias_attribute :parent_ids, :epic_id
alias_attribute :parent, :epic
scope :in_epic, ->(epic_id) { where(epic_id: epic_id) } scope :in_epic, ->(epic_id) { where(epic_id: epic_id) }
end end
...@@ -39,11 +39,20 @@ module Epics ...@@ -39,11 +39,20 @@ module Epics
end end
def validate_objects def validate_objects
unless moving_object.is_a?(EpicIssue) || moving_object.is_a?(Epic) unless supported_type?(moving_object) && supported_type?(adjacent_reference)
return 'Only epics and epic_issues are supported.' return 'Only epics and epic_issues are supported.'
end end
return 'You don\'t have permissions to move the objects.' unless authorized? return 'You don\'t have permissions to move the objects.' unless authorized?
return 'Both object have to belong to same parent epic.' unless same_parent?
end
def same_parent?
moving_object.parent == adjacent_reference.parent
end
def supported_type?(object)
object.is_a?(EpicIssue) || object.is_a?(Epic)
end end
def authorized? def authorized?
......
...@@ -8,7 +8,6 @@ describe EpicTreeSorting do ...@@ -8,7 +8,6 @@ describe EpicTreeSorting do
let!(:epic_issue1) { create(:epic_issue, epic: base_epic, relative_position: 10) } let!(:epic_issue1) { create(:epic_issue, epic: base_epic, relative_position: 10) }
let!(:epic_issue2) { create(:epic_issue, epic: base_epic, relative_position: 500) } let!(:epic_issue2) { create(:epic_issue, epic: base_epic, relative_position: 500) }
let!(:epic_issue3) { create(:epic_issue, epic: base_epic, relative_position: 1002) } let!(:epic_issue3) { create(:epic_issue, epic: base_epic, relative_position: 1002) }
let!(:epic_issue4) { create(:epic_issue, epic: base_epic, relative_position: 1500) }
let!(:epic1) { create(:epic, parent: base_epic, group: group, relative_position: 100) } let!(:epic1) { create(:epic, parent: base_epic, group: group, relative_position: 100) }
let!(:epic2) { create(:epic, parent: base_epic, group: group, relative_position: 1000) } let!(:epic2) { create(:epic, parent: base_epic, group: group, relative_position: 1000) }
let!(:epic3) { create(:epic, parent: base_epic, group: group, relative_position: 1001) } let!(:epic3) { create(:epic, parent: base_epic, group: group, relative_position: 1001) }
......
...@@ -10,31 +10,4 @@ describe EpicIssue do ...@@ -10,31 +10,4 @@ describe EpicIssue do
let(:default_params) { { epic: epic } } let(:default_params) { { epic: epic } }
end end
end end
context 'relative positioning with 2 classes' do
let(:group) { create(:group) }
let(:base_epic) { create(:epic, group: group) }
let!(:epic_issue1) { create(:epic_issue, epic: base_epic, relative_position: 10) }
let!(:epic_issue2) { create(:epic_issue, epic: base_epic, relative_position: 50) }
let!(:epic_issue3) { create(:epic_issue, epic: base_epic, relative_position: 1500) }
let!(:epic1) { create(:epic, parent: base_epic, group: group, relative_position: 1000) }
let!(:epic2) { create(:epic, parent: base_epic, group: group, relative_position: 2000) }
context '#move_after' do
it 'moves the epic after an epic_issue' do
epic1.move_after(epic_issue3)
expect(epic1.relative_position).to be > epic_issue3.relative_position
end
end
context '#move_between' do
it 'moves the epic between epic_issues' do
epic1.move_between(epic_issue1, epic_issue2)
expect(epic1.relative_position).to be > epic_issue1.relative_position
expect(epic1.relative_position).to be < epic_issue2.relative_position
end
end
end
end end
...@@ -106,7 +106,7 @@ describe 'Updating an epic tree' do ...@@ -106,7 +106,7 @@ describe 'Updating an epic tree' do
end end
end end
context 'when moving an epic fails' do context 'when moving an epic fails due another parent' do
let(:epic2) { create(:epic, relative_position: 20) } let(:epic2) { create(:epic, relative_position: 20) }
it_behaves_like 'a mutation that does not update the tree' it_behaves_like 'a mutation that does not update the tree'
...@@ -114,7 +114,7 @@ describe 'Updating an epic tree' do ...@@ -114,7 +114,7 @@ describe 'Updating an epic tree' do
it 'returns the error message' do it 'returns the error message' do
post_graphql_mutation(mutation, current_user: current_user) post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response['errors']).to eq(['Epic not found for given params']) expect(mutation_response['errors']).to eq(['Both object have to belong to same parent epic.'])
end end
end end
end end
...@@ -138,7 +138,7 @@ describe 'Updating an epic tree' do ...@@ -138,7 +138,7 @@ describe 'Updating an epic tree' do
end end
end end
context 'when moving an issue fails' do context 'when moving an issue fails due to another parent' do
let(:epic_issue2) { create(:epic_issue, relative_position: 20) } let(:epic_issue2) { create(:epic_issue, relative_position: 20) }
before do before do
...@@ -151,7 +151,7 @@ describe 'Updating an epic tree' do ...@@ -151,7 +151,7 @@ describe 'Updating an epic tree' do
it 'returns the error message' do it 'returns the error message' do
post_graphql_mutation(mutation, current_user: current_user) post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response['errors']).to eq(['Epic issue not found for given params']) expect(mutation_response['errors']).to eq(['Both object have to belong to same parent epic.'])
end end
end end
end end
......
...@@ -87,8 +87,22 @@ describe Epics::TreeReorderService do ...@@ -87,8 +87,22 @@ describe Epics::TreeReorderService do
end end
end end
context 'when object being moved is from of another epic' do
before do
other_epic = create(:epic, group: group)
epic_issue2.update(epic: other_epic)
end
it_behaves_like 'error for the tree update', 'Both object have to belong to same parent epic.'
end
context 'when object being moved is not supported type' do context 'when object being moved is not supported type' do
let(:moving_object_id) { GitlabSchema.id_from_object(issue1) } let(:moving_object_id) { GitlabSchema.id_from_object(issue1) }
it_behaves_like 'error for the tree update', 'Only epics and epic_issues are supported.'
end
context 'when adjacent object is not supported type' do
let(:adjacent_reference_id) { GitlabSchema.id_from_object(issue2) } let(:adjacent_reference_id) { GitlabSchema.id_from_object(issue2) }
it_behaves_like 'error for the tree update', 'Only epics and epic_issues are supported.' it_behaves_like 'error for the tree update', 'Only epics and epic_issues are supported.'
...@@ -131,6 +145,15 @@ describe Epics::TreeReorderService do ...@@ -131,6 +145,15 @@ describe Epics::TreeReorderService do
it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.' it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
end end
context 'when object being moved is from of another epic' do
before do
other_epic = create(:epic, group: group)
epic2.update(parent: other_epic)
end
it_behaves_like 'error for the tree update', 'Both object have to belong to same parent epic.'
end
context 'when moving is successful' do context 'when moving is successful' do
it 'updates the links relative positions' do it 'updates the links relative positions' do
subject subject
......
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