diff options
author | Igor Drozdov <idrozdov@gitlab.com> | 2019-06-21 21:22:55 +0300 |
---|---|---|
committer | Igor Drozdov <idrozdov@gitlab.com> | 2019-06-28 10:52:48 +0300 |
commit | 07559fda51dd32cf23a14567cc7077c02c3900d4 (patch) | |
tree | 820165867e8c10af7705db80b7d868cc57c838eb | |
parent | 546355f734f74c040d0ef0917ade50751fd90731 (diff) | |
download | gitlab-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
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, |