summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIgor Drozdov <idrozdov@gitlab.com>2019-06-21 21:22:55 +0300
committerIgor Drozdov <idrozdov@gitlab.com>2019-06-28 10:52:48 +0300
commit07559fda51dd32cf23a14567cc7077c02c3900d4 (patch)
tree820165867e8c10af7705db80b7d868cc57c838eb
parent546355f734f74c040d0ef0917ade50751fd90731 (diff)
downloadgitlab-ce-id-extract-widget-into-different-request.tar.gz
Extract MR's widget into a separate endpointid-extract-widget-into-different-request
This commits extracts /merge_requests/1.json?serializer=widget Into a separate /merge_requests/1/widget.json endpoint This will allow to use caching for this request
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue3
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js4
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js3
-rw-r--r--app/controllers/projects/merge_requests/application_controller.rb7
-rw-r--r--app/controllers/projects/merge_requests/content_controller.rb22
-rw-r--r--app/controllers/projects/merge_requests_controller.rb6
-rw-r--r--app/serializers/merge_request_widget_entity.rb8
-rw-r--r--changelogs/unreleased/id-extract-widget-into-different-request.yml5
-rw-r--r--config/routes/project.rb1
-rw-r--r--spec/controllers/projects/merge_requests/content_controller_spec.rb60
-rw-r--r--spec/fixtures/api/schemas/entities/merge_request_widget.json3
-rw-r--r--spec/javascripts/vue_mr_widget/mock_data.js3
12 files changed, 111 insertions, 14 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
index 41386178a1e..a79da476890 100644
--- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
@@ -162,7 +162,8 @@ export default {
removeWIPPath: store.removeWIPPath,
sourceBranchPath: store.sourceBranchPath,
ciEnvironmentsStatusPath: store.ciEnvironmentsStatusPath,
- statusPath: store.statusPath,
+ mergeRequestBasicPath: store.mergeRequestBasicPath,
+ mergeRequestWidgetPath: store.mergeRequestWidgetPath,
mergeActionsContentPath: store.mergeActionsContentPath,
rebasePath: store.rebasePath,
};
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 0bb70bfd658..1dae53039d5 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
@@ -30,11 +30,11 @@ export default class MRWidgetService {
}
poll() {
- return axios.get(`${this.endpoints.statusPath}?serializer=basic`);
+ return axios.get(this.endpoints.mergeRequestBasicPath);
}
checkStatus() {
- return axios.get(`${this.endpoints.statusPath}?serializer=widget`);
+ return axios.get(this.endpoints.mergeRequestWidgetPath);
}
fetchMergeActionsContent() {
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
index bfa3e7f4a59..581fee7477f 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
@@ -86,7 +86,8 @@ export default class MergeRequestStore {
this.mergePath = data.merge_path;
this.ffOnlyEnabled = data.ff_only_enabled;
this.shouldBeRebased = Boolean(data.should_be_rebased);
- this.statusPath = data.status_path;
+ this.mergeRequestBasicPath = data.merge_request_basic_path;
+ this.mergeRequestWidgetPath = data.merge_request_widget_path;
this.emailPatchesPath = data.email_patches_path;
this.plainDiffPath = data.plain_diff_path;
this.newBlobPath = data.new_blob_path;
diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb
index f2a6268b3e9..dcc272aecff 100644
--- a/app/controllers/projects/merge_requests/application_controller.rb
+++ b/app/controllers/projects/merge_requests/application_controller.rb
@@ -51,4 +51,11 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
Ci::Pipeline.none
end
end
+
+ def close_merge_request_if_no_source_project
+ return if @merge_request.source_project
+ return unless @merge_request.open?
+
+ @merge_request.close
+ end
end
diff --git a/app/controllers/projects/merge_requests/content_controller.rb b/app/controllers/projects/merge_requests/content_controller.rb
new file mode 100644
index 00000000000..6e026b83ee3
--- /dev/null
+++ b/app/controllers/projects/merge_requests/content_controller.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+class Projects::MergeRequests::ContentController < Projects::MergeRequests::ApplicationController
+ # @merge_request.check_mergeability is not executed here since
+ # widget serializer calls it via mergeable? method
+ # but we might want to call @merge_request.check_mergeability
+ # for other types of serialization
+
+ before_action :close_merge_request_if_no_source_project
+ around_action :allow_gitaly_ref_name_caching
+
+ def widget
+ respond_to do |format|
+ format.json do
+ Gitlab::PollingInterval.set_header(response, interval: 10_000)
+
+ serializer = MergeRequestSerializer.new(current_user: current_user, project: merge_request.project)
+ render json: serializer.represent(merge_request, serializer: 'widget')
+ end
+ end
+ end
+end
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index fc37ce1dbc4..7ee8e0ea8f8 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -235,12 +235,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
params[:auto_merge_strategy].present? || params[:merge_when_pipeline_succeeds].present?
end
- def close_merge_request_if_no_source_project
- if !@merge_request.source_project && @merge_request.open?
- @merge_request.close
- end
- end
-
private
def ci_environments_status_on_merge_result?
diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb
index 43aced598a9..fd2673fa0cc 100644
--- a/app/serializers/merge_request_widget_entity.rb
+++ b/app/serializers/merge_request_widget_entity.rb
@@ -217,8 +217,12 @@ class MergeRequestWidgetEntity < IssuableEntity
project_merge_request_path(merge_request.project, merge_request, format: :diff)
end
- expose :status_path do |merge_request|
- project_merge_request_path(merge_request.target_project, merge_request, format: :json)
+ expose :merge_request_basic_path do |merge_request|
+ project_merge_request_path(merge_request.target_project, merge_request, serializer: :basic, format: :json)
+ end
+
+ expose :merge_request_widget_path do |merge_request|
+ widget_project_json_merge_request_path(merge_request.target_project, merge_request, format: :json)
end
expose :ci_environments_status_path do |merge_request|
diff --git a/changelogs/unreleased/id-extract-widget-into-different-request.yml b/changelogs/unreleased/id-extract-widget-into-different-request.yml
new file mode 100644
index 00000000000..3b9f5fdd6bd
--- /dev/null
+++ b/changelogs/unreleased/id-extract-widget-into-different-request.yml
@@ -0,0 +1,5 @@
+---
+title: Add a separate endpoint for fetching MRs serialized as widgets
+merge_request: 29979
+author:
+type: performance
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 0e8e089c78a..898440e68eb 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -228,6 +228,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
get :commits
get :pipelines
get :diffs, to: 'merge_requests/diffs#show'
+ get :widget, to: 'merge_requests/content#widget'
end
get :diff_for_path, controller: 'merge_requests/diffs'
diff --git a/spec/controllers/projects/merge_requests/content_controller_spec.rb b/spec/controllers/projects/merge_requests/content_controller_spec.rb
new file mode 100644
index 00000000000..2879e06aee4
--- /dev/null
+++ b/spec/controllers/projects/merge_requests/content_controller_spec.rb
@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::MergeRequests::ContentController do
+ let(:project) { create(:project, :repository) }
+ let(:user) { create(:user) }
+ let(:merge_request) { create(:merge_request, target_project: project, source_project: project) }
+
+ before do
+ sign_in(user)
+ end
+
+ def do_request
+ get :widget, params: {
+ namespace_id: project.namespace.to_param,
+ project_id: project,
+ id: merge_request.iid,
+ format: :json
+ }
+ end
+
+ describe 'GET widget' do
+ context 'user has access to the project' do
+ before do
+ expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original
+
+ project.add_maintainer(user)
+ end
+
+ it 'renders widget MR entity as json' do
+ do_request
+
+ expect(response).to match_response_schema('entities/merge_request_widget')
+ end
+
+ it 'checks whether the MR can be merged' do
+ controller.instance_variable_set(:@merge_request, merge_request)
+
+ expect(merge_request).to receive(:check_mergeability)
+
+ do_request
+ end
+
+ it 'closes an MR with moved source project' do
+ merge_request.update_column(:source_project_id, nil)
+
+ expect { do_request }.to change { merge_request.reload.open? }.from(true).to(false)
+ end
+ end
+
+ context 'user does not have access to the project' do
+ it 'renders widget MR entity as json' do
+ do_request
+
+ expect(response).to have_http_status(:not_found)
+ end
+ end
+ end
+end
diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json
index 7018cb9a305..eac1dbc6474 100644
--- a/spec/fixtures/api/schemas/entities/merge_request_widget.json
+++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json
@@ -99,7 +99,8 @@
"revert_in_fork_path": { "type": ["string", "null"] },
"email_patches_path": { "type": "string" },
"plain_diff_path": { "type": "string" },
- "status_path": { "type": "string" },
+ "merge_request_basic_path": { "type": "string" },
+ "merge_request_widget_path": { "type": "string" },
"new_blob_path": { "type": ["string", "null"] },
"merge_check_path": { "type": "string" },
"ci_environments_status_path": { "type": "string" },
diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js
index 48f812f0db4..253413ae43e 100644
--- a/spec/javascripts/vue_mr_widget/mock_data.js
+++ b/spec/javascripts/vue_mr_widget/mock_data.js
@@ -218,7 +218,8 @@ export default {
'/root/acets-app/forks?continue%5Bnotice%5D=You%27re+not+allowed+to+make+changes+to+this+project+directly.+A+fork+of+this+project+has+been+created+that+you+can+make+changes+in%2C+so+you+can+submit+a+merge+request.+Try+to+cherry-pick+this+commit+again.&continue%5Bnotice_now%5D=You%27re+not+allowed+to+make+changes+to+this+project+directly.+A+fork+of+this+project+is+being+created+that+you+can+make+changes+in%2C+so+you+can+submit+a+merge+request.&continue%5Bto%5D=%2Froot%2Facets-app%2Fmerge_requests%2F22&namespace_key=1',
email_patches_path: '/root/acets-app/merge_requests/22.patch',
plain_diff_path: '/root/acets-app/merge_requests/22.diff',
- status_path: '/root/acets-app/merge_requests/22.json',
+ merge_request_basic_path: '/root/acets-app/merge_requests/22.json?serializer=basic',
+ merge_request_widget_path: '/root/acets-app/merge_requests/22/widget.json',
merge_check_path: '/root/acets-app/merge_requests/22/merge_check',
ci_environments_status_url: '/root/acets-app/merge_requests/22/ci_environments_status',
project_archived: false,