From c2d406bca13d2242a9e3a8bc2a0593ea15ce5689 Mon Sep 17 00:00:00 2001 From: Ash McKenzie <amckenzie@gitlab.com> Date: Tue, 25 Jul 2023 16:27:21 +1000 Subject: [PATCH] Support removing time estimate with /estimate 0 --- .../issue_and_merge_request_actions.rb | 12 +++++- .../quick_actions/interpret_service_spec.rb | 40 +++++++++++++++++-- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/lib/gitlab/quick_actions/issue_and_merge_request_actions.rb b/lib/gitlab/quick_actions/issue_and_merge_request_actions.rb index 597312edcd52d..c94deea0dfbfd 100644 --- a/lib/gitlab/quick_actions/issue_and_merge_request_actions.rb +++ b/lib/gitlab/quick_actions/issue_and_merge_request_actions.rb @@ -145,10 +145,18 @@ module IssueAndMergeRequestActions desc { _('Set time estimate') } explanation do |time_estimate| - formatted_time_estimate = format_time_estimate(time_estimate) - _("Sets time estimate to %{time_estimate}.") % { time_estimate: formatted_time_estimate } if formatted_time_estimate + next unless time_estimate + + if time_estimate == 0 + _('Removes time estimate.') + elsif time_estimate > 0 + formatted_time_estimate = format_time_estimate(time_estimate) + _("Sets time estimate to %{time_estimate}.") % { time_estimate: formatted_time_estimate } if formatted_time_estimate + end end execution_message do |time_estimate| + next _('Removed time estimate.') if time_estimate == 0 + formatted_time_estimate = format_time_estimate(time_estimate) _("Set time estimate to %{time_estimate}.") % { time_estimate: formatted_time_estimate } if formatted_time_estimate end diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index 86e2340b9fb8d..c78a304eda15e 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -2690,12 +2690,44 @@ end describe 'estimate command' do - let(:content) { '/estimate 79d' } + context 'positive estimation' do + let(:content) { '/estimate 79d' } - it 'includes the formatted duration' do - _, explanations = service.explain(content, merge_request) + it 'includes the formatted duration' do + _, explanations = service.explain(content, merge_request) + + expect(explanations).to eq(['Sets time estimate to 3mo 3w 4d.']) + end + end + + context 'zero estimation' do + let(:content) { '/estimate 0' } + + it 'includes the formatted duration' do + _, explanations = service.explain(content, merge_request) + + expect(explanations).to eq(['Removes time estimate.']) + end + end + + context 'negative estimation' do + let(:content) { '/estimate -79d' } - expect(explanations).to eq(['Sets time estimate to 3mo 3w 4d.']) + it 'does not explain' do + _, explanations = service.explain(content, merge_request) + + expect(explanations).to be_empty + end + end + + context 'invalid estimation' do + let(:content) { '/estimate a' } + + it 'does not explain' do + _, explanations = service.explain(content, merge_request) + + expect(explanations).to be_empty + end end end -- GitLab