From 2ceebdadbd950968b903707c5b198b0ad9a7c1f5 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Mon, 7 Oct 2019 10:55:47 -0500 Subject: [PATCH] feat(server): use new ancestor logic in build view UI --- packages/server/src/ui/hooks/use-api-data.jsx | 21 ++++++++++++++++++- .../src/ui/routes/build-view/build-view.jsx | 21 ++++++++++++------- .../ui/routes/build-view/build-view.test.jsx | 8 +++---- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/packages/server/src/ui/hooks/use-api-data.jsx b/packages/server/src/ui/hooks/use-api-data.jsx index 821177c03..21352a74b 100644 --- a/packages/server/src/ui/hooks/use-api-data.jsx +++ b/packages/server/src/ui/hooks/use-api-data.jsx @@ -151,12 +151,31 @@ export function useOptionalBuildRepresentativeRuns(projectId, buildId, url) { ); } +/** + * @param {string|undefined} projectId + * @param {string|undefined} buildId + * @return {[LoadingState, LHCI.ServerCommand.Build | null | undefined]} + */ +export function useAncestorBuild(projectId, buildId) { + const [apiLoadingState, build] = useApiData( + 'findAncestorBuildById', + projectId && buildId ? [projectId, buildId] : undefined + ); + + // If we couldn't find an ancestor build but we tried, consider it loaded with `null` to differentiate from `undefined`. + if (apiLoadingState === 'loaded' && !build) { + return ['loaded', null]; + } + + return [apiLoadingState, build]; +} + /** * @param {string|undefined} projectId * @param {{ancestorHash?: string} | undefined} build * @return {[LoadingState, LHCI.ServerCommand.Build | null | undefined]} */ -export function useOptionalAncestorBuild(projectId, build) { +export function useOptionalBuildByHash(projectId, build) { // Construct this options object in a `useMemo` to prevent infinitely re-requesting. const getBuildsOptions = useMemo(() => build && {hash: build.ancestorHash}, [ build && build.ancestorHash, diff --git a/packages/server/src/ui/routes/build-view/build-view.jsx b/packages/server/src/ui/routes/build-view/build-view.jsx index a8845595f..defa4b666 100644 --- a/packages/server/src/ui/routes/build-view/build-view.jsx +++ b/packages/server/src/ui/routes/build-view/build-view.jsx @@ -12,8 +12,9 @@ import {Dropdown} from '../../components/dropdown'; import { useProject, useBuild, - useOptionalAncestorBuild, + useOptionalBuildByHash, useOptionalBuildRepresentativeRuns, + useAncestorBuild, } from '../../hooks/use-api-data'; import {BuildHashSelector} from './build-hash-selector'; import {BuildSelectorPill} from './build-selector-pill'; @@ -241,16 +242,22 @@ const BuildView_ = props => { export const BuildView = props => { const projectLoadingData = useProject(props.projectId); const buildLoadingData = useBuild(props.projectId, props.buildId); + const ancestorBuildData = useAncestorBuild(props.projectId, props.buildId); - const ancestorHashOptions = props.baseHash ? {ancestorHash: props.baseHash} : buildLoadingData[1]; - const ancestorBuildData = useOptionalAncestorBuild(props.projectId, ancestorHashOptions); - const ancestorBuildId = ancestorBuildData[1] && ancestorBuildData[1].id; + const baseOverrideOptions = props.baseHash ? {ancestorHash: props.baseHash} : buildLoadingData[1]; + const baseOverrideData = useOptionalBuildByHash(props.projectId, baseOverrideOptions); + + const baseBuildData = + ancestorBuildData[0] === 'loaded' && !ancestorBuildData[1] + ? baseOverrideData + : ancestorBuildData; + const baseBuildId = baseBuildData[1] && baseBuildData[1].id; const runData = useOptionalBuildRepresentativeRuns(props.projectId, props.buildId, null); const baseRunData = useOptionalBuildRepresentativeRuns( props.projectId, - ancestorBuildId === null ? 'EMPTY_QUERY' : ancestorBuildId, + baseBuildId === null ? 'EMPTY_QUERY' : baseBuildId, null ); @@ -259,14 +266,14 @@ export const BuildView = props => { loadingState={combineLoadingStates( projectLoadingData, buildLoadingData, - ancestorBuildData, + baseBuildData, runData, baseRunData )} asyncData={combineAsyncData( projectLoadingData, buildLoadingData, - ancestorBuildData, + baseBuildData, runData, baseRunData )} diff --git a/packages/server/test/ui/routes/build-view/build-view.test.jsx b/packages/server/test/ui/routes/build-view/build-view.test.jsx index 21efce17d..224cb4d5f 100644 --- a/packages/server/test/ui/routes/build-view/build-view.test.jsx +++ b/packages/server/test/ui/routes/build-view/build-view.test.jsx @@ -28,9 +28,8 @@ describe('BuildView', () => { it('should render the build and missing comparison build', async () => { fetchMock.mockResponseOnce(JSON.stringify({name: 'My Project'})); // getProject - fetchMock.mockResponseOnce( - JSON.stringify({hash: 'abcd', commitMessage: 'test: write some tests'}) - ); // getBuild + fetchMock.mockResponseOnce(JSON.stringify({hash: 'abcd', commitMessage: 'write some tests'})); // getBuild + fetchMock.mockResponseOnce('null', {status: 404}); // findAncestor fetchMock.mockResponseOnce(JSON.stringify([])); // getBuilds - ancestors fetchMock.mockResponseOnce(JSON.stringify([])); // getRuns - compare fetchMock.mockResponseOnce(JSON.stringify([])); // getRuns - base @@ -44,7 +43,8 @@ describe('BuildView', () => { fetchMock.mockResponseOnce( JSON.stringify({hash: 'abcd', commitMessage: 'test: write some tests', ancestorHash: '1234'}) ); // getBuild - fetchMock.mockResponseOnce(JSON.stringify([{hash: '1234', commitMessage: 'fix: master'}])); // getBuilds - ancestors + fetchMock.mockResponseOnce(JSON.stringify({id: 'a', hash: '1234', commitMessage: 'fix it'})); // findAncestor + fetchMock.mockResponseOnce(JSON.stringify([])); // getBuilds - ancestors fetchMock.mockResponseOnce(JSON.stringify([])); // getRuns - compare fetchMock.mockResponseOnce(JSON.stringify([])); // getRuns - base