Merge branch 'dc-board-move-id-check' into 'master'

Fix issue position when creating issue in issue boards

See merge request gitlab-org/gitlab!80429

app/assets/javascripts/boards/components/board_new_issue.vue

@@ -25,7 +25,7 @@ export default {
   },
   computed: {
     ...mapState(['selectedProject', 'fullPath']),
-    ...mapGetters(['isGroupBoard']),
+    ...mapGetters(['isGroupBoard', 'getBoardItemsByList']),
     formEventPrefix() {
       return toggleFormEventPrefix.issue;
     },
@@ -42,6 +42,7 @@ export default {
       const labels = this.list.label ? [this.list.label] : [];
       const assignees = this.list.assignee ? [this.list.assignee] : [];
       const milestone = getMilestone(this.list);
+      const firstItemId = this.getBoardItemsByList(this.list.id)[0]?.id;
 
       return this.addListNewIssue({
         list: this.list,
@@ -51,6 +52,7 @@ export default {
           assigneeIds: assignees?.map((a) => a?.id),
           milestoneId: milestone?.id,
           projectPath: this.projectPath,
+          moveAfterId: firstItemId,
         },
       }).then(() => {
         this.cancel();

app/services/issues/base_service.rb

@@ -74,9 +74,7 @@ module Issues
     def issuable_for_positioning(id, positioning_scope)
       return unless id
 
-      issue = positioning_scope.find(id)
-
-      issue if can?(current_user, :update_issue, issue)
+      positioning_scope.find(id)
     end
 
     def create_assignee_note(issue, old_assignees)

spec/features/boards/new_issue_spec.rb

@@ -3,12 +3,13 @@
 require 'spec_helper'
 
 RSpec.describe 'Issue Boards new issue', :js do
-  let_it_be(:project)      { create(:project, :public) }
-  let_it_be(:board)        { create(:board, project: project) }
-  let_it_be(:backlog_list) { create(:backlog_list, board: board) }
-  let_it_be(:label)        { create(:label, project: project, name: 'Label 1') }
-  let_it_be(:list)         { create(:list, board: board, label: label, position: 0) }
-  let_it_be(:user)         { create(:user) }
+  let_it_be(:project)        { create(:project, :public) }
+  let_it_be(:board)          { create(:board, project: project) }
+  let_it_be(:backlog_list)   { create(:backlog_list, board: board) }
+  let_it_be(:label)          { create(:label, project: project, name: 'Label 1') }
+  let_it_be(:list)           { create(:list, board: board, label: label, position: 0) }
+  let_it_be(:user)           { create(:user) }
+  let_it_be(:existing_issue) { create(:issue, project: project, title: 'other issue', relative_position: 50) }
 
   let(:board_list_header) { first('[data-testid="board-list-header"]') }
   let(:project_select_dropdown) { find('[data-testid="project-select-dropdown"]') }
@@ -56,7 +57,7 @@ RSpec.describe 'Issue Boards new issue', :js do
       end
     end
 
-    it 'creates new issue and opens sidebar' do
+    it 'creates new issue, places it on top of the list, and opens sidebar' do
       page.within(first('.board')) do
         click_button 'New issue'
       end
@@ -69,12 +70,14 @@ RSpec.describe 'Issue Boards new issue', :js do
       wait_for_requests
 
       page.within(first('.board [data-testid="issue-count-badge"]')) do
-        expect(page).to have_content('1')
+        expect(page).to have_content('2')
       end
 
       page.within(first('.board-card')) do
         issue = project.issues.find_by_title('bug')
 
+        expect(issue.relative_position).to be < existing_issue.relative_position
+
         expect(page).to have_content(issue.to_reference)
         expect(page).to have_link(issue.title, href: /#{issue_path(issue)}/)
       end

spec/frontend/boards/components/board_new_issue_spec.js

@@ -6,7 +6,7 @@ import BoardNewItem from '~/boards/components/board_new_item.vue';
 import ProjectSelect from '~/boards/components/project_select.vue';
 import eventHub from '~/boards/eventhub';
 
-import { mockList, mockGroupProjects } from '../mock_data';
+import { mockList, mockGroupProjects, mockIssue, mockIssue2 } from '../mock_data';
 
 Vue.use(Vuex);
 
@@ -16,7 +16,7 @@ const mockActions = { addListNewIssue: addListNewIssuesSpy };
 const createComponent = ({
   state = { selectedProject: mockGroupProjects[0], fullPath: mockGroupProjects[0].fullPath },
   actions = mockActions,
-  getters = { isGroupBoard: () => true, isProjectBoard: () => false },
+  getters = { isGroupBoard: () => true, getBoardItemsByList: () => () => [] },
 } = {}) =>
   shallowMount(BoardNewIssue, {
     store: new Vuex.Store({
@@ -75,10 +75,39 @@ describe('Issue boards new issue form', () => {
         assigneeIds: [],
         milestoneId: undefined,
         projectPath: mockGroupProjects[0].fullPath,
+        moveAfterId: undefined,
       },
     });
   });
 
+  describe('when list has an existing issues', () => {
+    beforeEach(() => {
+      wrapper = createComponent({
+        getters: {
+          isGroupBoard: () => true,
+          getBoardItemsByList: () => () => [mockIssue, mockIssue2],
+        },
+      });
+    });
+
+    it('it uses the first issue ID as moveAfterId', async () => {
+      findBoardNewItem().vm.$emit('form-submit', { title: 'Foo' });
+
+      await nextTick();
+      expect(addListNewIssuesSpy).toHaveBeenCalledWith(expect.any(Object), {
+        list: mockList,
+        issueInput: {
+          title: 'Foo',
+          labelIds: [],
+          assigneeIds: [],
+          milestoneId: undefined,
+          projectPath: mockGroupProjects[0].fullPath,
+          moveAfterId: mockIssue.id,
+        },
+      });
+    });
+  });
+
   it('emits event `toggle-issue-form` with current list Id suffix on eventHub when `board-new-item` emits form-cancel event', async () => {
     jest.spyOn(eventHub, '$emit').mockImplementation();
     findBoardNewItem().vm.$emit('form-cancel');
@@ -99,7 +128,7 @@ describe('Issue boards new issue form', () => {
   describe('when in project issue board', () => {
     beforeEach(() => {
       wrapper = createComponent({
-        getters: { isGroupBoard: () => false, isProjectBoard: () => true },
+        getters: { isGroupBoard: () => false },
       });
     });
 

spec/services/issues/create_service_spec.rb

@@ -97,8 +97,6 @@ RSpec.describe Issues::CreateService do
         let(:issue_after) { create(:issue, project: project, relative_position: 50) }
 
         before do
-          project.add_reporter(user)
-
           opts.merge!(move_between_ids: [issue_before.id, issue_after.id])
         end