Commit 056d77dc authored by Alex Kalderimis's avatar Alex Kalderimis

Remove adjacency checks

This removes an extra check on moving designs that fails frequently
in the presence of soft-deletes.

If we have `[a b (c) d e]` (where `c` is a deleted design, not visible
to the user), then an attempt to move `e -> (b,d)` will fail due to the
adjacency check, even though from the user's perspective, `b` and `d`
are adjacent.

As long as clients are sensible, this extra level of sanity checking
is not necessary.
parent b6427f29
...@@ -228,17 +228,6 @@ module DesignManagement ...@@ -228,17 +228,6 @@ module DesignManagement
project project
end end
def immediately_before?(next_design)
return false if next_design.relative_position <= relative_position
interloper = self.class.on_issue(issue).where(
"relative_position <@ int4range(?, ?, '()')",
*[self, next_design].map(&:relative_position)
)
!interloper.exists?
end
def notes_with_associations def notes_with_associations
notes.includes(:author) notes.includes(:author)
end end
......
...@@ -16,7 +16,6 @@ module DesignManagement ...@@ -16,7 +16,6 @@ module DesignManagement
return error(:cannot_move) unless current_user.can?(:move_design, current_design) return error(:cannot_move) unless current_user.can?(:move_design, current_design)
return error(:no_neighbors) unless neighbors.present? return error(:no_neighbors) unless neighbors.present?
return error(:not_distinct) unless all_distinct? return error(:not_distinct) unless all_distinct?
return error(:not_adjacent) if any_in_gap?
return error(:not_same_issue) unless all_same_issue? return error(:not_same_issue) unless all_same_issue?
move_nulls_to_end move_nulls_to_end
...@@ -54,12 +53,6 @@ module DesignManagement ...@@ -54,12 +53,6 @@ module DesignManagement
ids.uniq.size == ids.size ids.uniq.size == ids.size
end end
def any_in_gap?
return false unless previous_design&.relative_position && next_design&.relative_position
!previous_design.immediately_before?(next_design)
end
def all_same_issue? def all_same_issue?
issue.designs.id_in(ids).count == ids.size issue.designs.id_in(ids).count == ids.size
end end
......
---
title: Fix spurious not-adjacent error when moving designs
merge_request: 53771
author:
type: fixed
...@@ -629,25 +629,4 @@ RSpec.describe DesignManagement::Design do ...@@ -629,25 +629,4 @@ RSpec.describe DesignManagement::Design do
end end
end end
end end
describe '#immediately_before' do
let_it_be(:design) { create(:design, issue: issue, relative_position: 100) }
let_it_be(:next_design) { create(:design, issue: issue, relative_position: 200) }
it 'is true when there is no element positioned between this item and the next' do
expect(design.immediately_before?(next_design)).to be true
end
it 'is false when there is an element positioned between this item and the next' do
create(:design, issue: issue, relative_position: 150)
expect(design.immediately_before?(next_design)).to be false
end
it 'is false when the next design is to the left of this design' do
further_left = create(:design, issue: issue, relative_position: 50)
expect(design.immediately_before?(further_left)).to be false
end
end
end end
...@@ -76,18 +76,6 @@ RSpec.describe DesignManagement::MoveDesignsService do ...@@ -76,18 +76,6 @@ RSpec.describe DesignManagement::MoveDesignsService do
end end
end end
context 'the designs are not adjacent' do
let(:current_design) { designs.first }
let(:previous_design) { designs.second }
let(:next_design) { designs.third }
it 'raises not_adjacent' do
create(:design, issue: issue, relative_position: next_design.relative_position - 1)
expect(subject).to be_error.and(have_attributes(message: :not_adjacent))
end
end
context 'moving a design with neighbours' do context 'moving a design with neighbours' do
let(:current_design) { designs.first } let(:current_design) { designs.first }
let(:previous_design) { designs.second } let(:previous_design) { designs.second }
......
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