Commit 9729e06b authored by Eulyeon Ko's avatar Eulyeon Ko Committed by Eulyeon Ko

Update relative positions on querying board issues

When board issues are queried via graphql,
check and initialize the relative positions of issues
if they are null.

Why?

The issues appearing in a board list must not have
their relative positions as `nil` to allow
repositioning of the issues.

Boards::IssuesController set the relative positions
of the issues being fetched but we did not do the same
when board issues were queried via graphql.

Changelog: fixed
parent 1c82e9a4
...@@ -15,8 +15,17 @@ module Resolvers ...@@ -15,8 +15,17 @@ module Resolvers
def resolve(**args) def resolve(**args)
filter_params = item_filters(args[:filters]).merge(board_id: list.board.id, id: list.id) filter_params = item_filters(args[:filters]).merge(board_id: list.board.id, id: list.id)
service = ::Boards::Issues::ListService.new(list.board.resource_parent, context[:current_user], filter_params) service = ::Boards::Issues::ListService.new(list.board.resource_parent, context[:current_user], filter_params)
pagination_connections = Gitlab::Graphql::Pagination::Keyset::Connection.new(service.execute)
service.execute initialize_relative_positions(pagination_connections.items, list.board)
pagination_connections
end
def initialize_relative_positions(issues, board)
if Gitlab::Database.read_write? && !board.disabled_for?(current_user)
Issue.move_nulls_to_end(issues)
end
end end
# https://gitlab.com/gitlab-org/gitlab/-/issues/235681 # https://gitlab.com/gitlab-org/gitlab/-/issues/235681
......
...@@ -134,6 +134,6 @@ RSpec.describe Resolvers::BoardListIssuesResolver do ...@@ -134,6 +134,6 @@ RSpec.describe Resolvers::BoardListIssuesResolver do
end end
def resolve_board_list_issues(args) def resolve_board_list_issues(args)
resolve(described_class, obj: list, args: args, ctx: { current_user: user }) resolve(described_class, obj: list, args: args, ctx: { current_user: user }).items
end end
end end
...@@ -20,18 +20,20 @@ RSpec.describe Resolvers::BoardListIssuesResolver do ...@@ -20,18 +20,20 @@ RSpec.describe Resolvers::BoardListIssuesResolver do
let!(:issue1) { create(:issue, project: project, labels: [label], relative_position: 10) } let!(:issue1) { create(:issue, project: project, labels: [label], relative_position: 10) }
let!(:issue2) { create(:issue, project: project, labels: [label, label2], relative_position: 12) } let!(:issue2) { create(:issue, project: project, labels: [label, label2], relative_position: 12) }
let!(:issue3) { create(:issue, project: project, labels: [label, label3], relative_position: 10) } let!(:issue3) { create(:issue, project: project, labels: [label, label3], relative_position: 10) }
let!(:issue4) { create(:issue, project: project, labels: [label], relative_position: nil) }
it 'returns the issues in the correct order' do it 'returns issues in the correct order with non-nil relative positions', :aggregate_failures do
# by relative_position and then ID # by relative_position and then ID
issues = resolve_board_list_issues result = resolve_board_list_issues
expect(issues.map(&:id)).to eq [issue3.id, issue1.id, issue2.id] expect(result.map(&:id)).to eq [issue3.id, issue1.id, issue2.id, issue4.id]
expect(result.map(&:relative_position)).not_to include(nil)
end end
it 'finds only issues matching filters' do it 'finds only issues matching filters' do
result = resolve_board_list_issues(args: { filters: { label_name: [label.title], not: { label_name: [label2.title] } } }) result = resolve_board_list_issues(args: { filters: { label_name: [label.title], not: { label_name: [label2.title] } } })
expect(result).to match_array([issue1, issue3]) expect(result).to match_array([issue1, issue3, issue4])
end end
it 'finds only issues matching search param' do it 'finds only issues matching search param' do
...@@ -49,7 +51,7 @@ RSpec.describe Resolvers::BoardListIssuesResolver do ...@@ -49,7 +51,7 @@ RSpec.describe Resolvers::BoardListIssuesResolver do
it 'accepts assignee wildcard id NONE' do it 'accepts assignee wildcard id NONE' do
result = resolve_board_list_issues(args: { filters: { assignee_wildcard_id: 'NONE' } }) result = resolve_board_list_issues(args: { filters: { assignee_wildcard_id: 'NONE' } })
expect(result).to match_array([issue1, issue2, issue3]) expect(result).to match_array([issue1, issue2, issue3, issue4])
end end
it 'accepts assignee wildcard id ANY' do it 'accepts assignee wildcard id ANY' do
...@@ -89,6 +91,6 @@ RSpec.describe Resolvers::BoardListIssuesResolver do ...@@ -89,6 +91,6 @@ RSpec.describe Resolvers::BoardListIssuesResolver do
end end
def resolve_board_list_issues(args: {}, current_user: user) def resolve_board_list_issues(args: {}, current_user: user)
resolve(described_class, obj: list, args: args, ctx: { current_user: current_user }) resolve(described_class, obj: list, args: args, ctx: { current_user: current_user }).items
end end
end end
...@@ -48,13 +48,18 @@ RSpec.describe 'get board lists' do ...@@ -48,13 +48,18 @@ RSpec.describe 'get board lists' do
issues_data.map { |i| i['title'] } issues_data.map { |i| i['title'] }
end end
def issue_relative_positions
issues_data.map { |i| i['relativePosition'] }
end
shared_examples 'group and project board list issues query' do shared_examples 'group and project board list issues query' do
let!(:board) { create(:board, resource_parent: board_parent) } let!(:board) { create(:board, resource_parent: board_parent) }
let!(:label_list) { create(:list, board: board, label: label, position: 10) } let!(:label_list) { create(:list, board: board, label: label, position: 10) }
let!(:issue1) { create(:issue, project: issue_project, labels: [label, label2], relative_position: 9) } let!(:issue1) { create(:issue, project: issue_project, labels: [label, label2], relative_position: 9) }
let!(:issue2) { create(:issue, project: issue_project, labels: [label, label2], relative_position: 2) } let!(:issue2) { create(:issue, project: issue_project, labels: [label, label2], relative_position: 2) }
let!(:issue3) { create(:issue, project: issue_project, labels: [label], relative_position: 9) } let!(:issue3) { create(:issue, project: issue_project, labels: [label, label2], relative_position: nil) }
let!(:issue4) { create(:issue, project: issue_project, labels: [label2], relative_position: 432) } let!(:issue4) { create(:issue, project: issue_project, labels: [label], relative_position: 9) }
let!(:issue5) { create(:issue, project: issue_project, labels: [label2], relative_position: 432) }
context 'when the user does not have access to the board' do context 'when the user does not have access to the board' do
it 'returns nil' do it 'returns nil' do
...@@ -64,7 +69,7 @@ RSpec.describe 'get board lists' do ...@@ -64,7 +69,7 @@ RSpec.describe 'get board lists' do
end end
end end
context 'when user can read the board' do context 'when user can read the board', :aggregate_failures do
before do before do
board_parent.add_reporter(user) board_parent.add_reporter(user)
end end
...@@ -72,7 +77,8 @@ RSpec.describe 'get board lists' do ...@@ -72,7 +77,8 @@ RSpec.describe 'get board lists' do
it 'can access the issues' do it 'can access the issues' do
post_graphql(query("id: \"#{global_id_of(label_list)}\""), current_user: user) post_graphql(query("id: \"#{global_id_of(label_list)}\""), current_user: user)
expect(issue_titles).to eq([issue2.title, issue1.title]) expect(issue_titles).to eq([issue2.title, issue1.title, issue3.title])
expect(issue_relative_positions).not_to include(nil)
end end
end end
end end
......
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