Commit 41285af4 authored by Phil Hughes's avatar Phil Hughes

Merge branch '42923-close-issue' into 'master'

Resolve "Can't close issue through buttons in the textarea on mobile"

Closes #42923

See merge request gitlab-org/gitlab-ce!17043
parents 4e846c73 d8d0f668
...@@ -24,32 +24,29 @@ export default class Issue { ...@@ -24,32 +24,29 @@ export default class Issue {
if (Issue.createMrDropdownWrap) { if (Issue.createMrDropdownWrap) {
this.createMergeRequestDropdown = new CreateMergeRequestDropdown(Issue.createMrDropdownWrap); this.createMergeRequestDropdown = new CreateMergeRequestDropdown(Issue.createMrDropdownWrap);
} }
}
initIssueBtnEventListeners() {
const issueFailMessage = 'Unable to update this issue at this time.';
return $(document).on('click', '.js-issuable-actions a.btn-close, .js-issuable-actions a.btn-reopen', (e) => { // Listen to state changes in the Vue app
var $button, shouldSubmit, url; document.addEventListener('issuable_vue_app:change', (event) => {
e.preventDefault(); this.updateTopState(event.detail.isClosed, event.detail.data);
e.stopImmediatePropagation(); });
$button = $(e.currentTarget);
shouldSubmit = $button.hasClass('btn-comment');
if (shouldSubmit) {
Issue.submitNoteForm($button.closest('form'));
} }
this.disableCloseReopenButton($button); /**
* This method updates the top area of the issue.
url = $button.attr('href'); *
return axios.put(url) * Once the issue state changes, either through a click on the top area (jquery)
.then(({ data }) => { * or a click on the bottom area (Vue) we need to update the top area.
*
* @param {Boolean} isClosed
* @param {Array} data
* @param {String} issueFailMessage
*/
updateTopState(isClosed, data, issueFailMessage = 'Unable to update this issue at this time.') {
if ('id' in data) {
const isClosedBadge = $('div.status-box-issue-closed'); const isClosedBadge = $('div.status-box-issue-closed');
const isOpenBadge = $('div.status-box-open'); const isOpenBadge = $('div.status-box-open');
const projectIssuesCounter = $('.issue_counter'); const projectIssuesCounter = $('.issue_counter');
if ('id' in data) {
const isClosed = $button.hasClass('btn-close');
isClosedBadge.toggleClass('hidden', !isClosed); isClosedBadge.toggleClass('hidden', !isClosed);
isOpenBadge.toggleClass('hidden', isClosed); isOpenBadge.toggleClass('hidden', isClosed);
...@@ -72,6 +69,28 @@ export default class Issue { ...@@ -72,6 +69,28 @@ export default class Issue {
} else { } else {
flash(issueFailMessage); flash(issueFailMessage);
} }
}
initIssueBtnEventListeners() {
const issueFailMessage = 'Unable to update this issue at this time.';
return $(document).on('click', '.js-issuable-actions a.btn-close, .js-issuable-actions a.btn-reopen', (e) => {
var $button, shouldSubmit, url;
e.preventDefault();
e.stopImmediatePropagation();
$button = $(e.currentTarget);
shouldSubmit = $button.hasClass('btn-comment');
if (shouldSubmit) {
Issue.submitNoteForm($button.closest('form'));
}
this.disableCloseReopenButton($button);
url = $button.attr('href');
return axios.put(url)
.then(({ data }) => {
const isClosed = $button.hasClass('btn-close');
this.updateTopState(isClosed, data);
}) })
.catch(() => flash(issueFailMessage)) .catch(() => flash(issueFailMessage))
.then(() => { .then(() => {
......
...@@ -2,16 +2,18 @@ ...@@ -2,16 +2,18 @@
import { mapActions, mapGetters } from 'vuex'; import { mapActions, mapGetters } from 'vuex';
import _ from 'underscore'; import _ from 'underscore';
import Autosize from 'autosize'; import Autosize from 'autosize';
import { __ } from '~/locale';
import Flash from '../../flash'; import Flash from '../../flash';
import Autosave from '../../autosave'; import Autosave from '../../autosave';
import TaskList from '../../task_list'; import TaskList from '../../task_list';
import * as constants from '../constants'; import * as constants from '../constants';
import eventHub from '../event_hub'; import eventHub from '../event_hub';
import issueWarning from '../../vue_shared/components/issue/issue_warning.vue'; import issueWarning from '../../vue_shared/components/issue/issue_warning.vue';
import noteSignedOutWidget from './note_signed_out_widget.vue';
import discussionLockedWidget from './discussion_locked_widget.vue';
import markdownField from '../../vue_shared/components/markdown/field.vue'; import markdownField from '../../vue_shared/components/markdown/field.vue';
import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue';
import loadingButton from '../../vue_shared/components/loading_button.vue';
import noteSignedOutWidget from './note_signed_out_widget.vue';
import discussionLockedWidget from './discussion_locked_widget.vue';
import issuableStateMixin from '../mixins/issuable_state'; import issuableStateMixin from '../mixins/issuable_state';
export default { export default {
...@@ -22,6 +24,7 @@ ...@@ -22,6 +24,7 @@
discussionLockedWidget, discussionLockedWidget,
markdownField, markdownField,
userAvatarLink, userAvatarLink,
loadingButton,
}, },
mixins: [ mixins: [
issuableStateMixin, issuableStateMixin,
...@@ -30,9 +33,6 @@ ...@@ -30,9 +33,6 @@
return { return {
note: '', note: '',
noteType: constants.COMMENT, noteType: constants.COMMENT,
// Can't use mapGetters,
// this needs to be in the data object because it belongs to the state
issueState: this.$store.getters.getNoteableData.state,
isSubmitting: false, isSubmitting: false,
isSubmitButtonDisabled: true, isSubmitButtonDisabled: true,
}; };
...@@ -43,6 +43,7 @@ ...@@ -43,6 +43,7 @@
'getUserData', 'getUserData',
'getNoteableData', 'getNoteableData',
'getNotesData', 'getNotesData',
'issueState',
]), ]),
isLoggedIn() { isLoggedIn() {
return this.getUserData.id; return this.getUserData.id;
...@@ -105,7 +106,7 @@ ...@@ -105,7 +106,7 @@
mounted() { mounted() {
// jQuery is needed here because it is a custom event being dispatched with jQuery. // jQuery is needed here because it is a custom event being dispatched with jQuery.
$(document).on('issuable:change', (e, isClosed) => { $(document).on('issuable:change', (e, isClosed) => {
this.issueState = isClosed ? constants.CLOSED : constants.REOPENED; this.toggleIssueLocalState(isClosed ? constants.CLOSED : constants.REOPENED);
}); });
this.initAutoSave(); this.initAutoSave();
...@@ -117,6 +118,9 @@ ...@@ -117,6 +118,9 @@
'stopPolling', 'stopPolling',
'restartPolling', 'restartPolling',
'removePlaceholderNotes', 'removePlaceholderNotes',
'closeIssue',
'reopenIssue',
'toggleIssueLocalState',
]), ]),
setIsSubmitButtonDisabled(note, isSubmitting) { setIsSubmitButtonDisabled(note, isSubmitting) {
if (!_.isEmpty(note) && !isSubmitting) { if (!_.isEmpty(note) && !isSubmitting) {
...@@ -126,6 +130,8 @@ ...@@ -126,6 +130,8 @@
} }
}, },
handleSave(withIssueAction) { handleSave(withIssueAction) {
this.isSubmitting = true;
if (this.note.length) { if (this.note.length) {
const noteData = { const noteData = {
endpoint: this.endpoint, endpoint: this.endpoint,
...@@ -142,7 +148,6 @@ ...@@ -142,7 +148,6 @@
if (this.noteType === constants.DISCUSSION) { if (this.noteType === constants.DISCUSSION) {
noteData.data.note.type = constants.DISCUSSION_NOTE; noteData.data.note.type = constants.DISCUSSION_NOTE;
} }
this.isSubmitting = true;
this.note = ''; // Empty textarea while being requested. Repopulate in catch this.note = ''; // Empty textarea while being requested. Repopulate in catch
this.resizeTextarea(); this.resizeTextarea();
this.stopPolling(); this.stopPolling();
...@@ -184,13 +189,25 @@ Please check your network connection and try again.`; ...@@ -184,13 +189,25 @@ Please check your network connection and try again.`;
this.toggleIssueState(); this.toggleIssueState();
} }
}, },
enableButton() {
this.isSubmitting = false;
},
toggleIssueState() { toggleIssueState() {
this.issueState = this.isIssueOpen ? constants.CLOSED : constants.REOPENED; if (this.isIssueOpen) {
this.closeIssue()
// This is out of scope for the Notes Vue component. .then(() => this.enableButton())
// It was the shortest path to update the issue state and relevant places. .catch(() => {
const btnClass = this.isIssueOpen ? 'btn-reopen' : 'btn-close'; this.enableButton();
$(`.js-btn-issue-action.${btnClass}:visible`).trigger('click'); Flash(__('Something went wrong while closing the issue. Please try again later'));
});
} else {
this.reopenIssue()
.then(() => this.enableButton())
.catch(() => {
this.enableButton();
Flash(__('Something went wrong while reopening the issue. Please try again later'));
});
}
}, },
discard(shouldClear = true) { discard(shouldClear = true) {
// `blur` is needed to clear slash commands autocomplete cache if event fired. // `blur` is needed to clear slash commands autocomplete cache if event fired.
...@@ -367,15 +384,19 @@ append-right-10 comment-type-dropdown js-comment-type-dropdown droplab-dropdown" ...@@ -367,15 +384,19 @@ append-right-10 comment-type-dropdown js-comment-type-dropdown droplab-dropdown"
</li> </li>
</ul> </ul>
</div> </div>
<button
type="button" <loading-button
@click="handleSave(true)"
v-if="canUpdateIssue" v-if="canUpdateIssue"
:class="actionButtonClassNames" :loading="isSubmitting"
@click="handleSave(true)"
:container-class="[
actionButtonClassNames,
'btn btn-comment btn-comment-and-close js-action-button'
]"
:disabled="isSubmitting" :disabled="isSubmitting"
class="btn btn-comment btn-comment-and-close js-action-button"> :label="issueActionButtonTitle"
{{ issueActionButtonTitle }} />
</button>
<button <button
type="button" type="button"
v-if="note.length" v-if="note.length"
......
...@@ -28,6 +28,8 @@ document.addEventListener('DOMContentLoaded', () => new Vue({ ...@@ -28,6 +28,8 @@ document.addEventListener('DOMContentLoaded', () => new Vue({
notesPath: notesDataset.notesPath, notesPath: notesDataset.notesPath,
markdownDocsPath: notesDataset.markdownDocsPath, markdownDocsPath: notesDataset.markdownDocsPath,
quickActionsDocsPath: notesDataset.quickActionsDocsPath, quickActionsDocsPath: notesDataset.quickActionsDocsPath,
closeIssuePath: notesDataset.closeIssuePath,
reopenIssuePath: notesDataset.reopenIssuePath,
}, },
}; };
}, },
......
...@@ -32,4 +32,7 @@ export default { ...@@ -32,4 +32,7 @@ export default {
toggleAward(endpoint, data) { toggleAward(endpoint, data) {
return Vue.http.post(endpoint, data, { emulateJSON: true }); return Vue.http.post(endpoint, data, { emulateJSON: true });
}, },
toggleIssueState(endpoint, data) {
return Vue.http.put(endpoint, data);
},
}; };
...@@ -61,6 +61,39 @@ export const createNewNote = ({ commit }, { endpoint, data }) => service ...@@ -61,6 +61,39 @@ export const createNewNote = ({ commit }, { endpoint, data }) => service
export const removePlaceholderNotes = ({ commit }) => export const removePlaceholderNotes = ({ commit }) =>
commit(types.REMOVE_PLACEHOLDER_NOTES); commit(types.REMOVE_PLACEHOLDER_NOTES);
export const closeIssue = ({ commit, dispatch, state }) => service
.toggleIssueState(state.notesData.closeIssuePath)
.then(res => res.json())
.then((data) => {
commit(types.CLOSE_ISSUE);
dispatch('emitStateChangedEvent', data);
});
export const reopenIssue = ({ commit, dispatch, state }) => service
.toggleIssueState(state.notesData.reopenIssuePath)
.then(res => res.json())
.then((data) => {
commit(types.REOPEN_ISSUE);
dispatch('emitStateChangedEvent', data);
});
export const emitStateChangedEvent = ({ commit, getters }, data) => {
const event = new CustomEvent('issuable_vue_app:change', { detail: {
data,
isClosed: getters.issueState === constants.CLOSED,
} });
document.dispatchEvent(event);
};
export const toggleIssueLocalState = ({ commit }, newState) => {
if (newState === constants.CLOSED) {
commit(types.CLOSE_ISSUE);
} else if (newState === constants.REOPENED) {
commit(types.REOPEN_ISSUE);
}
};
export const saveNote = ({ commit, dispatch }, noteData) => { export const saveNote = ({ commit, dispatch }, noteData) => {
const { note } = noteData.data.note; const { note } = noteData.data.note;
let placeholderText = note; let placeholderText = note;
......
...@@ -8,6 +8,7 @@ export const getNotesDataByProp = state => prop => state.notesData[prop]; ...@@ -8,6 +8,7 @@ export const getNotesDataByProp = state => prop => state.notesData[prop];
export const getNoteableData = state => state.noteableData; export const getNoteableData = state => state.noteableData;
export const getNoteableDataByProp = state => prop => state.noteableData[prop]; export const getNoteableDataByProp = state => prop => state.noteableData[prop];
export const issueState = state => state.noteableData.state;
export const getUserData = state => state.userData || {}; export const getUserData = state => state.userData || {};
export const getUserDataByProp = state => prop => state.userData && state.userData[prop]; export const getUserDataByProp = state => prop => state.userData && state.userData[prop];
......
...@@ -12,3 +12,7 @@ export const SHOW_PLACEHOLDER_NOTE = 'SHOW_PLACEHOLDER_NOTE'; ...@@ -12,3 +12,7 @@ export const SHOW_PLACEHOLDER_NOTE = 'SHOW_PLACEHOLDER_NOTE';
export const TOGGLE_AWARD = 'TOGGLE_AWARD'; export const TOGGLE_AWARD = 'TOGGLE_AWARD';
export const TOGGLE_DISCUSSION = 'TOGGLE_DISCUSSION'; export const TOGGLE_DISCUSSION = 'TOGGLE_DISCUSSION';
export const UPDATE_NOTE = 'UPDATE_NOTE'; export const UPDATE_NOTE = 'UPDATE_NOTE';
// Issue
export const CLOSE_ISSUE = 'CLOSE_ISSUE';
export const REOPEN_ISSUE = 'REOPEN_ISSUE';
...@@ -152,4 +152,12 @@ export default { ...@@ -152,4 +152,12 @@ export default {
noteObj.notes.splice(noteObj.notes.indexOf(comment), 1, note); noteObj.notes.splice(noteObj.notes.indexOf(comment), 1, note);
} }
}, },
[types.CLOSE_ISSUE](state) {
Object.assign(state.noteableData, { state: constants.CLOSED });
},
[types.REOPEN_ISSUE](state) {
Object.assign(state.noteableData, { state: constants.REOPENED });
},
}; };
...@@ -40,7 +40,7 @@ ...@@ -40,7 +40,7 @@
required: false, required: false,
}, },
containerClass: { containerClass: {
type: String, type: [String, Array, Object],
required: false, required: false,
default: 'btn btn-align-content', default: 'btn btn-align-content',
}, },
......
...@@ -12,6 +12,8 @@ ...@@ -12,6 +12,8 @@
markdown_docs_path: help_page_path('user/markdown'), markdown_docs_path: help_page_path('user/markdown'),
quick_actions_docs_path: help_page_path('user/project/quick_actions'), quick_actions_docs_path: help_page_path('user/project/quick_actions'),
notes_path: notes_url, notes_path: notes_url,
close_issue_path: issue_path(@issue, issue: { state_event: :close }, format: 'json'),
reopen_issue_path: issue_path(@issue, issue: { state_event: :reopen }, format: 'json'),
last_fetched_at: Time.now.to_i, last_fetched_at: Time.now.to_i,
noteable_data: serialize_issuable(@issue), noteable_data: serialize_issuable(@issue),
current_user_data: UserSerializer.new.represent(current_user, only_path: true).to_json } } current_user_data: UserSerializer.new.represent(current_user, only_path: true).to_json } }
---
title: Fix close button on issues not working on mobile
merge_request:
author:
type: fixed
// eslint-disable-next-line import/prefer-default-export
export const resetStore = (store) => {
store.replaceState({
notes: [],
targetNoteHash: null,
lastFetchedAt: null,
notesData: {},
userData: {},
noteableData: {},
});
};
...@@ -7,6 +7,8 @@ export const notesDataMock = { ...@@ -7,6 +7,8 @@ export const notesDataMock = {
notesPath: '/gitlab-org/gitlab-ce/noteable/issue/98/notes', notesPath: '/gitlab-org/gitlab-ce/noteable/issue/98/notes',
quickActionsDocsPath: '/help/user/project/quick_actions', quickActionsDocsPath: '/help/user/project/quick_actions',
registerPath: '/users/sign_in?redirect_to_referer=yes#register-pane', registerPath: '/users/sign_in?redirect_to_referer=yes#register-pane',
closeIssuePath: '/twitter/flight/issues/9.json?issue%5Bstate_event%5D=close',
reopenIssuePath: '/twitter/flight/issues/9.json?issue%5Bstate_event%5D=reopen',
}; };
export const userDataMock = { export const userDataMock = {
......
import Vue from 'vue';
import _ from 'underscore';
import * as actions from '~/notes/stores/actions'; import * as actions from '~/notes/stores/actions';
import store from '~/notes/stores';
import testAction from '../../helpers/vuex_action_helper'; import testAction from '../../helpers/vuex_action_helper';
import { resetStore } from '../helpers';
import { discussionMock, notesDataMock, userDataMock, noteableDataMock, individualNote } from '../mock_data'; import { discussionMock, notesDataMock, userDataMock, noteableDataMock, individualNote } from '../mock_data';
describe('Actions Notes Store', () => { describe('Actions Notes Store', () => {
afterEach(() => {
resetStore(store);
});
describe('setNotesData', () => { describe('setNotesData', () => {
it('should set received notes data', (done) => { it('should set received notes data', (done) => {
testAction(actions.setNotesData, null, { notesData: {} }, [ testAction(actions.setNotesData, null, { notesData: {} }, [
...@@ -58,4 +66,67 @@ describe('Actions Notes Store', () => { ...@@ -58,4 +66,67 @@ describe('Actions Notes Store', () => {
], done); ], done);
}); });
}); });
describe('async methods', () => {
const interceptor = (request, next) => {
next(request.respondWith(JSON.stringify({}), {
status: 200,
}));
};
beforeEach(() => {
Vue.http.interceptors.push(interceptor);
});
afterEach(() => {
Vue.http.interceptors = _.without(Vue.http.interceptors, interceptor);
});
describe('closeIssue', () => {
it('sets state as closed', (done) => {
store.dispatch('closeIssue', { notesData: { closeIssuePath: '' } })
.then(() => {
expect(store.state.noteableData.state).toEqual('closed');
done();
})
.catch(done.fail);
});
});
describe('reopenIssue', () => {
it('sets state as reopened', (done) => {
store.dispatch('reopenIssue', { notesData: { reopenIssuePath: '' } })
.then(() => {
expect(store.state.noteableData.state).toEqual('reopened');
done();
})
.catch(done.fail);
});
});
});
describe('emitStateChangedEvent', () => {
it('emits an event on the document', () => {
document.addEventListener('issuable_vue_app:change', (event) => {
expect(event.detail.data).toEqual({ id: '1', state: 'closed' });
expect(event.detail.isClosed).toEqual(false);
});
store.dispatch('emitStateChangedEvent', { id: '1', state: 'closed' });
});
});
describe('toggleIssueLocalState', () => {
it('sets issue state as closed', (done) => {
testAction(actions.toggleIssueLocalState, 'closed', {}, [
{ type: 'CLOSE_ISSUE', payload: 'closed' },
], done);
});
it('sets issue state as reopened', (done) => {
testAction(actions.toggleIssueLocalState, 'reopened', {}, [
{ type: 'REOPEN_ISSUE', payload: 'reopened' },
], done);
});
});
}); });
...@@ -55,4 +55,10 @@ describe('Getters Notes Store', () => { ...@@ -55,4 +55,10 @@ describe('Getters Notes Store', () => {
expect(getters.getCurrentUserLastNote(state)).toEqual(individualNote.notes[0]); expect(getters.getCurrentUserLastNote(state)).toEqual(individualNote.notes[0]);
}); });
}); });
describe('issueState', () => {
it('should return the issue state', () => {
expect(getters.issueState(state)).toEqual(noteableDataMock.state);
});
});
}); });
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