diff options
| -rw-r--r-- | app/models/concerns/time_trackable.rb | 32 | ||||
| -rw-r--r-- | app/services/issuable_base_service.rb | 10 | ||||
| -rw-r--r-- | app/services/slash_commands/interpret_service.rb | 7 | ||||
| -rw-r--r-- | changelogs/unreleased/time-tracking-api.yml | 4 | ||||
| -rw-r--r-- | doc/api/issues.md | 140 | ||||
| -rw-r--r-- | doc/api/merge_requests.md | 139 | ||||
| -rw-r--r-- | lib/api/entities.rb | 7 | ||||
| -rw-r--r-- | lib/api/helpers.rb | 4 | ||||
| -rw-r--r-- | lib/api/issues.rb | 2 | ||||
| -rw-r--r-- | lib/api/merge_requests.rb | 22 | ||||
| -rw-r--r-- | lib/api/time_tracking_endpoints.rb | 114 | ||||
| -rw-r--r-- | lib/gitlab/time_tracking_formatter.rb | 6 | ||||
| -rw-r--r-- | spec/models/concerns/issuable_spec.rb | 10 | ||||
| -rw-r--r-- | spec/requests/api/issues_spec.rb | 6 | ||||
| -rw-r--r-- | spec/requests/api/merge_requests_spec.rb | 8 | ||||
| -rw-r--r-- | spec/services/slash_commands/interpret_service_spec.rb | 6 | ||||
| -rw-r--r-- | spec/services/system_note_service_spec.rb | 4 | ||||
| -rw-r--r-- | spec/support/api/time_tracking_shared_examples.rb | 132 |
18 files changed, 608 insertions, 45 deletions
diff --git a/app/models/concerns/time_trackable.rb b/app/models/concerns/time_trackable.rb index 6fa2af4e4e6..040e3a2884e 100644 --- a/app/models/concerns/time_trackable.rb +++ b/app/models/concerns/time_trackable.rb @@ -9,27 +9,32 @@ module TimeTrackable extend ActiveSupport::Concern included do - attr_reader :time_spent + attr_reader :time_spent, :time_spent_user alias_method :time_spent?, :time_spent default_value_for :time_estimate, value: 0, allows_nil: false + validates :time_estimate, numericality: { message: 'has an invalid format' }, allow_nil: false + validate :check_negative_time_spent + has_many :timelogs, as: :trackable, dependent: :destroy end - def spend_time(seconds, user) - return if seconds == 0 + def spend_time(options) + @time_spent = options[:duration] + @time_spent_user = options[:user] + @original_total_time_spent = nil - @time_spent = seconds - @time_spent_user = user + return if @time_spent == 0 - if seconds == :reset + if @time_spent == :reset reset_spent_time else add_or_subtract_spent_time end end + alias_method :spend_time=, :spend_time def total_time_spent timelogs.sum(:time_spent) @@ -50,9 +55,18 @@ module TimeTrackable end def add_or_subtract_spent_time - # Exit if time to subtract exceeds the total time spent. - return if time_spent < 0 && (time_spent.abs > total_time_spent) - timelogs.new(time_spent: time_spent, user: @time_spent_user) end + + def check_negative_time_spent + return if time_spent.nil? || time_spent == :reset + + # we need to cache the total time spent so multiple calls to #valid? + # doesn't give a false error + @original_total_time_spent ||= total_time_spent + + if time_spent < 0 && (time_spent.abs > @original_total_time_spent) + errors.add(:time_spent, 'Time to subtract exceeds the total time spent') + end + end end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 7491c256b99..5f3ced49665 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -164,7 +164,6 @@ class IssuableBaseService < BaseService def create(issuable) merge_slash_commands_into_params!(issuable) filter_params(issuable) - change_time_spent(issuable) params.delete(:state_event) params[:author] ||= current_user @@ -207,14 +206,13 @@ class IssuableBaseService < BaseService change_subscription(issuable) change_todo(issuable) filter_params(issuable) - time_spent = change_time_spent(issuable) old_labels = issuable.labels.to_a old_mentioned_users = issuable.mentioned_users.to_a label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids) params[:label_ids] = label_ids if labels_changing?(issuable.label_ids, label_ids) - if (params.present? || time_spent) && update_issuable(issuable, params) + if params.present? && update_issuable(issuable, params) # We do not touch as it will affect a update on updated_at field ActiveRecord::Base.no_touching do handle_common_system_notes(issuable, old_labels: old_labels) @@ -261,12 +259,6 @@ class IssuableBaseService < BaseService end end - def change_time_spent(issuable) - time_spent = params.delete(:spend_time) - - issuable.spend_time(time_spent, current_user) if time_spent - end - def has_changes?(issuable, old_labels: []) valid_attrs = [:title, :description, :assignee_id, :milestone_id, :target_branch] diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb index ea00415ae1f..ab69ce91d18 100644 --- a/app/services/slash_commands/interpret_service.rb +++ b/app/services/slash_commands/interpret_service.rb @@ -262,13 +262,10 @@ module SlashCommands current_user.can?(:"admin_#{issuable.to_ability_name}", issuable) end command :spend do |raw_duration| - reduce_time = raw_duration.sub!(/\A-/, '') time_spent = Gitlab::TimeTrackingFormatter.parse(raw_duration) if time_spent - time_spent *= -1 if reduce_time - - @updates[:spend_time] = time_spent + @updates[:spend_time] = { duration: time_spent, user: current_user } end end @@ -287,7 +284,7 @@ module SlashCommands current_user.can?(:"admin_#{issuable.to_ability_name}", project) end command :remove_time_spent do - @updates[:spend_time] = :reset + @updates[:spend_time] = { duration: :reset, user: current_user } end # This is a dummy command, so that it appears in the autocomplete commands diff --git a/changelogs/unreleased/time-tracking-api.yml b/changelogs/unreleased/time-tracking-api.yml new file mode 100644 index 00000000000..b58d73bef81 --- /dev/null +++ b/changelogs/unreleased/time-tracking-api.yml @@ -0,0 +1,4 @@ +--- +title: Add new endpoints for Time Tracking. +merge_request: 8483 +author: diff --git a/doc/api/issues.md b/doc/api/issues.md index dd84afd7c73..b276d1ad918 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -712,6 +712,146 @@ Example response: } ``` +## Set a time estimate for an issue + +Sets an estimated time of work for this issue. + +``` +POST /projects/:id/issues/:issue_id/time_estimate +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer | yes | The ID of a project | +| `issue_id` | integer | yes | The ID of a project's issue | +| `duration` | string | yes | The duration in human format. e.g: 3h30m | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/93/time_estimate?duration=3h30m +``` + +Example response: + +```json +{ + "human_time_estimate": "3h 30m", + "human_total_time_spent": null, + "time_estimate": 12600, + "total_time_spent": 0 +} +``` + +## Reset the time estimate for an issue + +Resets the estimated time for this issue to 0 seconds. + +``` +POST /projects/:id/issues/:issue_id/reset_time_estimate +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer | yes | The ID of a project | +| `issue_id` | integer | yes | The ID of a project's issue | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/93/reset_time_estimate +``` + +Example response: + +```json +{ + "human_time_estimate": null, + "human_total_time_spent": null, + "time_estimate": 0, + "total_time_spent": 0 +} +``` + +## Add spent time for an issue + +Adds spent time for this issue + +``` +POST /projects/:id/issues/:issue_id/add_spent_time +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer | yes | The ID of a project | +| `issue_id` | integer | yes | The ID of a project's issue | +| `duration` | string | yes | The duration in human format. e.g: 3h30m | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/93/add_spent_time?duration=1h +``` + +Example response: + +```json +{ + "human_time_estimate": null, + "human_total_time_spent": "1h", + "time_estimate": 0, + "total_time_spent": 3600 +} +``` + +## Reset spent time for an issue + +Resets the total spent time for this issue to 0 seconds. + +``` +POST /projects/:id/issues/:issue_id/reset_spent_time +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer | yes | The ID of a project | +| `issue_id` | integer | yes | The ID of a project's issue | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/93/reset_spent_time +``` + +Example response: + +```json +{ + "human_time_estimate": null, + "human_total_time_spent": null, + "time_estimate": 0, + "total_time_spent": 0 +} +``` + +## Get time tracking stats + +``` +GET /projects/:id/issues/:issue_id/time_stats +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer | yes | The ID of a project | +| `issue_id` | integer | yes | The ID of a project's issue | + +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/93/time_stats +``` + +Example response: + +```json +{ + "human_time_estimate": "2h", + "human_total_time_spent": "1h", + "time_estimate": 7200, + "total_time_spent": 3600 +} +``` + ## Comments on issues Comments are done via the [notes](notes.md) resource. diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 662cc9da733..7b005591545 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -1018,3 +1018,142 @@ Example response: }] } ``` +## Set a time estimate for a merge request + +Sets an estimated time of work for this merge request. + +``` +POST /projects/:id/merge_requests/:merge_request_id/time_estimate +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer | yes | The ID of a project | +| `merge_request_id` | integer | yes | The ID of a project's merge request | +| `duration` | string | yes | The duration in human format. e.g: 3h30m | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/merge_requests/93/time_estimate?duration=3h30m +``` + +Example response: + +```json +{ + "human_time_estimate": "3h 30m", + "human_total_time_spent": null, + "time_estimate": 12600, + "total_time_spent": 0 +} +``` + +## Reset the time estimate for a merge request + +Resets the estimated time for this merge request to 0 seconds. + +``` +POST /projects/:id/merge_requests/:merge_request_id/reset_time_estimate +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer | yes | The ID of a project | +| `merge_request_id` | integer | yes | The ID of a project's merge_request | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/merge_requests/93/reset_time_estimate +``` + +Example response: + +```json +{ + "human_time_estimate": null, + "human_total_time_spent": null, + "time_estimate": 0, + "total_time_spent": 0 +} +``` + +## Add spent time for a merge request + +Adds spent time for this merge request + +``` +POST /projects/:id/merge_requests/:merge_request_id/add_spent_time +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer | yes | The ID of a project | +| `merge_request_id` | integer | yes | The ID of a project's merge request | +| `duration` | string | yes | The duration in human format. e.g: 3h30m | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/merge_requests/93/add_spent_time?duration=1h +``` + +Example response: + +```json +{ + "human_time_estimate": null, + "human_total_time_spent": "1h", + "time_estimate": 0, + "total_time_spent": 3600 +} +``` + +## Reset spent time for a merge request + +Resets the total spent time for this merge request to 0 seconds. + +``` +POST /projects/:id/merge_requests/:merge_request_id/reset_spent_time +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer | yes | The ID of a project | +| `merge_request_id` | integer | yes | The ID of a project's merge_request | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/merge_requests/93/reset_spent_time +``` + +Example response: + +```json +{ + "human_time_estimate": null, + "human_total_time_spent": null, + "time_estimate": 0, + "total_time_spent": 0 +} +``` + +## Get time tracking stats + +``` +GET /projects/:id/merge_requests/:merge_request_id/time_stats +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer | yes | The ID of a project | +| `merge_request_id` | integer | yes | The ID of a project's merge request | + +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/merge_requests/93/time_stats +``` + +Example response: + +```json +{ + "human_time_estimate": "2h", + "human_total_time_spent": "1h", + "time_estimate": 7200, + "total_time_spent": 3600 +} +``` diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 885ce7d44bc..9f59939e9ae 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -268,6 +268,13 @@ module API end end + class IssuableTimeStats < Grape::Entity + expose :time_estimate + expose :total_time_spent + expose :human_time_estimate + expose :human_total_time_spent + end + class ExternalIssue < Grape::Entity expose :title expose :id diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 20b5bc1502a..0fcb352000a 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -86,6 +86,10 @@ module API IssuesFinder.new(current_user, project_id: user_project.id).find(id) end + def find_project_merge_request(id) + MergeRequestsFinder.new(current_user, project_id: user_project.id).find(id) + end + def authenticate! unauthorized! unless current_user end diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 161269cbd41..fe016c1ec0a 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -89,6 +89,8 @@ module API requires :id, type: String, desc: 'The ID of a project' end resource :projects do + include TimeTrackingEndpoints + desc 'Get a list of project issues' do success Entities::Issue end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 5d1fe22f2df..e77af4b7a0d 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -10,6 +10,8 @@ module API requires :id, type: String, desc: 'The ID of a project' end resource :projects do + include TimeTrackingEndpoints + helpers do def handle_merge_request_errors!(errors) if errors[:project_access].any? @@ -96,7 +98,7 @@ module API requires :merge_request_id, type: Integer, desc: 'The ID of a merge request' end delete ":id/merge_requests/:merge_request_id" do - merge_request = user_project.merge_requests.find_by(id: params[:merge_request_id]) + merge_request = find_project_merge_request(params[:merge_request_id]) authorize!(:destroy_merge_request, merge_request) merge_request.destroy @@ -116,7 +118,7 @@ module API success Entities::MergeRequest end get path do - merge_request = user_project.merge_requests.find(params[:merge_request_id]) + merge_request = find_project_merge_request(params[:merge_request_id]) authorize! :read_merge_request, merge_request present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project end @@ -125,7 +127,7 @@ module API success Entities::RepoCommit end get "#{path}/commits" do - merge_request = user_project.merge_requests.find(params[:merge_request_id]) + merge_request = find_project_merge_request(params[:merge_request_id]) authorize! :read_merge_request, merge_request present merge_request.commits, with: Entities::RepoCommit end @@ -134,7 +136,7 @@ module API success Entities::MergeRequestChanges end get "#{path}/changes" do - merge_request = user_project.merge_requests.find(params[:merge_request_id]) + merge_request = find_project_merge_request(params[:merge_request_id]) authorize! :read_merge_request, merge_request present merge_request, with: Entities::MergeRequestChanges, current_user: current_user end @@ -153,7 +155,7 @@ module API :remove_source_branch end put path do - merge_request = user_project.merge_requests.find(params.delete(:merge_request_id)) + merge_request = find_project_merge_request(params.delete(:merge_request_id)) authorize! :update_merge_request, merge_request mr_params = declared_params(include_missing: false) @@ -180,7 +182,7 @@ module API optional :sha, type: String, desc: 'When present, must have the HEAD SHA of the source branch' end put "#{path}/merge" do - merge_request = user_project.merge_requests.find(params[:merge_request_id]) + merge_request = find_project_merge_request(params[:merge_request_id]) # Merge request can not be merged # because user dont have permissions to push into target branch @@ -216,7 +218,7 @@ module API success Entities::MergeRequest end post "#{path}/cancel_merge_when_build_succeeds" do - merge_request = user_project.merge_requests.find(params[:merge_request_id]) + merge_request = find_project_merge_request(params[:merge_request_id]) unauthorized! unless merge_request.can_cancel_merge_when_build_succeeds?(current_user) @@ -233,7 +235,7 @@ module API use :pagination end get "#{path}/comments" do - merge_request = user_project.merge_requests.find(params[:merge_request_id]) + merge_request = find_project_merge_request(params[:merge_request_id]) authorize! :read_merge_request, merge_request @@ -248,7 +250,7 @@ module API requires :note, type: String, desc: 'The text of the comment' end post "#{path}/comments" do - merge_request = user_project.merge_requests.find(params[:merge_request_id]) + merge_request = find_project_merge_request(params[:merge_request_id]) authorize! :create_note, merge_request opts = { @@ -273,7 +275,7 @@ module API use :pagination end get "#{path}/closes_issues" do - merge_request = user_project.merge_requests.find(params[:merge_request_id]) + merge_request = find_project_merge_request(params[:merge_request_id]) issues = ::Kaminari.paginate_array(merge_request.closes_issues(current_user)) present paginate(issues), with: issue_entity(user_project), current_user: current_user end diff --git a/lib/api/time_tracking_endpoints.rb b/lib/api/time_tracking_endpoints.rb new file mode 100644 index 00000000000..85b5f7d98b8 --- /dev/null +++ b/lib/api/time_tracking_endpoints.rb @@ -0,0 +1,114 @@ +module API + module TimeTrackingEndpoints + extend ActiveSupport::Concern + + included do + helpers do + def issuable_name + declared_params.has_key?(:issue_id) ? 'issue' : 'merge_request' + end + + def issuable_key + "#{issuable_name}_id".to_sym + end + + def update_issuable_key + "update_#{issuable_name}".to_sym + end + + def read_issuable_key + "read_#{issuable_name}".to_sym + end + + def load_issuable + @issuable ||= begin + case issuable_name + when 'issue' + find_project_issue(params.delete(issuable_key)) + when 'merge_request' + find_project_merge_request(params.delete(issuable_key)) + end + end + end + + def update_issuable(attrs) + custom_params = declared_params(include_missing: false) + custom_params.merge!(attrs) + + issuable = update_service.new(user_project, current_user, custom_params).execute(load_issuable) + if issuable.valid? + present issuable, with: Entities::IssuableTimeStats + else + render_validation_error!(issuable) + end + end + + def update_service + issuable_name == 'issue' ? ::Issues::UpdateService : ::MergeRequests::UpdateService + end + end + + issuable_name = name.end_with?('Issues') ? 'issue' : 'merge_request' + issuable_collection_name = issuable_name.pluralize + issuable_key = "#{issuable_name}_id".to_sym + + desc "Set a time estimate for a project #{issuable_name}" + params do + requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}" + requires :duration, type: String, desc: 'The duration to be parsed' + end + post ":id/#{issuable_collection_name}/:#{issuable_key}/time_estimate" do + authorize! update_issuable_key, load_issuable + + status :ok + update_issuable(time_estimate: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration))) + end + + desc "Reset the time estimate for a project #{issuable_name}" + params do + requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}" + end + post ":id/#{issuable_collection_name}/:#{issuable_key}/reset_time_estimate" do + authorize! update_issuable_key, load_issuable + + status :ok + update_issuable(time_estimate: 0) + end + + desc "Add spent time for a project #{issuable_name}" + params do + requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}" + requires :duration, type: String, desc: 'The duration to be parsed' + end + post ":id/#{issuable_collection_name}/:#{issuable_key}/add_spent_time" do + authorize! update_issuable_key, load_issuable + + update_issuable(spend_time: { + duration: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration)), + user: current_user + }) + end + + desc "Reset spent time for a project #{issuable_name}" + params do + requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}" + end + post ":id/#{issuable_collection_name}/:#{issuable_key}/reset_spent_time" do + authorize! update_issuable_key, load_issuable + + status :ok + update_issuable(spend_time: { duration: :reset, user: current_user }) + end + + desc "Show time stats for a project #{issuable_name}" + params do + requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}" + end + get ":id/#{issuable_collection_name}/:#{issuable_key}/time_stats" do + authorize! read_issuable_key, load_issuable + + present load_issuable, with: Entities::IssuableTimeStats + end + end + end +end diff --git a/lib/gitlab/time_tracking_formatter.rb b/lib/gitlab/time_tracking_formatter.rb index d09063c6c8f..d615c24149a 100644 --- a/lib/gitlab/time_tracking_formatter.rb +++ b/lib/gitlab/time_tracking_formatter.rb @@ -4,7 +4,11 @@ module Gitlab def parse(string) with_custom_config do - ChronicDuration.parse(string, default_unit: 'hours') rescue nil + string.sub!(/\A-/, '') + + seconds = ChronicDuration.parse(string, default_unit: 'hours') rescue nil + seconds *= -1 if seconds && Regexp.last_match + seconds end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 344906c581b..d7d31892e12 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -414,7 +414,7 @@ describe Issue, "Issuable" do let(:issue) { create(:issue) } def spend_time(seconds) - issue.spend_time(seconds, user) + issue.spend_time(duration: seconds, user: user) issue.save! end @@ -438,10 +438,10 @@ describe Issue, "Issuable" do end context 'when time to substract exceeds the total time spent' do - it 'should not alter the total time spent' do - spend_time(-3600) - - expect(issue.total_time_spent).to eq(1800) + it 'raise a validation error' do + expect do + spend_time(-3600) + end.to raise_error(ActiveRecord::RecordInvalid) end end end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 12dd4bd83f7..807c999b84a 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -1193,4 +1193,10 @@ describe API::Issues, api: true do expect(response).to have_http_status(404) end end + + describe 'time tracking endpoints' do + let(:issuable) { issue } + + include_examples 'time tracking endpoints', 'issue' + end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index f032d1b683d..4e4fea1dad8 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -6,7 +6,7 @@ describe API::MergeRequests, api: true do let(:user) { create(:user) } let(:admin) { create(:user, :admin) } let(:non_member) { create(:user) } - let!(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } + let!(:project) { create(:project, :public, creator_id: user.id, namespace: user.namespace) } let!(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) } let!(:merge_request_closed) { create(:merge_request, state: "closed", author: user, assignee: user, source_project: project, target_project: project, title: "Closed test", created_at: base_time + 1.second) } let!(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignee: user, source_project: project, target_project: project, title: "Merged test", created_at: base_time + 2.seconds, merge_commit_sha: '9999999999999999999999999999999999999999') } @@ -671,6 +671,12 @@ describe API::MergeRequests, api: true do end end + describe 'Time tracking' do + let(:issuable) { merge_request } + + include_examples 'time tracking endpoints', 'merge_request' + end + def mr_with_later_created_and_updated_at_time merge_request merge_request.created_at += 1.hour diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb index e1358acd7c1..80ae285ef64 100644 --- a/spec/services/slash_commands/interpret_service_spec.rb +++ b/spec/services/slash_commands/interpret_service_spec.rb @@ -222,7 +222,7 @@ describe SlashCommands::InterpretService, services: true do it 'populates spend_time: 3600 if content contains /spend 1h' do _, updates = service.execute(content, issuable) - expect(updates).to eq(spend_time: 3600) + expect(updates).to eq(spend_time: { duration: 3600, user: developer }) end end @@ -230,7 +230,7 @@ describe SlashCommands::InterpretService, services: true do it 'populates spend_time: -1800 if content contains /spend -30m' do _, updates = service.execute(content, issuable) - expect(updates).to eq(spend_time: -1800) + expect(updates).to eq(spend_time: { duration: -1800, user: developer }) end end @@ -246,7 +246,7 @@ describe SlashCommands::InterpretService, services: true do it 'populates spend_time: :reset if content contains /remove_time_spent' do _, updates = service.execute(content, issuable) - expect(updates).to eq(spend_time: :reset) + expect(updates).to eq(spend_time: { duration: :reset, user: developer }) end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index e85545f46dc..4042f2b0512 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -765,7 +765,7 @@ describe SystemNoteService, services: true do # We need a custom noteable in order to the shared examples to be green. let(:noteable) do mr = create(:merge_request, source_project: project) - mr.spend_time(1, author) + mr.spend_time(duration: 360000, user: author) mr.save! mr end @@ -801,7 +801,7 @@ describe SystemNoteService, services: true do end def spend_time!(seconds) - noteable.spend_time(seconds, author) + noteable.spend_time(duration: seconds, user: author) noteable.save! end end diff --git a/spec/support/api/time_tracking_shared_examples.rb b/spec/support/api/time_tracking_shared_examples.rb new file mode 100644 index 00000000000..210cd5817e0 --- /dev/null +++ b/spec/support/api/time_tracking_shared_examples.rb @@ -0,0 +1,132 @@ +shared_examples 'an unauthorized API user' do + it { is_expected.to eq(403) } +end + +shared_examples 'time tracking endpoints' do |issuable_name| + issuable_collection_name = issuable_name.pluralize + + describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/time_estimate" do + context 'with an unauthorized user' do + subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", non_member), duration: '1w') } + + it_behaves_like 'an unauthorized API user' + end + + it "sets the time estimate for #{issuable_name}" do + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '1w' + + expect(response).to have_http_status(200) + expect(json_response['human_time_estimate']).to eq('1w') + end + + describe 'updating the current estimate' do + before do + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '1w' + 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.id}/time_estimate", user), duration: 'foo' + + expect(response).to have_http_status(400) + expect(issuable.reload.human_time_estimate).to eq('1w') + end + end + + context 'with a valid duration' do + it 'updates the estimate' do + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '3w1h' + + expect(response).to have_http_status(200) + expect(issuable.reload.human_time_estimate).to eq('3w 1h') + end + end + end + end + + describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/reset_time_estimate" do + context 'with an unauthorized user' do + subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_time_estimate", non_member)) } + + it_behaves_like 'an unauthorized API user' + end + + it "resets the time estimate for #{issuable_name}" do + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_time_estimate", user) + + expect(response).to have_http_status(200) + expect(json_response['time_estimate']).to eq(0) + end + end + + describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/add_spent_time" do + context 'with an unauthorized user' do + subject do + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", non_member), + duration: '2h' + end + + it_behaves_like 'an unauthorized API user' + end + + it "add spent time for #{issuable_name}" do + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user), + duration: '2h' + + expect(response).to have_http_status(201) + expect(json_response['human_total_time_spent']).to eq('2h') + end + + context 'when subtracting time' do + it 'subtracts time of the total spent time' do + issuable.update_attributes!(spend_time: { duration: 7200, user: user }) + + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user), + duration: '-1h' + + expect(response).to have_http_status(201) + expect(json_response['total_time_spent']).to eq(3600) + end + end + + context 'when time to subtract is greater than the total spent time' do + it 'does not modify the total time spent' do + issuable.update_attributes!(spend_time: { duration: 7200, user: user }) + + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user), + duration: '-1w' + + expect(response).to have_http_status(400) + expect(json_response['message']['time_spent'].first).to match(/exceeds the total time spent/) + end + end + end + + describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/reset_spent_time" do + context 'with an unauthorized user' do + subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_spent_time", non_member)) } + + it_behaves_like 'an unauthorized API user' + end + + it "resets spent time for #{issuable_name}" do + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_spent_time", user) + + expect(response).to have_http_status(200) + expect(json_response['total_time_spent']).to eq(0) + end + end + + describe "GET /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/time_stats" do + it "returns the time stats for #{issuable_name}" do + issuable.update_attributes!(spend_time: { duration: 1800, user: user }, + time_estimate: 3600) + + get api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_stats", user) + + expect(response).to have_http_status(200) + expect(json_response['total_time_spent']).to eq(1800) + expect(json_response['time_estimate']).to eq(3600) + end + end +end |