diff --git a/ee/app/assets/javascripts/security_dashboard/components/first_class_group_security_dashboard_vulnerabilities.vue b/ee/app/assets/javascripts/security_dashboard/components/first_class_group_security_dashboard_vulnerabilities.vue index 7898e10e56c61679c8e422fd1ea08a514243b369..1f0dd9e007c288b477694c90dde06af3e6b0d9f0 100644 --- a/ee/app/assets/javascripts/security_dashboard/components/first_class_group_security_dashboard_vulnerabilities.vue +++ b/ee/app/assets/javascripts/security_dashboard/components/first_class_group_security_dashboard_vulnerabilities.vue @@ -64,6 +64,16 @@ export default { return `${this.sortBy}_${this.sortDirection}`; }, }, + watch: { + filters() { + // Clear out the existing vulnerabilities so that the skeleton loader is shown. + this.vulnerabilities = []; + }, + sort() { + // Clear out the existing vulnerabilities so that the skeleton loader is shown. + this.vulnerabilities = []; + }, + }, methods: { onErrorDismiss() { this.errorLoadingVulnerabilities = false; @@ -73,14 +83,13 @@ export default { this.$apollo.queries.vulnerabilities.fetchMore({ variables: { after: this.pageInfo.endCursor }, updateQuery: (previousResult, { fetchMoreResult }) => { - const results = produce(fetchMoreResult, (draftData) => { + return produce(fetchMoreResult, (draftData) => { // eslint-disable-next-line no-param-reassign draftData.group.vulnerabilities.nodes = [ ...previousResult.group.vulnerabilities.nodes, ...draftData.group.vulnerabilities.nodes, ]; }); - return results; }, }); } diff --git a/ee/app/assets/javascripts/security_dashboard/components/first_class_instance_security_dashboard_vulnerabilities.vue b/ee/app/assets/javascripts/security_dashboard/components/first_class_instance_security_dashboard_vulnerabilities.vue index a52292418fa31d220e697522b8ddaf47f1a0f4bf..84ebcaf22e1941334bfe907a80000f5b173fda91 100644 --- a/ee/app/assets/javascripts/security_dashboard/components/first_class_instance_security_dashboard_vulnerabilities.vue +++ b/ee/app/assets/javascripts/security_dashboard/components/first_class_instance_security_dashboard_vulnerabilities.vue @@ -24,7 +24,6 @@ export default { data() { return { pageInfo: {}, - isFirstResultLoading: true, vulnerabilities: [], errorLoadingVulnerabilities: false, sortBy: 'severity', @@ -35,6 +34,9 @@ export default { isLoadingQuery() { return this.$apollo.queries.vulnerabilities.loading; }, + isLoadingFirstResult() { + return this.isLoadingQuery && this.vulnerabilities.length === 0; + }, sort() { return `${this.sortBy}_${this.sortDirection}`; }, @@ -51,8 +53,7 @@ export default { }; }, update: ({ vulnerabilities }) => vulnerabilities.nodes, - result({ data, loading }) { - this.isFirstResultLoading = loading; + result({ data }) { this.pageInfo = preparePageInfo(data?.vulnerabilities?.pageInfo); }, error() { @@ -60,6 +61,16 @@ export default { }, }, }, + watch: { + filters() { + // Clear out the existing vulnerabilities so that the skeleton loader is shown. + this.vulnerabilities = []; + }, + sort() { + // Clear out the existing vulnerabilities so that the skeleton loader is shown. + this.vulnerabilities = []; + }, + }, methods: { onErrorDismiss() { this.errorLoadingVulnerabilities = false; @@ -69,14 +80,13 @@ export default { this.$apollo.queries.vulnerabilities.fetchMore({ variables: { after: this.pageInfo.endCursor }, updateQuery: (previousResult, { fetchMoreResult }) => { - const results = produce(fetchMoreResult, (draftData) => { + return produce(fetchMoreResult, (draftData) => { // eslint-disable-next-line no-param-reassign draftData.vulnerabilities.nodes = [ ...previousResult.vulnerabilities.nodes, ...draftData.vulnerabilities.nodes, ]; }); - return results; }, }); } @@ -106,7 +116,7 @@ export default { <vulnerability-list v-else :filters="filters" - :is-loading="isFirstResultLoading" + :is-loading="isLoadingFirstResult" :vulnerabilities="vulnerabilities" should-show-project-namespace @sort-changed="handleSortChange" diff --git a/ee/app/assets/javascripts/security_dashboard/components/project_vulnerabilities.vue b/ee/app/assets/javascripts/security_dashboard/components/project_vulnerabilities.vue index 3e3798fc944fc96d2c5be177baf4be00252d7592..860ba8910db2aa8b40e54aec299eaad9c8c1fbb8 100644 --- a/ee/app/assets/javascripts/security_dashboard/components/project_vulnerabilities.vue +++ b/ee/app/assets/javascripts/security_dashboard/components/project_vulnerabilities.vue @@ -95,20 +95,29 @@ export default { return `${this.sortBy}_${this.sortDirection}`; }, }, + watch: { + filters() { + // Clear out the existing vulnerabilities so that the skeleton loader is shown. + this.vulnerabilities = []; + }, + sort() { + // Clear out the existing vulnerabilities so that the skeleton loader is shown. + this.vulnerabilities = []; + }, + }, methods: { fetchNextPage() { if (this.pageInfo.hasNextPage) { this.$apollo.queries.vulnerabilities.fetchMore({ variables: { after: this.pageInfo.endCursor }, updateQuery: (previousResult, { fetchMoreResult }) => { - const results = produce(fetchMoreResult, (draftData) => { + return produce(fetchMoreResult, (draftData) => { // eslint-disable-next-line no-param-reassign draftData.project.vulnerabilities.nodes = [ ...previousResult.project.vulnerabilities.nodes, ...draftData.project.vulnerabilities.nodes, ]; }); - return results; }, }); } diff --git a/ee/app/assets/javascripts/security_dashboard/components/vulnerability_list.vue b/ee/app/assets/javascripts/security_dashboard/components/vulnerability_list.vue index b29d3c0255f3df6b80f46695fe6e8ae27a32e33b..14e334c8200feaa49b7dc2896af37174072d0873 100644 --- a/ee/app/assets/javascripts/security_dashboard/components/vulnerability_list.vue +++ b/ee/app/assets/javascripts/security_dashboard/components/vulnerability_list.vue @@ -458,7 +458,7 @@ export default { <template #cell(activity)="{ item }"> <div class="gl-display-flex gl-justify-content-end"> - <auto-fix-help-text v-if="item.hasSolutions" :merge-request="item.mergeRequest" /> + <auto-fix-help-text v-if="item.mergeRequest" :merge-request="item.mergeRequest" /> <issues-badge v-if="badgeIssues(item).length > 0" :issues="badgeIssues(item)" diff --git a/ee/changelogs/unreleased/320885-show-loading-for-vulnerability-list.yml b/ee/changelogs/unreleased/320885-show-loading-for-vulnerability-list.yml new file mode 100644 index 0000000000000000000000000000000000000000..d6c5708a79b2a4259cc5aedabe8cbfea4ff580e5 --- /dev/null +++ b/ee/changelogs/unreleased/320885-show-loading-for-vulnerability-list.yml @@ -0,0 +1,5 @@ +--- +title: Show loading state for vulnerability list when filter/sort is changed +merge_request: 53934 +author: +type: changed diff --git a/ee/spec/frontend/security_dashboard/components/first_class_group_security_dashboard_vulnerabilities_spec.js b/ee/spec/frontend/security_dashboard/components/first_class_group_security_dashboard_vulnerabilities_spec.js index fb3f33a6329fdcdd4bc1041d8c4536d5239b3306..9487aa66884e4e8ade2369b4b8e9f92e305d5e95 100644 --- a/ee/spec/frontend/security_dashboard/components/first_class_group_security_dashboard_vulnerabilities_spec.js +++ b/ee/spec/frontend/security_dashboard/components/first_class_group_security_dashboard_vulnerabilities_spec.js @@ -6,6 +6,9 @@ import { generateVulnerabilities } from './mock_data'; describe('First Class Group Dashboard Vulnerabilities Component', () => { let wrapper; + const apolloMock = { + queries: { vulnerabilities: { loading: true } }, + }; const groupFullPath = 'group-full-path'; @@ -14,7 +17,12 @@ describe('First Class Group Dashboard Vulnerabilities Component', () => { const findAlert = () => wrapper.find(GlAlert); const findLoadingIcon = () => wrapper.find(GlLoadingIcon); - const createWrapper = ({ $apollo, stubs }) => { + const expectLoadingState = ({ initial = false, nextPage = false }) => { + expect(findVulnerabilities().props('isLoading')).toBe(initial); + expect(findLoadingIcon().exists()).toBe(nextPage); + }; + + const createWrapper = ({ $apollo = apolloMock, stubs } = {}) => { return shallowMount(FirstClassGroupVulnerabilities, { propsData: { groupFullPath, @@ -44,12 +52,8 @@ describe('First Class Group Dashboard Vulnerabilities Component', () => { }); }); - it('passes down isLoading correctly', () => { - expect(findVulnerabilities().props()).toMatchObject({ isLoading: true }); - }); - - it('does not show the loading spinner', () => { - expect(findLoadingIcon().exists()).toBe(false); + it('shows the initial loading state', () => { + expectLoadingState({ initial: true }); }); }); @@ -135,6 +139,10 @@ describe('First Class Group Dashboard Vulnerabilities Component', () => { expect(wrapper.vm.sortBy).toBe('description'); expect(wrapper.vm.sortDirection).toBe('asc'); }); + + it('does not show loading any state', () => { + expectLoadingState({ initial: false, nextPage: false }); + }); }); describe('when there is more than a page of vulnerabilities', () => { @@ -160,7 +168,7 @@ describe('First Class Group Dashboard Vulnerabilities Component', () => { }); }); - describe('when the query is loading and there is another page', () => { + describe('when the query is loading the next page', () => { beforeEach(() => { wrapper = createWrapper({ $apollo: { @@ -169,6 +177,7 @@ describe('First Class Group Dashboard Vulnerabilities Component', () => { }); wrapper.setData({ + vulnerabilities: generateVulnerabilities(), pageInfo: { hasNextPage: true, }, @@ -176,7 +185,28 @@ describe('First Class Group Dashboard Vulnerabilities Component', () => { }); it('should render the loading spinner', () => { - expect(findLoadingIcon().exists()).toBe(true); + expectLoadingState({ nextPage: true }); + }); + }); + + describe('when filter or sort is changed', () => { + beforeEach(() => { + wrapper = createWrapper(); + }); + + it('should show the initial loading state when the filter is changed', () => { + wrapper.setProps({ filter: {} }); + + expectLoadingState({ initial: true }); + }); + + it('should show the initial loading state when the sort is changed', () => { + findVulnerabilities().vm.$emit('sort-changed', { + sortBy: 'description', + sortDesc: false, + }); + + expectLoadingState({ initial: true }); }); }); }); diff --git a/ee/spec/frontend/security_dashboard/components/first_class_instance_security_dashboard_vulnerabilities_spec.js b/ee/spec/frontend/security_dashboard/components/first_class_instance_security_dashboard_vulnerabilities_spec.js index e74f02092c0eb73415088a548158ded1b432d4d0..c02b1eff10964276e118cf298612abdc8b85fd3f 100644 --- a/ee/spec/frontend/security_dashboard/components/first_class_instance_security_dashboard_vulnerabilities_spec.js +++ b/ee/spec/frontend/security_dashboard/components/first_class_instance_security_dashboard_vulnerabilities_spec.js @@ -1,44 +1,24 @@ import { GlAlert, GlTable, GlEmptyState, GlIntersectionObserver, GlLoadingIcon } from '@gitlab/ui'; -import { shallowMount, createLocalVue } from '@vue/test-utils'; -import Vuex from 'vuex'; +import { shallowMount } from '@vue/test-utils'; import FirstClassInstanceVulnerabilities from 'ee/security_dashboard/components/first_class_instance_security_dashboard_vulnerabilities.vue'; import VulnerabilityList from 'ee/security_dashboard/components/vulnerability_list.vue'; import { generateVulnerabilities } from './mock_data'; -const localVue = createLocalVue(); -localVue.use(Vuex); - describe('First Class Instance Dashboard Vulnerabilities Component', () => { let wrapper; - let store; const findIntersectionObserver = () => wrapper.find(GlIntersectionObserver); const findVulnerabilities = () => wrapper.find(VulnerabilityList); const findAlert = () => wrapper.find(GlAlert); const findLoadingIcon = () => wrapper.find(GlLoadingIcon); - const createWrapper = ({ stubs, loading = false, isUpdatingProjects, data } = {}) => { - store = new Vuex.Store({ - modules: { - projectSelector: { - namespaced: true, - actions: { - fetchProjects() {}, - setProjectEndpoints() {}, - }, - getters: { - isUpdatingProjects: jest.fn().mockReturnValue(isUpdatingProjects), - }, - state: { - projects: [], - }, - }, - }, - }); + const expectLoadingState = ({ initial = false, nextPage = false }) => { + expect(findVulnerabilities().props('isLoading')).toBe(initial); + expect(findLoadingIcon().exists()).toBe(nextPage); + }; + const createWrapper = ({ stubs, loading = false, data } = {}) => { return shallowMount(FirstClassInstanceVulnerabilities, { - localVue, - store, stubs, mocks: { $apollo: { @@ -61,17 +41,11 @@ describe('First Class Instance Dashboard Vulnerabilities Component', () => { describe('when the query is loading', () => { beforeEach(() => { - wrapper = createWrapper({ - loading: true, - }); - }); - - it('passes down isLoading correctly', () => { - expect(findVulnerabilities().props()).toMatchObject({ isLoading: true }); + wrapper = createWrapper({ loading: true }); }); - it('does not render the loading spinner', () => { - expect(findLoadingIcon().exists()).toBe(false); + it('shows the initial loading state', () => { + expectLoadingState({ initial: true }); }); }); @@ -81,10 +55,7 @@ describe('First Class Instance Dashboard Vulnerabilities Component', () => { stubs: { GlAlert, }, - data: () => ({ - isFirstResultLoading: false, - errorLoadingVulnerabilities: true, - }), + data: () => ({ errorLoadingVulnerabilities: true }), }); }); @@ -117,10 +88,7 @@ describe('First Class Instance Dashboard Vulnerabilities Component', () => { GlTable, GlEmptyState, }, - data: () => ({ - vulnerabilities, - isFirstResultLoading: false, - }), + data: () => ({ vulnerabilities }), }); }); @@ -151,6 +119,10 @@ describe('First Class Instance Dashboard Vulnerabilities Component', () => { expect(wrapper.vm.sortBy).toBe('description'); expect(wrapper.vm.sortDirection).toBe('asc'); }); + + it('does not show loading any state', () => { + expectLoadingState({ initial: false, nextPage: false }); + }); }); describe('when there is more than a page of vulnerabilities', () => { @@ -177,6 +149,7 @@ describe('First Class Instance Dashboard Vulnerabilities Component', () => { wrapper = createWrapper({ loading: true, data: () => ({ + vulnerabilities: generateVulnerabilities(), pageInfo: { hasNextPage: true, }, @@ -188,8 +161,29 @@ describe('First Class Instance Dashboard Vulnerabilities Component', () => { expect(findIntersectionObserver().exists()).toBe(true); }); - it('should render the loading spinner', () => { - expect(findLoadingIcon().exists()).toBe(true); + it('should render the next page loading spinner', () => { + expectLoadingState({ nextPage: true }); + }); + }); + + describe('when filter or sort is changed', () => { + beforeEach(() => { + wrapper = createWrapper({ loading: true }); + }); + + it('should show the initial loading state when the filter is changed', () => { + wrapper.setProps({ filter: {} }); + + expectLoadingState({ initial: true }); + }); + + it('should show the initial loading state when the sort is changed', () => { + findVulnerabilities().vm.$emit('sort-changed', { + sortBy: 'description', + sortDesc: false, + }); + + expectLoadingState({ initial: true }); }); }); }); diff --git a/ee/spec/frontend/security_dashboard/components/project_vulnerabilities_spec.js b/ee/spec/frontend/security_dashboard/components/project_vulnerabilities_spec.js index d1f0a87d7ecbdaacb4400d40130320762a3d7893..9a54adc6ad7f696544b21d8bb942855e91586936 100644 --- a/ee/spec/frontend/security_dashboard/components/project_vulnerabilities_spec.js +++ b/ee/spec/frontend/security_dashboard/components/project_vulnerabilities_spec.js @@ -34,6 +34,11 @@ describe('Vulnerabilities app component', () => { const findVulnerabilityList = () => wrapper.find(VulnerabilityList); const findLoadingIcon = () => wrapper.find(GlLoadingIcon); + const expectLoadingState = ({ initial = false, nextPage = false }) => { + expect(findVulnerabilityList().props('isLoading')).toBe(initial); + expect(findLoadingIcon().exists()).toBe(nextPage); + }; + beforeEach(() => { createWrapper(); }); @@ -48,12 +53,8 @@ describe('Vulnerabilities app component', () => { createWrapper(); }); - it('should be in the loading state', () => { - expect(findVulnerabilityList().props().isLoading).toBe(true); - }); - - it('should not render the loading spinner', () => { - expect(findLoadingIcon().exists()).toBe(false); + it('should show the initial loading state', () => { + expectLoadingState({ initial: true }); }); }); @@ -64,13 +65,11 @@ describe('Vulnerabilities app component', () => { createWrapper(); vulnerabilities = generateVulnerabilities(); - wrapper.setData({ - vulnerabilities, - }); + wrapper.setData({ vulnerabilities }); }); - it('should not be in the loading state', () => { - expect(findVulnerabilityList().props().isLoading).toBe(false); + it('should not show any loading state', () => { + expectLoadingState({ initial: false, nextPage: false }); }); it('should pass the vulnerabilities to the vulnerabilities list', () => { @@ -94,17 +93,14 @@ describe('Vulnerabilities app component', () => { }); it('handles sorting', () => { - findVulnerabilityList().vm.$listeners['sort-changed']({ + findVulnerabilityList().vm.$emit('sort-changed', { sortBy: 'description', sortDesc: false, }); + expect(wrapper.vm.sortBy).toBe('description'); expect(wrapper.vm.sortDirection).toBe('asc'); }); - - it('should render the loading spinner', () => { - expect(findLoadingIcon().exists()).toBe(false); - }); }); describe('with more than a page of vulnerabilities', () => { @@ -126,12 +122,12 @@ describe('Vulnerabilities app component', () => { expect(findIntersectionObserver().exists()).toBe(true); }); - it('should render the loading spinner', () => { - expect(findLoadingIcon().exists()).toBe(true); + it('should render the next page loading spinner', () => { + expectLoadingState({ nextPage: true }); }); }); - describe("when there's an error loading vulnerabilities", () => { + describe(`when there's an error loading vulnerabilities`, () => { beforeEach(() => { createWrapper(); wrapper.setData({ errorLoadingVulnerabilities: true }); @@ -142,6 +138,27 @@ describe('Vulnerabilities app component', () => { }); }); + describe('when filter or sort is changed', () => { + beforeEach(() => { + createWrapper(); + }); + + it('should show the initial loading state when the filter is changed', () => { + wrapper.setProps({ filter: {} }); + + expectLoadingState({ initial: true }); + }); + + it('should show the initial loading state when the sort is changed', () => { + findVulnerabilityList().vm.$emit('sort-changed', { + sortBy: 'description', + sortDesc: false, + }); + + expectLoadingState({ initial: true }); + }); + }); + describe('security scanners', () => { const notEnabledScannersHelpPath = '#not-enabled'; const noPipelineRunScannersHelpPath = '#no-pipeline';