diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 9ccba88f7e65980e9ee92092036972af5967bbf4..5951912af55537f2b3375920ec28b428624d9415 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -45,6 +45,7 @@ import { import diffsEventHub from '../event_hub'; import { reviewStatuses } from '../utils/file_reviews'; import { diffsApp } from '../utils/performance'; +import { updateChangesTabCount } from '../utils/merge_request'; import { queueRedisHllEvents } from '../utils/queue_events'; import CollapsedFilesWarning from './collapsed_files_warning.vue'; import CommitWidget from './commit_widget.vue'; @@ -337,8 +338,6 @@ export default { mrReviews: this.rehydratedMrReviews, }); - this.interfaceWithDOM(); - if (this.endpointCodequality) { this.setCodequalityEndpoint(this.endpointCodequality); } @@ -446,16 +445,6 @@ export default { notesEventHub.$off('refetchDiffData', this.refetchDiffData); notesEventHub.$off('fetchDiffData', this.fetchData); }, - interfaceWithDOM() { - this.diffsTab = document.querySelector('.js-diffs-tab'); - }, - updateChangesTabCount() { - const badge = this.diffsTab.querySelector('.gl-badge'); - - if (this.diffsTab && badge) { - badge.textContent = this.diffFilesLength; - } - }, navigateToDiffFileNumber(number) { this.navigateToDiffFileIndex(number - 1); }, @@ -482,7 +471,9 @@ export default { this.setTreeDisplay(); } - this.updateChangesTabCount(); + updateChangesTabCount({ + count: this.diffFilesLength, + }); }) .catch(() => { createAlert({ diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index 873c48196698b88c2aee28be8baeb2afe3009d2d..a459def6b4b9f05eb8e199f8a2c99544c4b319b8 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -112,3 +112,6 @@ export const TRACKING_WHITESPACE_HIDE = 'i_code_review_diff_hide_whitespace'; export const TRACKING_CLICK_SINGLE_FILE_SETTING = 'i_code_review_click_single_file_mode_setting'; export const TRACKING_SINGLE_FILE_MODE = 'i_code_review_diff_single_file'; export const TRACKING_MULTIPLE_FILES_MODE = 'i_code_review_diff_multiple_files'; + +// UI +export const ZERO_CHANGES_ALT_DISPLAY = '-'; diff --git a/app/assets/javascripts/diffs/utils/merge_request.js b/app/assets/javascripts/diffs/utils/merge_request.js index 43e04a814c5ad18e0753eef97cc2858c4fd185fa..6847b8900d2f29ef9054a9739d49b3d8a3cd7795 100644 --- a/app/assets/javascripts/diffs/utils/merge_request.js +++ b/app/assets/javascripts/diffs/utils/merge_request.js @@ -1,3 +1,5 @@ +import { ZERO_CHANGES_ALT_DISPLAY } from '../constants'; + const endpointRE = /^(\/?(.+?)\/(.+?)\/-\/merge_requests\/(\d+)).*$/i; function getVersionInfo({ endpoint } = {}) { @@ -13,6 +15,17 @@ function getVersionInfo({ endpoint } = {}) { }; } +export function updateChangesTabCount({ + count, + badge = document.querySelector('.js-diffs-tab .gl-badge'), +} = {}) { + if (badge) { + // The purpose of this function is to assign to this parameter + /* eslint-disable-next-line no-param-reassign */ + badge.textContent = count || ZERO_CHANGES_ALT_DISPLAY; + } +} + export function getDerivedMergeRequestInformation({ endpoint } = {}) { let mrPath; let userOrGroup; diff --git a/app/views/projects/merge_requests/_page.html.haml b/app/views/projects/merge_requests/_page.html.haml index 4477a30d607cc1a2decddf0c6a5b2862abf5df0b..858bf2bf4c5557c882f7b0dbaa8650fa4caa0c45 100644 --- a/app/views/projects/merge_requests/_page.html.haml +++ b/app/views/projects/merge_requests/_page.html.haml @@ -8,6 +8,8 @@ - page_card_attributes @merge_request.card_attributes - suggest_changes_help_path = help_page_path('user/project/merge_requests/reviews/suggestions.md') - mr_action = j(params[:tab].presence || 'show') +-# @diffs_count is a number when the value is 0, but a string when there are other values +- diffs_count_display = @diffs_count.to_s == "0" ? "-" : @diffs_count - add_page_specific_style 'page_bundles/issuable' - add_page_specific_style 'page_bundles/design_management' - add_page_specific_style 'page_bundles/merge_requests' @@ -45,7 +47,7 @@ = render "projects/merge_requests/tabs/tab", name: "diffs", class: "diffs-tab js-diffs-tab", id: "diffs-tab", qa_selector: "diffs_tab" do = tab_link_for @merge_request, :diffs do = _("Changes") - = gl_badge_tag @diffs_count, { size: :sm } + = gl_badge_tag diffs_count_display, { size: :sm } .d-flex.flex-wrap.align-items-center.justify-content-lg-end #js-vue-discussion-counter{ data: { blocks_merge: @project.only_allow_merge_if_all_discussions_are_resolved?.to_s } } - if moved_mr_sidebar_enabled? diff --git a/spec/frontend/diffs/utils/merge_request_spec.js b/spec/frontend/diffs/utils/merge_request_spec.js index c070e8c004da4c0f86b2468afb5a218ecce7064c..21599a3be45cc83d79020ed321d7133bc036720c 100644 --- a/spec/frontend/diffs/utils/merge_request_spec.js +++ b/spec/frontend/diffs/utils/merge_request_spec.js @@ -1,4 +1,8 @@ -import { getDerivedMergeRequestInformation } from '~/diffs/utils/merge_request'; +import { + updateChangesTabCount, + getDerivedMergeRequestInformation, +} from '~/diffs/utils/merge_request'; +import { ZERO_CHANGES_ALT_DISPLAY } from '~/diffs/constants'; import { diffMetadata } from '../mock_data/diff_metadata'; describe('Merge Request utilities', () => { @@ -24,6 +28,56 @@ describe('Merge Request utilities', () => { ...noVersion, }; + describe('updateChangesTabCount', () => { + let dummyTab; + let badge; + + beforeEach(() => { + dummyTab = document.createElement('div'); + dummyTab.classList.add('js-diffs-tab'); + dummyTab.insertAdjacentHTML('afterbegin', 'ERROR'); + badge = dummyTab.querySelector('.gl-badge'); + }); + + afterEach(() => { + dummyTab.remove(); + dummyTab = null; + badge = null; + }); + + it('uses the alt hyphen display when the new changes are falsey', () => { + updateChangesTabCount({ count: 0, badge }); + + expect(dummyTab.textContent).toBe(ZERO_CHANGES_ALT_DISPLAY); + + updateChangesTabCount({ badge }); + + expect(dummyTab.textContent).toBe(ZERO_CHANGES_ALT_DISPLAY); + + updateChangesTabCount({ count: false, badge }); + + expect(dummyTab.textContent).toBe(ZERO_CHANGES_ALT_DISPLAY); + }); + + it('uses the actual value for display when the value is truthy', () => { + updateChangesTabCount({ count: 42, badge }); + + expect(dummyTab.textContent).toBe('42'); + + updateChangesTabCount({ count: '999+', badge }); + + expect(dummyTab.textContent).toBe('999+'); + }); + + it('selects the proper element to modify by default', () => { + document.body.insertAdjacentElement('afterbegin', dummyTab); + + updateChangesTabCount({ count: 42 }); + + expect(dummyTab.textContent).toBe('42'); + }); + }); + describe('getDerivedMergeRequestInformation', () => { let endpoint = `${diffMetadata.latest_version_path}.json?searchParam=irrelevant`;