Extract MR's widget into a separate endpoint

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

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,
       };

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() {

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;

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

app/controllers/projects/merge_requests/content_controller.rb

+# 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

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?

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|

changelogs/unreleased/id-extract-widget-into-different-request.yml

+---
+title: Add a separate endpoint for fetching MRs serialized as widgets
+merge_request: 29979
+author:
+type: performance

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'

spec/controllers/projects/merge_requests/content_controller_spec.rb

+# 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

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" },

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,