From 8824dd30e884193b25ab8ac353d7c476ce4325c1 Mon Sep 17 00:00:00 2001 From: Jeremy Davis Date: Wed, 15 Jul 2020 18:35:25 +0200 Subject: [PATCH] fix FP in CodePageTest --- .../js/apps/code/__tests__/utils-test.tsx | 62 +++++++++++++++++-- .../src/main/js/apps/code/components/App.tsx | 61 +++++++++--------- .../js/apps/code/components/Components.tsx | 33 +++++----- .../__snapshots__/Components-test.tsx.snap | 4 -- .../sonar-web/src/main/js/apps/code/utils.ts | 41 ++++++++---- 5 files changed, 134 insertions(+), 67 deletions(-) diff --git a/server/sonar-web/src/main/js/apps/code/__tests__/utils-test.tsx b/server/sonar-web/src/main/js/apps/code/__tests__/utils-test.tsx index e5cd3151006..6c516f5f7df 100644 --- a/server/sonar-web/src/main/js/apps/code/__tests__/utils-test.tsx +++ b/server/sonar-web/src/main/js/apps/code/__tests__/utils-test.tsx @@ -17,13 +17,25 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -import { getChildren } from '../../../api/components'; +import { getBreadcrumbs, getChildren, getComponent } from '../../../api/components'; import { mockMainBranch, mockPullRequest } from '../../../helpers/mocks/branch-like'; -import { addComponent, addComponentChildren, getComponentBreadcrumbs } from '../bucket'; -import { getCodeMetrics, loadMoreChildren, retrieveComponentChildren } from '../utils'; +import { + addComponent, + addComponentBreadcrumbs, + addComponentChildren, + getComponentBreadcrumbs +} from '../bucket'; +import { + getCodeMetrics, + loadMoreChildren, + retrieveComponent, + retrieveComponentChildren +} from '../utils'; jest.mock('../../../api/components', () => ({ - getChildren: jest.fn().mockRejectedValue({}) + getBreadcrumbs: jest.fn().mockRejectedValue({}), + getChildren: jest.fn().mockRejectedValue({}), + getComponent: jest.fn().mockRejectedValue({}) })); jest.mock('../bucket', () => ({ @@ -63,7 +75,7 @@ describe('retrieveComponentChildren', () => { paging: { total: 2, pageIndex: 0 } }); - await retrieveComponentChildren('key', 'TRK', mockMainBranch()); + await retrieveComponentChildren('key', 'TRK', { mounted: true }, mockMainBranch()); expect(addComponentChildren).toHaveBeenCalledWith('key', components, 2, 0); expect(addComponent).toHaveBeenCalledTimes(2); @@ -71,6 +83,44 @@ describe('retrieveComponentChildren', () => { }); }); +describe('retrieveComponent', () => { + it('should update bucket when component is mounted', async () => { + const components = [{}, {}]; + (getChildren as jest.Mock).mockResolvedValueOnce({ + components, + paging: { total: 2, pageIndex: 0 } + }); + (getComponent as jest.Mock).mockResolvedValueOnce({ + component: {} + }); + (getBreadcrumbs as jest.Mock).mockResolvedValueOnce([]); + + await retrieveComponent('key', 'TRK', { mounted: true }, mockMainBranch()); + + expect(addComponentChildren).toHaveBeenCalled(); + expect(addComponent).toHaveBeenCalledTimes(3); + expect(addComponentBreadcrumbs).toHaveBeenCalled(); + }); + + it('should not update bucket when component is not mounted', async () => { + const components = [{}, {}]; + (getChildren as jest.Mock).mockResolvedValueOnce({ + components, + paging: { total: 2, pageIndex: 0 } + }); + (getComponent as jest.Mock).mockResolvedValueOnce({ + component: {} + }); + (getBreadcrumbs as jest.Mock).mockResolvedValueOnce([]); + + await retrieveComponent('key', 'TRK', { mounted: false }, mockMainBranch()); + + expect(addComponentChildren).not.toHaveBeenCalled(); + expect(addComponent).not.toHaveBeenCalled(); + expect(addComponentBreadcrumbs).not.toHaveBeenCalled(); + }); +}); + describe('loadMoreChildren', () => { it('should load more children', async () => { const components = [{}, {}, {}]; @@ -79,7 +129,7 @@ describe('loadMoreChildren', () => { paging: { total: 6, pageIndex: 1 } }); - await loadMoreChildren('key', 1, 'TRK', mockMainBranch()); + await loadMoreChildren('key', 1, 'TRK', { mounted: true }, mockMainBranch()); expect(addComponentChildren).toHaveBeenCalledWith('key', components, 6, 1); expect(addComponent).toHaveBeenCalledTimes(3); diff --git a/server/sonar-web/src/main/js/apps/code/components/App.tsx b/server/sonar-web/src/main/js/apps/code/components/App.tsx index c1a65d7aca1..be226adadf4 100644 --- a/server/sonar-web/src/main/js/apps/code/components/App.tsx +++ b/server/sonar-web/src/main/js/apps/code/components/App.tsx @@ -110,35 +110,37 @@ export class App extends React.PureComponent { loadComponent = (componentKey: string) => { this.setState({ loading: true }); - retrieveComponent(componentKey, this.props.component.qualifier, this.props.branchLike).then( - r => { - if (this.mounted) { - if (['FIL', 'UTS'].includes(r.component.qualifier)) { - this.setState({ - breadcrumbs: r.breadcrumbs, - components: r.components, - loading: false, - page: 0, - searchResults: undefined, - sourceViewer: r.component, - total: 0 - }); - } else { - this.setState({ - baseComponent: r.component, - breadcrumbs: r.breadcrumbs, - components: r.components, - loading: false, - page: r.page, - searchResults: undefined, - sourceViewer: undefined, - total: r.total - }); - } + retrieveComponent( + componentKey, + this.props.component.qualifier, + this, + this.props.branchLike + ).then(r => { + if (this.mounted) { + if (['FIL', 'UTS'].includes(r.component.qualifier)) { + this.setState({ + breadcrumbs: r.breadcrumbs, + components: r.components, + loading: false, + page: 0, + searchResults: undefined, + sourceViewer: r.component, + total: 0 + }); + } else { + this.setState({ + baseComponent: r.component, + breadcrumbs: r.breadcrumbs, + components: r.components, + loading: false, + page: r.page, + searchResults: undefined, + sourceViewer: undefined, + total: r.total + }); } - }, - this.stopLoading - ); + } + }, this.stopLoading); }; stopLoading = () => { @@ -154,7 +156,7 @@ export class App extends React.PureComponent { addComponentBreadcrumbs(component.key, component.breadcrumbs); this.setState({ loading: true }); - retrieveComponentChildren(component.key, component.qualifier, branchLike).then(() => { + retrieveComponentChildren(component.key, component.qualifier, this, branchLike).then(() => { addComponent(component); if (this.mounted) { this.handleUpdate(); @@ -171,6 +173,7 @@ export class App extends React.PureComponent { baseComponent.key, page + 1, this.props.component.qualifier, + this, this.props.branchLike ).then(r => { if (this.mounted && r.components.length) { diff --git a/server/sonar-web/src/main/js/apps/code/components/Components.tsx b/server/sonar-web/src/main/js/apps/code/components/Components.tsx index 69cead60d7b..4a6e1b0cae3 100644 --- a/server/sonar-web/src/main/js/apps/code/components/Components.tsx +++ b/server/sonar-web/src/main/js/apps/code/components/Components.tsx @@ -56,23 +56,24 @@ export class Components extends React.PureComponent { rootComponent={rootComponent} /> )} - {baseComponent && ( - - - -   -   - - - )} + {baseComponent && ( + <> + + +   +   + + + )} + {components.length ? ( components.map((component, index, list) => ( - - - - { - addComponent(component); + if (instance.mounted) { + addComponent(component); + } return component; }); } @@ -140,6 +147,7 @@ function retrieveComponentBase(componentKey: string, qualifier: string, branchLi export function retrieveComponentChildren( componentKey: string, qualifier: string, + instance: { mounted: boolean }, branchLike?: BranchLike ): Promise<{ components: T.ComponentMeasure[]; page: number; total: number }> { const existing = getComponentChildren(componentKey); @@ -160,15 +168,18 @@ export function retrieveComponentChildren( }) .then(prepareChildren) .then(r => { - addComponentChildren(componentKey, r.components, r.total, r.page); - storeChildrenBase(r.components); - storeChildrenBreadcrumbs(componentKey, r.components); + if (instance.mounted) { + addComponentChildren(componentKey, r.components, r.total, r.page); + storeChildrenBase(r.components); + storeChildrenBreadcrumbs(componentKey, r.components); + } return r; }); } function retrieveComponentBreadcrumbs( component: string, + instance: { mounted: boolean }, branchLike?: BranchLike ): Promise { const existing = getComponentBreadcrumbs(component); @@ -179,7 +190,9 @@ function retrieveComponentBreadcrumbs( return getBreadcrumbs({ component, ...getBranchLikeQuery(branchLike) }) .then(skipRootDir) .then(breadcrumbs => { - addComponentBreadcrumbs(component, breadcrumbs); + if (instance.mounted) { + addComponentBreadcrumbs(component, breadcrumbs); + } return breadcrumbs; }); } @@ -187,6 +200,7 @@ function retrieveComponentBreadcrumbs( export function retrieveComponent( componentKey: string, qualifier: string, + instance: { mounted: boolean }, branchLike?: BranchLike ): Promise<{ breadcrumbs: T.Breadcrumb[]; @@ -196,9 +210,9 @@ export function retrieveComponent( total: number; }> { return Promise.all([ - retrieveComponentBase(componentKey, qualifier, branchLike), - retrieveComponentChildren(componentKey, qualifier, branchLike), - retrieveComponentBreadcrumbs(componentKey, branchLike) + retrieveComponentBase(componentKey, qualifier, instance, branchLike), + retrieveComponentChildren(componentKey, qualifier, instance, branchLike), + retrieveComponentBreadcrumbs(componentKey, instance, branchLike) ]).then(r => { return { breadcrumbs: r[2], @@ -214,6 +228,7 @@ export function loadMoreChildren( componentKey: string, page: number, qualifier: string, + instance: { mounted: boolean }, branchLike?: BranchLike ): Promise { const metrics = getCodeMetrics(qualifier, branchLike, { includeQGStatus: true }); @@ -226,9 +241,11 @@ export function loadMoreChildren( }) .then(prepareChildren) .then(r => { - addComponentChildren(componentKey, r.components, r.total, r.page); - storeChildrenBase(r.components); - storeChildrenBreadcrumbs(componentKey, r.components); + if (instance.mounted) { + addComponentChildren(componentKey, r.components, r.total, r.page); + storeChildrenBase(r.components); + storeChildrenBreadcrumbs(componentKey, r.components); + } return r; }); } -- 2.39.5