From 03dba01273d548ab8c0dd82408400a834fb4e10f Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Fri, 9 Nov 2018 13:19:48 +1300 Subject: [PATCH] Revert "Extract code into IssueBoardEntity" This reverts commit 8c126525faba40032244328187ba73a53b6eaf4c. --- app/controllers/boards/issues_controller.rb | 16 ++++-- app/models/issue.rb | 14 +++++ app/serializers/README.md | 4 +- app/serializers/issue_board_entity.rb | 52 ------------------- app/serializers/issue_serializer.rb | 6 +-- app/serializers/label_entity.rb | 4 -- .../boards/issues_controller_spec.rb | 6 +-- .../api/schemas/entities/issue_boards.json | 15 ------ spec/models/concerns/awardable_spec.rb | 4 +- spec/serializers/issue_board_entity_spec.rb | 23 -------- spec/serializers/issue_serializer_spec.rb | 8 --- 11 files changed, 34 insertions(+), 118 deletions(-) delete mode 100644 app/serializers/issue_board_entity.rb delete mode 100644 spec/fixtures/api/schemas/entities/issue_boards.json delete mode 100644 spec/serializers/issue_board_entity_spec.rb diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb index 0dd7500623d..7f874687212 100644 --- a/app/controllers/boards/issues_controller.rb +++ b/app/controllers/boards/issues_controller.rb @@ -100,12 +100,18 @@ module Boards .merge(board_id: params[:board_id], list_id: params[:list_id], request: request) end - def serializer - IssueSerializer.new(current_user: current_user) - end - def serialize_as_json(resource) - serializer.represent(resource, serializer: 'board', include_full_project_path: board.group_board?) + resource.as_json( + only: [:id, :iid, :project_id, :title, :confidential, :due_date, :relative_position, :weight], + labels: true, + issue_endpoints: true, + include_full_project_path: board.group_board?, + include: { + project: { only: [:id, :path] }, + assignees: { only: [:id, :name, :username], methods: [:avatar_url] }, + milestone: { only: [:id, :title] } + } + ) end def whitelist_query_limiting diff --git a/app/models/issue.rb b/app/models/issue.rb index abdb3448d4e..0de5e434b02 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -231,6 +231,20 @@ class Issue < ActiveRecord::Base def as_json(options = {}) super(options).tap do |json| + if options.key?(:issue_endpoints) && project + url_helper = Gitlab::Routing.url_helpers + + issue_reference = options[:include_full_project_path] ? to_reference(full: true) : to_reference + + json.merge!( + reference_path: issue_reference, + real_path: url_helper.project_issue_path(project, self), + issue_sidebar_endpoint: url_helper.project_issue_path(project, self, format: :json, serializer: 'sidebar'), + toggle_subscription_endpoint: url_helper.toggle_subscription_project_issue_path(project, self), + assignable_labels_endpoint: url_helper.project_labels_path(project, format: :json, include_ancestor_groups: true) + ) + end + if options.key?(:labels) json[:labels] = labels.as_json( project: project, diff --git a/app/serializers/README.md b/app/serializers/README.md index bb94745b0b5..0337f88db5f 100644 --- a/app/serializers/README.md +++ b/app/serializers/README.md @@ -180,7 +180,7 @@ def index render json: MyResourceSerializer .new(current_user: @current_user) .represent_details(@project.resources) - end + nd end ``` @@ -196,7 +196,7 @@ def index .represent_details(@project.resources), count: @project.resources.count } - end + nd end ``` diff --git a/app/serializers/issue_board_entity.rb b/app/serializers/issue_board_entity.rb deleted file mode 100644 index 4e3d03b236b..00000000000 --- a/app/serializers/issue_board_entity.rb +++ /dev/null @@ -1,52 +0,0 @@ -# frozen_string_literal: true - -class IssueBoardEntity < Grape::Entity - include RequestAwareEntity - - expose :id - expose :iid - expose :title - - expose :confidential - expose :due_date - expose :project_id - expose :relative_position - expose :weight, if: -> (*) { respond_to?(:weight) } - expose :time_estimate - - expose :project do |issue| - API::Entities::Project.represent issue.project, only: [:id, :path] - end - - expose :milestone, expose_nil: false do |issue| - API::Entities::Project.represent issue.milestone, only: [:id, :title] - end - - expose :assignees do |issue| - API::Entities::UserBasic.represent issue.assignees, only: [:id, :name, :username, :avatar_url] - end - - expose :labels do |issue| - LabelEntity.represent issue.labels, project: issue.project, only: [:id, :title, :description, :color, :priority, :text_color] - end - - expose :reference_path, if: -> (issue) { issue.project } do |issue, options| - options[:include_full_project_path] ? issue.to_reference(full: true) : issue.to_reference - end - - expose :real_path, if: -> (issue) { issue.project } do |issue| - project_issue_path(issue.project, issue) - end - - expose :issue_sidebar_endpoint, if: -> (issue) { issue.project } do |issue| - project_issue_path(issue.project, issue, format: :json, serializer: 'sidebar') - end - - expose :toggle_subscription_endpoint, if: -> (issue) { issue.project } do |issue| - toggle_subscription_project_issue_path(issue.project, issue) - end - - expose :assignable_labels_endpoint, if: -> (issue) { issue.project } do |issue| - project_labels_path(issue.project, format: :json, include_ancestor_groups: true) - end -end diff --git a/app/serializers/issue_serializer.rb b/app/serializers/issue_serializer.rb index d66f0a5acb7..37cf5e28396 100644 --- a/app/serializers/issue_serializer.rb +++ b/app/serializers/issue_serializer.rb @@ -4,17 +4,15 @@ class IssueSerializer < BaseSerializer # This overrided method takes care of which entity should be used # to serialize the `issue` based on `basic` key in `opts` param. # Hence, `entity` doesn't need to be declared on the class scope. - def represent(issue, opts = {}) + def represent(merge_request, opts = {}) entity = case opts[:serializer] when 'sidebar' IssueSidebarEntity - when 'board' - IssueBoardEntity else IssueEntity end - super(issue, opts, entity) + super(merge_request, opts, entity) end end diff --git a/app/serializers/label_entity.rb b/app/serializers/label_entity.rb index 5082245dda9..98743d62b50 100644 --- a/app/serializers/label_entity.rb +++ b/app/serializers/label_entity.rb @@ -12,8 +12,4 @@ class LabelEntity < Grape::Entity expose :text_color expose :created_at expose :updated_at - - expose :priority, if: -> (*) { options.key?(:project) } do |label| - label.priority(options[:project]) - end end diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb index 6d0483f0032..98946e4287b 100644 --- a/spec/controllers/boards/issues_controller_spec.rb +++ b/spec/controllers/boards/issues_controller_spec.rb @@ -50,7 +50,7 @@ describe Boards::IssuesController do parsed_response = JSON.parse(response.body) - expect(response).to match_response_schema('entities/issue_boards') + expect(response).to match_response_schema('issues') expect(parsed_response['issues'].length).to eq 2 expect(development.issues.map(&:relative_position)).not_to include(nil) end @@ -121,7 +121,7 @@ describe Boards::IssuesController do parsed_response = JSON.parse(response.body) - expect(response).to match_response_schema('entities/issue_boards') + expect(response).to match_response_schema('issues') expect(parsed_response['issues'].length).to eq 2 end end @@ -168,7 +168,7 @@ describe Boards::IssuesController do it 'returns the created issue' do create_issue user: user, board: board, list: list1, title: 'New issue' - expect(response).to match_response_schema('entities/issue_board') + expect(response).to match_response_schema('issue') end end diff --git a/spec/fixtures/api/schemas/entities/issue_boards.json b/spec/fixtures/api/schemas/entities/issue_boards.json deleted file mode 100644 index 0ac1d9468c8..00000000000 --- a/spec/fixtures/api/schemas/entities/issue_boards.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "type": "object", - "required" : [ - "issues", - "size" - ], - "properties" : { - "issues": { - "type": "array", - "items": { "$ref": "issue_board.json" } - }, - "size": { "type": "integer" } - }, - "additionalProperties": false -} diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb index 5713106418d..debc02fa51f 100644 --- a/spec/models/concerns/awardable_spec.rb +++ b/spec/models/concerns/awardable_spec.rb @@ -37,8 +37,8 @@ describe Awardable do create(:award_emoji, awardable: issue3, name: "star", user: award_emoji.user) create(:award_emoji, awardable: issue3, name: "star", user: award_emoji2.user) - expect(Issue.awarded(award_emoji.user)).to contain_exactly(issue, issue3) - expect(Issue.awarded(award_emoji2.user)).to contain_exactly(issue2, issue3) + expect(Issue.awarded(award_emoji.user)).to eq [issue, issue3] + expect(Issue.awarded(award_emoji2.user)).to eq [issue2, issue3] end end diff --git a/spec/serializers/issue_board_entity_spec.rb b/spec/serializers/issue_board_entity_spec.rb deleted file mode 100644 index 06d9d3657e6..00000000000 --- a/spec/serializers/issue_board_entity_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe IssueBoardEntity do - let(:project) { create(:project) } - let(:resource) { create(:issue, project: project) } - let(:user) { create(:user) } - - let(:request) { double('request', current_user: user) } - - subject { described_class.new(resource, request: request).as_json } - - it 'has basic attributes' do - expect(subject).to include(:id, :iid, :title, :confidential, :due_date, :project_id, :relative_position, - :project, :labels) - end - - it 'has path and endpoints' do - expect(subject).to include(:reference_path, :real_path, :issue_sidebar_endpoint, - :toggle_subscription_endpoint, :assignable_labels_endpoint) - end -end diff --git a/spec/serializers/issue_serializer_spec.rb b/spec/serializers/issue_serializer_spec.rb index e8c46c0cdee..75578816e75 100644 --- a/spec/serializers/issue_serializer_spec.rb +++ b/spec/serializers/issue_serializer_spec.rb @@ -24,12 +24,4 @@ describe IssueSerializer do expect(json_entity).to match_schema('entities/issue_sidebar') end end - - context 'board issue serialization' do - let(:serializer) { 'board' } - - it 'matches board issue json schema' do - expect(json_entity).to match_schema('entities/issue_board') - end - end end