From fd3868967dac6eee7f9957ae1db634d351e460ca Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 14 Sep 2022 16:30:25 -0600 Subject: [PATCH 1/5] Use the MR data to determine if the branch was deleted Changelog: fixed --- .../components/states/ready_to_merge.vue | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue index 78430abcfe983b..f689076d0a1ec3 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue @@ -325,15 +325,21 @@ export default { ); }, sourceBranchDeletedText() { - if (this.removeSourceBranch) { - return this.mr.state === 'merged' - ? __('Deleted the source branch.') - : __('Source branch will be deleted.'); - } + const premerge = { + true: __('Source branch will be deleted.'), + false: __('Source branch will not be deleted.'), + }; + const postmerge = { + true: __('Deleted the source branch.'), + false: __('Did not delete the source branch.'), + }; + const isPreMerge = this.mr.state !== 'merged'; + const strings = isPreMerge ? premerge : postmerge; + const doDelete = isPreMerge + ? this.mr.shouldRemoveSourceBranch || this.removeSourceBranch + : this.mr.sourceBranchRemoved; - return this.mr.state === 'merged' - ? __('Did not delete the source branch.') - : __('Source branch will not be deleted.'); + return strings[doDelete]; }, showMergeDetailsHeader() { return ['readyToMerge'].indexOf(this.mr.state) >= 0; -- GitLab From b81583c4e8f1b33658ae93177e1a58385b84c67e Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 19 Sep 2022 19:50:12 -0600 Subject: [PATCH 2/5] Switch to dual-ternary, dual-exit format --- .../components/states/ready_to_merge.vue | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue index f689076d0a1ec3..e0588dd37e0bff 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue @@ -325,21 +325,19 @@ export default { ); }, sourceBranchDeletedText() { - const premerge = { - true: __('Source branch will be deleted.'), - false: __('Source branch will not be deleted.'), - }; - const postmerge = { - true: __('Deleted the source branch.'), - false: __('Did not delete the source branch.'), - }; const isPreMerge = this.mr.state !== 'merged'; - const strings = isPreMerge ? premerge : postmerge; - const doDelete = isPreMerge - ? this.mr.shouldRemoveSourceBranch || this.removeSourceBranch - : this.mr.sourceBranchRemoved; + const shouldDelete = this.mr.shouldRemoveSourceBranch || this.removeSourceBranch; + const isDeleted = this.mr.sourceBranchRemoved; - return strings[doDelete]; + if (isPreMerge) { + return shouldDelete + ? __('Source branch will be deleted.') + : __('Source branch will not be deleted.'); + } else { + return isDeleted + ? __('Deleted the source branch.') + : __('Did not delete the source branch.'); + } }, showMergeDetailsHeader() { return ['readyToMerge'].indexOf(this.mr.state) >= 0; -- GitLab From 478f3fd155aa29a069b1383e6895e4b6e40d94e0 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 19 Sep 2022 21:28:51 -0600 Subject: [PATCH 3/5] Remove fallback to this.removeSourceBranch as it is duplicative this.removeSourceBranch is identical in its source to this.mr.shouldRemoveSourceBranch, so it's useless to fall back to it. --- .../components/states/ready_to_merge.vue | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue index e0588dd37e0bff..9aa352a8dee7c0 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue @@ -326,15 +326,13 @@ export default { }, sourceBranchDeletedText() { const isPreMerge = this.mr.state !== 'merged'; - const shouldDelete = this.mr.shouldRemoveSourceBranch || this.removeSourceBranch; - const isDeleted = this.mr.sourceBranchRemoved; if (isPreMerge) { - return shouldDelete + return this.mr.shouldRemoveSourceBranch ? __('Source branch will be deleted.') : __('Source branch will not be deleted.'); } else { - return isDeleted + return this.mr.sourceBranchRemoved ? __('Deleted the source branch.') : __('Did not delete the source branch.'); } -- GitLab From 11b255e1e589be9d2df8825ccc8d2d962378af0f Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 19 Sep 2022 21:29:18 -0600 Subject: [PATCH 4/5] Test potential string outputs for various source branch deletion options --- .../states/mr_widget_ready_to_merge_spec.js | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/spec/frontend/vue_merge_request_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/frontend/vue_merge_request_widget/components/states/mr_widget_ready_to_merge_spec.js index bc3dbe8c351aa0..b7c974991c55f7 100644 --- a/spec/frontend/vue_merge_request_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/frontend/vue_merge_request_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -300,6 +300,48 @@ describe('ReadyToMerge', () => { expect(wrapper.vm.isMergeButtonDisabled).toBe(true); }); }); + + describe('sourceBranchDeletedText', () => { + const should = 'Source branch will be deleted.'; + const shouldNot = 'Source branch will not be deleted.'; + const did = 'Deleted the source branch.'; + const didNot = 'Did not delete the source branch.'; + const scenarios = [ + "the MR hasn't merged yet, and the backend-provided value expects to delete the branch", + "the MR hasn't merged yet, and the backend-provided value expects to leave the branch", + "the MR hasn't merged yet, and the backend-provided value is a non-boolean falsey value", + "the MR hasn't merged yet, and the backend-provided value is a non-boolean truthy value", + 'the MR has merged, and the backend reports that the branch has been removed', + 'the MR has been merged, and the backend reports that the branch has not been removed', + 'the MR has been merged, and the backend reports a non-boolean falsey value', + 'the MR has been merged, and the backend reports a non-boolean truthy value', + ]; + + it.each` + describe | premerge | mrShould | mrRemoved | output + ${scenarios[0]} | ${true} | ${true} | ${null} | ${should} + ${scenarios[1]} | ${true} | ${false} | ${null} | ${shouldNot} + ${scenarios[2]} | ${true} | ${null} | ${null} | ${shouldNot} + ${scenarios[3]} | ${true} | ${'yeah'} | ${null} | ${should} + ${scenarios[4]} | ${false} | ${null} | ${true} | ${did} + ${scenarios[5]} | ${false} | ${null} | ${false} | ${didNot} + ${scenarios[6]} | ${false} | ${null} | ${null} | ${didNot} + ${scenarios[7]} | ${false} | ${null} | ${'yep'} | ${did} + `( + 'in the case that $describe, returns "$output"', + ({ premerge, mrShould, mrRemoved, output }) => { + createComponent({ + mr: { + state: !premerge ? 'merged' : 'literally-anything-else', + shouldRemoveSourceBranch: mrShould, + sourceBranchRemoved: mrRemoved, + }, + }); + + expect(wrapper.vm.sourceBranchDeletedText).toBe(output); + }, + ); + }); }); describe('methods', () => { -- GitLab From a98b64631c7a7f66fa6905a03a00d0d4492ac5b7 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 20 Sep 2022 11:34:13 -0600 Subject: [PATCH 5/5] Split the second return out of the if-clause --- .../components/states/ready_to_merge.vue | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue index 9aa352a8dee7c0..16a7119e1028b2 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue @@ -331,11 +331,11 @@ export default { return this.mr.shouldRemoveSourceBranch ? __('Source branch will be deleted.') : __('Source branch will not be deleted.'); - } else { - return this.mr.sourceBranchRemoved - ? __('Deleted the source branch.') - : __('Did not delete the source branch.'); } + + return this.mr.sourceBranchRemoved + ? __('Deleted the source branch.') + : __('Did not delete the source branch.'); }, showMergeDetailsHeader() { return ['readyToMerge'].indexOf(this.mr.state) >= 0; -- GitLab