From 656095632e433f11267711728871070b8dd91b86 Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Fri, 2 May 2025 21:58:41 +0400 Subject: [PATCH 1/3] Support loading collapsed files in Rapid Diffs --- .../javascripts/rapid_diffs/adapters.js | 3 +- .../javascripts/rapid_diffs/diff_file.js | 25 +++++++++++- .../rapid_diffs/load_file/adapter.js | 38 +++++++++++++++++++ app/components/rapid_diffs/app_component.rb | 3 ++ .../rapid_diffs/diff_file_component.rb | 4 +- .../viewers/no_preview_component.html.haml | 7 ++-- .../viewers/no_preview_component.rb | 2 +- app/controllers/projects/commit_controller.rb | 1 + .../projects/compare_controller.rb | 1 + .../merge_requests/creations_controller.rb | 1 + .../projects/merge_requests_controller.rb | 1 + .../projects/commit/rapid_diffs.html.haml | 2 +- .../projects/compare/rapid_diffs.html.haml | 2 +- .../creations/rapid_diffs.html.haml | 2 +- .../merge_requests/rapid_diffs.html.haml | 2 +- locale/gitlab.pot | 3 ++ 16 files changed, 85 insertions(+), 12 deletions(-) create mode 100644 app/assets/javascripts/rapid_diffs/load_file/adapter.js diff --git a/app/assets/javascripts/rapid_diffs/adapters.js b/app/assets/javascripts/rapid_diffs/adapters.js index 1911f55bcc49eb..4f03a2e46b40c8 100644 --- a/app/assets/javascripts/rapid_diffs/adapters.js +++ b/app/assets/javascripts/rapid_diffs/adapters.js @@ -3,6 +3,7 @@ import { OptionsMenuAdapter } from '~/rapid_diffs/options_menu/adapter'; import { ToggleFileAdapter } from '~/rapid_diffs/toggle_file/adapter'; import { DisableDiffSideAdapter } from '~/rapid_diffs/disable_diff_side/adapter'; import { ImageAdapter } from '~/rapid_diffs/image_viewer/adapter'; +import { LoadFileAdapter } from '~/rapid_diffs/load_file/adapter'; const HEADER_ADAPTERS = [OptionsMenuAdapter, ToggleFileAdapter]; @@ -10,5 +11,5 @@ export const VIEWER_ADAPTERS = { text_inline: [...HEADER_ADAPTERS, ExpandLinesAdapter], text_parallel: [...HEADER_ADAPTERS, ExpandLinesAdapter, DisableDiffSideAdapter], image: [...HEADER_ADAPTERS, ImageAdapter], - no_preview: HEADER_ADAPTERS, + no_preview: [...HEADER_ADAPTERS, LoadFileAdapter], }; diff --git a/app/assets/javascripts/rapid_diffs/diff_file.js b/app/assets/javascripts/rapid_diffs/diff_file.js index 31314f96ed0783..0fc17683bd63f5 100644 --- a/app/assets/javascripts/rapid_diffs/diff_file.js +++ b/app/assets/javascripts/rapid_diffs/diff_file.js @@ -64,6 +64,10 @@ export class DiffFile extends HTMLElement { this.app.observe(this); } + unobserveVisibility() { + this.app.unobserve(this); + } + // Delegated to Rapid Diffs App onVisible(entry) { this.trigger(events.VISIBLE, entry); @@ -98,6 +102,24 @@ export class DiffFile extends HTMLElement { // TODO: add outline for active file } + focusFirstButton(options) { + this.diffElement.querySelector('button').focus(options); + } + + selfReplace(node) { + // 'mount' is automagically called by the component in the Rapid Diffs app + this.replaceWith(node); + this.disconnect(); + node.focusFirstButton({ focusVisible: false }); + } + + disconnect() { + this.diffElement.removeEventListener('click', this.onClickHandler); + this.unobserveVisibility(); + this.app = null; + this.sink = null; + } + get data() { if (!this[dataCacheKey]) this[dataCacheKey] = camelizeKeys(JSON.parse(this.dataset.fileData)); return this[dataCacheKey]; @@ -109,7 +131,8 @@ export class DiffFile extends HTMLElement { diffElement: this.diffElement, sink: this.sink, data: this.data, - trigger: this.trigger, + trigger: this.trigger.bind(this), + replaceWith: this.selfReplace.bind(this), }; } diff --git a/app/assets/javascripts/rapid_diffs/load_file/adapter.js b/app/assets/javascripts/rapid_diffs/load_file/adapter.js new file mode 100644 index 00000000000000..a11475886f6356 --- /dev/null +++ b/app/assets/javascripts/rapid_diffs/load_file/adapter.js @@ -0,0 +1,38 @@ +import axios from '~/lib/utils/axios_utils'; +import { useDiffsView } from '~/rapid_diffs/stores/diffs_view'; +import { pinia } from '~/pinia/instance'; +import { createAlert } from '~/alert'; +import { s__ } from '~/locale'; + +function htmlToElement(html) { + const parser = new DOMParser(); + const doc = parser.parseFromString(html, 'text/html'); + return doc.body.firstChild; +} + +export const LoadFileAdapter = { + clicks: { + async showChanges(event, button) { + const { parallelView, showWhitespace } = useDiffsView(pinia); + const url = new URL(this.appData.diffFileEndpoint, window.location.origin); + if (this.data.oldPath) url.searchParams.set('old_path', this.data.oldPath); + if (this.data.newPath) url.searchParams.set('new_path', this.data.newPath); + url.searchParams.set('ignore_whitespace_changes', !showWhitespace); + if (parallelView) url.searchParams.set('view', 'parallel'); + button.setAttribute('disabled', true); + let response; + try { + response = await axios.get(url); + } catch (error) { + button.removeAttribute('disabled'); + createAlert({ + message: s__('RapidDiffs|Failed to load changes, please try again.'), + error, + }); + return; + } + const node = htmlToElement(response.data); + this.replaceWith(node); + }, + }, +}; diff --git a/app/components/rapid_diffs/app_component.rb b/app/components/rapid_diffs/app_component.rb index e63e8d2d1b7f85..52aa42066c7250 100644 --- a/app/components/rapid_diffs/app_component.rb +++ b/app/components/rapid_diffs/app_component.rb @@ -13,6 +13,7 @@ def initialize( update_user_endpoint:, diffs_stats_endpoint:, diff_files_endpoint:, + diff_file_endpoint:, should_sort_metadata_files: false, lazy: false ) @@ -25,6 +26,7 @@ def initialize( @diffs_stats_endpoint = diffs_stats_endpoint @diff_files_endpoint = diff_files_endpoint @should_sort_metadata_files = should_sort_metadata_files + @diff_file_endpoint = diff_file_endpoint @lazy = lazy end @@ -37,6 +39,7 @@ def app_data should_sort_metadata_files: @should_sort_metadata_files, show_whitespace: @show_whitespace, diff_view_type: @diff_view, + diff_file_endpoint: @diff_file_endpoint, update_user_endpoint: @update_user_endpoint } end diff --git a/app/components/rapid_diffs/diff_file_component.rb b/app/components/rapid_diffs/diff_file_component.rb index 837a9abd16825d..9a53d80d531179 100644 --- a/app/components/rapid_diffs/diff_file_component.rb +++ b/app/components/rapid_diffs/diff_file_component.rb @@ -21,7 +21,9 @@ def file_data params = tree_join(@diff_file.content_sha, @diff_file.file_path) { viewer: viewer_component.viewer_name, - diff_lines_path: project_blob_diff_lines_path(project, params) + diff_lines_path: project_blob_diff_lines_path(project, params), + old_path: @diff_file.old_path, + new_path: @diff_file.new_path } end diff --git a/app/components/rapid_diffs/viewers/no_preview_component.html.haml b/app/components/rapid_diffs/viewers/no_preview_component.html.haml index 8390d9e18ceb62..71a9f05af412fc 100644 --- a/app/components/rapid_diffs/viewers/no_preview_component.html.haml +++ b/app/components/rapid_diffs/viewers/no_preview_component.html.haml @@ -6,12 +6,11 @@ %p.rd-no-preview-paragraph = no_preview_reason .rd-no-preview-actions - - if @diff_file.collapsed? && expandable? || @diff_file.whitespace_only? - - click = @diff_file.whitespace_only? ? 'showWhitespaceChanges' : 'showChanges' - = action_button(button_options: { data: { click: click } }) do + - if @diff_file.collapsed? && expandable? || @diff_file.whitespace_only? && !@diff_file.too_large? + = action_button(button_options: { data: { click: 'showChanges' } }) do = _('Show changes') - elsif expandable? - = action_button(button_options: { data: { click: 'showFileContents' } }) do + = action_button(button_options: { data: { click: 'showChanges' } }) do = _('Show file contents') - elsif @diff_file.content_changed? = action_button(href: old_blob_path) do diff --git a/app/components/rapid_diffs/viewers/no_preview_component.rb b/app/components/rapid_diffs/viewers/no_preview_component.rb index b7ad2bff798d46..cc7eff69995d7a 100644 --- a/app/components/rapid_diffs/viewers/no_preview_component.rb +++ b/app/components/rapid_diffs/viewers/no_preview_component.rb @@ -45,7 +45,7 @@ def no_preview_reason end def expandable? - @diff_file.diffable_text? + @diff_file.diffable_text? && !@diff_file.too_large? end def important? diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index de8e9b0020c628..a5402f94e12eb2 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -167,6 +167,7 @@ def rapid_diffs @stream_url = diffs_stream_url(@commit, streaming_offset, diff_view) @diffs_slice = @commit.first_diffs_slice(streaming_offset, commit_diff_options) @diff_files_endpoint = diff_files_metadata_namespace_project_commit_path + @diff_file_endpoint = diff_file_namespace_project_commit_path @diffs_stats_endpoint = diffs_stats_namespace_project_commit_path @update_current_user_path = expose_path(api_v4_user_preferences_path) diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index adb8615c1c21d8..5f97d59cbd5def 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -84,6 +84,7 @@ def rapid_diffs @show_whitespace_default = current_user.nil? || current_user.show_whitespace_in_diffs @reload_stream_url = diffs_stream_namespace_project_compare_index_path(**compare_params) @diff_files_endpoint = diff_files_metadata_namespace_project_compare_index_path(**compare_params) + @diff_file_endpoint = diff_file_namespace_project_compare_index_path(**compare_params) @diffs_stats_endpoint = diffs_stats_namespace_project_compare_index_path(**compare_params) @update_current_user_path = expose_path(api_v4_user_preferences_path) diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index ec68a14001e757..68098b19d616d8 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/app/controllers/projects/merge_requests/creations_controller.rb @@ -161,6 +161,7 @@ def define_rapid_diffs_vars @stream_url = project_new_merge_request_diffs_stream_path(@project, merge_request: merge_request) @reload_stream_url = project_new_merge_request_diffs_stream_path(@project, merge_request: merge_request) @diff_files_endpoint = project_new_merge_request_diff_files_metadata_path(@project, merge_request: merge_request) + @diff_file_endpoint = project_new_merge_request_diff_file_path(@project, merge_request: merge_request) @diffs_stats_endpoint = project_new_merge_request_diffs_stats_path(@project, merge_request: merge_request) end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index b81154e288135c..d02e4719458f92 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -113,6 +113,7 @@ def rapid_diffs @stream_url = diffs_stream_url(@merge_request, streaming_offset, diff_view) @diffs_slice = @merge_request.first_diffs_slice(streaming_offset, diff_options) @diff_files_endpoint = diff_files_metadata_namespace_project_merge_request_path + @diff_file_endpoint = diff_file_namespace_project_merge_request_path @diffs_stats_endpoint = diffs_stats_namespace_project_merge_request_path show_merge_request diff --git a/app/views/projects/commit/rapid_diffs.html.haml b/app/views/projects/commit/rapid_diffs.html.haml index 76b0206c463542..9b8445949271fc 100644 --- a/app/views/projects/commit/rapid_diffs.html.haml +++ b/app/views/projects/commit/rapid_diffs.html.haml @@ -13,5 +13,5 @@ .container-fluid{ class: [container_class] } = render "commit_box" = render "ci_menu" - - 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, diffs_stats_endpoint: @diffs_stats_endpoint, diff_files_endpoint: @diff_files_endpoint } + - 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, diffs_stats_endpoint: @diffs_stats_endpoint, diff_files_endpoint: @diff_files_endpoint, diff_file_endpoint: @diff_file_endpoint } = render ::RapidDiffs::AppComponent.new(**args) diff --git a/app/views/projects/compare/rapid_diffs.html.haml b/app/views/projects/compare/rapid_diffs.html.haml index f9558da164e1b0..ebbe1c2f7577da 100644 --- a/app/views/projects/compare/rapid_diffs.html.haml +++ b/app/views/projects/compare/rapid_diffs.html.haml @@ -17,7 +17,7 @@ .container-fluid{ class: [container_class] } = render "projects/commits/commit_list" unless hide_commit_list .container-fluid - - args = { diffs_slice: nil, reload_stream_url: @reload_stream_url, stream_url: nil, show_whitespace: @show_whitespace_default, diff_view: diff_view, diffs_stats_endpoint: @diffs_stats_endpoint, update_user_endpoint: @update_current_user_path, diff_files_endpoint: @diff_files_endpoint, lazy: true } + - args = { diffs_slice: nil, reload_stream_url: @reload_stream_url, stream_url: nil, show_whitespace: @show_whitespace_default, diff_view: diff_view, diffs_stats_endpoint: @diffs_stats_endpoint, update_user_endpoint: @update_current_user_path, diff_files_endpoint: @diff_files_endpoint, diff_file_endpoint: @diff_file_endpoint, lazy: true } = render ::RapidDiffs::AppComponent.new(**args) - else .container-fluid diff --git a/app/views/projects/merge_requests/creations/rapid_diffs.html.haml b/app/views/projects/merge_requests/creations/rapid_diffs.html.haml index bfe466641f8bd2..caf82d8feca1d8 100644 --- a/app/views/projects/merge_requests/creations/rapid_diffs.html.haml +++ b/app/views/projects/merge_requests/creations/rapid_diffs.html.haml @@ -3,5 +3,5 @@ - add_page_specific_style 'page_bundles/merge_request_creation_rapid_diffs' = render "page" do - - args = { diffs_slice: nil, reload_stream_url: @reload_stream_url, stream_url: @stream_url, show_whitespace: @show_whitespace_default, diff_view: diff_view, update_user_endpoint: expose_path(api_v4_user_preferences_path), diffs_stats_endpoint: @diffs_stats_endpoint, diff_files_endpoint: @diff_files_endpoint, lazy: true } + - args = { diffs_slice: nil, reload_stream_url: @reload_stream_url, stream_url: @stream_url, show_whitespace: @show_whitespace_default, diff_view: diff_view, update_user_endpoint: expose_path(api_v4_user_preferences_path), diffs_stats_endpoint: @diffs_stats_endpoint, diff_files_endpoint: @diff_files_endpoint, diff_file_endpoint: @diff_file_endpoint, lazy: true } = render ::RapidDiffs::AppComponent.new(**args) diff --git a/app/views/projects/merge_requests/rapid_diffs.html.haml b/app/views/projects/merge_requests/rapid_diffs.html.haml index 792252e7a4e5e9..e9cdd8f6a2dbc9 100644 --- a/app/views/projects/merge_requests/rapid_diffs.html.haml +++ b/app/views/projects/merge_requests/rapid_diffs.html.haml @@ -3,7 +3,7 @@ - @content_class = 'rd-page-container diffs-container-limited' = 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, diffs_stats_endpoint: @diffs_stats_endpoint, diff_files_endpoint: @diff_files_endpoint, should_sort_metadata_files: true } +- 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, diffs_stats_endpoint: @diffs_stats_endpoint, diff_files_endpoint: @diff_files_endpoint, should_sort_metadata_files: true, diff_file_endpoint: @diff_file_endpoint } = 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) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ea98e2cfb056bb..70e96e5961292f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -50271,6 +50271,9 @@ msgstr "" msgid "RapidDiffs|Failed to expand lines, please try again." msgstr "" +msgid "RapidDiffs|Failed to load changes, please try again." +msgstr "" + msgid "RapidDiffs|File moved from %{old} to %{new}" msgstr "" -- GitLab From b958d9fb3a171c5bffb5b490e88e6b792f965bb1 Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Wed, 21 May 2025 17:48:19 +0400 Subject: [PATCH 2/3] Move expand data to the collapsed component --- app/assets/javascripts/rapid_diffs/load_file/adapter.js | 5 +++-- app/components/rapid_diffs/diff_file_component.rb | 4 +--- .../rapid_diffs/viewers/no_preview_component.html.haml | 4 ++-- app/components/rapid_diffs/viewers/no_preview_component.rb | 7 +++++++ 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/rapid_diffs/load_file/adapter.js b/app/assets/javascripts/rapid_diffs/load_file/adapter.js index a11475886f6356..4ec362f0ac452d 100644 --- a/app/assets/javascripts/rapid_diffs/load_file/adapter.js +++ b/app/assets/javascripts/rapid_diffs/load_file/adapter.js @@ -15,8 +15,9 @@ export const LoadFileAdapter = { async showChanges(event, button) { const { parallelView, showWhitespace } = useDiffsView(pinia); const url = new URL(this.appData.diffFileEndpoint, window.location.origin); - if (this.data.oldPath) url.searchParams.set('old_path', this.data.oldPath); - if (this.data.newPath) url.searchParams.set('new_path', this.data.newPath); + const { old_path: oldPath, new_path: newPath } = JSON.parse(button.dataset.paths); + if (oldPath) url.searchParams.set('old_path', oldPath); + if (newPath) url.searchParams.set('new_path', newPath); url.searchParams.set('ignore_whitespace_changes', !showWhitespace); if (parallelView) url.searchParams.set('view', 'parallel'); button.setAttribute('disabled', true); diff --git a/app/components/rapid_diffs/diff_file_component.rb b/app/components/rapid_diffs/diff_file_component.rb index 9a53d80d531179..837a9abd16825d 100644 --- a/app/components/rapid_diffs/diff_file_component.rb +++ b/app/components/rapid_diffs/diff_file_component.rb @@ -21,9 +21,7 @@ def file_data params = tree_join(@diff_file.content_sha, @diff_file.file_path) { viewer: viewer_component.viewer_name, - diff_lines_path: project_blob_diff_lines_path(project, params), - old_path: @diff_file.old_path, - new_path: @diff_file.new_path + diff_lines_path: project_blob_diff_lines_path(project, params) } end diff --git a/app/components/rapid_diffs/viewers/no_preview_component.html.haml b/app/components/rapid_diffs/viewers/no_preview_component.html.haml index 71a9f05af412fc..4c3bb96d93cd39 100644 --- a/app/components/rapid_diffs/viewers/no_preview_component.html.haml +++ b/app/components/rapid_diffs/viewers/no_preview_component.html.haml @@ -7,10 +7,10 @@ = no_preview_reason .rd-no-preview-actions - if @diff_file.collapsed? && expandable? || @diff_file.whitespace_only? && !@diff_file.too_large? - = action_button(button_options: { data: { click: 'showChanges' } }) do + = action_button(button_options: { data: { click: 'showChanges', paths: paths.to_json } }) do = _('Show changes') - elsif expandable? - = action_button(button_options: { data: { click: 'showChanges' } }) do + = action_button(button_options: { data: { click: 'showChanges', paths: paths.to_json } }) do = _('Show file contents') - elsif @diff_file.content_changed? = action_button(href: old_blob_path) do diff --git a/app/components/rapid_diffs/viewers/no_preview_component.rb b/app/components/rapid_diffs/viewers/no_preview_component.rb index cc7eff69995d7a..081b1dcd404713 100644 --- a/app/components/rapid_diffs/viewers/no_preview_component.rb +++ b/app/components/rapid_diffs/viewers/no_preview_component.rb @@ -72,6 +72,13 @@ def action_button(**args, &) end end + def paths + { + old_path: @diff_file.old_path, + new_path: @diff_file.new_path + } + end + def project @diff_file.repository.project end -- GitLab From e10476f7d5bd8f1961b99ce9dbec33dee3ad83a9 Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Wed, 21 May 2025 20:40:48 +0400 Subject: [PATCH 3/3] Add tests --- .../javascripts/rapid_diffs/diff_file.js | 12 +- .../rapid_diffs/load_file/adapter.js | 2 +- .../rapid_diffs/app_component_spec.rb | 3 + .../viewers/no_preview_component_spec.rb | 16 ++- spec/frontend/rapid_diffs/diff_file_spec.js | 69 ++++++----- .../rapid_diffs/load_file/adapter_spec.js | 108 ++++++++++++++++++ 6 files changed, 166 insertions(+), 44 deletions(-) create mode 100644 spec/frontend/rapid_diffs/load_file/adapter_spec.js diff --git a/app/assets/javascripts/rapid_diffs/diff_file.js b/app/assets/javascripts/rapid_diffs/diff_file.js index 0fc17683bd63f5..0658e7e700fbeb 100644 --- a/app/assets/javascripts/rapid_diffs/diff_file.js +++ b/app/assets/javascripts/rapid_diffs/diff_file.js @@ -43,7 +43,7 @@ export class DiffFile extends HTMLElement { disconnectedCallback() { // app might be missing if the file was destroyed before mounting // for example: changing view settings in the middle of the streaming - if (this.app) this.app.unobserve(this); + if (this.app) this.unobserveVisibility(); this.app = undefined; this.diffElement = undefined; this.sink = undefined; @@ -107,19 +107,11 @@ export class DiffFile extends HTMLElement { } selfReplace(node) { - // 'mount' is automagically called by the component in the Rapid Diffs app + // 'mount' is automagically called by the component inside the diff file this.replaceWith(node); - this.disconnect(); node.focusFirstButton({ focusVisible: false }); } - disconnect() { - this.diffElement.removeEventListener('click', this.onClickHandler); - this.unobserveVisibility(); - this.app = null; - this.sink = null; - } - get data() { if (!this[dataCacheKey]) this[dataCacheKey] = camelizeKeys(JSON.parse(this.dataset.fileData)); return this[dataCacheKey]; diff --git a/app/assets/javascripts/rapid_diffs/load_file/adapter.js b/app/assets/javascripts/rapid_diffs/load_file/adapter.js index 4ec362f0ac452d..2737cd7e58e7f2 100644 --- a/app/assets/javascripts/rapid_diffs/load_file/adapter.js +++ b/app/assets/javascripts/rapid_diffs/load_file/adapter.js @@ -23,7 +23,7 @@ export const LoadFileAdapter = { button.setAttribute('disabled', true); let response; try { - response = await axios.get(url); + response = await axios.get(url.toString()); } catch (error) { button.removeAttribute('disabled'); createAlert({ diff --git a/spec/components/rapid_diffs/app_component_spec.rb b/spec/components/rapid_diffs/app_component_spec.rb index 21868651e2dfed..1c3f80d6ef4542 100644 --- a/spec/components/rapid_diffs/app_component_spec.rb +++ b/spec/components/rapid_diffs/app_component_spec.rb @@ -11,6 +11,7 @@ let(:update_user_endpoint) { '/update_user' } let(:diffs_stats_endpoint) { '/diffs_stats' } let(:diff_files_endpoint) { '/diff_files_metadata' } + let(:diff_file_endpoint) { '/diff_file' } let(:should_sort_metadata_files) { false } let(:lazy) { false } @@ -35,6 +36,7 @@ expect(data['diff_view_type']).to eq(diff_view) expect(data['update_user_endpoint']).to eq(update_user_endpoint) expect(data['diffs_stream_url']).to eq(stream_url) + expect(data['diff_file_endpoint']).to eq(diff_file_endpoint) end context "with should_sort_metadata_files set to true" do @@ -150,6 +152,7 @@ def create_instance update_user_endpoint:, diffs_stats_endpoint:, diff_files_endpoint:, + diff_file_endpoint:, should_sort_metadata_files:, lazy: ) diff --git a/spec/components/rapid_diffs/viewers/no_preview_component_spec.rb b/spec/components/rapid_diffs/viewers/no_preview_component_spec.rb index 43cb22f05fd974..74f355e334a653 100644 --- a/spec/components/rapid_diffs/viewers/no_preview_component_spec.rb +++ b/spec/components/rapid_diffs/viewers/no_preview_component_spec.rb @@ -167,15 +167,27 @@ expect(page).to have_button("Show file contents") end - context 'when diff is too large' do + context 'when diff is collapsed' do before do - allow(diff_file).to receive(:collapsed?).and_return(true) + allow(diff_file).to receive_messages(collapsed?: true, too_large?: false) end it 'shows preview button' do render_component expect(page).to have_button("Show changes") end + + context 'when diff is too large' do + before do + allow(diff_file).to receive_messages(too_large?: true) + end + + it 'shows preview button' do + render_component + expect(page).to have_link("View original file") + expect(page).to have_link("View changed file") + end + end end context 'when diff is whitespace only' do diff --git a/spec/frontend/rapid_diffs/diff_file_spec.js b/spec/frontend/rapid_diffs/diff_file_spec.js index 0afb5eefbfb225..51eb549d15b4a7 100644 --- a/spec/frontend/rapid_diffs/diff_file_spec.js +++ b/spec/frontend/rapid_diffs/diff_file_spec.js @@ -11,7 +11,7 @@ describe('DiffFile Web Component', () => { `; let app; - let defaultAdapter; + let adapter; const getDiffElement = () => document.querySelector('[id=foo]'); const getWebComponentElement = () => document.querySelector('diff-file'); @@ -22,13 +22,20 @@ describe('DiffFile Web Component', () => { isIntersecting ? target.onVisible({}) : target.onInvisible({}); }; - const createDefaultAdapter = (customAdapter) => { - defaultAdapter = customAdapter; - }; + const createDefaultAdapter = () => ({ + click: jest.fn(), + clicks: { + foo: jest.fn(), + }, + visible: jest.fn(), + invisible: jest.fn(), + mounted: jest.fn(), + }); - const initRapidDiffsApp = (adapterConfig = { current: [defaultAdapter] }, appData = {}) => { + const initRapidDiffsApp = (currentAdapter = createDefaultAdapter(), appData = {}) => { + adapter = currentAdapter; app = { - adapterConfig, + adapterConfig: { current: [currentAdapter] }, appData, observe: jest.fn(), unobserve: jest.fn(), @@ -48,7 +55,8 @@ describe('DiffFile Web Component', () => { getWebComponentElement().onClick(event); }; - const mount = () => { + const mount = (customAdapter) => { + initRapidDiffsApp(customAdapter); document.body.innerHTML = html; getWebComponentElement().mount(app); }; @@ -61,26 +69,14 @@ describe('DiffFile Web Component', () => { viewer: 'current', }, sink: {}, - trigger: getWebComponentElement().trigger, + trigger: expect.any(Function), + replaceWith: expect.any(Function), }); beforeAll(() => { customElements.define('diff-file', DiffFile); }); - beforeEach(() => { - createDefaultAdapter({ - click: jest.fn(), - clicks: { - foo: jest.fn(), - }, - visible: jest.fn(), - invisible: jest.fn(), - mounted: jest.fn(), - }); - initRapidDiffsApp(); - }); - it('observes diff element', () => { mount(); expect(app.observe).toHaveBeenCalledWith(getWebComponentElement()); @@ -92,8 +88,8 @@ describe('DiffFile Web Component', () => { emitted = true; }); mount(); - expect(defaultAdapter.mounted).toHaveBeenCalled(); - expect(defaultAdapter.mounted.mock.instances[0]).toStrictEqual(getContext()); + expect(adapter.mounted).toHaveBeenCalled(); + expect(adapter.mounted.mock.instances[0]).toStrictEqual(getContext()); expect(emitted).toBe(true); }); @@ -104,6 +100,17 @@ describe('DiffFile Web Component', () => { expect(app.unobserve).toHaveBeenCalledWith(element); }); + it('can self replace', () => { + const focusFirstButton = jest.fn(); + const mockNode = { focusFirstButton }; + mount({ + mounted() { + this.replaceWith(mockNode); + }, + }); + expect(focusFirstButton).toHaveBeenCalled(); + }); + it('#selectFile', () => { mount(); const spy = jest.spyOn(getWebComponentElement(), 'scrollIntoView'); @@ -119,28 +126,28 @@ describe('DiffFile Web Component', () => { it('handles all clicks', () => { triggerVisibility(true); delegatedClick(getDiffElement()); - expect(defaultAdapter.click).toHaveBeenCalledWith(expect.any(MouseEvent)); - expect(defaultAdapter.click.mock.instances[0]).toStrictEqual(getContext()); + expect(adapter.click).toHaveBeenCalledWith(expect.any(MouseEvent)); + expect(adapter.click.mock.instances[0]).toStrictEqual(getContext()); }); it('handles specific clicks', () => { triggerVisibility(true); const clickTarget = getDiffElement().querySelector('[data-click=foo]'); delegatedClick(clickTarget); - expect(defaultAdapter.clicks.foo).toHaveBeenCalledWith(expect.any(MouseEvent), clickTarget); - expect(defaultAdapter.clicks.foo.mock.instances[0]).toStrictEqual(getContext()); + expect(adapter.clicks.foo).toHaveBeenCalledWith(expect.any(MouseEvent), clickTarget); + expect(adapter.clicks.foo.mock.instances[0]).toStrictEqual(getContext()); }); it('handles visible event', () => { triggerVisibility(true); - expect(defaultAdapter.visible).toHaveBeenCalled(); - expect(defaultAdapter.visible.mock.instances[0]).toStrictEqual(getContext()); + expect(adapter.visible).toHaveBeenCalled(); + expect(adapter.visible.mock.instances[0]).toStrictEqual(getContext()); }); it('handles invisible event', () => { triggerVisibility(false); - expect(defaultAdapter.invisible).toHaveBeenCalled(); - expect(defaultAdapter.invisible.mock.instances[0]).toStrictEqual(getContext()); + expect(adapter.invisible).toHaveBeenCalled(); + expect(adapter.invisible.mock.instances[0]).toStrictEqual(getContext()); }); }); diff --git a/spec/frontend/rapid_diffs/load_file/adapter_spec.js b/spec/frontend/rapid_diffs/load_file/adapter_spec.js new file mode 100644 index 00000000000000..9112975643f825 --- /dev/null +++ b/spec/frontend/rapid_diffs/load_file/adapter_spec.js @@ -0,0 +1,108 @@ +import MockAxiosAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; +import { DiffFile } from '~/rapid_diffs/diff_file'; +import { setHTMLFixture } from 'helpers/fixtures'; +import { LoadFileAdapter } from '~/rapid_diffs/load_file/adapter'; +import { HTTP_STATUS_INTERNAL_SERVER_ERROR, HTTP_STATUS_OK } from '~/lib/utils/http_status'; +import { TEST_HOST } from 'spec/test_constants'; +import { useDiffsView } from '~/rapid_diffs/stores/diffs_view'; +import { pinia } from '~/pinia/instance'; +import waitForPromises from 'helpers/wait_for_promises'; +import { createAlert } from '~/alert'; + +jest.mock('~/alert'); + +describe('LoadFileAdapter', () => { + const viewer = 'any'; + const diffFileEndpoint = '/diff-file'; + + let mockAdapter; + + const getDiffFile = () => document.querySelector('diff-file'); + const getButton = () => document.querySelector('button[data-click="showChanges"]'); + const getExpandedContent = () => document.querySelector('#expanded'); + const getRequestUrl = () => + `${TEST_HOST}${diffFileEndpoint}?old_path=foo&new_path=bar&ignore_whitespace_changes=${!useDiffsView(pinia).showWhitespace}`; + + const mountComponent = (component = getDiffFile()) => { + component.mount({ + adapterConfig: { [viewer]: [LoadFileAdapter] }, + appData: { diffFileEndpoint }, + unobserve: jest.fn(), + }); + }; + + const createComponentHtml = (name, content) => ` + <${name} data-file-data='${JSON.stringify({ viewer })}'> + ${content} + + `; + + const mount = () => { + setHTMLFixture( + createComponentHtml( + 'diff-file', + ``, + ), + ); + mountComponent(); + }; + + const delegatedClick = (element) => { + let event; + element.addEventListener( + 'click', + (e) => { + event = e; + }, + { once: true }, + ); + element.click(); + getDiffFile().onClick(event); + }; + + beforeAll(() => { + customElements.define('diff-file', DiffFile); + customElements.define( + 'new-diff-file', + class extends DiffFile { + constructor(...args) { + super(...args); + mountComponent(this); + } + }, + ); + }); + + beforeEach(() => { + mockAdapter = new MockAxiosAdapter(axios); + }); + + it.each([true, false])('expands file with hide whitespace %s', async (whitespace) => { + useDiffsView(pinia).showWhitespace = whitespace; + mockAdapter + .onGet(getRequestUrl()) + .reply( + HTTP_STATUS_OK, + createComponentHtml( + 'new-diff-file', + '
Expanded Content
', + ), + ); + mount(); + delegatedClick(getButton()); + expect(getButton().disabled).toBe(true); + await waitForPromises(); + expect(getExpandedContent()).not.toBeFalsy(); + }); + + it('handles error', async () => { + mockAdapter.onGet(getRequestUrl()).reply(HTTP_STATUS_INTERNAL_SERVER_ERROR); + mount(); + delegatedClick(getButton()); + expect(getButton().disabled).toBe(true); + await waitForPromises(); + expect(getButton().disabled).toBe(false); + expect(createAlert).toHaveBeenCalled(); + }); +}); -- GitLab