From 4f331fca975f641880dc093c6681c75b280b1b59 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 11:00:20 -0600 Subject: [PATCH 01/25] Draft --- ...re_revisions_diff_file_component.html.haml | 4 ++ .../compare_revisions_diff_file_component.rb | 39 +++++++++++++++++++ .../projects/compare/rapid_diffs.html.haml | 4 +- 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 app/components/rapid_diffs/compare_revisions_diff_file_component.html.haml create mode 100644 app/components/rapid_diffs/compare_revisions_diff_file_component.rb diff --git a/app/components/rapid_diffs/compare_revisions_diff_file_component.html.haml b/app/components/rapid_diffs/compare_revisions_diff_file_component.html.haml new file mode 100644 index 00000000000000..e0b66381d37d37 --- /dev/null +++ b/app/components/rapid_diffs/compare_revisions_diff_file_component.html.haml @@ -0,0 +1,4 @@ += render RapidDiffs::DiffFileComponent.new(diff_file:, parallel_view: @parallel_view) do |c| + - c.with_header do + = render RapidDiffs::DiffFileHeaderComponent.new(diff_file:, additional_menu_items:) + end \ No newline at end of file diff --git a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb new file mode 100644 index 00000000000000..81f7cbb8d6d0c7 --- /dev/null +++ b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module RapidDiffs + class CompareRevisionsDiffFileComponent < ViewComponent::Base + with_collection_parameter :diff_file + + attr_reader :diff_file + + def initialize(diff_file:, parallel_view: false) + @diff_file = diff_file + @parallel_view = parallel_view + end + + def additional_menu_items + [ + view_file_item + ].compact + end + + private + + def view_file_item + { + text: view_file_button_text, + href: @diff_file.view_path, + extraAttrs: { target: '_blank' }, + position: 1 + } + end + + def view_file_button_text + truncated_sha = helpers.truncate_sha(@diff_file.content_sha) + helpers.sprintf( + s__('MergeRequests|View file @ %{commitId}'), + { commitId: truncated_sha } + ) + end + end +end diff --git a/app/views/projects/compare/rapid_diffs.html.haml b/app/views/projects/compare/rapid_diffs.html.haml index b1589dfdeb8309..d6e56238b793f0 100644 --- a/app/views/projects/compare/rapid_diffs.html.haml +++ b/app/views/projects/compare/rapid_diffs.html.haml @@ -16,7 +16,9 @@ .container-fluid{ class: [container_class] } = render "projects/commits/commit_list" unless hide_commit_list .container-fluid - = render ::RapidDiffs::AppComponent.new(@rapid_diffs_presenter) + = render ::RapidDiffs::AppComponent.new(@rapid_diffs_presenter) do |c| + - c.with_diffs_list do + = render RapidDiffs::CompareRevisionsDiffFileComponent.with_collection(c.diffs_slice, parallel_view: c.diff_view == :parallel) - else .container-fluid = render Pajamas::CardComponent.new(card_options: { class: "gl-bg-subtle" }) do |c| -- GitLab From 0514fb2effafc9768cfe882faf7d5c21c46b92c3 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 11:53:12 -0600 Subject: [PATCH 02/25] Working implementation with debug --- .../compare_revisions_diff_file_component.rb | 26 ++++++++++++++----- .../rapid_diffs/diff_file_header_component.rb | 9 ++++++- .../projects/compare/rapid_diffs.html.haml | 15 ++++++++++- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb index 81f7cbb8d6d0c7..2102fa0b91b5fc 100644 --- a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb +++ b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb @@ -12,26 +12,40 @@ def initialize(diff_file:, parallel_view: false) end def additional_menu_items - [ + Rails.logger.error "[KEBAB_DEBUG] CompareRevisionsDiffFileComponent: additional_menu_items called" + + items = [ view_file_item ].compact + + Rails.logger.error "[KEBAB_DEBUG] menu items: #{items.inspect}" + items end private def view_file_item - { + project = @diff_file.repository.project + file_path = @diff_file.new_path || @diff_file.old_path + commit_sha = @diff_file.content_sha + + view_path = helpers.project_blob_path(project, helpers.tree_join(commit_sha, file_path)) + + item = { text: view_file_button_text, - href: @diff_file.view_path, + href: view_path, extraAttrs: { target: '_blank' }, position: 1 } + + Rails.logger.error "[KEBAB_DEBUG] view_file_item: #{item.inspect}" + item end def view_file_button_text - truncated_sha = helpers.truncate_sha(@diff_file.content_sha) - helpers.sprintf( - s__('MergeRequests|View file @ %{commitId}'), + truncated_sha = @diff_file.content_sha[0..7] + sprintf( + s_('MergeRequests|View file @ %{commitId}'), { commitId: truncated_sha } ) end diff --git a/app/components/rapid_diffs/diff_file_header_component.rb b/app/components/rapid_diffs/diff_file_header_component.rb index 86cacd21878458..66f86ad2a5a068 100644 --- a/app/components/rapid_diffs/diff_file_header_component.rb +++ b/app/components/rapid_diffs/diff_file_header_component.rb @@ -36,7 +36,14 @@ def copy_path_button end def menu_items - @additional_menu_items.sort_by { |item| item[:position] || Float::INFINITY } + Rails.logger.error "[KEBAB_DEBUG] DiffFileHeaderComponent: menu_items called" + Rails.logger.error "[KEBAB_DEBUG] @additional_menu_items: #{@additional_menu_items.inspect}" + + sorted_menu_items = @additional_menu_items.sort_by { |item| item[:position] || Float::INFINITY } + + Rails.logger.error "[KEBAB_DEBUG] sorted menu_items: #{sorted_menu_items.inspect}" + + sorted_menu_items end def heading_id diff --git a/app/views/projects/compare/rapid_diffs.html.haml b/app/views/projects/compare/rapid_diffs.html.haml index d6e56238b793f0..10bc5b9745b916 100644 --- a/app/views/projects/compare/rapid_diffs.html.haml +++ b/app/views/projects/compare/rapid_diffs.html.haml @@ -18,7 +18,20 @@ .container-fluid = render ::RapidDiffs::AppComponent.new(@rapid_diffs_presenter) do |c| - c.with_diffs_list do - = render RapidDiffs::CompareRevisionsDiffFileComponent.with_collection(c.diffs_slice, parallel_view: c.diff_view == :parallel) + - Rails.logger.error "[KEBAB_DEBUG] diffs_slice: #{c.diffs_slice.nil? ? 'nil' : 'present with ' + c.diffs_slice.size.to_s + ' items'}" + - Rails.logger.error "[KEBAB_DEBUG] resource.diffs present?: #{c.presenter.resource.diffs.present?}" + - if c.presenter.resource.diffs.present? + - Rails.logger.error "[KEBAB_DEBUG] resource.diffs.diff_files count: #{c.presenter.resource.diffs.diff_files.size}" + - Rails.logger.error "[KEBAB_DEBUG] empty_state_visible?: #{c.empty_state_visible?}" + + - # Try the fallback approach + - diffs_to_render = c.diffs_slice || (c.presenter.resource.diffs.present? ? c.presenter.resource.diffs.diff_files : []) + - Rails.logger.error "[KEBAB_DEBUG] diffs_to_render count: #{diffs_to_render.size}" + + - if diffs_to_render.any? + = render RapidDiffs::CompareRevisionsDiffFileComponent.with_collection(diffs_to_render, parallel_view: c.diff_view == :parallel) + - else + - Rails.logger.error "[KEBAB_DEBUG] No diffs to render - falling back to default" - else .container-fluid = render Pajamas::CardComponent.new(card_options: { class: "gl-bg-subtle" }) do |c| -- GitLab From 27d6a29508433d850095beabc4efc05d92454fc4 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 11:58:13 -0600 Subject: [PATCH 03/25] Implementation without debug --- .../compare_revisions_diff_file_component.rb | 12 ++---------- .../rapid_diffs/diff_file_header_component.rb | 9 +-------- app/views/projects/compare/rapid_diffs.html.haml | 11 ----------- 3 files changed, 3 insertions(+), 29 deletions(-) diff --git a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb index 2102fa0b91b5fc..720d7c7f3d06c7 100644 --- a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb +++ b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb @@ -12,14 +12,9 @@ def initialize(diff_file:, parallel_view: false) end def additional_menu_items - Rails.logger.error "[KEBAB_DEBUG] CompareRevisionsDiffFileComponent: additional_menu_items called" - - items = [ + [ view_file_item ].compact - - Rails.logger.error "[KEBAB_DEBUG] menu items: #{items.inspect}" - items end private @@ -31,15 +26,12 @@ def view_file_item view_path = helpers.project_blob_path(project, helpers.tree_join(commit_sha, file_path)) - item = { + { text: view_file_button_text, href: view_path, extraAttrs: { target: '_blank' }, position: 1 } - - Rails.logger.error "[KEBAB_DEBUG] view_file_item: #{item.inspect}" - item end def view_file_button_text diff --git a/app/components/rapid_diffs/diff_file_header_component.rb b/app/components/rapid_diffs/diff_file_header_component.rb index 66f86ad2a5a068..86cacd21878458 100644 --- a/app/components/rapid_diffs/diff_file_header_component.rb +++ b/app/components/rapid_diffs/diff_file_header_component.rb @@ -36,14 +36,7 @@ def copy_path_button end def menu_items - Rails.logger.error "[KEBAB_DEBUG] DiffFileHeaderComponent: menu_items called" - Rails.logger.error "[KEBAB_DEBUG] @additional_menu_items: #{@additional_menu_items.inspect}" - - sorted_menu_items = @additional_menu_items.sort_by { |item| item[:position] || Float::INFINITY } - - Rails.logger.error "[KEBAB_DEBUG] sorted menu_items: #{sorted_menu_items.inspect}" - - sorted_menu_items + @additional_menu_items.sort_by { |item| item[:position] || Float::INFINITY } end def heading_id diff --git a/app/views/projects/compare/rapid_diffs.html.haml b/app/views/projects/compare/rapid_diffs.html.haml index 10bc5b9745b916..3b603fea46ce42 100644 --- a/app/views/projects/compare/rapid_diffs.html.haml +++ b/app/views/projects/compare/rapid_diffs.html.haml @@ -18,20 +18,9 @@ .container-fluid = render ::RapidDiffs::AppComponent.new(@rapid_diffs_presenter) do |c| - c.with_diffs_list do - - Rails.logger.error "[KEBAB_DEBUG] diffs_slice: #{c.diffs_slice.nil? ? 'nil' : 'present with ' + c.diffs_slice.size.to_s + ' items'}" - - Rails.logger.error "[KEBAB_DEBUG] resource.diffs present?: #{c.presenter.resource.diffs.present?}" - - if c.presenter.resource.diffs.present? - - Rails.logger.error "[KEBAB_DEBUG] resource.diffs.diff_files count: #{c.presenter.resource.diffs.diff_files.size}" - - Rails.logger.error "[KEBAB_DEBUG] empty_state_visible?: #{c.empty_state_visible?}" - - - # Try the fallback approach - diffs_to_render = c.diffs_slice || (c.presenter.resource.diffs.present? ? c.presenter.resource.diffs.diff_files : []) - - Rails.logger.error "[KEBAB_DEBUG] diffs_to_render count: #{diffs_to_render.size}" - - if diffs_to_render.any? = render RapidDiffs::CompareRevisionsDiffFileComponent.with_collection(diffs_to_render, parallel_view: c.diff_view == :parallel) - - else - - Rails.logger.error "[KEBAB_DEBUG] No diffs to render - falling back to default" - else .container-fluid = render Pajamas::CardComponent.new(card_options: { class: "gl-bg-subtle" }) do |c| -- GitLab From 1ef317d16753d8b021e294099b4f3523a693be88 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 12:15:11 -0600 Subject: [PATCH 04/25] Make sure streaming renders the correct views and doesn't duplicate files --- .../projects/compare_diffs_stream_controller.rb | 8 ++++++++ app/presenters/rapid_diffs/compare_presenter.rb | 7 ------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/app/controllers/projects/compare_diffs_stream_controller.rb b/app/controllers/projects/compare_diffs_stream_controller.rb index f73e9b91c5059f..6f385ca1f6b8f5 100644 --- a/app/controllers/projects/compare_diffs_stream_controller.rb +++ b/app/controllers/projects/compare_diffs_stream_controller.rb @@ -9,5 +9,13 @@ class CompareDiffsStreamController < Projects::CompareController def resource compare end + + def diff_file_component(diff_file) + ::RapidDiffs::CompareRevisionsDiffFileComponent.new(diff_file: diff_file, parallel_view: view == :parallel) + end + + def diff_files_collection(diff_files) + ::RapidDiffs::CompareRevisionsDiffFileComponent.with_collection(diff_files, parallel_view: view == :parallel) + end end end diff --git a/app/presenters/rapid_diffs/compare_presenter.rb b/app/presenters/rapid_diffs/compare_presenter.rb index be04c1ebfb379b..21e7cc22122ff0 100644 --- a/app/presenters/rapid_diffs/compare_presenter.rb +++ b/app/presenters/rapid_diffs/compare_presenter.rb @@ -27,12 +27,5 @@ def reload_stream_url(offset: nil, diff_view: nil) view: diff_view ) end - - protected - - override(:offset) - def offset - 0 - end end end -- GitLab From 448ecd10e43a2da1265a1d396c2667d2bb6fc332 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 13:12:57 -0600 Subject: [PATCH 05/25] Add tests for updated controllers and components --- ...pare_revisions_diff_file_component_spec.rb | 128 ++++++++++++++++++ .../compare_diffs_stream_controller_spec.rb | 31 +++++ 2 files changed, 159 insertions(+) create mode 100644 spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb diff --git a/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb b/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb new file mode 100644 index 00000000000000..b4d119e7c931cf --- /dev/null +++ b/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require "spec_helper" + +require_relative './shared' + +RSpec.describe RapidDiffs::CompareRevisionsDiffFileComponent, type: :component, feature_category: :code_review_workflow do + include_context "with diff file component tests" + + let(:content_sha) { 'abc123' } + + before do + allow(diff_file).to receive(:repository).and_return(repository) + allow(repository).to receive(:commit).with(RepoHelpers.sample_commit.id).and_return(sample_commit) + allow(diff_file).to receive_messages( + new_path: 'path/to/file.rb', + old_path: 'old/path/to/file.rb', + content_sha: content_sha, + repository: repository + ) + end + + describe '#view_file_button_text' do + it 'formats the button text with truncated SHA' do + component = described_class.new(diff_file: diff_file) + + expect(component.send(:view_file_button_text)).to eq("View file @ #{content_sha[0..7]}") + end + end + + describe '#view_file_item' do + it 'returns correctly formatted menu item hash' do + component = described_class.new(diff_file: diff_file) + item = component.send(:view_file_item) + + expect(item[:text]).to eq("View file @ #{content_sha[0..7]}") + expect(item[:href]).to include("/#{project.full_path}/-/blob/#{content_sha}/path/to/file.rb") + expect(item[:extraAttrs][:target]).to eq('_blank') + expect(item[:position]).to eq(1) + end + end + + describe 'header menu options' do + context 'with text diff file' do + before do + allow(diff_file).to receive(:text?).and_return(true) + end + + it 'renders view file menu item' do + render_component + + options_menu_items = Gitlab::Json.parse(page.find('script', visible: false).text) + + expect(options_menu_items[0]['text']).to eq("View file @ #{content_sha[0..7]}") + expect(options_menu_items[0]['href']).to include("/#{project.full_path}/-/blob/#{content_sha}/path/to/file.rb") + expect(options_menu_items[0]['extraAttrs']['target']).to eq('_blank') + expect(options_menu_items[0]['position']).to eq(1) + end + + context 'when file has old_path only (deleted file)' do + before do + allow(diff_file).to receive(:new_path).and_return(nil) + end + + it 'uses old_path for the view file link' do + render_component + + options_menu_items = Gitlab::Json.parse(page.find('script', visible: false).text) + + expect(options_menu_items[0]['href']).to include("/#{project.full_path}/-/blob/#{content_sha}/old/path/to/file.rb") + end + end + end + end + + describe 'edge cases' do + context 'when content_sha is nil' do + before do + allow(diff_file).to receive(:content_sha).and_return(nil) + end + + it 'handles nil content_sha gracefully' do + expect { render_component }.not_to raise_error + end + + it 'does not include view file item when content_sha is nil' do + component = described_class.new(diff_file: diff_file) + + expect { component.send(:view_file_item) }.to raise_error(NoMethodError) + end + end + + context 'when both new_path and old_path are nil' do + before do + allow(diff_file).to receive_messages( + new_path: nil, + old_path: nil, + content_sha: 'abc123' + ) + end + + it 'handles missing paths gracefully' do + expect { render_component }.not_to raise_error + end + + it 'uses nil file_path in view_file_item' do + component = described_class.new(diff_file: diff_file) + item = component.send(:view_file_item) + + expect(item[:href]).to include('/-/blob/abc123/') + end + end + + context 'when content_sha is empty string' do + before do + allow(diff_file).to receive(:content_sha).and_return('') + end + + it 'handles empty content_sha gracefully' do + expect { render_component }.not_to raise_error + end + end + end + + def render_component(**args) + render_inline(described_class.new(diff_file:, **args)) + end +end \ No newline at end of file diff --git a/spec/requests/projects/compare_diffs_stream_controller_spec.rb b/spec/requests/projects/compare_diffs_stream_controller_spec.rb index c12b2e859acd72..02a8f16468e6c8 100644 --- a/spec/requests/projects/compare_diffs_stream_controller_spec.rb +++ b/spec/requests/projects/compare_diffs_stream_controller_spec.rb @@ -54,4 +54,35 @@ def go(**extra_params) include_examples 'with diffs_blobs param' end + + describe 'component integration' do + it 'uses CompareRevisionsDiffFileComponent for diff files' do + go + + expect(response.body).to include('js-options-menu') + expect(response.body).to include('js-options-button') + expect(response.body).to include('data-click="toggleOptionsMenu"') + + expect(response.body).to include('View file @') + expect(response.body).to include('application/json') + end + + it 'renders view file menu items with correct paths' do + go + + doc = Nokogiri::HTML(response.body) + script_tags = doc.css('script[type="application/json"]') + + expect(script_tags).not_to be_empty + + script_tags.each do |script| + menu_items = JSON.parse(script.content) + menu_items.each do |item| + expect(item['text']).to match(/View file @ \w{8}/) + expect(item['href']).to include('/-/blob/') + expect(item['extraAttrs']['target']).to eq('_blank') + end + end + end + end end -- GitLab From 61cb15ff091f3753d2c8c6805e5b9fb409f8d916 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 13:15:41 -0600 Subject: [PATCH 06/25] Add newlines to new component files --- .../rapid_diffs/compare_revisions_diff_file_component.html.haml | 2 +- .../rapid_diffs/compare_revisions_diff_file_component_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/components/rapid_diffs/compare_revisions_diff_file_component.html.haml b/app/components/rapid_diffs/compare_revisions_diff_file_component.html.haml index e0b66381d37d37..8de679514080ab 100644 --- a/app/components/rapid_diffs/compare_revisions_diff_file_component.html.haml +++ b/app/components/rapid_diffs/compare_revisions_diff_file_component.html.haml @@ -1,4 +1,4 @@ = render RapidDiffs::DiffFileComponent.new(diff_file:, parallel_view: @parallel_view) do |c| - c.with_header do = render RapidDiffs::DiffFileHeaderComponent.new(diff_file:, additional_menu_items:) - end \ No newline at end of file + end diff --git a/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb b/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb index b4d119e7c931cf..b9028583898baa 100644 --- a/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb +++ b/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb @@ -125,4 +125,4 @@ def render_component(**args) render_inline(described_class.new(diff_file:, **args)) end -end \ No newline at end of file +end -- GitLab From f4a1df642926de5534f3dc15ccd880a8dd441d88 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 13:17:29 -0600 Subject: [PATCH 07/25] If content sha is nil, we can't provide anything meaningful --- .../rapid_diffs/compare_revisions_diff_file_component.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb index 720d7c7f3d06c7..cda6e207d8c2b9 100644 --- a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb +++ b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb @@ -20,6 +20,8 @@ def additional_menu_items private def view_file_item + return nil unless @diff_file.content_sha + project = @diff_file.repository.project file_path = @diff_file.new_path || @diff_file.old_path commit_sha = @diff_file.content_sha -- GitLab From c9a3af3a2bdf0298a7ad7a6c2f4e044ad6d0d89a Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 13:19:56 -0600 Subject: [PATCH 08/25] Fix test for graceful menu item omission --- .../rapid_diffs/compare_revisions_diff_file_component_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb b/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb index b9028583898baa..13bdb886935083 100644 --- a/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb +++ b/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb @@ -86,7 +86,7 @@ it 'does not include view file item when content_sha is nil' do component = described_class.new(diff_file: diff_file) - expect { component.send(:view_file_item) }.to raise_error(NoMethodError) + expect(component.send(:view_file_item)).to be_nil end end -- GitLab From 3f11014c5754e3674923bd8859314388a2e58597 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 14:21:49 -0600 Subject: [PATCH 09/25] Fix linting issues --- .../rapid_diffs/compare_revisions_diff_file_component.rb | 4 ++-- .../compare_revisions_diff_file_component_spec.rb | 5 +++-- .../projects/compare_diffs_stream_controller_spec.rb | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb index cda6e207d8c2b9..b05df1b2ff442e 100644 --- a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb +++ b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb @@ -20,7 +20,7 @@ def additional_menu_items private def view_file_item - return nil unless @diff_file.content_sha + return unless @diff_file.content_sha project = @diff_file.repository.project file_path = @diff_file.new_path || @diff_file.old_path @@ -38,7 +38,7 @@ def view_file_item def view_file_button_text truncated_sha = @diff_file.content_sha[0..7] - sprintf( + format( s_('MergeRequests|View file @ %{commitId}'), { commitId: truncated_sha } ) diff --git a/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb b/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb index 13bdb886935083..9607c0cd7a34a3 100644 --- a/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb +++ b/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb @@ -4,7 +4,7 @@ require_relative './shared' -RSpec.describe RapidDiffs::CompareRevisionsDiffFileComponent, type: :component, feature_category: :code_review_workflow do +RSpec.describe RapidDiffs::CompareRevisionsDiffFileComponent, feature_category: :code_review_workflow do include_context "with diff file component tests" let(:content_sha) { 'abc123' } @@ -66,8 +66,9 @@ render_component options_menu_items = Gitlab::Json.parse(page.find('script', visible: false).text) + expected_path = "/#{project.full_path}/-/blob/#{content_sha}/old/path/to/file.rb" - expect(options_menu_items[0]['href']).to include("/#{project.full_path}/-/blob/#{content_sha}/old/path/to/file.rb") + expect(options_menu_items[0]['href']).to include(expected_path) end end end diff --git a/spec/requests/projects/compare_diffs_stream_controller_spec.rb b/spec/requests/projects/compare_diffs_stream_controller_spec.rb index 02a8f16468e6c8..e19aeea4e1295b 100644 --- a/spec/requests/projects/compare_diffs_stream_controller_spec.rb +++ b/spec/requests/projects/compare_diffs_stream_controller_spec.rb @@ -76,7 +76,7 @@ def go(**extra_params) expect(script_tags).not_to be_empty script_tags.each do |script| - menu_items = JSON.parse(script.content) + menu_items = Gitlab::Json.parse(script.content) menu_items.each do |item| expect(item['text']).to match(/View file @ \w{8}/) expect(item['href']).to include('/-/blob/') -- GitLab From 89405c2b60701731f6fb6f7bd821d6a7641c2d30 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 14:27:05 -0600 Subject: [PATCH 10/25] Next the component integration tests under the GET block --- .../compare_diffs_stream_controller_spec.rb | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/spec/requests/projects/compare_diffs_stream_controller_spec.rb b/spec/requests/projects/compare_diffs_stream_controller_spec.rb index e19aeea4e1295b..77303c2ca3755e 100644 --- a/spec/requests/projects/compare_diffs_stream_controller_spec.rb +++ b/spec/requests/projects/compare_diffs_stream_controller_spec.rb @@ -50,39 +50,39 @@ def go(**extra_params) end end - include_examples 'diffs stream tests' + describe 'proper components integration' do + it 'uses CompareRevisionsDiffFileComponent for diff files' do + go - include_examples 'with diffs_blobs param' - end + expect(response.body).to include('js-options-menu') + expect(response.body).to include('js-options-button') + expect(response.body).to include('data-click="toggleOptionsMenu"') - describe 'component integration' do - it 'uses CompareRevisionsDiffFileComponent for diff files' do - go - - expect(response.body).to include('js-options-menu') - expect(response.body).to include('js-options-button') - expect(response.body).to include('data-click="toggleOptionsMenu"') - - expect(response.body).to include('View file @') - expect(response.body).to include('application/json') - end + expect(response.body).to include('View file @') + expect(response.body).to include('application/json') + end - it 'renders view file menu items with correct paths' do - go + it 'renders view file menu items with correct paths' do + go - doc = Nokogiri::HTML(response.body) - script_tags = doc.css('script[type="application/json"]') + doc = Nokogiri::HTML(response.body) + script_tags = doc.css('script[type="application/json"]') - expect(script_tags).not_to be_empty + expect(script_tags).not_to be_empty - script_tags.each do |script| - menu_items = Gitlab::Json.parse(script.content) - menu_items.each do |item| - expect(item['text']).to match(/View file @ \w{8}/) - expect(item['href']).to include('/-/blob/') - expect(item['extraAttrs']['target']).to eq('_blank') + script_tags.each do |script| + menu_items = Gitlab::Json.parse(script.content) + menu_items.each do |item| + expect(item['text']).to match(/View file @ \w{8}/) + expect(item['href']).to include('/-/blob/') + expect(item['extraAttrs']['target']).to eq('_blank') + end end end end + + include_examples 'diffs stream tests' + + include_examples 'with diffs_blobs param' end end -- GitLab From c5c03b091a0e40e683c421244e57628da41a069a Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 18:10:50 -0600 Subject: [PATCH 11/25] Fix format string syntax --- .../rapid_diffs/compare_revisions_diff_file_component.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb index b05df1b2ff442e..5becd5349515f4 100644 --- a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb +++ b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb @@ -38,9 +38,10 @@ def view_file_item def view_file_button_text truncated_sha = @diff_file.content_sha[0..7] - format( + + helpers.safe_format( s_('MergeRequests|View file @ %{commitId}'), - { commitId: truncated_sha } + commitId: truncated_sha ) end end -- GitLab From 70a08e8c778ea1085e92abcafcf664856b122a97 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 19:07:05 -0600 Subject: [PATCH 12/25] Check for actual returned HTML values --- .../requests/projects/compare_diffs_stream_controller_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/requests/projects/compare_diffs_stream_controller_spec.rb b/spec/requests/projects/compare_diffs_stream_controller_spec.rb index 77303c2ca3755e..5c626b8f9ed661 100644 --- a/spec/requests/projects/compare_diffs_stream_controller_spec.rb +++ b/spec/requests/projects/compare_diffs_stream_controller_spec.rb @@ -54,8 +54,8 @@ def go(**extra_params) it 'uses CompareRevisionsDiffFileComponent for diff files' do go - expect(response.body).to include('js-options-menu') - expect(response.body).to include('js-options-button') + expect(response.body).to include('rd-diff-file-options-menu') + expect(response.body).to include('Show options') expect(response.body).to include('data-click="toggleOptionsMenu"') expect(response.body).to include('View file @') -- GitLab From 09d8d28d4a8b28791a348bc9fcb9184e7142cb89 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 19:43:13 -0600 Subject: [PATCH 13/25] Re-add the 0 offset override --- app/presenters/rapid_diffs/compare_presenter.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/presenters/rapid_diffs/compare_presenter.rb b/app/presenters/rapid_diffs/compare_presenter.rb index 21e7cc22122ff0..be04c1ebfb379b 100644 --- a/app/presenters/rapid_diffs/compare_presenter.rb +++ b/app/presenters/rapid_diffs/compare_presenter.rb @@ -27,5 +27,12 @@ def reload_stream_url(offset: nil, diff_view: nil) view: diff_view ) end + + protected + + override(:offset) + def offset + 0 + end end end -- GitLab From 954349c0677b628dade5ae3becfec67920aaac69 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 19:54:49 -0600 Subject: [PATCH 14/25] Stop forcing the BE to render diffs when the compare page starts streaming at 0 --- app/views/projects/compare/rapid_diffs.html.haml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/views/projects/compare/rapid_diffs.html.haml b/app/views/projects/compare/rapid_diffs.html.haml index 3b603fea46ce42..d6e56238b793f0 100644 --- a/app/views/projects/compare/rapid_diffs.html.haml +++ b/app/views/projects/compare/rapid_diffs.html.haml @@ -18,9 +18,7 @@ .container-fluid = render ::RapidDiffs::AppComponent.new(@rapid_diffs_presenter) do |c| - c.with_diffs_list do - - diffs_to_render = c.diffs_slice || (c.presenter.resource.diffs.present? ? c.presenter.resource.diffs.diff_files : []) - - if diffs_to_render.any? - = render RapidDiffs::CompareRevisionsDiffFileComponent.with_collection(diffs_to_render, parallel_view: c.diff_view == :parallel) + = render RapidDiffs::CompareRevisionsDiffFileComponent.with_collection(c.diffs_slice, parallel_view: c.diff_view == :parallel) - else .container-fluid = render Pajamas::CardComponent.new(card_options: { class: "gl-bg-subtle" }) do |c| -- GitLab From 05f6bae7cb1024384e1a10b69d69fe5de03abb18 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 16 Sep 2025 22:18:18 -0600 Subject: [PATCH 15/25] Test that the CompareRevisions DiffFile component renders the menu --- .../compare_revisions_diff_file_component.rb | 14 +-- ...pare_revisions_diff_file_component_spec.rb | 98 ++----------------- 2 files changed, 11 insertions(+), 101 deletions(-) diff --git a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb index 5becd5349515f4..58e5094a3eadb7 100644 --- a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb +++ b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb @@ -29,20 +29,14 @@ def view_file_item view_path = helpers.project_blob_path(project, helpers.tree_join(commit_sha, file_path)) { - text: view_file_button_text, + text: helpers.safe_format( + s_('MergeRequests|View file @ %{commitId}'), + commitId: @diff_file.content_sha[0..7] + ), href: view_path, extraAttrs: { target: '_blank' }, position: 1 } end - - def view_file_button_text - truncated_sha = @diff_file.content_sha[0..7] - - helpers.safe_format( - s_('MergeRequests|View file @ %{commitId}'), - commitId: truncated_sha - ) - end end end diff --git a/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb b/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb index 9607c0cd7a34a3..059cb78cb75a68 100644 --- a/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb +++ b/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb @@ -7,7 +7,7 @@ RSpec.describe RapidDiffs::CompareRevisionsDiffFileComponent, feature_category: :code_review_workflow do include_context "with diff file component tests" - let(:content_sha) { 'abc123' } + let(:content_sha) { 'abc123def456' } before do allow(diff_file).to receive(:repository).and_return(repository) @@ -16,109 +16,25 @@ new_path: 'path/to/file.rb', old_path: 'old/path/to/file.rb', content_sha: content_sha, - repository: repository + repository: repository, + diffable_text?: true ) end - describe '#view_file_button_text' do - it 'formats the button text with truncated SHA' do - component = described_class.new(diff_file: diff_file) - - expect(component.send(:view_file_button_text)).to eq("View file @ #{content_sha[0..7]}") - end - end - - describe '#view_file_item' do - it 'returns correctly formatted menu item hash' do - component = described_class.new(diff_file: diff_file) - item = component.send(:view_file_item) - - expect(item[:text]).to eq("View file @ #{content_sha[0..7]}") - expect(item[:href]).to include("/#{project.full_path}/-/blob/#{content_sha}/path/to/file.rb") - expect(item[:extraAttrs][:target]).to eq('_blank') - expect(item[:position]).to eq(1) - end - end - describe 'header menu options' do context 'with text diff file' do before do - allow(diff_file).to receive(:text?).and_return(true) + allow(diff_file).to receive_messages( + text?: true + ) end - it 'renders view file menu item' do + it 'renders additional options' do render_component options_menu_items = Gitlab::Json.parse(page.find('script', visible: false).text) expect(options_menu_items[0]['text']).to eq("View file @ #{content_sha[0..7]}") - expect(options_menu_items[0]['href']).to include("/#{project.full_path}/-/blob/#{content_sha}/path/to/file.rb") - expect(options_menu_items[0]['extraAttrs']['target']).to eq('_blank') - expect(options_menu_items[0]['position']).to eq(1) - end - - context 'when file has old_path only (deleted file)' do - before do - allow(diff_file).to receive(:new_path).and_return(nil) - end - - it 'uses old_path for the view file link' do - render_component - - options_menu_items = Gitlab::Json.parse(page.find('script', visible: false).text) - expected_path = "/#{project.full_path}/-/blob/#{content_sha}/old/path/to/file.rb" - - expect(options_menu_items[0]['href']).to include(expected_path) - end - end - end - end - - describe 'edge cases' do - context 'when content_sha is nil' do - before do - allow(diff_file).to receive(:content_sha).and_return(nil) - end - - it 'handles nil content_sha gracefully' do - expect { render_component }.not_to raise_error - end - - it 'does not include view file item when content_sha is nil' do - component = described_class.new(diff_file: diff_file) - - expect(component.send(:view_file_item)).to be_nil - end - end - - context 'when both new_path and old_path are nil' do - before do - allow(diff_file).to receive_messages( - new_path: nil, - old_path: nil, - content_sha: 'abc123' - ) - end - - it 'handles missing paths gracefully' do - expect { render_component }.not_to raise_error - end - - it 'uses nil file_path in view_file_item' do - component = described_class.new(diff_file: diff_file) - item = component.send(:view_file_item) - - expect(item[:href]).to include('/-/blob/abc123/') - end - end - - context 'when content_sha is empty string' do - before do - allow(diff_file).to receive(:content_sha).and_return('') - end - - it 'handles empty content_sha gracefully' do - expect { render_component }.not_to raise_error end end end -- GitLab From b5afad9a1ca096d0088a108dfc2aeec2cf17ebfc Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 18 Sep 2025 08:46:30 -0600 Subject: [PATCH 16/25] Test that empty content_sha does not render menu items --- .../compare_revisions_diff_file_component_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb b/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb index 059cb78cb75a68..4dd1bd2d50888a 100644 --- a/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb +++ b/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb @@ -37,6 +37,16 @@ expect(options_menu_items[0]['text']).to eq("View file @ #{content_sha[0..7]}") end end + + context 'when content_sha is nil' do + let(:content_sha) { nil } + + it 'does not include view file menu item in additional_menu_items' do + component = described_class.new(diff_file: diff_file) + + expect(component.additional_menu_items).to be_empty + end + end end def render_component(**args) -- GitLab From 829755efeda95c59db777655ea5a6465c11f578d Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 24 Sep 2025 13:31:03 -0600 Subject: [PATCH 17/25] Move "View file at [SHA]" item to show on all Rapid Diffs diff files --- ...re_revisions_diff_file_component.html.haml | 4 -- .../compare_revisions_diff_file_component.rb | 42 ------------------- .../rapid_diffs/diff_file_header_component.rb | 25 ++++++++++- .../merge_request_diff_file_component.rb | 2 +- .../compare_diffs_stream_controller.rb | 8 ---- .../projects/compare/rapid_diffs.html.haml | 4 +- 6 files changed, 26 insertions(+), 59 deletions(-) delete mode 100644 app/components/rapid_diffs/compare_revisions_diff_file_component.html.haml delete mode 100644 app/components/rapid_diffs/compare_revisions_diff_file_component.rb diff --git a/app/components/rapid_diffs/compare_revisions_diff_file_component.html.haml b/app/components/rapid_diffs/compare_revisions_diff_file_component.html.haml deleted file mode 100644 index 8de679514080ab..00000000000000 --- a/app/components/rapid_diffs/compare_revisions_diff_file_component.html.haml +++ /dev/null @@ -1,4 +0,0 @@ -= render RapidDiffs::DiffFileComponent.new(diff_file:, parallel_view: @parallel_view) do |c| - - c.with_header do - = render RapidDiffs::DiffFileHeaderComponent.new(diff_file:, additional_menu_items:) - end diff --git a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb b/app/components/rapid_diffs/compare_revisions_diff_file_component.rb deleted file mode 100644 index 58e5094a3eadb7..00000000000000 --- a/app/components/rapid_diffs/compare_revisions_diff_file_component.rb +++ /dev/null @@ -1,42 +0,0 @@ -# frozen_string_literal: true - -module RapidDiffs - class CompareRevisionsDiffFileComponent < ViewComponent::Base - with_collection_parameter :diff_file - - attr_reader :diff_file - - def initialize(diff_file:, parallel_view: false) - @diff_file = diff_file - @parallel_view = parallel_view - end - - def additional_menu_items - [ - view_file_item - ].compact - end - - private - - def view_file_item - return unless @diff_file.content_sha - - project = @diff_file.repository.project - file_path = @diff_file.new_path || @diff_file.old_path - commit_sha = @diff_file.content_sha - - view_path = helpers.project_blob_path(project, helpers.tree_join(commit_sha, file_path)) - - { - text: helpers.safe_format( - s_('MergeRequests|View file @ %{commitId}'), - commitId: @diff_file.content_sha[0..7] - ), - href: view_path, - extraAttrs: { target: '_blank' }, - position: 1 - } - end - end -end diff --git a/app/components/rapid_diffs/diff_file_header_component.rb b/app/components/rapid_diffs/diff_file_header_component.rb index 86cacd21878458..12965d7a00cd84 100644 --- a/app/components/rapid_diffs/diff_file_header_component.rb +++ b/app/components/rapid_diffs/diff_file_header_component.rb @@ -36,7 +36,10 @@ def copy_path_button end def menu_items - @additional_menu_items.sort_by { |item| item[:position] || Float::INFINITY } + [ + view_file_menu_item, + *@additional_menu_items + ].sort_by { |item| item[:position] || Float::INFINITY } end def heading_id @@ -69,5 +72,25 @@ def stats_label def pretty_print_bytes(size) ActiveSupport::NumberHelper.number_to_human_size(size) end + + def view_file_menu_item + return unless @diff_file.content_sha + + project = @diff_file.repository.project + file_path = @diff_file.new_path || @diff_file.old_path + commit_sha = @diff_file.content_sha + + view_path = helpers.project_blob_path(project, helpers.tree_join(commit_sha, file_path)) + + { + text: helpers.safe_format( + s_('RapidDiffs|View file at %{commitId}'), + commitId: @diff_file.content_sha[0..7] + ), + href: view_path, + extraAttrs: { target: '_blank' }, + position: 1 + } + end end end diff --git a/app/components/rapid_diffs/merge_request_diff_file_component.rb b/app/components/rapid_diffs/merge_request_diff_file_component.rb index 866389afe556d0..e39019394fc03a 100644 --- a/app/components/rapid_diffs/merge_request_diff_file_component.rb +++ b/app/components/rapid_diffs/merge_request_diff_file_component.rb @@ -28,7 +28,7 @@ def edit_in_sfe { text: _('Edit in single-file editor'), href: editor_path, - position: 1 + position: 2 } end end diff --git a/app/controllers/projects/compare_diffs_stream_controller.rb b/app/controllers/projects/compare_diffs_stream_controller.rb index 6f385ca1f6b8f5..f73e9b91c5059f 100644 --- a/app/controllers/projects/compare_diffs_stream_controller.rb +++ b/app/controllers/projects/compare_diffs_stream_controller.rb @@ -9,13 +9,5 @@ class CompareDiffsStreamController < Projects::CompareController def resource compare end - - def diff_file_component(diff_file) - ::RapidDiffs::CompareRevisionsDiffFileComponent.new(diff_file: diff_file, parallel_view: view == :parallel) - end - - def diff_files_collection(diff_files) - ::RapidDiffs::CompareRevisionsDiffFileComponent.with_collection(diff_files, parallel_view: view == :parallel) - end end end diff --git a/app/views/projects/compare/rapid_diffs.html.haml b/app/views/projects/compare/rapid_diffs.html.haml index d6e56238b793f0..b1589dfdeb8309 100644 --- a/app/views/projects/compare/rapid_diffs.html.haml +++ b/app/views/projects/compare/rapid_diffs.html.haml @@ -16,9 +16,7 @@ .container-fluid{ class: [container_class] } = render "projects/commits/commit_list" unless hide_commit_list .container-fluid - = render ::RapidDiffs::AppComponent.new(@rapid_diffs_presenter) do |c| - - c.with_diffs_list do - = render RapidDiffs::CompareRevisionsDiffFileComponent.with_collection(c.diffs_slice, parallel_view: c.diff_view == :parallel) + = render ::RapidDiffs::AppComponent.new(@rapid_diffs_presenter) - else .container-fluid = render Pajamas::CardComponent.new(card_options: { class: "gl-bg-subtle" }) do |c| -- GitLab From 1d56f6c81d47d185c8f27dc493527dc17e5d7d29 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 24 Sep 2025 13:32:37 -0600 Subject: [PATCH 18/25] Remove unnecessary stream controller tests --- .../compare_diffs_stream_controller_spec.rb | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/spec/requests/projects/compare_diffs_stream_controller_spec.rb b/spec/requests/projects/compare_diffs_stream_controller_spec.rb index 5c626b8f9ed661..c12b2e859acd72 100644 --- a/spec/requests/projects/compare_diffs_stream_controller_spec.rb +++ b/spec/requests/projects/compare_diffs_stream_controller_spec.rb @@ -50,37 +50,6 @@ def go(**extra_params) end end - describe 'proper components integration' do - it 'uses CompareRevisionsDiffFileComponent for diff files' do - go - - expect(response.body).to include('rd-diff-file-options-menu') - expect(response.body).to include('Show options') - expect(response.body).to include('data-click="toggleOptionsMenu"') - - expect(response.body).to include('View file @') - expect(response.body).to include('application/json') - end - - it 'renders view file menu items with correct paths' do - go - - doc = Nokogiri::HTML(response.body) - script_tags = doc.css('script[type="application/json"]') - - expect(script_tags).not_to be_empty - - script_tags.each do |script| - menu_items = Gitlab::Json.parse(script.content) - menu_items.each do |item| - expect(item['text']).to match(/View file @ \w{8}/) - expect(item['href']).to include('/-/blob/') - expect(item['extraAttrs']['target']).to eq('_blank') - end - end - end - end - include_examples 'diffs stream tests' include_examples 'with diffs_blobs param' -- GitLab From 8cd17c392dc2b64ed3245617f199995d6ae05f38 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 24 Sep 2025 14:10:21 -0600 Subject: [PATCH 19/25] Filter out nil entries in the header component menu_items --- app/components/rapid_diffs/diff_file_header_component.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/components/rapid_diffs/diff_file_header_component.rb b/app/components/rapid_diffs/diff_file_header_component.rb index 12965d7a00cd84..2cc6223f5a5dd5 100644 --- a/app/components/rapid_diffs/diff_file_header_component.rb +++ b/app/components/rapid_diffs/diff_file_header_component.rb @@ -39,7 +39,9 @@ def menu_items [ view_file_menu_item, *@additional_menu_items - ].sort_by { |item| item[:position] || Float::INFINITY } + ] + .filter_map { |item| item unless item.nil? } + .sort_by { |item| item[:position] || Float::INFINITY } end def heading_id -- GitLab From cf5bb8fad05536ecfb2a6e73f311fc5fed3c3741 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 24 Sep 2025 14:10:58 -0600 Subject: [PATCH 20/25] Remove test that should always fail The header component should now always render 1 item (View file at [SHA]) --- .../rapid_diffs/diff_file_header_component_spec.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/spec/components/rapid_diffs/diff_file_header_component_spec.rb b/spec/components/rapid_diffs/diff_file_header_component_spec.rb index d9eb4c920adae0..a87dd6b000b3b0 100644 --- a/spec/components/rapid_diffs/diff_file_header_component_spec.rb +++ b/spec/components/rapid_diffs/diff_file_header_component_spec.rb @@ -155,12 +155,6 @@ allow(diff_file).to receive(:content_sha).and_return(content_sha) end - it 'does not render menu toggle without options' do - render_component - - expect(page).not_to have_css('button[data-click="toggleOptionsMenu"][aria-label="Show options"]') - end - it 'renders additional menu items with respective order' do menu_items = [ { -- GitLab From 38bce6a99bb8b6cc39f3260af5bfaf31214a20c4 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 24 Sep 2025 14:11:49 -0600 Subject: [PATCH 21/25] Remove unnecessary nil return There are other parts of the header that throw an error if the content_sha is nil, so this shouldn't ever happen to the menu items. --- app/components/rapid_diffs/diff_file_header_component.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/components/rapid_diffs/diff_file_header_component.rb b/app/components/rapid_diffs/diff_file_header_component.rb index 2cc6223f5a5dd5..b69da0abf14586 100644 --- a/app/components/rapid_diffs/diff_file_header_component.rb +++ b/app/components/rapid_diffs/diff_file_header_component.rb @@ -76,8 +76,6 @@ def pretty_print_bytes(size) end def view_file_menu_item - return unless @diff_file.content_sha - project = @diff_file.repository.project file_path = @diff_file.new_path || @diff_file.old_path commit_sha = @diff_file.content_sha -- GitLab From e593a8053c81c5dfb282407158ce193f821d31d2 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 24 Sep 2025 14:23:53 -0600 Subject: [PATCH 22/25] Remove tests for removed component --- ...pare_revisions_diff_file_component_spec.rb | 55 ------------------- 1 file changed, 55 deletions(-) delete mode 100644 spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb diff --git a/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb b/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb deleted file mode 100644 index 4dd1bd2d50888a..00000000000000 --- a/spec/components/rapid_diffs/compare_revisions_diff_file_component_spec.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -require_relative './shared' - -RSpec.describe RapidDiffs::CompareRevisionsDiffFileComponent, feature_category: :code_review_workflow do - include_context "with diff file component tests" - - let(:content_sha) { 'abc123def456' } - - before do - allow(diff_file).to receive(:repository).and_return(repository) - allow(repository).to receive(:commit).with(RepoHelpers.sample_commit.id).and_return(sample_commit) - allow(diff_file).to receive_messages( - new_path: 'path/to/file.rb', - old_path: 'old/path/to/file.rb', - content_sha: content_sha, - repository: repository, - diffable_text?: true - ) - end - - describe 'header menu options' do - context 'with text diff file' do - before do - allow(diff_file).to receive_messages( - text?: true - ) - end - - it 'renders additional options' do - render_component - - options_menu_items = Gitlab::Json.parse(page.find('script', visible: false).text) - - expect(options_menu_items[0]['text']).to eq("View file @ #{content_sha[0..7]}") - end - end - - context 'when content_sha is nil' do - let(:content_sha) { nil } - - it 'does not include view file menu item in additional_menu_items' do - component = described_class.new(diff_file: diff_file) - - expect(component.additional_menu_items).to be_empty - end - end - end - - def render_component(**args) - render_inline(described_class.new(diff_file:, **args)) - end -end -- GitLab From 569b59d0621232047002694b73a5f50d866addb5 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 24 Sep 2025 14:24:14 -0600 Subject: [PATCH 23/25] Test that the Diff File Header always renders the View File menu item --- .../rapid_diffs/diff_file_header_component_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/components/rapid_diffs/diff_file_header_component_spec.rb b/spec/components/rapid_diffs/diff_file_header_component_spec.rb index a87dd6b000b3b0..8ad831bcb6242b 100644 --- a/spec/components/rapid_diffs/diff_file_header_component_spec.rb +++ b/spec/components/rapid_diffs/diff_file_header_component_spec.rb @@ -155,6 +155,14 @@ allow(diff_file).to receive(:content_sha).and_return(content_sha) end + it 'always renders the "View file at [SHA]" menu item' do + render_component + + options_menu_items = Gitlab::Json.parse(page.find('script', visible: false).text) + + expect(options_menu_items.first['text']).to eq('View file at abc123') + end + it 'renders additional menu items with respective order' do menu_items = [ { -- GitLab From 6e5afa66a3b2dbdbe08609488cd41d850737b43a Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 24 Sep 2025 17:24:07 -0600 Subject: [PATCH 24/25] Update localized text to use RapidDiffs prefix --- locale/gitlab.pot | 3 +++ 1 file changed, 3 insertions(+) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a0c2a978c59e12..7255e955365523 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -53063,6 +53063,9 @@ msgstr "" msgid "RapidDiffs|View controls" msgstr "" +msgid "RapidDiffs|View file at %{commitId}" +msgstr "" + msgid "Rate Limits" msgstr "" -- GitLab From a837a7b3f0d49ba5cfe2b1f4b0056614a49b549c Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 24 Sep 2025 17:24:42 -0600 Subject: [PATCH 25/25] Update MergeRequestDiff-specific menu item position test --- .../rapid_diffs/merge_request_diff_file_component_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/components/rapid_diffs/merge_request_diff_file_component_spec.rb b/spec/components/rapid_diffs/merge_request_diff_file_component_spec.rb index 529219c936537f..47d0d1893d23ea 100644 --- a/spec/components/rapid_diffs/merge_request_diff_file_component_spec.rb +++ b/spec/components/rapid_diffs/merge_request_diff_file_component_spec.rb @@ -36,8 +36,8 @@ options_menu_items = Gitlab::Json.parse(page.find('script', visible: false).text) - expect(options_menu_items[0]['text']).to eq('Edit in single-file editor') - expect(options_menu_items[0]['href']).to include("#{edit_path_base}#{merge_request.iid}") + expect(options_menu_items[1]['text']).to eq('Edit in single-file editor') + expect(options_menu_items[1]['href']).to include("#{edit_path_base}#{merge_request.iid}") end end end -- GitLab