Merge branch '28453-add-time-estimate-time-spent-to-api-issue-output' into 'master'

Add time stats to Issue and Merge Request API

Closes #28453

See merge request !13335

Author: Robert Speicher

changelogs/unreleased/28453-add-time-estimate-time-spent-to-api-issue-output.yml

@@ -0,0 +1,4 @@
+---
+title: Add time stats to Issue and Merge Request API
+merge_request: 13335
+author: @travismiller

doc/api/issues.md

@@ -101,6 +101,12 @@ Example response:
       "user_notes_count": 1,
       "due_date": "2016-07-22",
       "web_url": "http://example.com/example/example/issues/6",
+      "time_stats": {
+         "time_estimate": 0,
+         "total_time_spent": 0,
+         "human_time_estimate": null,
+         "human_total_time_spent": null
+      },
       "confidential": false
    }
 ]
@@ -198,6 +204,12 @@ Example response:
       "user_notes_count": 1,
       "due_date": null,
       "web_url": "http://example.com/example/example/issues/1",
+      "time_stats": {
+         "time_estimate": 0,
+         "total_time_spent": 0,
+         "human_time_estimate": null,
+         "human_total_time_spent": null
+      },
       "confidential": false
    }
 ]
@@ -296,6 +308,12 @@ Example response:
       "user_notes_count": 1,
       "due_date": "2016-07-22",
       "web_url": "http://example.com/example/example/issues/1",
+      "time_stats": {
+         "time_estimate": 0,
+         "total_time_spent": 0,
+         "human_time_estimate": null,
+         "human_total_time_spent": null
+      },
       "confidential": false
    }
 ]
@@ -372,6 +390,12 @@ Example response:
    "user_notes_count": 1,
    "due_date": null,
    "web_url": "http://example.com/example/example/issues/1",
+   "time_stats": {
+      "time_estimate": 0,
+      "total_time_spent": 0,
+      "human_time_estimate": null,
+      "human_total_time_spent": null
+   },
    "confidential": false,
    "_links": {
       "self": "http://example.com/api/v4/projects/1/issues/2",
@@ -440,6 +464,12 @@ Example response:
    "user_notes_count": 0,
    "due_date": null,
    "web_url": "http://example.com/example/example/issues/14",
+   "time_stats": {
+      "time_estimate": 0,
+      "total_time_spent": 0,
+      "human_time_estimate": null,
+      "human_total_time_spent": null
+   },
    "confidential": false,
    "_links": {
       "self": "http://example.com/api/v4/projects/1/issues/2",
@@ -509,6 +539,12 @@ Example response:
    "user_notes_count": 0,
    "due_date": "2016-07-22",
    "web_url": "http://example.com/example/example/issues/15",
+   "time_stats": {
+      "time_estimate": 0,
+      "total_time_spent": 0,
+      "human_time_estimate": null,
+      "human_total_time_spent": null
+   },
    "confidential": false,
    "_links": {
       "self": "http://example.com/api/v4/projects/1/issues/2",
@@ -601,6 +637,12 @@ Example response:
   },
   "due_date": null,
   "web_url": "http://example.com/example/example/issues/11",
+  "time_stats": {
+    "time_estimate": 0,
+    "total_time_spent": 0,
+    "human_time_estimate": null,
+    "human_total_time_spent": null
+  },
   "confidential": false,
   "_links": {
     "self": "http://example.com/api/v4/projects/1/issues/2",
@@ -672,6 +714,12 @@ Example response:
   },
   "due_date": null,
   "web_url": "http://example.com/example/example/issues/11",
+  "time_stats": {
+    "time_estimate": 0,
+    "total_time_spent": 0,
+    "human_time_estimate": null,
+    "human_total_time_spent": null
+  },
   "confidential": false,
   "_links": {
     "self": "http://example.com/api/v4/projects/1/issues/2",
@@ -1001,7 +1049,13 @@ Example response:
     "user_notes_count": 1,
     "should_remove_source_branch": null,
     "force_remove_source_branch": false,
-    "web_url": "https://gitlab.example.com/gitlab-org/gitlab-test/merge_requests/6432"
+    "web_url": "https://gitlab.example.com/gitlab-org/gitlab-test/merge_requests/6432",
+    "time_stats": {
+      "time_estimate": 0,
+      "total_time_spent": 0,
+      "human_time_estimate": null,
+      "human_total_time_spent": null
+    }
   }
 ]
 ```

doc/api/merge_requests.md

@@ -92,7 +92,13 @@ Parameters:
     "user_notes_count": 1,
     "should_remove_source_branch": true,
     "force_remove_source_branch": false,
-    "web_url": "http://example.com/example/example/merge_requests/1"
+    "web_url": "http://example.com/example/example/merge_requests/1",
+    "time_stats": {
+      "time_estimate": 0,
+      "total_time_spent": 0,
+      "human_time_estimate": null,
+      "human_total_time_spent": null
+    }
   }
 ]
 ```
@@ -181,7 +187,13 @@ Parameters:
     "user_notes_count": 1,
     "should_remove_source_branch": true,
     "force_remove_source_branch": false,
-    "web_url": "http://example.com/example/example/merge_requests/1"
+    "web_url": "http://example.com/example/example/merge_requests/1",
+    "time_stats": {
+      "time_estimate": 0,
+      "total_time_spent": 0,
+      "human_time_estimate": null,
+      "human_total_time_spent": null
+    }
   }
 ]
 ```
@@ -250,7 +262,13 @@ Parameters:
   "user_notes_count": 1,
   "should_remove_source_branch": true,
   "force_remove_source_branch": false,
-  "web_url": "http://example.com/example/example/merge_requests/1"
+  "web_url": "http://example.com/example/example/merge_requests/1",
+  "time_stats": {
+    "time_estimate": 0,
+    "total_time_spent": 0,
+    "human_time_estimate": null,
+    "human_total_time_spent": null
+  }
 }
 ```
 
@@ -356,6 +374,12 @@ Parameters:
   "should_remove_source_branch": true,
   "force_remove_source_branch": false,
   "web_url": "http://example.com/example/example/merge_requests/1",
+  "time_stats": {
+    "time_estimate": 0,
+    "total_time_spent": 0,
+    "human_time_estimate": null,
+    "human_total_time_spent": null
+  }
   "changes": [
     {
     "old_path": "VERSION",
@@ -442,7 +466,13 @@ POST /projects/:id/merge_requests
   "user_notes_count": 0,
   "should_remove_source_branch": true,
   "force_remove_source_branch": false,
-  "web_url": "http://example.com/example/example/merge_requests/1"
+  "web_url": "http://example.com/example/example/merge_requests/1",
+  "time_stats": {
+    "time_estimate": 0,
+    "total_time_spent": 0,
+    "human_time_estimate": null,
+    "human_total_time_spent": null
+  }
 }
 ```
 
@@ -519,7 +549,13 @@ Must include at least one non-required attribute from above.
   "user_notes_count": 1,
   "should_remove_source_branch": true,
   "force_remove_source_branch": false,
-  "web_url": "http://example.com/example/example/merge_requests/1"
+  "web_url": "http://example.com/example/example/merge_requests/1",
+  "time_stats": {
+    "time_estimate": 0,
+    "total_time_spent": 0,
+    "human_time_estimate": null,
+    "human_total_time_spent": null
+  }
 }
 ```
 
@@ -617,7 +653,13 @@ Parameters:
   "user_notes_count": 1,
   "should_remove_source_branch": true,
   "force_remove_source_branch": false,
-  "web_url": "http://example.com/example/example/merge_requests/1"
+  "web_url": "http://example.com/example/example/merge_requests/1",
+  "time_stats": {
+    "time_estimate": 0,
+    "total_time_spent": 0,
+    "human_time_estimate": null,
+    "human_total_time_spent": null
+  }
 }
 ```
 
@@ -687,7 +729,13 @@ Parameters:
   "user_notes_count": 1,
   "should_remove_source_branch": true,
   "force_remove_source_branch": false,
-  "web_url": "http://example.com/example/example/merge_requests/1"
+  "web_url": "http://example.com/example/example/merge_requests/1",
+  "time_stats": {
+    "time_estimate": 0,
+    "total_time_spent": 0,
+    "human_time_estimate": null,
+    "human_total_time_spent": null
+  }
 }
 ```
 

lib/api/entities.rb

@@ -320,7 +320,10 @@ class Milestone < Grape::Entity
     end
 
     class IssueBasic < ProjectEntity
-      expose :label_names, as: :labels
+      expose :labels do |issue, options|
+        # Avoids an N+1 query since labels are preloaded
+        issue.labels.map(&:title).sort
+      end
       expose :milestone, using: Entities::Milestone
       expose :assignees, :author, using: Entities::UserBasic
 
@@ -329,13 +332,32 @@ class IssueBasic < ProjectEntity
       end
 
       expose :user_notes_count
-      expose :upvotes, :downvotes
+      expose :upvotes do |issue, options|
+        if options[:issuable_metadata]
+          # Avoids an N+1 query when metadata is included
+          options[:issuable_metadata][issue.id].upvotes
+        else
+          issue.upvotes
+        end
+      end
+      expose :downvotes do |issue, options|
+        if options[:issuable_metadata]
+          # Avoids an N+1 query when metadata is included
+          options[:issuable_metadata][issue.id].downvotes
+        else
+          issue.downvotes
+        end
+      end
       expose :due_date
       expose :confidential
 
       expose :web_url do |issue, options|
         Gitlab::UrlBuilder.build(issue)
       end
+
+      expose :time_stats, using: 'API::Entities::IssuableTimeStats' do |issue|
+        issue
+      end
     end
 
     class Issue < IssueBasic
@@ -365,10 +387,22 @@ class Issue < IssueBasic
     end
 
     class IssuableTimeStats < Grape::Entity
+      format_with(:time_tracking_formatter) do |time_spent|
+        Gitlab::TimeTrackingFormatter.output(time_spent)
+      end
+
       expose :time_estimate
       expose :total_time_spent
       expose :human_time_estimate
-      expose :human_total_time_spent
+
+      with_options(format_with: :time_tracking_formatter) do
+        expose :total_time_spent, as: :human_total_time_spent
+      end
+
+      def total_time_spent
+        # Avoids an N+1 query since timelogs are preloaded
+        object.timelogs.map(&:time_spent).sum
+      end
     end
 
     class ExternalIssue < Grape::Entity
@@ -418,6 +452,10 @@ class MergeRequestBasic < ProjectEntity
       expose :web_url do |merge_request, options|
         Gitlab::UrlBuilder.build(merge_request)
       end
+
+      expose :time_stats, using: 'API::Entities::IssuableTimeStats' do |merge_request|
+        merge_request
+      end
     end
 
     class MergeRequest < MergeRequestBasic

lib/api/issues.rb

@@ -4,6 +4,8 @@ class Issues < Grape::API
 
     before { authenticate! }
 
+    helpers ::Gitlab::IssuableMetadata
+
     helpers do
       def find_issues(args = {})
         args = params.merge(args)
@@ -13,6 +15,7 @@ def find_issues(args = {})
         args[:label_name] = args.delete(:labels)
 
         issues = IssuesFinder.new(current_user, args).execute
+          .preload(:assignees, :labels, :notes, :timelogs)
 
         issues.reorder(args[:order_by] => args[:sort])
       end
@@ -65,7 +68,13 @@ def find_issues(args = {})
       get do
         issues = find_issues
 
-        present paginate(issues), with: Entities::IssueBasic, current_user: current_user
+        options = {
+          with: Entities::IssueBasic,
+          current_user: current_user,
+          issuable_metadata: issuable_meta_data(issues, 'Issue')
+        }
+
+        present paginate(issues), options
       end
     end
 
@@ -86,7 +95,13 @@ def find_issues(args = {})
 
         issues = find_issues(group_id: group.id)
 
-        present paginate(issues), with: Entities::IssueBasic, current_user: current_user
+        options = {
+          with: Entities::IssueBasic,
+          current_user: current_user,
+          issuable_metadata: issuable_meta_data(issues, 'Issue')
+        }
+
+        present paginate(issues), options
       end
     end
 
@@ -109,7 +124,14 @@ def find_issues(args = {})
 
         issues = find_issues(project_id: project.id)
 
-        present paginate(issues), with: Entities::IssueBasic, current_user: current_user, project: user_project
+        options = {
+          with: Entities::IssueBasic,
+          current_user: current_user,
+          project: user_project,
+          issuable_metadata: issuable_meta_data(issues, 'Issue')
+        }
+
+        present paginate(issues), options
       end
 
       desc 'Get a single project issue' do

lib/api/merge_requests.rb

@@ -21,7 +21,7 @@ def find_merge_requests(args = {})
         return merge_requests if args[:view] == 'simple'
 
         merge_requests
-          .preload(:notes, :author, :assignee, :milestone, :merge_request_diff, :labels)
+          .preload(:notes, :author, :assignee, :milestone, :merge_request_diff, :labels, :timelogs)
       end
 
       params :merge_requests_params do