From cb56015ca1ed1bb4bab8a911396aa1a7c6c6d4f6 Mon Sep 17 00:00:00 2001
From: Stanislav Lashmanov <slashmanov@gitlab.com>
Date: Wed, 12 Mar 2025 12:28:31 +0400
Subject: [PATCH] Display diff stats on Rapid Diffs
---
.../diffs/components/diff_app_controls.vue | 2 +-
.../diffs/components/diff_stats.vue | 2 +-
.../javascripts/rapid_diffs/app/index.js | 20 +++++++++++---
.../rapid_diffs/app/init_file_browser.js | 7 +----
.../rapid_diffs/app/view_settings.js | 11 +++++++-
.../rapid_diffs/stores/diffs_view.js | 17 ++++++++++++
.../rapid_diffs/app_component.html.haml | 4 +--
locale/gitlab.pot | 3 +++
.../rapid_diffs/app_component_spec.rb | 2 +-
spec/frontend/rapid_diffs/app/app_spec.js | 18 ++++++++++---
.../rapid_diffs/app/init_file_browser_spec.js | 14 ++--------
.../rapid_diffs/app/view_settings_spec.js | 11 ++++++++
.../rapid_diffs/stores/diffs_view_spec.js | 26 +++++++++++++++++++
13 files changed, 107 insertions(+), 30 deletions(-)
diff --git a/app/assets/javascripts/diffs/components/diff_app_controls.vue b/app/assets/javascripts/diffs/components/diff_app_controls.vue
index 22935806c908b..30364fb6ab5a9 100644
--- a/app/assets/javascripts/diffs/components/diff_app_controls.vue
+++ b/app/assets/javascripts/diffs/components/diff_app_controls.vue
@@ -24,7 +24,7 @@ export default {
required: true,
},
diffsCount: {
- type: String,
+ type: [String, Number],
default: '',
required: false,
},
diff --git a/app/assets/javascripts/diffs/components/diff_stats.vue b/app/assets/javascripts/diffs/components/diff_stats.vue
index ba97f0d5b2167..0dfb998630d63 100644
--- a/app/assets/javascripts/diffs/components/diff_stats.vue
+++ b/app/assets/javascripts/diffs/components/diff_stats.vue
@@ -21,7 +21,7 @@ export default {
required: true,
},
diffsCount: {
- type: String,
+ type: [String, Number],
required: false,
default: null,
},
diff --git a/app/assets/javascripts/rapid_diffs/app/index.js b/app/assets/javascripts/rapid_diffs/app/index.js
index 53e63859f551f..81571e1e28241 100644
--- a/app/assets/javascripts/rapid_diffs/app/index.js
+++ b/app/assets/javascripts/rapid_diffs/app/index.js
@@ -5,6 +5,9 @@ import { DiffFileMounted } from '~/rapid_diffs/diff_file_mounted';
import { useDiffsList } from '~/rapid_diffs/stores/diffs_list';
import { initFileBrowser } from '~/rapid_diffs/app/init_file_browser';
import { StreamingError } from '~/rapid_diffs/streaming_error';
+import { useDiffsView } from '~/rapid_diffs/stores/diffs_view';
+import { createAlert } from '~/alert';
+import { __ } from '~/locale';
// This facade interface joins together all the bits and pieces of Rapid Diffs: DiffFile, Settings, File browser, etc.
// It's a unified entrypoint for Rapid Diffs and all external communications should happen through this interface.
@@ -15,9 +18,20 @@ class RapidDiffsFacade {
init() {
this.#registerCustomElements();
- const appElement = document.querySelector('[data-rapid-diffs]');
- initViewSettings({ pinia, streamUrl: appElement.dataset.reloadStreamUrl });
- initFileBrowser();
+ const { reloadStreamUrl, metadataEndpoint } =
+ document.querySelector('[data-rapid-diffs]').dataset;
+ useDiffsView(pinia).metadataEndpoint = metadataEndpoint;
+ useDiffsView(pinia)
+ .loadMetadata()
+ .then(() => {
+ initFileBrowser();
+ })
+ .catch(() => {
+ createAlert({
+ message: __('Failed to load additional diffs information. Try reloading the page.'),
+ });
+ });
+ initViewSettings({ pinia, streamUrl: reloadStreamUrl });
}
// eslint-disable-next-line class-methods-use-this
diff --git a/app/assets/javascripts/rapid_diffs/app/init_file_browser.js b/app/assets/javascripts/rapid_diffs/app/init_file_browser.js
index f9ff48c8f6459..bee0bf06b4b2c 100644
--- a/app/assets/javascripts/rapid_diffs/app/init_file_browser.js
+++ b/app/assets/javascripts/rapid_diffs/app/init_file_browser.js
@@ -4,13 +4,8 @@ import { pinia } from '~/pinia/instance';
import { DiffFile } from '~/rapid_diffs/diff_file';
import FileBrowser from './file_browser.vue';
-export async function initFileBrowser() {
+export function initFileBrowser() {
const el = document.querySelector('[data-file-browser]');
- const { metadataEndpoint } = el.dataset;
-
- store.state.diffs.endpointMetadata = metadataEndpoint;
- await store.dispatch('diffs/fetchDiffFilesMeta');
-
// eslint-disable-next-line no-new
new Vue({
el,
diff --git a/app/assets/javascripts/rapid_diffs/app/view_settings.js b/app/assets/javascripts/rapid_diffs/app/view_settings.js
index 3e1edf227c3db..819ad296308a0 100644
--- a/app/assets/javascripts/rapid_diffs/app/view_settings.js
+++ b/app/assets/javascripts/rapid_diffs/app/view_settings.js
@@ -21,7 +21,13 @@ const initSettingsApp = (el, pinia) => {
pinia,
computed: {
...mapState(useDiffsList, ['isLoading', 'isEmpty']),
- ...mapState(useDiffsView, ['showWhitespace', 'viewType', 'fileByFileMode', 'singleFileMode']),
+ ...mapState(useDiffsView, [
+ 'showWhitespace',
+ 'viewType',
+ 'fileByFileMode',
+ 'singleFileMode',
+ 'diffStats',
+ ]),
},
methods: {
...mapActions(useDiffsView, ['updateViewType', 'updateShowWhitespace']),
@@ -34,6 +40,9 @@ const initSettingsApp = (el, pinia) => {
diffViewType: this.viewType,
viewDiffsFileByFile: this.singleFileMode,
isLoading: this.isLoading,
+ addedLines: this.diffStats?.addedLines,
+ removedLines: this.diffStats?.removedLines,
+ diffsCount: this.diffStats?.diffsCount,
},
on: {
updateDiffViewType: this.updateViewType,
diff --git a/app/assets/javascripts/rapid_diffs/stores/diffs_view.js b/app/assets/javascripts/rapid_diffs/stores/diffs_view.js
index edb64bd2ff5ff..2773624d7e124 100644
--- a/app/assets/javascripts/rapid_diffs/stores/diffs_view.js
+++ b/app/assets/javascripts/rapid_diffs/stores/diffs_view.js
@@ -11,6 +11,7 @@ import { queueRedisHllEvents } from '~/diffs/utils/queue_events';
import { mergeUrlParams } from '~/lib/utils/url_utility';
import axios from '~/lib/utils/axios_utils';
import { useDiffsList } from '~/rapid_diffs/stores/diffs_list';
+import store from '~/mr_notes/stores';
export const useDiffsView = defineStore('diffsView', {
state() {
@@ -20,9 +21,24 @@ export const useDiffsView = defineStore('diffsView', {
singleFileMode: false,
updateUserEndpoint: undefined,
streamUrl: undefined,
+ metadataEndpoint: undefined,
+ diffStats: null,
};
},
actions: {
+ async loadMetadata() {
+ // TODO: refactor this to our own Pinia stores
+ store.state.diffs.endpointMetadata = this.metadataEndpoint;
+ store.state.diffs.diffViewType = this.viewType;
+ store.state.diffs.showWhitespace = this.showWhitespace;
+ await store.dispatch('diffs/fetchDiffFilesMeta');
+ this.diffStats = {
+ addedLines: store.state.diffs.addedLines,
+ removedLines: store.state.diffs.removedLines,
+ // we will be using a number for that after refactoring
+ diffsCount: parseInt(store.state.diffs.realSize, 10),
+ };
+ },
updateDiffView() {
if (this.singleFileMode) {
// TODO: implement single file mode
@@ -47,6 +63,7 @@ export const useDiffsView = defineStore('diffsView', {
// we don't have to wait for the setting to be saved since whitespace param is passed explicitly
axios.put(this.updateUserEndpoint, { show_whitespace_in_diffs: value });
}
+ this.loadMetadata();
this.updateDiffView();
},
},
diff --git a/app/components/rapid_diffs/app_component.html.haml b/app/components/rapid_diffs/app_component.html.haml
index b4ef595f53c3d..8995d3554dd4a 100644
--- a/app/components/rapid_diffs/app_component.html.haml
+++ b/app/components/rapid_diffs/app_component.html.haml
@@ -1,9 +1,9 @@
-.rd-app{ data: { rapid_diffs: true, reload_stream_url: @reload_stream_url } }
+.rd-app{ data: { rapid_diffs: true, reload_stream_url: @reload_stream_url, metadata_endpoint: @metadata_endpoint } }
.rd-app-header
.rd-app-settings
%div{ data: { view_settings: true, show_whitespace: @show_whitespace.to_json, diff_view_type: @diff_view, update_user_endpoint: @update_user_endpoint } }
.rd-app-body
- .rd-app-sidebar{ data: { file_browser: true, metadata_endpoint: @metadata_endpoint }, style: ("width: #{initial_sidebar_width}px" if initial_sidebar_width) }
+ .rd-app-sidebar{ data: { file_browser: true }, style: ("width: #{initial_sidebar_width}px" if initial_sidebar_width) }
.rd-app-sidebar-loading
= helpers.gl_loading_icon(size: 'sm')
.rd-app-content{ data: { sidebar_visible: true } }
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 6c26122e45a11..0fce6fe0732e7 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -24724,6 +24724,9 @@ msgstr ""
msgid "Failed to load Roadmap"
msgstr ""
+msgid "Failed to load additional diffs information. Try reloading the page."
+msgstr ""
+
msgid "Failed to load assignees. Please try again."
msgstr ""
diff --git a/spec/components/rapid_diffs/app_component_spec.rb b/spec/components/rapid_diffs/app_component_spec.rb
index f49697c2ed3a6..b4ce774bd2b7d 100644
--- a/spec/components/rapid_diffs/app_component_spec.rb
+++ b/spec/components/rapid_diffs/app_component_spec.rb
@@ -21,6 +21,7 @@
app = page.find('[data-rapid-diffs]')
expect(app).not_to be_nil
expect(app['data-reload-stream-url']).to eq(reload_stream_url)
+ expect(app['data-metadata-endpoint']).to eq(metadata_endpoint)
end
it "renders view settings" do
@@ -36,7 +37,6 @@
render_component
container = page.find("[data-file-browser]")
expect(container).not_to be_nil
- expect(container['data-metadata-endpoint']).to eq(metadata_endpoint)
end
it "sets sidebar width" do
diff --git a/spec/frontend/rapid_diffs/app/app_spec.js b/spec/frontend/rapid_diffs/app/app_spec.js
index df04dd4d1be69..c689fec220209 100644
--- a/spec/frontend/rapid_diffs/app/app_spec.js
+++ b/spec/frontend/rapid_diffs/app/app_spec.js
@@ -8,7 +8,9 @@ import { useDiffsList } from '~/rapid_diffs/stores/diffs_list';
import { pinia } from '~/pinia/instance';
import { initFileBrowser } from '~/rapid_diffs/app/init_file_browser';
import { StreamingError } from '~/rapid_diffs/streaming_error';
+import { useDiffsView } from '~/rapid_diffs/stores/diffs_view';
+jest.mock('~/mr_notes/stores');
jest.mock('~/rapid_diffs/app/view_settings');
jest.mock('~/rapid_diffs/app/init_file_browser');
@@ -23,21 +25,31 @@ describe('Rapid Diffs App', () => {
createTestingPinia();
setHTMLFixture(
`
- <div data-rapid-diffs data-reload-stream-url="/reload">
+ <div data-rapid-diffs data-reload-stream-url="/reload" data-metadata-endpoint="/metadata">
<div id="js-stream-container" data-diffs-stream-url="/stream"></div>
</div>
`,
);
});
- it('initializes the app', () => {
+ it('initializes the app', async () => {
+ let res;
+ const mock = useDiffsView().loadMetadata.mockImplementationOnce(
+ () =>
+ new Promise((resolve) => {
+ res = resolve;
+ }),
+ );
createApp();
app.init();
+ expect(useDiffsView().metadataEndpoint).toBe('/metadata');
+ expect(mock).toHaveBeenCalled();
expect(initViewSettings).toHaveBeenCalledWith({ pinia, streamUrl: '/reload' });
- expect(initFileBrowser).toHaveBeenCalled();
expect(window.customElements.get('diff-file')).toBe(DiffFile);
expect(window.customElements.get('diff-file-mounted')).toBe(DiffFileMounted);
expect(window.customElements.get('streaming-error')).toBe(StreamingError);
+ await res();
+ expect(initFileBrowser).toHaveBeenCalled();
});
it('streams remaining diffs', () => {
diff --git a/spec/frontend/rapid_diffs/app/init_file_browser_spec.js b/spec/frontend/rapid_diffs/app/init_file_browser_spec.js
index 3fe7da030b862..00f0b5626dae0 100644
--- a/spec/frontend/rapid_diffs/app/init_file_browser_spec.js
+++ b/spec/frontend/rapid_diffs/app/init_file_browser_spec.js
@@ -1,5 +1,4 @@
import { resetHTMLFixture, setHTMLFixture } from 'helpers/fixtures';
-import store from '~/mr_notes/stores';
import { initFileBrowser } from '~/rapid_diffs/app/init_file_browser';
import createEventHub from '~/helpers/event_hub_factory';
import waitForPromises from 'helpers/wait_for_promises';
@@ -22,13 +21,9 @@ jest.mock('~/rapid_diffs/app/file_browser.vue', () => ({
}));
describe('Init file browser', () => {
- let dispatch;
-
- const getMountElement = () => document.querySelector('[data-file-browser]');
const getFileBrowser = () => document.querySelector('[data-file-browser-component]');
beforeEach(() => {
- dispatch = jest.spyOn(store, 'dispatch').mockResolvedValue();
window.mrTabs = { eventHub: createEventHub() };
setHTMLFixture(
`
@@ -46,14 +41,9 @@ describe('Init file browser', () => {
resetHTMLFixture();
});
- it('sets metadata endpoint', () => {
- initFileBrowser();
- expect(store.state.diffs.endpointMetadata).toBe(getMountElement().dataset.metadataEndpoint);
- });
-
- it('fetches metadata', () => {
+ it('mounts the component', () => {
initFileBrowser();
- expect(dispatch).toHaveBeenCalledWith('diffs/fetchDiffFilesMeta');
+ expect(getFileBrowser()).not.toBe(null);
});
it('handles file clicks', async () => {
diff --git a/spec/frontend/rapid_diffs/app/view_settings_spec.js b/spec/frontend/rapid_diffs/app/view_settings_spec.js
index b3c4696cabebf..865ed154d2b52 100644
--- a/spec/frontend/rapid_diffs/app/view_settings_spec.js
+++ b/spec/frontend/rapid_diffs/app/view_settings_spec.js
@@ -20,6 +20,9 @@ jest.mock('~/diffs/components/diff_app_controls.vue', () => ({
'data-show-whitespace': JSON.stringify(this.showWhitespace),
'data-diff-view-type': JSON.stringify(this.diffViewType),
'data-is-loading': JSON.stringify(this.isLoading),
+ 'data-added-lines': JSON.stringify(this.addedLines),
+ 'data-removed-lines': JSON.stringify(this.removedLines),
+ 'data-diffs-count': JSON.stringify(this.diffsCount),
},
});
},
@@ -68,6 +71,11 @@ describe('View settings', () => {
it('sets diff app controls props', () => {
useDiffsList().loadedFiles = { foo: true };
+ useDiffsView().diffStats = {
+ addedLines: 1,
+ removedLines: 2,
+ diffsCount: 3,
+ };
initViewSettings({ pinia, streamUrl });
const el = getDiffAppControls();
const getProp = (prop) => JSON.parse(el.dataset[prop]);
@@ -75,6 +83,9 @@ describe('View settings', () => {
expect(getProp('showWhitespace')).toBe(true);
expect(getProp('diffViewType')).toBe('parallel');
expect(getProp('isLoading')).toBe(false);
+ expect(getProp('addedLines')).toBe(1);
+ expect(getProp('removedLines')).toBe(2);
+ expect(getProp('diffsCount')).toBe(3);
});
it('triggers collapse all files', () => {
diff --git a/spec/frontend/rapid_diffs/stores/diffs_view_spec.js b/spec/frontend/rapid_diffs/stores/diffs_view_spec.js
index 0c3b2cb45115e..5fde16c506ef2 100644
--- a/spec/frontend/rapid_diffs/stores/diffs_view_spec.js
+++ b/spec/frontend/rapid_diffs/stores/diffs_view_spec.js
@@ -14,6 +14,7 @@ import {
import { queueRedisHllEvents } from '~/diffs/utils/queue_events';
import axios from '~/lib/utils/axios_utils';
import { HTTP_STATUS_OK } from '~/lib/utils/http_status';
+import vuexStore from '~/mr_notes/stores';
const defaultState = {
updateUserEndpoint: '/update',
@@ -39,6 +40,31 @@ describe('Diffs view store', () => {
setActivePinia(pinia);
store = useDiffsView();
useDiffsList().reloadDiffs.mockResolvedValue();
+ jest.spyOn(vuexStore, 'dispatch').mockResolvedValue();
+ });
+
+ describe('#loadMetadata', () => {
+ it('uses Vuex store to load metadata', () => {
+ const spy = jest.spyOn(vuexStore, 'dispatch');
+ store.metadataEndpoint = '/metadata';
+ store.loadMetadata();
+ expect(vuexStore.state.diffs.endpointMetadata).toBe('/metadata');
+ expect(vuexStore.state.diffs.diffViewType).toBe('inline');
+ expect(vuexStore.state.diffs.showWhitespace).toBe(true);
+ expect(spy).toHaveBeenCalledWith('diffs/fetchDiffFilesMeta');
+ });
+
+ it('copies values from Vuex store for diff stats', async () => {
+ vuexStore.state.diffs.addedLines = 1;
+ vuexStore.state.diffs.removedLines = 2;
+ vuexStore.state.diffs.realSize = '3';
+ await store.loadMetadata();
+ expect(store.diffStats).toStrictEqual({
+ addedLines: 1,
+ removedLines: 2,
+ diffsCount: 3,
+ });
+ });
});
describe('#updateDiffView', () => {
--
GitLab