Merge branch...

Merge branch '292902-move-to-createboard-mutation-instead-of-rest-api-call-updateboard' into 'master'

[RUN-AS-IF-FOSS] Move to `createBoard` mutation instead of REST API call

See merge request gitlab-org/gitlab!50171

app/assets/javascripts/boards/boards_util.js

 import { sortBy } from 'lodash';
-import axios from '~/lib/utils/axios_utils';
 import { ListType } from './constants';
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 
@@ -121,15 +120,6 @@ export function moveIssueListHelper(issue, fromList, toList) {
   return updatedIssue;
 }
 
-export function getBoardsPath(endpoint, board) {
-  const path = `${endpoint}${board.id ? `/${board.id}` : ''}.json`;
-
-  if (board.id) {
-    return axios.put(path, { board });
-  }
-  return axios.post(path, { board });
-}
-
 export function isListDraggable(list) {
   return list.listType !== ListType.backlog && list.listType !== ListType.closed;
 }
@@ -146,6 +136,5 @@ export default {
   fullBoardId,
   fullLabelId,
   fullIterationId,
-  getBoardsPath,
   isListDraggable,
 };

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

@@ -6,36 +6,13 @@ export default {
     GlFormCheckbox,
   },
   props: {
-    currentBoard: {
-      type: Object,
-      required: true,
-    },
-    board: {
-      type: Object,
+    hideBacklogList: {
+      type: Boolean,
       required: true,
     },
-    isNewForm: {
+    hideClosedList: {
       type: Boolean,
-      required: false,
-      default: false,
-    },
-  },
-  data() {
-    const { hide_backlog_list: hideBacklogList, hide_closed_list: hideClosedList } = this.isNewForm
-      ? this.board
-      : this.currentBoard;
-
-    return {
-      hideClosedList,
-      hideBacklogList,
-    };
-  },
-  methods: {
-    changeClosedList(checked) {
-      this.board.hideClosedList = !checked;
-    },
-    changeBacklogList(checked) {
-      this.board.hideBacklogList = !checked;
+      required: true,
     },
   },
 };
@@ -52,13 +29,13 @@ export default {
     <gl-form-checkbox
       :checked="!hideBacklogList"
       data-testid="backlog-list-checkbox"
-      @change="changeBacklogList"
+      @change="$emit('update:hideBacklogList', !hideBacklogList)"
       >{{ __('Show the Open list') }}
     </gl-form-checkbox>
     <gl-form-checkbox
       :checked="!hideClosedList"
       data-testid="closed-list-checkbox"
-      @change="changeClosedList"
+      @change="$emit('update:hideClosedList', !hideClosedList)"
       >{{ __('Show the Closed list') }}
     </gl-form-checkbox>
   </div>

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

 <script>
 import { GlModal } from '@gitlab/ui';
-import { pick } from 'lodash';
 import { __, s__ } from '~/locale';
 import { deprecatedCreateFlash as Flash } from '~/flash';
-import { visitUrl } from '~/lib/utils/url_utility';
+import { visitUrl, stripFinalUrlSegment } from '~/lib/utils/url_utility';
+import { getIdFromGraphQLId, convertToGraphQLId } from '~/graphql_shared/utils';
 import boardsStore from '~/boards/stores/boards_store';
-import { fullBoardId, getBoardsPath } from '../boards_util';
+import { fullLabelId, fullBoardId } from '../boards_util';
 
 import BoardConfigurationOptions from './board_configuration_options.vue';
-import createBoardMutation from '../graphql/board.mutation.graphql';
+import updateBoardMutation from '../graphql/board_update.mutation.graphql';
+import createBoardMutation from '../graphql/board_create.mutation.graphql';
 
 const boardDefaults = {
   id: false,
@@ -91,8 +92,8 @@ export default {
     },
   },
   inject: {
-    endpoints: {
-      default: {},
+    fullPath: {
+      default: '',
     },
   },
   data() {
@@ -155,14 +156,38 @@ export default {
         text: this.$options.i18n.cancelButtonText,
       };
     },
-    boardPayload() {
-      const { assignee, milestone, labels } = this.board;
-      return {
-        ...this.board,
-        assignee_id: assignee?.id,
-        milestone_id: milestone?.id,
-        label_ids: labels.length ? labels.map(b => b.id) : [''],
+    currentMutation() {
+      return this.board.id ? updateBoardMutation : createBoardMutation;
+    },
+    mutationVariables() {
+      const { board } = this;
+      /* eslint-disable @gitlab/require-i18n-strings */
+      const baseMutationVariables = {
+        name: board.name,
+        weight: board.weight,
+        assigneeId: board.assignee?.id ? convertToGraphQLId('User', board.assignee.id) : null,
+        milestoneId:
+          board.milestone?.id || board.milestone?.id === 0
+            ? convertToGraphQLId('Milestone', board.milestone.id)
+            : null,
+        labelIds: board.labels.map(fullLabelId),
+        hideBacklogList: board.hide_backlog_list,
+        hideClosedList: board.hide_closed_list,
+        iterationId: board.iteration_id
+          ? convertToGraphQLId('Iteration', board.iteration_id)
+          : null,
       };
+      /* eslint-enable @gitlab/require-i18n-strings */
+      return board.id
+        ? {
+            ...baseMutationVariables,
+            id: fullBoardId(board.id),
+          }
+        : {
+            ...baseMutationVariables,
+            projectPath: this.projectId ? this.fullPath : null,
+            groupPath: this.groupId ? this.fullPath : null,
+          };
     },
   },
   mounted() {
@@ -175,55 +200,39 @@ export default {
     setIteration(iterationId) {
       this.board.iteration_id = iterationId;
     },
-    callBoardMutation(id) {
-      return this.$apollo.mutate({
-        mutation: createBoardMutation,
-        variables: {
-          ...pick(this.boardPayload, ['hideClosedList', 'hideBacklogList']),
-          id,
-        },
+    async createOrUpdateBoard() {
+      const response = await this.$apollo.mutate({
+        mutation: this.currentMutation,
+        variables: { input: this.mutationVariables },
       });
-    },
-    async updateBoard() {
-      const responses = await Promise.all([
-        // Remove unnecessary REST API call when https://gitlab.com/gitlab-org/gitlab/-/issues/282299#note_462996301 is resolved
-        getBoardsPath(this.endpoints.boardsEndpoint, this.boardPayload),
-        this.callBoardMutation(fullBoardId(this.boardPayload.id)),
-      ]);
 
-      return responses[0].data;
+      return this.board.id
+        ? getIdFromGraphQLId(response.data.updateBoard.board.id)
+        : getIdFromGraphQLId(response.data.createBoard.board.id);
     },
-    async createBoard() {
-      // TODO: change this to use `createBoard` mutation https://gitlab.com/gitlab-org/gitlab/-/issues/292466 is resolved
-      const boardData = await getBoardsPath(this.endpoints.boardsEndpoint, this.boardPayload);
-      this.callBoardMutation(fullBoardId(boardData.data.id));
-
-      return boardData.data || boardData;
-    },
-    submit() {
+    async submit() {
       if (this.board.name.length === 0) return;
       this.isLoading = true;
       if (this.isDeleteForm) {
-        boardsStore
-          .deleteBoard(this.currentBoard)
-          .then(() => {
-            this.isLoading = false;
-            visitUrl(boardsStore.rootPath);
-          })
-          .catch(() => {
-            Flash(this.$options.i18n.deleteErrorMessage);
-            this.isLoading = false;
-          });
+        try {
+          await boardsStore.deleteBoard(this.currentBoard);
+          visitUrl(boardsStore.rootPath);
+        } catch {
+          Flash(this.$options.i18n.deleteErrorMessage);
+        } finally {
+          this.isLoading = false;
+        }
       } else {
-        const boardAction = this.boardPayload.id ? this.updateBoard : this.createBoard;
-        boardAction()
-          .then(data => {
-            visitUrl(data.board_path);
-          })
-          .catch(() => {
-            Flash(this.$options.i18n.saveErrorMessage);
-            this.isLoading = false;
-          });
+        try {
+          const path = await this.createOrUpdateBoard();
+          const strippedUrl = stripFinalUrlSegment(window.location.href);
+          const url = strippedUrl.includes('boards') ? `${path}` : `boards/${path}`;
+          visitUrl(url);
+        } catch {
+          Flash(this.$options.i18n.saveErrorMessage);
+        } finally {
+          this.isLoading = false;
+        }
       }
     },
     cancel() {
@@ -277,9 +286,8 @@ export default {
       </div>
 
       <board-configuration-options
-        :is-new-form="isNewForm"
-        :board="board"
-        :current-board="currentBoard"
+        :hide-backlog-list.sync="board.hide_backlog_list"
+        :hide-closed-list.sync="board.hide_closed_list"
       />
 
       <board-scope

app/assets/javascripts/boards/graphql/board.mutation.graphql

-mutation UpdateBoard($id: BoardID!, $hideClosedList: Boolean, $hideBacklogList: Boolean) {
-  updateBoard(
-    input: { id: $id, hideClosedList: $hideClosedList, hideBacklogList: $hideBacklogList }
-  ) {
-    board {
-      id
-      hideClosedList
-      hideBacklogList
-    }
-  }
-}

app/assets/javascripts/boards/graphql/board_create.mutation.graphql

+mutation createBoard($input: CreateBoardInput!) {
+  createBoard(input: $input) {
+    board {
+      id
+    }
+  }
+}

app/assets/javascripts/boards/graphql/board_update.mutation.graphql

+mutation UpdateBoard($input: UpdateBoardInput!) {
+  updateBoard(input: $input) {
+    board {
+      id
+      hideClosedList
+      hideBacklogList
+    }
+  }
+}

app/assets/javascripts/boards/index.js

@@ -335,7 +335,6 @@ export default () => {
   }
 
   mountMultipleBoardsSwitcher({
-    boardsEndpoint: $boardApp.dataset.boardsEndpoint,
-    recentBoardsEndpoint: $boardApp.dataset.recentBoardsEndpoint,
+    fullPath: $boardApp.dataset.fullPath,
   });
 };

app/assets/javascripts/boards/mount_multiple_boards_switcher.js

@@ -10,7 +10,7 @@ const apolloProvider = new VueApollo({
   defaultClient: createDefaultClient(),
 });
 
-export default (endpoints = {}) => {
+export default (params = {}) => {
   const boardsSwitcherElement = document.getElementById('js-multiple-boards-switcher');
   return new Vue({
     el: boardsSwitcherElement,
@@ -36,7 +36,7 @@ export default (endpoints = {}) => {
       return { boardsSelectorProps };
     },
     provide: {
-      endpoints,
+      fullPath: params.fullPath,
     },
     render(createElement) {
       return createElement(BoardsSelector, {

changelogs/unreleased/292902-move-to-createboard-mutation-instead-of-rest-api-call-updateboard.yml

+---
+title: "[RUN-AS-IF-FOSS] Move to `createBoard` mutation instead of REST API call +
+  `updateBoard`"
+merge_request: 50171
+author:
+type: changed

spec/frontend/boards/components/board_configuration_options_spec.js

@@ -3,38 +3,30 @@ import BoardConfigurationOptions from '~/boards/components/board_configuration_o
 
 describe('BoardConfigurationOptions', () => {
   let wrapper;
-  const board = { hide_backlog_list: false, hide_closed_list: false };
 
   const defaultProps = {
-    currentBoard: board,
-    board,
-    isNewForm: false,
+    hideBacklogList: false,
+    hideClosedList: false,
   };
 
-  const createComponent = () => {
+  const createComponent = (props = {}) => {
     wrapper = shallowMount(BoardConfigurationOptions, {
-      propsData: { ...defaultProps },
+      propsData: { ...defaultProps, ...props },
     });
   };
 
-  beforeEach(() => {
-    createComponent();
-  });
-
   afterEach(() => {
     wrapper.destroy();
   });
 
-  const backlogListCheckbox = el => el.find('[data-testid="backlog-list-checkbox"]');
-  const closedListCheckbox = el => el.find('[data-testid="closed-list-checkbox"]');
+  const backlogListCheckbox = () => wrapper.find('[data-testid="backlog-list-checkbox"]');
+  const closedListCheckbox = () => wrapper.find('[data-testid="closed-list-checkbox"]');
 
   const checkboxAssert = (backlogCheckbox, closedCheckbox) => {
-    expect(backlogListCheckbox(wrapper).attributes('checked')).toEqual(
+    expect(backlogListCheckbox().attributes('checked')).toEqual(
       backlogCheckbox ? undefined : 'true',
     );
-    expect(closedListCheckbox(wrapper).attributes('checked')).toEqual(
-      closedCheckbox ? undefined : 'true',
-    );
+    expect(closedListCheckbox().attributes('checked')).toEqual(closedCheckbox ? undefined : 'true');
   };
 
   it.each`
@@ -45,15 +37,28 @@ describe('BoardConfigurationOptions', () => {
     ${false}             | ${false}
   `(
     'renders two checkbox when one is $backlogCheckboxValue and other is $closedCheckboxValue',
-    async ({ backlogCheckboxValue, closedCheckboxValue }) => {
-      await wrapper.setData({
+    ({ backlogCheckboxValue, closedCheckboxValue }) => {
+      createComponent({
         hideBacklogList: backlogCheckboxValue,
         hideClosedList: closedCheckboxValue,
       });
-
-      return wrapper.vm.$nextTick().then(() => {
-        checkboxAssert(backlogCheckboxValue, closedCheckboxValue);
-      });
+      checkboxAssert(backlogCheckboxValue, closedCheckboxValue);
     },
   );
+
+  it('emits a correct value on backlog checkbox change', () => {
+    createComponent();
+
+    backlogListCheckbox().vm.$emit('change');
+
+    expect(wrapper.emitted('update:hideBacklogList')).toEqual([[true]]);
+  });
+
+  it('emits a correct value on closed checkbox change', () => {
+    createComponent();
+
+    closedListCheckbox().vm.$emit('change');
+
+    expect(wrapper.emitted('update:hideClosedList')).toEqual([[true]]);
+  });
 });

spec/frontend/boards/components/board_form_spec.js

 import { shallowMount } from '@vue/test-utils';
-import AxiosMockAdapter from 'axios-mock-adapter';
 
 import { TEST_HOST } from 'jest/helpers/test_constants';
 import { GlModal } from '@gitlab/ui';
 import waitForPromises from 'helpers/wait_for_promises';
 
-import axios from '~/lib/utils/axios_utils';
 import { visitUrl } from '~/lib/utils/url_utility';
 import boardsStore from '~/boards/stores/boards_store';
 import BoardForm from '~/boards/components/board_form.vue';
-import BoardConfigurationOptions from '~/boards/components/board_configuration_options.vue';
-import createBoardMutation from '~/boards/graphql/board.mutation.graphql';
+import updateBoardMutation from '~/boards/graphql/board_update.mutation.graphql';
+import createBoardMutation from '~/boards/graphql/board_create.mutation.graphql';
 
 jest.mock('~/lib/utils/url_utility', () => ({
   visitUrl: jest.fn().mockName('visitUrlMock'),
+  stripFinalUrlSegment: jest.requireActual('~/lib/utils/url_utility').stripFinalUrlSegment,
 }));
 
 const currentBoard = {
@@ -28,18 +27,6 @@ const currentBoard = {
   hide_closed_list: false,
 };
 
-const boardDefaults = {
-  id: false,
-  name: '',
-  labels: [],
-  milestone_id: undefined,
-  assignee: {},
-  assignee_id: undefined,
-  weight: null,
-  hide_backlog_list: false,
-  hide_closed_list: false,
-};
-
 const defaultProps = {
   canAdminBoard: false,
   labelsPath: `${TEST_HOST}/labels/path`,
@@ -51,18 +38,21 @@ const endpoints = {
   boardsEndpoint: 'test-endpoint',
 };
 
-const mutate = jest.fn().mockResolvedValue({});
+const mutate = jest.fn().mockResolvedValue({
+  data: {
+    createBoard: { board: { id: 'gid://gitlab/Board/123' } },
+    updateBoard: { board: { id: 'gid://gitlab/Board/321' } },
+  },
+});
 
 describe('BoardForm', () => {
   let wrapper;
-  let axiosMock;
 
   const findModal = () => wrapper.find(GlModal);
   const findModalActionPrimary = () => findModal().props('actionPrimary');
   const findForm = () => wrapper.find('[data-testid="board-form"]');
   const findFormWrapper = () => wrapper.find('[data-testid="board-form-wrapper"]');
   const findDeleteConfirmation = () => wrapper.find('[data-testid="delete-confirmation-message"]');
-  const findConfigurationOptions = () => wrapper.find(BoardConfigurationOptions);
   const findInput = () => wrapper.find('#board-new-name');
 
   const createComponent = (props, data) => {
@@ -86,13 +76,12 @@ describe('BoardForm', () => {
   };
 
   beforeEach(() => {
-    axiosMock = new AxiosMockAdapter(axios);
+    delete window.location;
   });
 
   afterEach(() => {
     wrapper.destroy();
     wrapper = null;
-    axiosMock.restore();
     boardsStore.state.currentPage = null;
   });
 
@@ -145,7 +134,7 @@ describe('BoardForm', () => {
       });
 
       it('clears the form', () => {
-        expect(findConfigurationOptions().props('board')).toEqual(boardDefaults);
+        expect(findInput().element.value).toBe('');
       });
 
       it('shows a correct title about creating a board', () => {
@@ -164,18 +153,9 @@ describe('BoardForm', () => {
       it('renders form wrapper', () => {
         expect(findFormWrapper().exists()).toBe(true);
       });
-
-      it('passes a true isNewForm prop to BoardConfigurationOptions component', () => {
-        expect(findConfigurationOptions().props('isNewForm')).toBe(true);
-      });
     });
 
     describe('when submitting a create event', () => {
-      beforeEach(() => {
-        const url = `${endpoints.boardsEndpoint}.json`;
-        axiosMock.onPost(url).reply(200, { id: '2', board_path: 'new path' });
-      });
-
       it('does not call API if board name is empty', async () => {
         createComponent({ canAdminBoard: true });
         findInput().trigger('keyup.enter', { metaKey: true });
@@ -185,7 +165,8 @@ describe('BoardForm', () => {
         expect(mutate).not.toHaveBeenCalled();
       });
 
-      it('calls REST and GraphQL API and redirects to correct page', async () => {
+      it('calls a correct GraphQL mutation and redirects to correct page from existing board', async () => {
+        window.location = new URL('https://test/boards/1');
         createComponent({ canAdminBoard: true });
 
         findInput().value = 'Test name';
@@ -194,19 +175,40 @@ describe('BoardForm', () => {
 
         await waitForPromises();
 
-        expect(axiosMock.history.post[0].data).toBe(
-          JSON.stringify({ board: { ...boardDefaults, name: 'test', label_ids: [''] } }),
-        );
+        expect(mutate).toHaveBeenCalledWith({
+          mutation: createBoardMutation,
+          variables: {
+            input: expect.objectContaining({
+              name: 'test',
+            }),
+          },
+        });
+
+        await waitForPromises();
+        expect(visitUrl).toHaveBeenCalledWith('123');
+      });
+
+      it('calls a correct GraphQL mutation and redirects to correct page from boards list', async () => {
+        window.location = new URL('https://test/boards');
+        createComponent({ canAdminBoard: true });
+
+        findInput().value = 'Test name';
+        findInput().trigger('input');
+        findInput().trigger('keyup.enter', { metaKey: true });
+
+        await waitForPromises();
 
         expect(mutate).toHaveBeenCalledWith({
           mutation: createBoardMutation,
           variables: {
-            id: 'gid://gitlab/Board/2',
+            input: expect.objectContaining({
+              name: 'test',
+            }),
           },
         });
 
         await waitForPromises();
-        expect(visitUrl).toHaveBeenCalledWith('new path');
+        expect(visitUrl).toHaveBeenCalledWith('boards/123');
       });
     });
   });
@@ -222,7 +224,7 @@ describe('BoardForm', () => {
       });
 
       it('clears the form', () => {
-        expect(findConfigurationOptions().props('board')).toEqual(currentBoard);
+        expect(findInput().element.value).toEqual(currentBoard.name);
       });
 
       it('shows a correct title about creating a board', () => {
@@ -241,35 +243,28 @@ describe('BoardForm', () => {
       it('renders form wrapper', () => {
         expect(findFormWrapper().exists()).toBe(true);
       });
-
-      it('passes a false isNewForm prop to BoardConfigurationOptions component', () => {
-        expect(findConfigurationOptions().props('isNewForm')).toBe(false);
-      });
     });
 
     describe('when submitting an update event', () => {
-      beforeEach(() => {
-        const url = endpoints.boardsEndpoint;
-        axiosMock.onPut(url).reply(200, { board_path: 'new path' });
-      });
-
       it('calls REST and GraphQL API with correct parameters', async () => {
+        window.location = new URL('https://test/boards/1');
         createComponent({ canAdminBoard: true });
 
         findInput().trigger('keyup.enter', { metaKey: true });
 
         await waitForPromises();
 
-        expect(axiosMock.history.put[0].data).toBe(
-          JSON.stringify({ board: { ...currentBoard, label_ids: [''] } }),
-        );
-
         expect(mutate).toHaveBeenCalledWith({
-          mutation: createBoardMutation,
+          mutation: updateBoardMutation,
           variables: {
-            id: `gid://gitlab/Board/${currentBoard.id}`,
+            input: expect.objectContaining({
+              id: `gid://gitlab/Board/${currentBoard.id}`,
+            }),
           },
         });
+
+        await waitForPromises();
+        expect(visitUrl).toHaveBeenCalledWith('321');
       });
     });
   });