diff --git a/lib/api/time_tracking_endpoints.rb b/lib/api/time_tracking_endpoints.rb index 3534edc38313867c29f6cad4862f50541ebd01c9..d68057a1947c623dd2f9630358e18ded5f35552d 100644 --- a/lib/api/time_tracking_endpoints.rb +++ b/lib/api/time_tracking_endpoints.rb @@ -55,10 +55,11 @@ def update_service issuable_key = "#{issuable_name}_iid".to_sym desc "Set a time estimate for a #{issuable_name}" do - detail " Sets an estimated time of work for this #{issuable_name}." + detail "Sets an estimated time of work for this #{issuable_name}." success Entities::IssuableTimeStats failure [ { code: 401, message: 'Unauthorized' }, + { code: 400, message: 'Bad request' }, { code: 404, message: 'Not found' } ] tags [issuable_collection_name] @@ -70,8 +71,14 @@ def update_service post ":id/#{issuable_collection_name}/:#{issuable_key}/time_estimate" do authorize! admin_issuable_key, load_issuable - status :ok - update_issuable(time_estimate: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration))) + time_estimate = Gitlab::TimeTrackingFormatter.parse(params.delete(:duration), keep_zero: true) + + if time_estimate && time_estimate >= 0 + status :ok + update_issuable(time_estimate: time_estimate) + else + bad_request!(reason: 'Time estimate must have a valid format and be greater than or equal to zero.') + end end desc "Reset the time estimate for a project #{issuable_name}" do diff --git a/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb b/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb index 398421c7a79e878c1c896a55f76078652a246531..dec15cb68b3219ed9212154c2fb1dd3e309638b8 100644 --- a/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb @@ -20,40 +20,49 @@ issuable_collection_name = issuable_name.pluralize describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/time_estimate" do + subject(:set_time_estimate) do + post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", user), params: { duration: duration }) + end + + let(:duration) { '2h' } + context 'with an unauthorized user' do - subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", non_member), params: { duration: '1w' }) } + let(:user) { non_member } it_behaves_like 'an unauthorized API user' it_behaves_like 'API user with insufficient permissions' end - it "sets the time estimate for #{issuable_name}" do - post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", user), params: { duration: '1w' } + context 'with an authorized user' do + it "sets the time estimate for #{issuable_name}" do + set_time_estimate - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['human_time_estimate']).to eq('1w') + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['time_estimate']).to eq(7200) + end end describe 'updating the current estimate' do before do - post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", user), params: { duration: '1w' } + post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", user), params: { duration: '2h' }) end - context 'when duration has a bad format' do - it 'does not modify the original estimate' do - post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", user), params: { duration: 'foo' } + using RSpec::Parameterized::TableSyntax - expect(response).to have_gitlab_http_status(:bad_request) - expect(issuable.reload.human_time_estimate).to eq('1w') - end + where(:updated_duration, :expected_http_status, :expected_time_estimate) do + 'foo' | :bad_request | 7200 + '-1' | :bad_request | 7200 + '1h' | :ok | 3600 + '0' | :ok | 0 end - context 'with a valid duration' do - it 'updates the estimate' do - post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", user), params: { duration: '3w1h' } + with_them do + let(:duration) { updated_duration } + it 'returns expected HTTP status and time estimate' do + set_time_estimate - expect(response).to have_gitlab_http_status(:ok) - expect(issuable.reload.human_time_estimate).to eq('3w 1h') + expect(response).to have_gitlab_http_status(expected_http_status) + expect(issuable.reload.time_estimate).to eq(expected_time_estimate) end end end