Commit 16dcca08 authored by Phil Hughes's avatar Phil Hughes

Merge branch 'tor/feature/unify-merging-states' into 'master'

Unify "merging" MR Widgets

See merge request gitlab-org/gitlab!68519
parents 3c5a3c7e 6b778dbf
...@@ -29,6 +29,7 @@ import { ...@@ -29,6 +29,7 @@ import {
WARNING, WARNING,
MT_MERGE_STRATEGY, MT_MERGE_STRATEGY,
PIPELINE_FAILED_STATE, PIPELINE_FAILED_STATE,
STATE_MACHINE,
} from '../../constants'; } from '../../constants';
import eventHub from '../../event_hub'; import eventHub from '../../event_hub';
import mergeRequestQueryVariablesMixin from '../../mixins/merge_request_query_variables'; import mergeRequestQueryVariablesMixin from '../../mixins/merge_request_query_variables';
...@@ -47,6 +48,9 @@ const MERGE_FAILED_STATUS = 'failed'; ...@@ -47,6 +48,9 @@ const MERGE_FAILED_STATUS = 'failed';
const MERGE_SUCCESS_STATUS = 'success'; const MERGE_SUCCESS_STATUS = 'success';
const MERGE_HOOK_VALIDATION_ERROR_STATUS = 'hook_validation_error'; const MERGE_HOOK_VALIDATION_ERROR_STATUS = 'hook_validation_error';
const { transitions } = STATE_MACHINE;
const { MERGE, MERGED, MERGE_FAILURE } = transitions;
export default { export default {
name: 'ReadyToMerge', name: 'ReadyToMerge',
apollo: { apollo: {
...@@ -361,6 +365,7 @@ export default { ...@@ -361,6 +365,7 @@ export default {
} }
this.isMakingRequest = true; this.isMakingRequest = true;
this.mr.transitionStateMachine({ transition: MERGE });
this.service this.service
.merge(options) .merge(options)
.then((res) => res.data) .then((res) => res.data)
...@@ -375,6 +380,7 @@ export default { ...@@ -375,6 +380,7 @@ export default {
this.initiateMergePolling(); this.initiateMergePolling();
} else if (hasError) { } else if (hasError) {
eventHub.$emit('FailedToMerge', data.merge_error); eventHub.$emit('FailedToMerge', data.merge_error);
this.mr.transitionStateMachine({ transition: MERGE_FAILURE });
} }
if (this.glFeatures.mergeRequestWidgetGraphql) { if (this.glFeatures.mergeRequestWidgetGraphql) {
...@@ -383,6 +389,7 @@ export default { ...@@ -383,6 +389,7 @@ export default {
}) })
.catch(() => { .catch(() => {
this.isMakingRequest = false; this.isMakingRequest = false;
this.mr.transitionStateMachine({ transition: MERGE_FAILURE });
createFlash({ createFlash({
message: __('Something went wrong. Please try again.'), message: __('Something went wrong. Please try again.'),
}); });
...@@ -417,6 +424,7 @@ export default { ...@@ -417,6 +424,7 @@ export default {
eventHub.$emit('FetchActionsContent'); eventHub.$emit('FetchActionsContent');
MergeRequest.hideCloseButton(); MergeRequest.hideCloseButton();
MergeRequest.decreaseCounter(); MergeRequest.decreaseCounter();
this.mr.transitionStateMachine({ transition: MERGED });
stopPolling(); stopPolling();
refreshUserMergeRequestCounts(); refreshUserMergeRequestCounts();
...@@ -428,6 +436,7 @@ export default { ...@@ -428,6 +436,7 @@ export default {
} }
} else if (data.merge_error) { } else if (data.merge_error) {
eventHub.$emit('FailedToMerge', data.merge_error); eventHub.$emit('FailedToMerge', data.merge_error);
this.mr.transitionStateMachine({ transition: MERGE_FAILURE });
stopPolling(); stopPolling();
} else { } else {
// MR is not merged yet, continue polling until the state becomes 'merged' // MR is not merged yet, continue polling until the state becomes 'merged'
...@@ -438,6 +447,7 @@ export default { ...@@ -438,6 +447,7 @@ export default {
createFlash({ createFlash({
message: __('Something went wrong while merging this merge request. Please try again.'), message: __('Something went wrong while merging this merge request. Please try again.'),
}); });
this.mr.transitionStateMachine({ transition: MERGE_FAILURE });
stopPolling(); stopPolling();
}); });
}, },
......
import { s__ } from '~/locale'; import { s__ } from '~/locale';
import { stateToComponentMap as classStateMap, stateKey } from './stores/state_maps';
export const SUCCESS = 'success'; export const SUCCESS = 'success';
export const WARNING = 'warning'; export const WARNING = 'warning';
...@@ -52,3 +53,42 @@ export const MERGE_ACTIVE_STATUS_PHRASES = [ ...@@ -52,3 +53,42 @@ export const MERGE_ACTIVE_STATUS_PHRASES = [
emoji: 'heart_eyes', emoji: 'heart_eyes',
}, },
]; ];
const STATE_MACHINE = {
states: {
IDLE: 'IDLE',
MERGING: 'MERGING',
},
transitions: {
MERGE: 'start-merge',
MERGE_FAILURE: 'merge-failed',
MERGED: 'merge-done',
},
};
const { states, transitions } = STATE_MACHINE;
STATE_MACHINE.definition = {
initial: states.IDLE,
states: {
[states.IDLE]: {
on: {
[transitions.MERGE]: states.MERGING,
},
},
[states.MERGING]: {
on: {
[transitions.MERGED]: states.IDLE,
[transitions.MERGE_FAILURE]: states.IDLE,
},
},
},
};
export const stateToTransitionMap = {
[stateKey.merging]: transitions.MERGE,
[stateKey.merged]: transitions.MERGED,
};
export const stateToComponentMap = {
[states.MERGING]: classStateMap[stateKey.merging],
};
export { STATE_MACHINE };
...@@ -4,7 +4,7 @@ import { isEmpty } from 'lodash'; ...@@ -4,7 +4,7 @@ import { isEmpty } from 'lodash';
import MrWidgetApprovals from 'ee_else_ce/vue_merge_request_widget/components/approvals/approvals.vue'; import MrWidgetApprovals from 'ee_else_ce/vue_merge_request_widget/components/approvals/approvals.vue';
import MRWidgetService from 'ee_else_ce/vue_merge_request_widget/services/mr_widget_service'; import MRWidgetService from 'ee_else_ce/vue_merge_request_widget/services/mr_widget_service';
import MRWidgetStore from 'ee_else_ce/vue_merge_request_widget/stores/mr_widget_store'; import MRWidgetStore from 'ee_else_ce/vue_merge_request_widget/stores/mr_widget_store';
import stateMaps from 'ee_else_ce/vue_merge_request_widget/stores/state_maps'; import { stateToComponentMap as classState } from 'ee_else_ce/vue_merge_request_widget/stores/state_maps';
import createFlash from '~/flash'; import createFlash from '~/flash';
import { secondsToMilliseconds } from '~/lib/utils/datetime_utility'; import { secondsToMilliseconds } from '~/lib/utils/datetime_utility';
import notify from '~/lib/utils/notify'; import notify from '~/lib/utils/notify';
...@@ -39,6 +39,7 @@ import ShaMismatch from './components/states/sha_mismatch.vue'; ...@@ -39,6 +39,7 @@ import ShaMismatch from './components/states/sha_mismatch.vue';
import UnresolvedDiscussionsState from './components/states/unresolved_discussions.vue'; import UnresolvedDiscussionsState from './components/states/unresolved_discussions.vue';
import WorkInProgressState from './components/states/work_in_progress.vue'; import WorkInProgressState from './components/states/work_in_progress.vue';
import ExtensionsContainer from './components/extensions/container'; import ExtensionsContainer from './components/extensions/container';
import { STATE_MACHINE, stateToComponentMap } from './constants';
import eventHub from './event_hub'; import eventHub from './event_hub';
import mergeRequestQueryVariablesMixin from './mixins/merge_request_query_variables'; import mergeRequestQueryVariablesMixin from './mixins/merge_request_query_variables';
import getStateQuery from './queries/get_state.query.graphql'; import getStateQuery from './queries/get_state.query.graphql';
...@@ -124,7 +125,9 @@ export default { ...@@ -124,7 +125,9 @@ export default {
mr: store, mr: store,
state: store && store.state, state: store && store.state,
service: store && this.createService(store), service: store && this.createService(store),
machineState: store?.machineValue || STATE_MACHINE.definition.initial,
loading: true, loading: true,
recomputeComponentName: 0,
}; };
}, },
computed: { computed: {
...@@ -139,7 +142,7 @@ export default { ...@@ -139,7 +142,7 @@ export default {
return this.mr.state !== 'nothingToMerge'; return this.mr.state !== 'nothingToMerge';
}, },
componentName() { componentName() {
return stateMaps.stateToComponentMap[this.mr.state]; return stateToComponentMap[this.machineState] || classState[this.mr.state];
}, },
hasPipelineMustSucceedConflict() { hasPipelineMustSucceedConflict() {
return !this.mr.hasCI && this.mr.onlyAllowMergeIfPipelineSucceeds; return !this.mr.hasCI && this.mr.onlyAllowMergeIfPipelineSucceeds;
...@@ -206,6 +209,11 @@ export default { ...@@ -206,6 +209,11 @@ export default {
}, },
}, },
watch: { watch: {
'mr.machineValue': {
handler(newValue) {
this.machineState = newValue;
},
},
state(newVal, oldVal) { state(newVal, oldVal) {
if (newVal !== oldVal && this.shouldRenderMergedPipeline) { if (newVal !== oldVal && this.shouldRenderMergedPipeline) {
// init polling // init polling
...@@ -247,6 +255,8 @@ export default { ...@@ -247,6 +255,8 @@ export default {
this.mr = new MRWidgetStore({ ...window.gl.mrWidgetData, ...data }); this.mr = new MRWidgetStore({ ...window.gl.mrWidgetData, ...data });
} }
this.machineState = this.mr.machineValue;
if (!this.state) { if (!this.state) {
this.state = this.mr.state; this.state = this.mr.state;
} }
......
import getStateKey from 'ee_else_ce/vue_merge_request_widget/stores/get_state_key'; import getStateKey from 'ee_else_ce/vue_merge_request_widget/stores/get_state_key';
import { statusBoxState } from '~/issuable/components/status_box.vue'; import { statusBoxState } from '~/issuable/components/status_box.vue';
import { formatDate, getTimeago } from '~/lib/utils/datetime_utility'; import { formatDate, getTimeago } from '~/lib/utils/datetime_utility';
import { MTWPS_MERGE_STRATEGY, MT_MERGE_STRATEGY, MWPS_MERGE_STRATEGY } from '../constants'; import { machine } from '~/lib/utils/finite_state_machine';
import {
MTWPS_MERGE_STRATEGY,
MT_MERGE_STRATEGY,
MWPS_MERGE_STRATEGY,
STATE_MACHINE,
stateToTransitionMap,
} from '../constants';
import { stateKey } from './state_maps'; import { stateKey } from './state_maps';
const { format } = getTimeago(); const { format } = getTimeago();
const { states } = STATE_MACHINE;
const { IDLE } = states;
export default class MergeRequestStore { export default class MergeRequestStore {
constructor(data) { constructor(data) {
this.sha = data.diff_head_sha; this.sha = data.diff_head_sha;
...@@ -16,6 +26,9 @@ export default class MergeRequestStore { ...@@ -16,6 +26,9 @@ export default class MergeRequestStore {
this.apiUnapprovePath = data.api_unapprove_path; this.apiUnapprovePath = data.api_unapprove_path;
this.hasApprovalsAvailable = data.has_approvals_available; this.hasApprovalsAvailable = data.has_approvals_available;
this.stateMachine = machine(STATE_MACHINE.definition);
this.machineValue = this.stateMachine.value;
this.setPaths(data); this.setPaths(data);
this.setData(data); this.setData(data);
...@@ -215,10 +228,7 @@ export default class MergeRequestStore { ...@@ -215,10 +228,7 @@ export default class MergeRequestStore {
setState() { setState() {
if (this.mergeOngoing) { if (this.mergeOngoing) {
this.state = 'merging'; this.state = 'merging';
return; } else if (this.isOpen) {
}
if (this.isOpen) {
this.state = getStateKey.call(this); this.state = getStateKey.call(this);
} else { } else {
switch (this.mergeRequestState) { switch (this.mergeRequestState) {
...@@ -232,6 +242,8 @@ export default class MergeRequestStore { ...@@ -232,6 +242,8 @@ export default class MergeRequestStore {
this.state = null; this.state = null;
} }
} }
this.translateStateToMachine();
} }
setPaths(data) { setPaths(data) {
...@@ -356,4 +368,32 @@ export default class MergeRequestStore { ...@@ -356,4 +368,32 @@ export default class MergeRequestStore {
(this.onlyAllowMergeIfPipelineSucceeds && this.isPipelineFailed) (this.onlyAllowMergeIfPipelineSucceeds && this.isPipelineFailed)
); );
} }
// Because the state machine doesn't yet handle every state and transition,
// some use-cases will need to force a state that can't be reached by
// a known transition. This is undesirable long-term (as it subverts
// the intent of a state machine), but is necessary until the machine
// can handle all possible combinations. (unsafeForce)
transitionStateMachine({ transition, state, unsafeForce = false } = {}) {
if (unsafeForce && state) {
this.stateMachine.value = state;
} else {
this.stateMachine.send(transition);
}
this.machineValue = this.stateMachine.value;
}
translateStateToMachine() {
const transition = stateToTransitionMap[this.state];
let transitionOptions = {
state: IDLE,
unsafeForce: true,
};
if (transition) {
transitionOptions = { transition };
}
this.transitionStateMachine(transitionOptions);
}
} }
const stateToComponentMap = { export const stateToComponentMap = {
merged: 'mr-widget-merged', merged: 'mr-widget-merged',
closed: 'mr-widget-closed', closed: 'mr-widget-closed',
merging: 'mr-widget-merging', merging: 'mr-widget-merging',
...@@ -21,7 +21,7 @@ const stateToComponentMap = { ...@@ -21,7 +21,7 @@ const stateToComponentMap = {
mergeChecksFailed: 'mergeChecksFailed', mergeChecksFailed: 'mergeChecksFailed',
}; };
const statesToShowHelpWidget = [ export const statesToShowHelpWidget = [
'merging', 'merging',
'conflicts', 'conflicts',
'workInProgress', 'workInProgress',
...@@ -50,11 +50,7 @@ export const stateKey = { ...@@ -50,11 +50,7 @@ export const stateKey = {
notAllowedToMerge: 'notAllowedToMerge', notAllowedToMerge: 'notAllowedToMerge',
readyToMerge: 'readyToMerge', readyToMerge: 'readyToMerge',
rebase: 'rebase', rebase: 'rebase',
merging: 'merging',
merged: 'merged', merged: 'merged',
mergeChecksFailed: 'mergeChecksFailed', mergeChecksFailed: 'mergeChecksFailed',
}; };
export default {
stateToComponentMap,
statesToShowHelpWidget,
};
import stateMaps from '~/vue_merge_request_widget/stores/state_maps'; import { stateToComponentMap as ceStateMap } from '~/vue_merge_request_widget/stores/state_maps';
stateMaps.stateToComponentMap.geoSecondaryNode = 'mr-widget-geo-secondary-node';
stateMaps.stateToComponentMap.policyViolation = 'mr-widget-policy-violation';
stateMaps.stateToComponentMap.jiraAssociationMissing = 'mr-widget-jira-association-missing';
export { statesToShowHelpWidget } from '~/vue_merge_request_widget/stores/state_maps';
export const stateKey = { export const stateKey = {
policyViolation: 'policyViolation', policyViolation: 'policyViolation',
jiraAssociationMissing: 'jiraAssociationMissing', jiraAssociationMissing: 'jiraAssociationMissing',
}; };
export const stateToComponentMap = {
export default { ...ceStateMap,
stateToComponentMap: stateMaps.stateToComponentMap, geoSecondaryNode: 'mr-widget-geo-secondary-node',
statesToShowHelpWidget: stateMaps.statesToShowHelpWidget, policyViolation: 'mr-widget-policy-violation',
jiraAssociationMissing: 'mr-widget-jira-association-missing',
}; };
...@@ -52,7 +52,7 @@ RSpec.describe 'Merge requests > User merges immediately', :js do ...@@ -52,7 +52,7 @@ RSpec.describe 'Merge requests > User merges immediately', :js do
find(':focus').send_keys :enter find(':focus').send_keys :enter
expect(merge_button).to have_no_content('Merge in progress') expect(merge_button).to have_content('Start merge train')
end end
end end
...@@ -62,7 +62,7 @@ RSpec.describe 'Merge requests > User merges immediately', :js do ...@@ -62,7 +62,7 @@ RSpec.describe 'Merge requests > User merges immediately', :js do
click_button 'Merge immediately' click_button 'Merge immediately'
expect(merge_button).to have_content('Merge in progress') expect(find('.media-body h4')).to have_content('Merging!')
end end
end end
end end
......
...@@ -36,7 +36,7 @@ RSpec.describe 'Merge requests > User merges immediately', :js do ...@@ -36,7 +36,7 @@ RSpec.describe 'Merge requests > User merges immediately', :js do
Sidekiq::Testing.fake! do Sidekiq::Testing.fake! do
click_button 'Merge immediately' click_button 'Merge immediately'
expect(find('.accept-merge-request.btn-confirm')).to have_content('Merge in progress') expect(find('.media-body h4')).to have_content('Merging!')
wait_for_requests wait_for_requests
end end
......
...@@ -45,6 +45,8 @@ const createTestMr = (customConfig) => { ...@@ -45,6 +45,8 @@ const createTestMr = (customConfig) => {
preferredAutoMergeStrategy: MWPS_MERGE_STRATEGY, preferredAutoMergeStrategy: MWPS_MERGE_STRATEGY,
availableAutoMergeStrategies: [MWPS_MERGE_STRATEGY], availableAutoMergeStrategies: [MWPS_MERGE_STRATEGY],
mergeImmediatelyDocsPath: 'path/to/merge/immediately/docs', mergeImmediatelyDocsPath: 'path/to/merge/immediately/docs',
transitionStateMachine: () => eventHub.$emit('StateMachineValueChanged', { value: 'value' }),
translateStateToMachine: () => this.transitionStateMachine(),
}; };
Object.assign(mr, customConfig.mr); Object.assign(mr, customConfig.mr);
......
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