summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOswaldo Ferreira <oswaldo@gitlab.com>2017-10-17 13:07:32 +0200
committerOswaldo Ferreira <oswaldo@gitlab.com>2017-10-31 12:49:57 +0100
commitf77e26d4fbf97f42bf4d3d64c3f4146110725832 (patch)
treef197fafd60683c600305a18f82644c00bfedb504
parent41bdb29644799398a4b14d781f7d7696c7d276c0 (diff)
downloadgitlab-ce-23206-load-participants-async.tar.gz
Load participants async BE changes23206-load-participants-async
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js2
-rw-r--r--app/controllers/projects/issues_controller.rb2
-rw-r--r--app/controllers/projects/merge_requests_controller.rb2
-rw-r--r--app/helpers/issuables_helper.rb22
-rw-r--r--app/models/concerns/subscribable.rb2
-rw-r--r--app/serializers/issuable_entity.rb10
-rw-r--r--app/serializers/issuable_sidebar_entity.rb16
-rw-r--r--app/serializers/issue_serializer.rb15
-rw-r--r--app/serializers/issue_sidebar_entity.rb3
-rw-r--r--app/serializers/merge_request_basic_entity.rb18
-rw-r--r--app/serializers/merge_request_serializer.rb9
-rw-r--r--spec/controllers/projects/merge_requests_controller_spec.rb6
-rw-r--r--spec/features/boards/boards_spec.rb2
-rw-r--r--spec/fixtures/api/schemas/entities/issue.json44
-rw-r--r--spec/fixtures/api/schemas/entities/issue_sidebar.json21
-rw-r--r--spec/fixtures/api/schemas/entities/label.json26
-rw-r--r--spec/fixtures/api/schemas/entities/merge_request_basic.json4
-rw-r--r--spec/fixtures/api/schemas/issue.json27
-rw-r--r--spec/models/concerns/subscribable_spec.rb6
-rw-r--r--spec/serializers/issue_serializer_spec.rb27
-rw-r--r--spec/serializers/merge_request_basic_serializer_spec.rb10
-rw-r--r--spec/serializers/merge_request_serializer_spec.rb12
22 files changed, 204 insertions, 82 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js b/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js
index 79c3d335679..99f5c305df5 100644
--- a/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js
+++ b/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js
@@ -11,7 +11,7 @@ export default class MRWidgetService {
this.removeWIPResource = Vue.resource(endpoints.removeWIPPath);
this.removeSourceBranchResource = Vue.resource(endpoints.sourceBranchPath);
this.deploymentsResource = Vue.resource(endpoints.ciEnvironmentsStatusPath);
- this.pollResource = Vue.resource(`${endpoints.statusPath}?basic=true`);
+ this.pollResource = Vue.resource(`${endpoints.statusPath}?serializer=basic`);
this.mergeActionsContentResource = Vue.resource(endpoints.mergeActionsContentPath);
}
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index fe1334c0cfe..6a5e4538717 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -74,7 +74,7 @@ class Projects::IssuesController < Projects::ApplicationController
respond_to do |format|
format.html
format.json do
- render json: serializer.represent(@issue)
+ render json: serializer.represent(@issue, serializer: params[:serializer])
end
end
end
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index c5204080333..2b0294c8387 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -83,7 +83,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
format.json do
Gitlab::PollingInterval.set_header(response, interval: 10_000)
- render json: serializer.represent(@merge_request, basic: params[:basic])
+ render json: serializer.represent(@merge_request, serializer: params[:serializer])
end
format.patch do
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index 09621c886ba..d0069cd48cf 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -33,15 +33,17 @@ module IssuablesHelper
end
def serialize_issuable(issuable)
- case issuable
- when Issue
- IssueSerializer.new(current_user: current_user, project: issuable.project).represent(issuable).to_json
- when MergeRequest
- MergeRequestSerializer
- .new(current_user: current_user, project: issuable.project)
- .represent(issuable)
- .to_json
- end
+ serializer_klass = case issuable
+ when Issue
+ IssueSerializer
+ when MergeRequest
+ MergeRequestSerializer
+ end
+
+ serializer_klass
+ .new(current_user: current_user, project: issuable.project)
+ .represent(issuable)
+ .to_json
end
def template_dropdown_tag(issuable, &block)
@@ -357,7 +359,7 @@ module IssuablesHelper
def issuable_sidebar_options(issuable, can_edit_issuable)
{
- endpoint: "#{issuable_json_path(issuable)}?basic=true",
+ endpoint: "#{issuable_json_path(issuable)}?serializer=sidebar",
toggleSubscriptionEndpoint: toggle_subscription_path(issuable),
moveIssueEndpoint: move_namespace_project_issue_path(namespace_id: issuable.project.namespace.to_param, project_id: issuable.project, id: issuable),
projectsAutocompleteEndpoint: autocomplete_projects_path(project_id: @project.id),
diff --git a/app/models/concerns/subscribable.rb b/app/models/concerns/subscribable.rb
index 274b38a7708..f478c8ede18 100644
--- a/app/models/concerns/subscribable.rb
+++ b/app/models/concerns/subscribable.rb
@@ -13,6 +13,8 @@ module Subscribable
end
def subscribed?(user, project = nil)
+ return false unless user
+
if subscription = subscriptions.find_by(user: user, project: project)
subscription.subscribed
else
diff --git a/app/serializers/issuable_entity.rb b/app/serializers/issuable_entity.rb
index 91efad08ef8..61c7a428745 100644
--- a/app/serializers/issuable_entity.rb
+++ b/app/serializers/issuable_entity.rb
@@ -17,14 +17,4 @@ class IssuableEntity < Grape::Entity
expose :human_total_time_spent
expose :milestone, using: API::Entities::Milestone
expose :labels, using: LabelEntity
- expose :participants, using: ::API::Entities::UserBasic do |issuable|
- issuable.participants(current_user)
- end
- expose :subscribed do |issuable|
- issuable.subscribed?(current_user)
- end
-
- private
-
- delegate :current_user, to: :request
end
diff --git a/app/serializers/issuable_sidebar_entity.rb b/app/serializers/issuable_sidebar_entity.rb
new file mode 100644
index 00000000000..ff23d8bf0c7
--- /dev/null
+++ b/app/serializers/issuable_sidebar_entity.rb
@@ -0,0 +1,16 @@
+class IssuableSidebarEntity < Grape::Entity
+ include RequestAwareEntity
+
+ expose :participants, using: ::API::Entities::UserBasic do |issuable|
+ issuable.participants(request.current_user)
+ end
+
+ expose :subscribed do |issuable|
+ issuable.subscribed?(request.current_user, issuable.project)
+ end
+
+ expose :time_estimate
+ expose :total_time_spent
+ expose :human_time_estimate
+ expose :human_total_time_spent
+end
diff --git a/app/serializers/issue_serializer.rb b/app/serializers/issue_serializer.rb
index 4fff54a9126..2555595379b 100644
--- a/app/serializers/issue_serializer.rb
+++ b/app/serializers/issue_serializer.rb
@@ -1,3 +1,16 @@
class IssueSerializer < BaseSerializer
- entity IssueEntity
+ # 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(merge_request, opts = {})
+ entity =
+ case opts[:serializer]
+ when 'sidebar'
+ IssueSidebarEntity
+ else
+ IssueEntity
+ end
+
+ super(merge_request, opts, entity)
+ end
end
diff --git a/app/serializers/issue_sidebar_entity.rb b/app/serializers/issue_sidebar_entity.rb
new file mode 100644
index 00000000000..6c823dbfe95
--- /dev/null
+++ b/app/serializers/issue_sidebar_entity.rb
@@ -0,0 +1,3 @@
+class IssueSidebarEntity < IssuableSidebarEntity
+ expose :assignees, using: API::Entities::UserBasic
+end
diff --git a/app/serializers/merge_request_basic_entity.rb b/app/serializers/merge_request_basic_entity.rb
index 70a60f19d23..d54a6516aed 100644
--- a/app/serializers/merge_request_basic_entity.rb
+++ b/app/serializers/merge_request_basic_entity.rb
@@ -1,23 +1,7 @@
-class MergeRequestBasicEntity < Grape::Entity
- include RequestAwareEntity
-
+class MergeRequestBasicEntity < IssuableSidebarEntity
expose :assignee_id
expose :merge_status
expose :merge_error
expose :state
expose :source_branch_exists?, as: :source_branch_exists
- expose :time_estimate
- expose :total_time_spent
- expose :human_time_estimate
- expose :human_total_time_spent
- expose :participants, using: ::API::Entities::UserBasic do |issuable|
- issuable.participants(current_user)
- end
- expose :subscribed do |issuable|
- issuable.subscribed?(current_user)
- end
-
- private
-
- delegate :current_user, to: :request
end
diff --git a/app/serializers/merge_request_serializer.rb b/app/serializers/merge_request_serializer.rb
index f67034ce47a..e9d98d8baca 100644
--- a/app/serializers/merge_request_serializer.rb
+++ b/app/serializers/merge_request_serializer.rb
@@ -3,7 +3,14 @@ class MergeRequestSerializer < BaseSerializer
# to serialize the `merge_request` based on `basic` key in `opts` param.
# Hence, `entity` doesn't need to be declared on the class scope.
def represent(merge_request, opts = {})
- entity = opts[:basic] ? MergeRequestBasicEntity : MergeRequestEntity
+ entity =
+ case opts[:serializer]
+ when 'basic', 'sidebar'
+ MergeRequestBasicEntity
+ else
+ MergeRequestEntity
+ end
+
super(merge_request, opts, entity)
end
end
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index b7cccddefdd..52ef8c6a589 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -83,15 +83,15 @@ describe Projects::MergeRequestsController do
end
describe 'as json' do
- context 'with basic param' do
+ context 'with basic serializer param' do
it 'renders basic MR entity as json' do
- go(basic: true, format: :json)
+ go(serializer: 'basic', format: :json)
expect(response).to match_response_schema('entities/merge_request_basic')
end
end
- context 'without basic param' do
+ context 'without basic serializer param' do
it 'renders the merge request in the json format' do
go(format: :json)
diff --git a/spec/features/boards/boards_spec.rb b/spec/features/boards/boards_spec.rb
index 60ed17c0c81..ebe6939df4c 100644
--- a/spec/features/boards/boards_spec.rb
+++ b/spec/features/boards/boards_spec.rb
@@ -538,7 +538,7 @@ describe 'Issue Boards', :js do
end
it 'does not show create new list' do
- expect(page).not_to have_selector('.js-new-board-list')
+ expect(page).not_to have_button('.js-new-board-list')
end
it 'does not allow dragging' do
diff --git a/spec/fixtures/api/schemas/entities/issue.json b/spec/fixtures/api/schemas/entities/issue.json
new file mode 100644
index 00000000000..3d3329a3406
--- /dev/null
+++ b/spec/fixtures/api/schemas/entities/issue.json
@@ -0,0 +1,44 @@
+{
+ "type": "object",
+ "properties" : {
+ "id": { "type": "integer" },
+ "iid": { "type": "integer" },
+ "author_id": { "type": "integer" },
+ "description": { "type": ["string", "null"] },
+ "lock_version": { "type": ["string", "null"] },
+ "milestone_id": { "type": ["string", "null"] },
+ "title": { "type": "string" },
+ "moved_to_id": { "type": ["integer", "null"] },
+ "project_id": { "type": "integer" },
+ "web_url": { "type": "string" },
+ "state": { "type": "string" },
+ "create_note_path": { "type": "string" },
+ "preview_note_path": { "type": "string" },
+ "current_user": {
+ "type": "object",
+ "properties": {
+ "can_create_note": { "type": "boolean" },
+ "can_update": { "type": "boolean" }
+ }
+ },
+ "created_at": { "type": "date-time" },
+ "updated_at": { "type": "date-time" },
+ "branch_name": { "type": ["string", "null"] },
+ "due_date": { "type": "date" },
+ "confidential": { "type": "boolean" },
+ "discussion_locked": { "type": ["boolean", "null"] },
+ "updated_by_id": { "type": ["string", "null"] },
+ "deleted_at": { "type": ["string", "null"] },
+ "time_estimate": { "type": "integer" },
+ "total_time_spent": { "type": "integer" },
+ "human_time_estimate": { "type": ["integer", "null"] },
+ "human_total_time_spent": { "type": ["integer", "null"] },
+ "milestone": { "type": ["object", "null"] },
+ "labels": {
+ "type": "array",
+ "items": { "$ref": "label.json" }
+ },
+ "assignees": { "type": ["array", "null"] }
+ },
+ "additionalProperties": false
+}
diff --git a/spec/fixtures/api/schemas/entities/issue_sidebar.json b/spec/fixtures/api/schemas/entities/issue_sidebar.json
new file mode 100644
index 00000000000..682e345d5f5
--- /dev/null
+++ b/spec/fixtures/api/schemas/entities/issue_sidebar.json
@@ -0,0 +1,21 @@
+{
+ "type": "object",
+ "properties" : {
+ "id": { "type": "integer" },
+ "iid": { "type": "integer" },
+ "subscribed": { "type": "boolean" },
+ "time_estimate": { "type": "integer" },
+ "total_time_spent": { "type": "integer" },
+ "human_time_estimate": { "type": ["integer", "null"] },
+ "human_total_time_spent": { "type": ["integer", "null"] },
+ "participants": {
+ "type": "array",
+ "items": { "$ref": "../public_api/v4/user/basic.json" }
+ },
+ "assignees": {
+ "type": "array",
+ "items": { "$ref": "../public_api/v4/user/basic.json" }
+ }
+ },
+ "additionalProperties": false
+}
diff --git a/spec/fixtures/api/schemas/entities/label.json b/spec/fixtures/api/schemas/entities/label.json
new file mode 100644
index 00000000000..40dff764c17
--- /dev/null
+++ b/spec/fixtures/api/schemas/entities/label.json
@@ -0,0 +1,26 @@
+{
+ "type": "object",
+ "required": [
+ "id",
+ "color",
+ "description",
+ "title",
+ "priority"
+ ],
+ "properties": {
+ "id": { "type": "integer" },
+ "color": {
+ "type": "string",
+ "pattern": "^#[0-9A-Fa-f]{3}{1,2}$"
+ },
+ "description": { "type": ["string", "null"] },
+ "text_color": {
+ "type": "string",
+ "pattern": "^#[0-9A-Fa-f]{3}{1,2}$"
+ },
+ "type": { "type": "string" },
+ "title": { "type": "string" },
+ "priority": { "type": ["integer", "null"] }
+ },
+ "additionalProperties": false
+} \ No newline at end of file
diff --git a/spec/fixtures/api/schemas/entities/merge_request_basic.json b/spec/fixtures/api/schemas/entities/merge_request_basic.json
index 6b14188582a..995f13381ad 100644
--- a/spec/fixtures/api/schemas/entities/merge_request_basic.json
+++ b/spec/fixtures/api/schemas/entities/merge_request_basic.json
@@ -9,7 +9,9 @@
"human_time_estimate": { "type": ["string", "null"] },
"human_total_time_spent": { "type": ["string", "null"] },
"merge_error": { "type": ["string", "null"] },
- "assignee_id": { "type": ["integer", "null"] }
+ "assignee_id": { "type": ["integer", "null"] },
+ "subscribed": { "type": ["boolean", "null"] },
+ "participants": { "type": "array" }
},
"additionalProperties": false
}
diff --git a/spec/fixtures/api/schemas/issue.json b/spec/fixtures/api/schemas/issue.json
index e1f62508933..a55ecaa5697 100644
--- a/spec/fixtures/api/schemas/issue.json
+++ b/spec/fixtures/api/schemas/issue.json
@@ -19,32 +19,7 @@
},
"labels": {
"type": "array",
- "items": {
- "type": "object",
- "required": [
- "id",
- "color",
- "description",
- "title",
- "priority"
- ],
- "properties": {
- "id": { "type": "integer" },
- "color": {
- "type": "string",
- "pattern": "^#[0-9A-Fa-f]{3}{1,2}+$"
- },
- "description": { "type": ["string", "null"] },
- "text_color": {
- "type": "string",
- "pattern": "^#[0-9A-Fa-f]{3}{1,2}+$"
- },
- "type": { "type": "string" },
- "title": { "type": "string" },
- "priority": { "type": ["integer", "null"] }
- },
- "additionalProperties": false
- }
+ "items": { "$ref": "entities/label.json" }
},
"assignee": {
"id": { "type": "integet" },
diff --git a/spec/models/concerns/subscribable_spec.rb b/spec/models/concerns/subscribable_spec.rb
index 28ff8158e0e..45dfb136aea 100644
--- a/spec/models/concerns/subscribable_spec.rb
+++ b/spec/models/concerns/subscribable_spec.rb
@@ -6,6 +6,12 @@ describe Subscribable, 'Subscribable' do
let(:user_1) { create(:user) }
describe '#subscribed?' do
+ context 'without user' do
+ it 'returns false' do
+ expect(resource.subscribed?(nil, project)).to be_falsey
+ end
+ end
+
context 'without project' do
it 'returns false when no subscription exists' do
expect(resource.subscribed?(user_1)).to be_falsey
diff --git a/spec/serializers/issue_serializer_spec.rb b/spec/serializers/issue_serializer_spec.rb
new file mode 100644
index 00000000000..75578816e75
--- /dev/null
+++ b/spec/serializers/issue_serializer_spec.rb
@@ -0,0 +1,27 @@
+require 'spec_helper'
+
+describe IssueSerializer do
+ let(:resource) { create(:issue) }
+ let(:user) { create(:user) }
+ let(:json_entity) do
+ described_class.new(current_user: user)
+ .represent(resource, serializer: serializer)
+ .with_indifferent_access
+ end
+
+ context 'non-sidebar issue serialization' do
+ let(:serializer) { nil }
+
+ it 'matches issue json schema' do
+ expect(json_entity).to match_schema('entities/issue')
+ end
+ end
+
+ context 'sidebar issue serialization' do
+ let(:serializer) { 'sidebar' }
+
+ it 'matches sidebar issue json schema' do
+ expect(json_entity).to match_schema('entities/issue_sidebar')
+ end
+ end
+end
diff --git a/spec/serializers/merge_request_basic_serializer_spec.rb b/spec/serializers/merge_request_basic_serializer_spec.rb
index 4daf5a59d0c..1fad8e6bc5d 100644
--- a/spec/serializers/merge_request_basic_serializer_spec.rb
+++ b/spec/serializers/merge_request_basic_serializer_spec.rb
@@ -4,9 +4,13 @@ describe MergeRequestBasicSerializer do
let(:resource) { create(:merge_request) }
let(:user) { create(:user) }
- subject { described_class.new.represent(resource) }
+ let(:json_entity) do
+ described_class.new(current_user: user)
+ .represent(resource, serializer: 'basic')
+ .with_indifferent_access
+ end
- it 'has important MergeRequest attributes' do
- expect(subject).to include(:merge_status)
+ it 'matches basic merge request json' do
+ expect(json_entity).to match_schema('entities/merge_request_basic')
end
end
diff --git a/spec/serializers/merge_request_serializer_spec.rb b/spec/serializers/merge_request_serializer_spec.rb
index 73fbecc153d..e3abefa6d63 100644
--- a/spec/serializers/merge_request_serializer_spec.rb
+++ b/spec/serializers/merge_request_serializer_spec.rb
@@ -9,11 +9,11 @@ describe MergeRequestSerializer do
end
describe '#represent' do
- let(:opts) { { basic: basic } }
- subject { serializer.represent(merge_request, basic: basic) }
+ let(:opts) { { serializer: serializer_entity } }
+ subject { serializer.represent(merge_request, serializer: serializer_entity) }
- context 'when basic param is truthy' do
- let(:basic) { true }
+ context 'when passing basic serializer param' do
+ let(:serializer_entity) { 'basic' }
it 'calls super class #represent with correct params' do
expect_any_instance_of(BaseSerializer).to receive(:represent)
@@ -23,8 +23,8 @@ describe MergeRequestSerializer do
end
end
- context 'when basic param is falsy' do
- let(:basic) { false }
+ context 'when serializer param is falsy' do
+ let(:serializer_entity) { nil }
it 'calls super class #represent with correct params' do
expect_any_instance_of(BaseSerializer).to receive(:represent)