Commit 4df3b8b9 authored by Eulyeon Ko's avatar Eulyeon Ko

Add combine createNewIssue and addListNewIssue

addListNewIssue action lacked any spec coverage and
had a structure that was difficult to test.

This MR merges the two actions and improves the spec
for the combined action.

A placeholder issue should have a distinct id
in case a user creates a multiple issues in rapid
succession.
parent ca836edd
......@@ -12,6 +12,7 @@ import {
import { getIdFromGraphQLId } from '~/graphql_shared/utils';
import createGqClient, { fetchPolicies } from '~/lib/graphql';
import { convertObjectPropsToCamelCase, urlParamsToObject } from '~/lib/utils/common_utils';
import { s__ } from '~/locale';
import {
formatBoardLists,
formatListIssues,
......@@ -402,49 +403,51 @@ export default {
});
},
createNewIssue: ({ commit, state }, issueInput) => {
const { boardConfig } = state;
addListItem: ({ commit }, { list, item, position }) => {
commit(types.ADD_BOARD_ITEM_TO_LIST, { listId: list.id, itemId: item.id, atIndex: position });
commit(types.UPDATE_BOARD_ITEM, item);
},
removeListItem: ({ commit }, { listId, itemId }) => {
commit(types.REMOVE_BOARD_ITEM_FROM_LIST, { listId, itemId });
commit(types.REMOVE_BOARD_ITEM, itemId);
},
addListNewIssue: (
{ state: { boardConfig, boardType, fullPath }, dispatch, commit },
{ issueInput, list, placeholderId = `tmp-${new Date().getTime()}` },
) => {
const input = formatIssueInput(issueInput, boardConfig);
const { boardType, fullPath } = state;
if (boardType === BoardType.project) {
input.projectPath = fullPath;
}
return gqlClient
const placeholderIssue = formatIssue({ ...issueInput, id: placeholderId });
dispatch('addListItem', { list, item: placeholderIssue, position: 0 });
gqlClient
.mutate({
mutation: issueCreateMutation,
variables: { input },
})
.then(({ data }) => {
if (data.createIssue.errors.length) {
commit(types.CREATE_ISSUE_FAILURE);
} else {
return data.createIssue?.issue;
throw new Error();
}
return null;
})
.catch(() => commit(types.CREATE_ISSUE_FAILURE));
},
addListIssue: ({ commit }, { list, issue, position }) => {
commit(types.ADD_ISSUE_TO_LIST, { list, issue, position });
},
addListNewIssue: ({ commit, dispatch }, { issueInput, list }) => {
const issue = formatIssue({ ...issueInput, id: 'tmp' });
commit(types.ADD_ISSUE_TO_LIST, { list, issue, position: 0 });
dispatch('createNewIssue', issueInput)
.then((res) => {
commit(types.ADD_ISSUE_TO_LIST, {
list,
issue: formatIssue({ ...res, id: getIdFromGraphQLId(res.id) }),
});
commit(types.REMOVE_ISSUE_FROM_LIST, { list, issue });
const rawIssue = data.createIssue?.issue;
const formattedIssue = formatIssue({ ...rawIssue, id: getIdFromGraphQLId(rawIssue.id) });
dispatch('removeListItem', { listId: list.id, itemId: placeholderId });
dispatch('addListItem', { list, item: formattedIssue, position: 0 });
})
.catch(() => commit(types.ADD_ISSUE_TO_LIST_FAILURE, { list, issueId: issueInput.id }));
.catch(() => {
dispatch('removeListItem', { listId: list.id, itemId: placeholderId });
commit(
types.SET_ERROR,
s__('Boards|An error occurred while creating the issue. Please try again.'),
);
});
},
setActiveIssueLabels: async ({ commit, getters }, input) => {
......
......@@ -20,19 +20,19 @@ export const REMOVE_LIST_FAILURE = 'REMOVE_LIST_FAILURE';
export const REQUEST_ITEMS_FOR_LIST = 'REQUEST_ITEMS_FOR_LIST';
export const RECEIVE_ITEMS_FOR_LIST_FAILURE = 'RECEIVE_ITEMS_FOR_LIST_FAILURE';
export const RECEIVE_ITEMS_FOR_LIST_SUCCESS = 'RECEIVE_ITEMS_FOR_LIST_SUCCESS';
export const CREATE_ISSUE_FAILURE = 'CREATE_ISSUE_FAILURE';
export const REQUEST_ADD_ISSUE = 'REQUEST_ADD_ISSUE';
export const RECEIVE_ADD_ISSUE_SUCCESS = 'RECEIVE_ADD_ISSUE_SUCCESS';
export const RECEIVE_ADD_ISSUE_ERROR = 'RECEIVE_ADD_ISSUE_ERROR';
export const MOVE_ISSUE = 'MOVE_ISSUE';
export const MOVE_ISSUE_SUCCESS = 'MOVE_ISSUE_SUCCESS';
export const MOVE_ISSUE_FAILURE = 'MOVE_ISSUE_FAILURE';
export const UPDATE_BOARD_ITEM = 'UPDATE_BOARD_ITEM';
export const REMOVE_BOARD_ITEM = 'REMOVE_BOARD_ITEM';
export const REQUEST_UPDATE_ISSUE = 'REQUEST_UPDATE_ISSUE';
export const RECEIVE_UPDATE_ISSUE_SUCCESS = 'RECEIVE_UPDATE_ISSUE_SUCCESS';
export const RECEIVE_UPDATE_ISSUE_ERROR = 'RECEIVE_UPDATE_ISSUE_ERROR';
export const ADD_ISSUE_TO_LIST = 'ADD_ISSUE_TO_LIST';
export const ADD_ISSUE_TO_LIST_FAILURE = 'ADD_ISSUE_TO_LIST_FAILURE';
export const REMOVE_ISSUE_FROM_LIST = 'REMOVE_ISSUE_FROM_LIST';
export const ADD_BOARD_ITEM_TO_LIST = 'ADD_BOARD_ITEM_TO_LIST';
export const REMOVE_BOARD_ITEM_FROM_LIST = 'REMOVE_BOARD_ITEM_FROM_LIST';
export const SET_CURRENT_PAGE = 'SET_CURRENT_PAGE';
export const TOGGLE_EMPTY_STATE = 'TOGGLE_EMPTY_STATE';
export const SET_ACTIVE_ID = 'SET_ACTIVE_ID';
......
......@@ -229,28 +229,23 @@ export default {
notImplemented();
},
[mutationTypes.CREATE_ISSUE_FAILURE]: (state) => {
state.error = s__('Boards|An error occurred while creating the issue. Please try again.');
[mutationTypes.ADD_BOARD_ITEM_TO_LIST]: (
state,
{ itemId, listId, moveBeforeId, moveAfterId, atIndex },
) => {
addItemToList({ state, listId, itemId, moveBeforeId, moveAfterId, atIndex });
},
[mutationTypes.ADD_ISSUE_TO_LIST]: (state, { list, issue, position }) => {
addItemToList({
state,
listId: list.id,
itemId: issue.id,
atIndex: position,
});
Vue.set(state.boardItems, issue.id, issue);
[mutationTypes.REMOVE_BOARD_ITEM_FROM_LIST]: (state, { itemId, listId }) => {
removeItemFromList({ state, listId, itemId });
},
[mutationTypes.ADD_ISSUE_TO_LIST_FAILURE]: (state, { list, issueId }) => {
state.error = s__('Boards|An error occurred while creating the issue. Please try again.');
removeItemFromList({ state, listId: list.id, itemId: issueId });
[mutationTypes.UPDATE_BOARD_ITEM]: (state, item) => {
Vue.set(state.boardItems, item.id, item);
},
[mutationTypes.REMOVE_ISSUE_FROM_LIST]: (state, { list, issue }) => {
removeItemFromList({ state, listId: list.id, itemId: issue.id });
Vue.delete(state.boardItems, issue.id);
[mutationTypes.REMOVE_BOARD_ITEM]: (state, itemId) => {
Vue.delete(state.boardItems, itemId);
},
[mutationTypes.SET_CURRENT_PAGE]: () => {
......
---
title: If creating a new issue fails in boards, remove the issue card from a list
merge_request: 58558
author:
type: other
export * from '~/boards/stores/mutation_types';
export const REQUEST_AVAILABLE_BOARDS = 'REQUEST_AVAILABLE_BOARDS';
export const RECEIVE_AVAILABLE_BOARDS_SUCCESS = 'RECEIVE_AVAILABLE_BOARDS_SUCCESS';
export const RECEIVE_AVAILABLE_BOARDS_ERROR = 'RECEIVE_AVAILABLE_BOARDS_ERROR';
......@@ -8,29 +10,21 @@ export const REQUEST_REMOVE_BOARD = 'REQUEST_REMOVE_BOARD';
export const RECEIVE_REMOVE_BOARD_SUCCESS = 'RECEIVE_REMOVE_BOARD_SUCCESS';
export const RECEIVE_REMOVE_BOARD_ERROR = 'RECEIVE_REMOVE_BOARD_ERROR';
export const TOGGLE_PROMOTION_STATE = 'TOGGLE_PROMOTION_STATE';
export const REQUEST_ITEMS_FOR_LIST = 'REQUEST_ITEMS_FOR_LIST';
export const RECEIVE_ITEMS_FOR_LIST_FAILURE = 'RECEIVE_ITEMS_FOR_LIST_FAILURE';
export const RECEIVE_ITEMS_FOR_LIST_SUCCESS = 'RECEIVE_ITEMS_FOR_LIST_SUCCESS';
export const TOGGLE_EPICS_SWIMLANES = 'TOGGLE_EPICS_SWIMLANES';
export const SET_EPICS_SWIMLANES = 'SET_EPICS_SWIMLANES';
export const DONE_LOADING_SWIMLANES_ITEMS = 'DONE_LOADING_SWIMLANES_ITEMS';
export const RECEIVE_BOARD_LISTS_SUCCESS = 'RECEIVE_BOARD_LISTS_SUCCESS';
export const RECEIVE_BOARD_LISTS_FAILURE = 'RECEIVE_BOARD_LISTS_FAILURE';
export const REQUEST_ISSUES_FOR_EPIC = 'REQUEST_ISSUES_FOR_EPIC';
export const RECEIVE_ISSUES_FOR_EPIC_SUCCESS = 'RECEIVE_ISSUES_FOR_EPIC_SUCCESS';
export const RECEIVE_ISSUES_FOR_EPIC_FAILURE = 'RECEIVE_ISSUES_FOR_EPIC_FAILURE';
export const UPDATE_LIST_SUCCESS = 'UPDATE_LIST_SUCCESS';
export const UPDATE_LIST_FAILURE = 'UPDATE_LIST_FAILURE';
export const RECEIVE_SWIMLANES_FAILURE = 'RECEIVE_SWIMLANES_FAILURE';
export const RECEIVE_EPICS_SUCCESS = 'RECEIVE_EPICS_SUCCESS';
export const UPDATE_CACHED_EPICS = 'UPDATE_CACHED_EPICS';
export const SET_EPIC_FETCH_IN_PROGRESS = 'SET_EPIC_FETCH_IN_PROGRESS';
export const RESET_EPICS = 'RESET_EPICS';
export const SET_SHOW_LABELS = 'SET_SHOW_LABELS';
export const SET_FILTERS = 'SET_FILTERS';
export const MOVE_ISSUE = 'MOVE_ISSUE';
export const MOVE_ISSUE_SUCCESS = 'MOVE_ISSUE_SUCCESS';
export const MOVE_ISSUE_FAILURE = 'MOVE_ISSUE_FAILURE';
export const MOVE_EPIC = 'MOVE_EPIC';
export const MOVE_EPIC_FAILURE = 'MOVE_EPIC_FAILURE';
export const CREATE_LIST_FAILURE = 'CREATE_LIST_FAILURE';
export const SET_BOARD_EPIC_USER_PREFERENCES = 'SET_BOARD_EPIC_USER_PREFERENCES';
export const RECEIVE_MILESTONES_REQUEST = 'RECEIVE_MILESTONES_REQUEST';
export const RECEIVE_MILESTONES_SUCCESS = 'RECEIVE_MILESTONES_SUCCESS';
......
......@@ -5,6 +5,7 @@ import {
formatListIssues,
formatBoardLists,
formatIssueInput,
formatIssue,
} from '~/boards/boards_util';
import { inactiveId, ISSUABLE } from '~/boards/constants';
import destroyBoardListMutation from '~/boards/graphql/board_list_destroy.mutation.graphql';
......@@ -12,6 +13,7 @@ import issueCreateMutation from '~/boards/graphql/issue_create.mutation.graphql'
import issueMoveListMutation from '~/boards/graphql/issue_move_list.mutation.graphql';
import actions, { gqlClient } from '~/boards/stores/actions';
import * as types from '~/boards/stores/mutation_types';
import { getIdFromGraphQLId } from '~/graphql_shared/utils';
import {
mockLists,
mockListsById,
......@@ -816,7 +818,43 @@ describe('setAssignees', () => {
});
});
describe('createNewIssue', () => {
describe('addListItem', () => {
it('should commit ADD_BOARD_ITEM_TO_LIST and UPDATE_BOARD_ITEM mutations', () => {
const payload = {
list: mockLists[0],
item: mockIssue,
position: 0,
};
testAction(actions.addListItem, payload, {}, [
{
type: types.ADD_BOARD_ITEM_TO_LIST,
payload: {
listId: mockLists[0].id,
itemId: mockIssue.id,
atIndex: 0,
},
},
{ type: types.UPDATE_BOARD_ITEM, payload: mockIssue },
]);
});
});
describe('removeListItem', () => {
it('should commit REMOVE_BOARD_ITEM_FROM_LIST and REMOVE_BOARD_ITEM mutations', () => {
const payload = {
listId: mockLists[0].id,
itemId: mockIssue.id,
};
testAction(actions.removeListItem, payload, {}, [
{ type: types.REMOVE_BOARD_ITEM_FROM_LIST, payload },
{ type: types.REMOVE_BOARD_ITEM, payload: mockIssue.id },
]);
});
});
describe('addListNewIssue', () => {
const state = {
boardType: 'group',
fullPath: 'gitlab-org/gitlab',
......@@ -843,19 +881,7 @@ describe('createNewIssue', () => {
},
};
it('should return issue from API on success', async () => {
jest.spyOn(gqlClient, 'mutate').mockResolvedValue({
data: {
createIssue: {
issue: mockIssue,
errors: [],
},
},
});
const result = await actions.createNewIssue({ state }, mockIssue);
expect(result).toEqual(mockIssue);
});
const fakeList = { id: 'gid://gitlab/List/123' };
it('should add board scope to the issue being created', async () => {
jest.spyOn(gqlClient, 'mutate').mockResolvedValue({
......@@ -867,7 +893,11 @@ describe('createNewIssue', () => {
},
});
await actions.createNewIssue({ state: stateWithBoardConfig }, mockIssue);
await actions.addListNewIssue(
{ dispatch: jest.fn(), commit: jest.fn(), state: stateWithBoardConfig },
{ issueInput: mockIssue, list: fakeList },
);
expect(gqlClient.mutate).toHaveBeenCalledWith({
mutation: issueCreateMutation,
variables: {
......@@ -894,7 +924,11 @@ describe('createNewIssue', () => {
const payload = formatIssueInput(issue, stateWithBoardConfig.boardConfig);
await actions.createNewIssue({ state: stateWithBoardConfig }, issue);
await actions.addListNewIssue(
{ dispatch: jest.fn(), commit: jest.fn(), state: stateWithBoardConfig },
{ issueInput: issue, list: fakeList },
);
expect(gqlClient.mutate).toHaveBeenCalledWith({
mutation: issueCreateMutation,
variables: {
......@@ -905,45 +939,86 @@ describe('createNewIssue', () => {
expect(payload.assigneeIds).toEqual(['gid://gitlab/User/1', 'gid://gitlab/User/2']);
});
it('should commit CREATE_ISSUE_FAILURE mutation when API returns an error', (done) => {
jest.spyOn(gqlClient, 'mutate').mockResolvedValue({
data: {
createIssue: {
issue: mockIssue,
errors: [{ foo: 'bar' }],
describe('when issue creation mutation request succeeds', () => {
it('dispatches a correct set of mutations', () => {
jest.spyOn(gqlClient, 'mutate').mockResolvedValue({
data: {
createIssue: {
issue: mockIssue,
errors: [],
},
},
},
});
testAction({
action: actions.addListNewIssue,
payload: {
issueInput: mockIssue,
list: fakeList,
placeholderId: 'tmp',
},
state,
expectedActions: [
{
type: 'addListItem',
payload: {
list: fakeList,
item: formatIssue({ ...mockIssue, id: 'tmp' }),
position: 0,
},
},
{ type: 'removeListItem', payload: { listId: fakeList.id, itemId: 'tmp' } },
{
type: 'addListItem',
payload: {
list: fakeList,
item: formatIssue({ ...mockIssue, id: getIdFromGraphQLId(mockIssue.id) }),
position: 0,
},
},
],
});
});
const payload = mockIssue;
testAction(
actions.createNewIssue,
payload,
state,
[{ type: types.CREATE_ISSUE_FAILURE }],
[],
done,
);
});
});
describe('addListIssue', () => {
it('should commit ADD_ISSUE_TO_LIST mutation', (done) => {
const payload = {
list: mockLists[0],
issue: mockIssue,
position: 0,
};
testAction(
actions.addListIssue,
payload,
{},
[{ type: types.ADD_ISSUE_TO_LIST, payload }],
[],
done,
);
describe('when issue creation mutation request fails', () => {
it('dispatches a correct set of mutations', () => {
jest.spyOn(gqlClient, 'mutate').mockResolvedValue({
data: {
createIssue: {
issue: mockIssue,
errors: [{ foo: 'bar' }],
},
},
});
testAction({
action: actions.addListNewIssue,
payload: {
issueInput: mockIssue,
list: fakeList,
placeholderId: 'tmp',
},
state,
expectedActions: [
{
type: 'addListItem',
payload: {
list: fakeList,
item: formatIssue({ ...mockIssue, id: 'tmp' }),
position: 0,
},
},
{ type: 'removeListItem', payload: { listId: fakeList.id, itemId: 'tmp' } },
],
expectedMutations: [
{
type: types.SET_ERROR,
payload: 'An error occurred while creating the issue. Please try again.',
},
],
});
});
});
});
......
import { cloneDeep } from 'lodash';
import { issuableTypes } from '~/boards/constants';
import * as types from '~/boards/stores/mutation_types';
import mutations from '~/boards/stores/mutations';
......@@ -9,6 +10,7 @@ import {
mockIssue2,
mockGroupProjects,
labels,
mockList,
} from '../mock_data';
const expectNotImplemented = (action) => {
......@@ -25,6 +27,14 @@ describe('Board Store Mutations', () => {
'gid://gitlab/List/2': mockLists[1],
};
const setBoardsListsState = () => {
state = cloneDeep({
...state,
boardItemsByListId: { 'gid://gitlab/List/1': [mockIssue.id] },
boardLists: { 'gid://gitlab/List/1': mockList },
});
};
beforeEach(() => {
state = defaultState();
});
......@@ -467,6 +477,27 @@ describe('Board Store Mutations', () => {
});
});
describe('UPDATE_BOARD_ITEM', () => {
it('updates the given issue in state.boardItems', () => {
const updatedIssue = { id: 'some_gid', foo: 'bar' };
state = { boardItems: { some_gid: { id: 'some_gid' } } };
mutations.UPDATE_BOARD_ITEM(state, updatedIssue);
expect(state.boardItems.some_gid).toEqual(updatedIssue);
});
});
describe('REMOVE_BOARD_ITEM', () => {
it('removes the given issue from state.boardItems', () => {
state = { boardItems: { some_gid: {}, some_gid2: {} } };
mutations.REMOVE_BOARD_ITEM(state, 'some_gid');
expect(state.boardItems).toEqual({ some_gid2: {} });
});
});
describe('REQUEST_UPDATE_ISSUE', () => {
expectNotImplemented(mutations.REQUEST_UPDATE_ISSUE);
});
......@@ -479,85 +510,89 @@ describe('Board Store Mutations', () => {
expectNotImplemented(mutations.RECEIVE_UPDATE_ISSUE_ERROR);
});
describe('CREATE_ISSUE_FAILURE', () => {
it('sets error message on state', () => {
mutations.CREATE_ISSUE_FAILURE(state);
describe('ADD_BOARD_ITEM_TO_LIST', () => {
beforeEach(() => {
setBoardsListsState();
});
it.each([
[
'at position 0 by default',
{
payload: {
itemId: mockIssue2.id,
listId: mockList.id,
},
listState: [mockIssue2.id, mockIssue.id],
},
],
[
'at a given position',
{
payload: {
itemId: mockIssue2.id,
listId: mockList.id,
atIndex: 1,
},
listState: [mockIssue.id, mockIssue2.id],
},
],
[
"below the issue with id of 'moveBeforeId'",
{
payload: {
itemId: mockIssue2.id,
listId: mockList.id,
moveBeforeId: mockIssue.id,
},
listState: [mockIssue.id, mockIssue2.id],
},
],
[
"above the issue with id of 'moveAfterId'",
{
payload: {
itemId: mockIssue2.id,
listId: mockList.id,
moveAfterId: mockIssue.id,
},
listState: [mockIssue2.id, mockIssue.id],
},
],
])(`inserts an item into a list %s`, (_, { payload, listState }) => {
mutations.ADD_BOARD_ITEM_TO_LIST(state, payload);
expect(state.error).toBe('An error occurred while creating the issue. Please try again.');
expect(state.boardItemsByListId[payload.listId]).toEqual(listState);
});
});
describe('ADD_ISSUE_TO_LIST', () => {
it('adds issue to issues state and issue id in list in boardItemsByListId', () => {
const listIssues = {
'gid://gitlab/List/1': [mockIssue.id],
};
const issues = {
1: mockIssue,
};
state = {
...state,
boardItemsByListId: listIssues,
boardItems: issues,
boardLists: initialBoardListsState,
};
it("updates the list's items count", () => {
expect(state.boardLists['gid://gitlab/List/1'].issuesCount).toBe(1);
mutations.ADD_ISSUE_TO_LIST(state, { list: mockLists[0], issue: mockIssue2 });
mutations.ADD_BOARD_ITEM_TO_LIST(state, {
itemId: mockIssue2.id,
listId: mockList.id,
});
expect(state.boardItemsByListId['gid://gitlab/List/1']).toContain(mockIssue2.id);
expect(state.boardItems[mockIssue2.id]).toEqual(mockIssue2);
expect(state.boardLists['gid://gitlab/List/1'].issuesCount).toBe(2);
});
});
describe('ADD_ISSUE_TO_LIST_FAILURE', () => {
it('removes issue id from list in boardItemsByListId and sets error message', () => {
const listIssues = {
'gid://gitlab/List/1': [mockIssue.id, mockIssue2.id],
};
const issues = {
1: mockIssue,
2: mockIssue2,
};
state = {
...state,
boardItemsByListId: listIssues,
boardItems: issues,
boardLists: initialBoardListsState,
};
mutations.ADD_ISSUE_TO_LIST_FAILURE(state, { list: mockLists[0], issueId: mockIssue2.id });
expect(state.boardItemsByListId['gid://gitlab/List/1']).not.toContain(mockIssue2.id);
expect(state.error).toBe('An error occurred while creating the issue. Please try again.');
describe('REMOVE_BOARD_ITEM_FROM_LIST', () => {
beforeEach(() => {
setBoardsListsState();
});
});
describe('REMOVE_ISSUE_FROM_LIST', () => {
it('removes issue id from list in boardItemsByListId and deletes issue from state', () => {
const listIssues = {
'gid://gitlab/List/1': [mockIssue.id, mockIssue2.id],
};
const issues = {
1: mockIssue,
2: mockIssue2,
};
state = {
...state,
boardItemsByListId: listIssues,
boardItems: issues,
boardLists: initialBoardListsState,
};
it("removes an item from a list and updates the list's items count", () => {
expect(state.boardLists['gid://gitlab/List/1'].issuesCount).toBe(1);
expect(state.boardItemsByListId['gid://gitlab/List/1']).toContain(mockIssue.id);
mutations.ADD_ISSUE_TO_LIST_FAILURE(state, { list: mockLists[0], issueId: mockIssue2.id });
mutations.REMOVE_BOARD_ITEM_FROM_LIST(state, {
itemId: mockIssue.id,
listId: mockList.id,
});
expect(state.boardItemsByListId['gid://gitlab/List/1']).not.toContain(mockIssue2.id);
expect(state.boardItems).not.toContain(mockIssue2);
expect(state.boardItemsByListId['gid://gitlab/List/1']).not.toContain(mockIssue.id);
expect(state.boardLists['gid://gitlab/List/1'].issuesCount).toBe(0);
});
});
......
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