Move issue details to from IssueBasic to Issue entity

Cleanup IssueBasic entity to keep it basic and move extra attributes
to Issue entity which contains more details
This commit is contained in:
Alexandru Croitor 2019-04-19 10:55:36 +03:00
parent 5ec28dc387
commit a4fbf39eca
5 changed files with 30 additions and 20 deletions

View File

@ -542,14 +542,10 @@ module API
class IssueBasic < ProjectEntity class IssueBasic < ProjectEntity
expose :closed_at expose :closed_at
expose :closed_by, using: Entities::UserBasic expose :closed_by, using: Entities::UserBasic
expose :labels do |issue, options| expose :labels do |issue|
# Avoids an N+1 query since labels are preloaded # Avoids an N+1 query since labels are preloaded
if options[:with_labels_data]
::API::Entities::LabelBasic.represent(issue.labels.sort_by(&:title))
else
issue.labels.map(&:title).sort issue.labels.map(&:title).sort
end end
end
expose :milestone, using: Entities::Milestone expose :milestone, using: Entities::Milestone
expose :assignees, :author, using: Entities::UserBasic expose :assignees, :author, using: Entities::UserBasic
@ -564,8 +560,6 @@ module API
expose :due_date expose :due_date
expose :confidential expose :confidential
expose :discussion_locked expose :discussion_locked
expose(:has_tasks) {|issue, _| !issue.task_list_items.empty? }
expose :task_status, if: -> (issue, _) { !issue.task_list_items.empty? }
expose :web_url do |issue| expose :web_url do |issue|
Gitlab::UrlBuilder.build(issue) Gitlab::UrlBuilder.build(issue)
@ -579,6 +573,23 @@ module API
class Issue < IssueBasic class Issue < IssueBasic
include ::API::Helpers::RelatedResourcesHelpers include ::API::Helpers::RelatedResourcesHelpers
expose :labels do |issue, options|
# Avoids an N+1 query since labels are preloaded
if options[:with_labels_data]
::API::Entities::LabelBasic.represent(issue.labels.sort_by(&:title))
else
issue.labels.map(&:title).sort
end
end
expose(:has_tasks) do |issue, _|
!issue.task_list_items.empty?
end
expose :task_status, if: -> (issue, _) do
!issue.task_list_items.empty?
end
expose :_links do expose :_links do
expose :self do |issue| expose :self do |issue|
expose_url(api_v4_project_issue_path(id: issue.project_id, issue_iid: issue.iid)) expose_url(api_v4_project_issue_path(id: issue.project_id, issue_iid: issue.iid))

View File

@ -37,6 +37,7 @@ module API
issues = finder.execute.with_api_entity_associations issues = finder.execute.with_api_entity_associations
order_by = declared_params[:sort].present? && %w(asc desc).include?(declared_params[:sort].downcase) order_by = declared_params[:sort].present? && %w(asc desc).include?(declared_params[:sort].downcase)
issues = issues.reorder(order_options_with_tie_breaker) if order_by issues = issues.reorder(order_options_with_tie_breaker) if order_by
issues issues
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
end end

View File

@ -82,7 +82,7 @@ module API
resource :issues do resource :issues do
desc "Get currently authenticated user's issues" do desc "Get currently authenticated user's issues" do
success Entities::IssueBasic success Entities::Issue
end end
params do params do
use :issues_params use :issues_params
@ -94,7 +94,7 @@ module API
issues = paginate(find_issues) issues = paginate(find_issues)
options = { options = {
with: Entities::IssueBasic, with: Entities::Issue,
with_labels_data: declared_params[:with_labels_data], with_labels_data: declared_params[:with_labels_data],
current_user: current_user, current_user: current_user,
issuable_metadata: issuable_meta_data(issues, 'Issue') issuable_metadata: issuable_meta_data(issues, 'Issue')
@ -109,7 +109,7 @@ module API
end end
resource :groups, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do resource :groups, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
desc 'Get a list of group issues' do desc 'Get a list of group issues' do
success Entities::IssueBasic success Entities::Issue
end end
params do params do
use :issues_params use :issues_params
@ -120,7 +120,7 @@ module API
issues = paginate(find_issues(group_id: group.id, include_subgroups: true)) issues = paginate(find_issues(group_id: group.id, include_subgroups: true))
options = { options = {
with: Entities::IssueBasic, with: Entities::Issue,
with_labels_data: declared_params[:with_labels_data], with_labels_data: declared_params[:with_labels_data],
current_user: current_user, current_user: current_user,
issuable_metadata: issuable_meta_data(issues, 'Issue') issuable_metadata: issuable_meta_data(issues, 'Issue')
@ -149,7 +149,7 @@ module API
include TimeTrackingEndpoints include TimeTrackingEndpoints
desc 'Get a list of project issues' do desc 'Get a list of project issues' do
success Entities::IssueBasic success Entities::Issue
end end
params do params do
use :issues_params use :issues_params
@ -160,7 +160,7 @@ module API
issues = paginate(find_issues(project_id: project.id)) issues = paginate(find_issues(project_id: project.id))
options = { options = {
with: Entities::IssueBasic, with: Entities::Issue,
with_labels_data: declared_params[:with_labels_data], with_labels_data: declared_params[:with_labels_data],
current_user: current_user, current_user: current_user,
project: user_project, project: user_project,

View File

@ -14,7 +14,7 @@
"labels": { "labels": {
"type": "array", "type": "array",
"items": { "items": {
"$ref": "../../entities/label.json" "type": "string"
} }
}, },
"milestone": { "milestone": {
@ -79,8 +79,6 @@
"due_date": { "type": ["date", "null"] }, "due_date": { "type": ["date", "null"] },
"confidential": { "type": "boolean" }, "confidential": { "type": "boolean" },
"web_url": { "type": "uri" }, "web_url": { "type": "uri" },
"has_tasks": {"type": "boolean"},
"task_status": {"type": "string"},
"time_stats": { "time_stats": {
"time_estimate": { "type": "integer" }, "time_estimate": { "type": "integer" },
"total_time_spent": { "type": "integer" }, "total_time_spent": { "type": "integer" },
@ -93,6 +91,6 @@
"state", "created_at", "updated_at", "labels", "state", "created_at", "updated_at", "labels",
"milestone", "assignees", "author", "user_notes_count", "milestone", "assignees", "author", "user_notes_count",
"upvotes", "downvotes", "due_date", "confidential", "upvotes", "downvotes", "due_date", "confidential",
"has_tasks", "web_url" "web_url"
] ]
} }

View File

@ -160,12 +160,12 @@ describe API::Issues do
it 'avoids N+1 queries' do it 'avoids N+1 queries' do
get api("/projects/#{project.id}/issues", user) get api("/projects/#{project.id}/issues", user)
create_list(:issue, 3, project: project)
control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do
get api("/projects/#{project.id}/issues", user) get api("/projects/#{project.id}/issues", user)
end.count end.count
create_list(:issue, 3, project: project)
expect do expect do
get api("/projects/#{project.id}/issues", user) get api("/projects/#{project.id}/issues", user)
end.not_to exceed_all_query_limit(control_count) end.not_to exceed_all_query_limit(control_count)