Merge branch 'slashmanov/rapid-diffs-merge-request-diff-file-component' into 'master'

Add MergeRequestDiffFileComponent to Rapid Diffs

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/183592



Merged-by: Phil Hughes <me@iamphill.com>
Approved-by: Thomas Randolph <trandolph@gitlab.com>
Approved-by: Phil Hughes <me@iamphill.com>
Approved-by: Jacques Erasmus <jerasmus@gitlab.com>
Co-authored-by: Stanislav Lashmanov <slashmanov@gitlab.com>

app/components/rapid_diffs/app_component.html.haml

@@ -9,6 +9,9 @@
     .rd-app-content{ data: { sidebar_visible: true } }
       .code{ class: helpers.user_color_scheme }
         %div{ data: { diffs_list: true } }
-          = render RapidDiffs::DiffFileComponent.with_collection(@diffs_slice, parallel_view: @diff_view == :parallel)
+          - if diffs_list?
+            = diffs_list
+          - else
+            = render RapidDiffs::DiffFileComponent.with_collection(@diffs_slice, parallel_view: @diff_view == :parallel)
           - if @stream_url
             #js-stream-container{ data: { diffs_stream_url: @stream_url } }

app/components/rapid_diffs/app_component.rb

@@ -2,6 +2,8 @@
 
 module RapidDiffs
   class AppComponent < ViewComponent::Base
+    renders_one :diffs_list
+
     def initialize(
       diffs_slice:,
       reload_stream_url:,

app/components/rapid_diffs/merge_request_diff_file_component.html.haml

+= render RapidDiffs::DiffFileComponent.new(diff_file: @diff_file, parallel_view: @parallel_view)

app/components/rapid_diffs/merge_request_diff_file_component.rb

+# frozen_string_literal: true
+
+module RapidDiffs
+  class MergeRequestDiffFileComponent < ViewComponent::Base
+    with_collection_parameter :diff_file
+
+    def initialize(diff_file:, merge_request:, parallel_view: false)
+      @diff_file = diff_file
+      @merge_request = merge_request
+      @parallel_view = parallel_view
+    end
+  end
+end

app/controllers/projects/merge_requests/diffs_stream_controller.rb

@@ -10,6 +10,17 @@ class DiffsStreamController < Projects::MergeRequests::ApplicationController
       def resource
         @merge_request
       end
+
+      def render_diff_file(diff_file)
+        render_to_string(
+          ::RapidDiffs::MergeRequestDiffFileComponent.new(
+            diff_file: diff_file,
+            merge_request: @merge_request,
+            parallel_view: view == :parallel
+          ),
+          layout: false
+        )
+      end
     end
   end
 end

app/views/projects/merge_requests/rapid_diffs.html.haml

@@ -4,4 +4,6 @@
 
 = render 'page'
 - args = { diffs_slice: @diffs_slice, reload_stream_url: @reload_stream_url, stream_url: @stream_url, show_whitespace: @show_whitespace_default, diff_view: @diff_view, update_user_endpoint: @update_current_user_path, metadata_endpoint: @endpoint_metadata_url }
-= render ::RapidDiffs::AppComponent.new(**args)
+= render ::RapidDiffs::AppComponent.new(**args) do |c|
+  - c.with_diffs_list do
+    = render RapidDiffs::MergeRequestDiffFileComponent.with_collection(@diffs_slice, merge_request: @merge_request, parallel_view: @diff_view == :parallel)

spec/components/rapid_diffs/app_component_spec.rb

@@ -53,7 +53,16 @@
     expect(container['data-diffs-stream-url']).to eq(stream_url)
   end
 
-  def render_component
+  it "renders diffs_list slot" do
+    result = render_component do |c|
+      c.with_diffs_list do
+        'custom_list'
+      end
+    end
+    expect(result).to have_text('custom_list')
+  end
+
+  def render_component(&block)
     render_inline(described_class.new(
       diffs_slice:,
       stream_url:,
@@ -62,6 +71,6 @@ def render_component
       diff_view:,
       update_user_endpoint:,
       metadata_endpoint:
-    ))
+    ), &block)
   end
 end

spec/components/rapid_diffs/diff_file_component_spec.rb

@@ -2,74 +2,10 @@
 
 require "spec_helper"
 
-RSpec.describe RapidDiffs::DiffFileComponent, type: :component, feature_category: :code_review_workflow do
-  let_it_be(:diff_file) { build(:diff_file) }
-  let(:repository) { diff_file.repository }
-  let(:project) { repository.container }
-  let(:namespace) { project.namespace }
-  let(:web_component) { page.find('diff-file') }
-
-  it "renders" do
-    render_component
-    expect(page).to have_selector('diff-file')
-    expect(page).to have_selector('diff-file-mounted')
-  end
-
-  it "renders server data" do
-    render_component
-    diff_path = "/#{namespace.to_param}/#{project.to_param}/-/blob/#{diff_file.content_sha}/#{diff_file.file_path}"
-    expect(web_component['data-diff-lines-path']).to eq("#{diff_path}/diff_lines")
-  end
-
-  context "when is text diff" do
-    before do
-      allow(diff_file).to receive(:diffable_text?).and_return(true)
-    end
-
-    context "when file is not modified" do
-      before do
-        allow(diff_file).to receive(:modified_file?).and_return(false)
-      end
-
-      it "renders no preview" do
-        render_component
-        expect(web_component['data-viewer']).to eq('no_preview')
-      end
-    end
-
-    it "renders parallel text viewer" do
-      render_component
-      expect(web_component['data-viewer']).to eq('text_inline')
-    end
+require_relative './shared'
 
-    it "renders parallel text viewer" do
-      render_component(parallel_view: true)
-      expect(web_component['data-viewer']).to eq('text_parallel')
-    end
-  end
-
-  context "when no viewer found" do
-    before do
-      allow(diff_file).to receive(:text?).and_return(false)
-      allow(diff_file).to receive(:content_changed?).and_return(false)
-    end
-
-    it "renders no preview" do
-      render_component
-      expect(web_component['data-viewer']).to eq('no_preview')
-    end
-  end
-
-  context "when file is collapsed" do
-    before do
-      allow(diff_file).to receive(:collapsed?).and_return(true)
-    end
-
-    it "renders no preview" do
-      render_component
-      expect(web_component['data-viewer']).to eq('no_preview')
-    end
-  end
+RSpec.describe RapidDiffs::DiffFileComponent, type: :component, feature_category: :code_review_workflow do
+  include_context "with diff file component tests"
 
   def render_component(**args)
     render_inline(described_class.new(diff_file: diff_file, **args))

spec/components/rapid_diffs/merge_request_diff_file_component_spec.rb

+# frozen_string_literal: true
+
+require "spec_helper"
+
+require_relative './shared'
+
+RSpec.describe RapidDiffs::MergeRequestDiffFileComponent, type: :component, feature_category: :code_review_workflow do
+  include_context "with diff file component tests"
+  let_it_be(:merge_request) { build(:merge_request) }
+
+  def render_component(**args)
+    render_inline(described_class.new(diff_file:, merge_request:, **args))
+  end
+end

spec/components/rapid_diffs/shared.rb

+# frozen_string_literal: true
+
+require "spec_helper"
+
+RSpec.shared_context "with diff file component tests" do
+  let(:web_component_selector) { 'diff-file' }
+  let(:web_component) { page.find(web_component_selector) }
+  let(:diff_file) { build(:diff_file) }
+  let(:repository) { diff_file.repository }
+  let(:project) { repository.container }
+  let(:namespace) { project.namespace }
+
+  # This should be overridden in the including spec
+  def render_component
+    raise "Override render_component in the including spec"
+  end
+
+  it "renders" do
+    render_component
+    expect(page).to have_selector(web_component_selector)
+    expect(page).to have_selector("#{web_component_selector}-mounted")
+  end
+
+  it "renders server data" do
+    render_component
+    diff_path = "/#{namespace.to_param}/#{project.to_param}/-/blob/#{diff_file.content_sha}/#{diff_file.file_path}"
+    expect(web_component['data-diff-lines-path']).to eq("#{diff_path}/diff_lines")
+  end
+
+  context "when is text diff" do
+    before do
+      allow(diff_file).to receive(:diffable_text?).and_return(true)
+    end
+
+    context "when file is not modified" do
+      before do
+        allow(diff_file).to receive(:modified_file?).and_return(false)
+      end
+
+      it "renders no preview" do
+        render_component
+        expect(web_component['data-viewer']).to eq('no_preview')
+      end
+    end
+
+    it "renders inline text viewer" do
+      render_component
+      expect(web_component['data-viewer']).to eq('text_inline')
+    end
+
+    it "renders parallel text viewer" do
+      render_component(parallel_view: true)
+      expect(web_component['data-viewer']).to eq('text_parallel')
+    end
+  end
+
+  context "when no viewer found" do
+    before do
+      allow(diff_file).to receive_messages(text?: false, content_changed?: false)
+    end
+
+    it "renders no preview" do
+      render_component
+      expect(web_component['data-viewer']).to eq('no_preview')
+    end
+  end
+
+  context "when file is collapsed" do
+    before do
+      allow(diff_file).to receive(:collapsed?).and_return(true)
+    end
+
+    it "renders no preview" do
+      render_component
+      expect(web_component['data-viewer']).to eq('no_preview')
+    end
+  end
+end

spec/requests/projects/merge_requests/diffs_stream_spec.rb

@@ -51,6 +51,14 @@ def go(**extra_params)
 
         go
       end
+
+      it 'renders merge request diff file component' do
+        expect_any_instance_of(::RapidDiffs::MergeRequestDiffFileComponent) do |component|
+          expect(component).to receive(:render_in).and_call_original
+        end
+
+        go
+      end
     end
 
     context 'when offset is not given' do