diff --git a/src/pages/Compare/index.spec.tsx b/src/pages/Compare/index.spec.tsx index d3f303b3d..54617fc17 100644 --- a/src/pages/Compare/index.spec.tsx +++ b/src/pages/Compare/index.spec.tsx @@ -24,7 +24,7 @@ import Loading from '../../components/Loading'; import VersionChooser from '../../components/VersionChooser'; import styles from './styles.module.scss'; -import Compare, { CompareBase, PublicProps } from '.'; +import Compare, { CompareBase, PublicProps, mapStateToProps } from '.'; describe(__filename, () => { type GetRouteParamsParams = { @@ -256,10 +256,34 @@ describe(__filename, () => { }); it('renders an error when fetching a diff has failed', () => { + const addonId = 9999; + const baseVersionId = 1; + const headVersionId = baseVersionId + 1; + const store = configureStore(); - store.dispatch(versionsActions.abortFetchDiff()); + store.dispatch( + versionsActions.beginFetchDiff({ + addonId, + baseVersionId, + headVersionId, + }), + ); + store.dispatch( + versionsActions.abortFetchDiff({ + addonId, + baseVersionId, + headVersionId, + }), + ); - const root = render({ store }); + const root = render({ + ...getRouteParams({ + addonId, + baseVersionId, + headVersionId, + }), + store, + }); expect( getContentShellPanel(root, 'mainSidePanel').find(`.${styles.error}`), @@ -318,15 +342,15 @@ describe(__filename, () => { expect(dispatch).not.toHaveBeenCalled(); }); - it('does not dispatch fetchDiff() on update if no URL parameter has changed', () => { - const { dispatchSpy, root, params } = loadDiffAndRender(); + it('does not dispatch anything on update if nothing has changed', () => { + const { dispatchSpy, root } = loadDiffAndRender(); - root.setProps({ match: { params } }); + root.setProps({}); expect(dispatchSpy).not.toHaveBeenCalled(); }); - it('dispatches fetchDiff() on update if base version is different', () => { + it('dispatches fetchDiff() on update when baseVersionId changes', () => { const { _fetchDiff, addonId, @@ -335,30 +359,35 @@ describe(__filename, () => { fakeThunk, headVersionId, root, - version, + store, } = loadDiffAndRender(); - const newBaseVersionId = baseVersionId - 1; - root.setProps({ - match: { + const newBaseVersionId = baseVersionId + 1; + const routerProps = { + ...createFakeRouteComponentProps({ params: getRouteParams({ addonId, baseVersionId: newBaseVersionId, headVersionId, }), - }, + }), + }; + + root.setProps({ + ...routerProps, + ...mapStateToProps(store.getState(), { + ...root.instance().props, + ...routerProps, + }), }); expect(dispatchSpy).toHaveBeenCalledWith(fakeThunk.thunk); - expect(_fetchDiff).toHaveBeenCalledWith({ - addonId, - baseVersionId: newBaseVersionId, - headVersionId, - path: version.file.selected_file, - }); + expect(_fetchDiff).toHaveBeenCalledWith( + expect.objectContaining({ baseVersionId: newBaseVersionId }), + ); }); - it('dispatches fetchDiff() on update if head version is different', () => { + it('dispatches fetchDiff() on update when headVersionId changes', () => { const { _fetchDiff, addonId, @@ -367,30 +396,35 @@ describe(__filename, () => { fakeThunk, headVersionId, root, - version, + store, } = loadDiffAndRender(); const newHeadVersionId = headVersionId + 1; - root.setProps({ - match: { + const routerProps = { + ...createFakeRouteComponentProps({ params: getRouteParams({ addonId, baseVersionId, headVersionId: newHeadVersionId, }), - }, + }), + }; + + root.setProps({ + ...routerProps, + ...mapStateToProps(store.getState(), { + ...root.instance().props, + ...routerProps, + }), }); expect(dispatchSpy).toHaveBeenCalledWith(fakeThunk.thunk); - expect(_fetchDiff).toHaveBeenCalledWith({ - addonId, - baseVersionId, - headVersionId: newHeadVersionId, - path: version.file.selected_file, - }); + expect(_fetchDiff).toHaveBeenCalledWith( + expect.objectContaining({ headVersionId: newHeadVersionId }), + ); }); - it('dispatches fetchDiff() on update if addon ID is different', () => { + it('dispatches fetchDiff() on update when addonId changes', () => { const { _fetchDiff, addonId, @@ -399,27 +433,73 @@ describe(__filename, () => { fakeThunk, headVersionId, root, - version, + store, } = loadDiffAndRender(); const newAddonId = addonId + 1; - root.setProps({ - match: { + const routerProps = { + ...createFakeRouteComponentProps({ params: getRouteParams({ addonId: newAddonId, baseVersionId, headVersionId, }), - }, + }), + }; + + root.setProps({ + ...routerProps, + ...mapStateToProps(store.getState(), { + ...root.instance().props, + ...routerProps, + }), }); expect(dispatchSpy).toHaveBeenCalledWith(fakeThunk.thunk); - expect(_fetchDiff).toHaveBeenCalledWith({ - addonId: newAddonId, + expect(_fetchDiff).toHaveBeenCalledWith( + expect.objectContaining({ addonId: newAddonId }), + ); + }); + + it('dispatches fetchDiff() on update when path changes', () => { + const { + _fetchDiff, + addonId, baseVersionId, + dispatchSpy, + fakeThunk, headVersionId, - path: version.file.selected_file, + root, + store, + } = loadDiffAndRender(); + + const path = 'some-new-path'; + const history = createFakeHistory({ + location: createFakeLocation({ + search: queryString.stringify({ path }), + }), }); + const routerProps = { + ...createFakeRouteComponentProps({ + history, + params: getRouteParams({ + addonId, + baseVersionId, + headVersionId, + }), + }), + }; + + root.setProps({ + ...routerProps, + ...mapStateToProps(store.getState(), { + ...root.instance().props, + ...routerProps, + }), + }); + + expect(dispatchSpy).toHaveBeenCalledWith(fakeThunk.thunk); + expect(_fetchDiff).toHaveBeenCalledWith(expect.objectContaining({ path })); }); it('dispatches viewVersionFile() when a file is selected', () => { @@ -484,42 +564,6 @@ describe(__filename, () => { expect(dispatchSpy).not.toHaveBeenCalled(); }); - it('dispatches viewVersionFile() on mount if the query string contains a `path` that is different than `version.selectedPath`', () => { - const path = 'a/different/file.js'; - const history = createFakeHistory({ - location: createFakeLocation({ - search: queryString.stringify({ path }), - }), - }); - const { - _viewVersionFile, - dispatchSpy, - fakeThunk, - version, - } = loadDiffAndRender({ history }); - - expect(dispatchSpy).toHaveBeenCalledWith(fakeThunk.thunk); - expect(dispatchSpy).toHaveBeenCalledTimes(1); - expect(_viewVersionFile).toHaveBeenCalledWith({ - versionId: version.id, - selectedPath: path, - preserveHash: true, - }); - }); - - it('does not dispatch viewVersionFile() on mount when `path` is equal to the selected path', () => { - const version = fakeVersionWithDiff; - const history = createFakeHistory({ - location: createFakeLocation({ - search: queryString.stringify({ path: version.file.selected_file }), - }), - }); - - const { dispatchSpy } = loadDiffAndRender({ history }); - - expect(dispatchSpy).not.toHaveBeenCalled(); - }); - it('dispatches fetchDiff() with the path specified in the URL on mount', () => { const addonId = 9999; const baseVersionId = 1; @@ -552,45 +596,34 @@ describe(__filename, () => { }); }); - // This could happen when a keyboard navigation updates the selected path. - it('does not dispatch viewVersionFile() on update if the query string contains a `path` that is different than `version.selectedPath`', () => { - const { dispatchSpy, root } = loadDiffAndRender(); + it('does not dispatch anything on mount when an API error has occured', () => { + const addonId = 9999; + const baseVersionId = 1; + const headVersionId = baseVersionId + 1; - const path = 'a/different/file.js'; - const history = createFakeHistory({ - location: createFakeLocation({ - search: queryString.stringify({ path }), + const store = configureStore(); + store.dispatch( + versionsActions.beginFetchDiff({ + addonId, + baseVersionId, + headVersionId, }), - }); - - root.setProps({ history }); - - expect(dispatchSpy).not.toHaveBeenCalled(); - }); - - it('does not dispatch viewVersionFile() on update when `path` is equal to the selected path', () => { - const { dispatchSpy, root, version } = loadDiffAndRender(); - - const history = createFakeHistory({ - location: createFakeLocation({ - search: queryString.stringify({ path: version.file.selected_file }), + ); + // Simulate an API error. + store.dispatch( + versionsActions.abortFetchDiff({ + addonId, + baseVersionId, + headVersionId, }), - }); - - root.setProps({ history }); - - expect(dispatchSpy).not.toHaveBeenCalled(); - }); - - it('does not dispatch viewVersionFile() on update when the query string does not contain a `path`', () => { - const { dispatchSpy, root } = loadDiffAndRender(); + ); + const dispatch = spyOn(store, 'dispatch'); - const history = createFakeHistory({ - location: createFakeLocation({ search: '' }), + render({ + ...getRouteParams({ addonId, baseVersionId, headVersionId }), + store, }); - root.setProps({ history }); - - expect(dispatchSpy).not.toHaveBeenCalled(); + expect(dispatch).not.toHaveBeenCalled(); }); }); diff --git a/src/pages/Compare/index.tsx b/src/pages/Compare/index.tsx index b3ca8f1f7..66c103dd4 100644 --- a/src/pages/Compare/index.tsx +++ b/src/pages/Compare/index.tsx @@ -13,6 +13,7 @@ import { CompareInfo, Version, fetchDiff, + getCompareInfo, getVersionInfo, viewVersionFile, } from '../../reducers/versions'; @@ -34,7 +35,7 @@ type PropsFromRouter = { type PropsFromState = { addonId: number; compareInfo: CompareInfo | null | void; - isLoading: boolean; + path: string | undefined; version: Version | void | null; }; @@ -53,12 +54,19 @@ export class CompareBase extends React.Component { this.loadData(); } - componentDidUpdate(prevProps: Props) { - this.loadData(prevProps); + componentDidUpdate() { + this.loadData(); } - loadData(prevProps?: Props) { - const { _fetchDiff, dispatch, history, match, version } = this.props; + loadData() { + const { + _fetchDiff, + compareInfo, + dispatch, + history, + match, + path, + } = this.props; const { addonId, baseVersionId, headVersionId, lang } = match.params; const oldVersionId = parseInt(baseVersionId, 10); @@ -73,50 +81,24 @@ export class CompareBase extends React.Component { return; } - const path = getPathFromQueryString(history); - - if (!version) { - dispatch( - _fetchDiff({ - addonId: parseInt(addonId, 10), - baseVersionId: oldVersionId, - headVersionId: newVersionId, - path: path || undefined, - }), - ); + if (compareInfo === null) { + // An error has occured when fetching the compare info. return; } - // We do not want to update the selected path again (e.g., when a keyboard - // navigation updates the selected path), so we apply this logic to the - // first render (mount) only. - if (!prevProps) { - if (path && path !== version.selectedPath) { - // We preserve the hash in the URL (if any) when we load the file from - // an URL that has likely be shared. - this.viewVersionFile(path, { preserveHash: true }); - } - } else if ( - (prevProps.version && - version.selectedPath !== prevProps.version.selectedPath) || - addonId !== prevProps.match.params.addonId || - baseVersionId !== prevProps.match.params.baseVersionId || - headVersionId !== prevProps.match.params.headVersionId - ) { + if (compareInfo === undefined) { dispatch( _fetchDiff({ addonId: parseInt(addonId, 10), baseVersionId: oldVersionId, headVersionId: newVersionId, - path: version.selectedPath, + path: path || undefined, }), ); } } - // When selecting a new file to view, we do not want to preserve the hash in - // the URL (this hash highlights a specific line of code). - viewVersionFile = (path: string, { preserveHash = false } = {}) => { + viewVersionFile = (path: string) => { const { _viewVersionFile, dispatch, match } = this.props; const { headVersionId } = match.params; @@ -124,15 +106,17 @@ export class CompareBase extends React.Component { _viewVersionFile({ selectedPath: path, versionId: parseInt(headVersionId, 10), - preserveHash, + // When selecting a new file to view, we do not want to preserve the + // hash in the URL (this hash highlights a specific line of code). + preserveHash: false, }), ); }; renderLoadingMessageOrError(message: string) { - const { isLoading } = this.props; + const { compareInfo } = this.props; - if (!isLoading) { + if (compareInfo === null) { return (

{gettext('Ooops, an error has occured.')} @@ -171,16 +155,23 @@ export class CompareBase extends React.Component { } } -const mapStateToProps = ( +export const mapStateToProps = ( state: ApplicationState, ownProps: RouteComponentProps, ): PropsFromState => { - const { match } = ownProps; + const { history, match } = ownProps; const addonId = parseInt(match.params.addonId, 10); + const baseVersionId = parseInt(match.params.baseVersionId, 10); const headVersionId = parseInt(match.params.headVersionId, 10); + const path = getPathFromQueryString(history) || undefined; - const { compareInfo } = state.versions; - const isLoading = compareInfo === undefined; + const compareInfo = getCompareInfo( + state.versions, + addonId, + baseVersionId, + headVersionId, + path, + ); // The Compare API returns the version info of the head/newest version. const version = getVersionInfo(state.versions, headVersionId); @@ -188,7 +179,7 @@ const mapStateToProps = ( return { addonId, compareInfo, - isLoading, + path, version, }; }; diff --git a/src/reducers/versions.spec.tsx b/src/reducers/versions.spec.tsx index 7f3e6a9d8..c13b5c6cf 100644 --- a/src/reducers/versions.spec.tsx +++ b/src/reducers/versions.spec.tsx @@ -19,6 +19,8 @@ import reducer, { fetchVersion, fetchVersionFile, fetchVersionsList, + getCompareInfo, + getCompareInfoKey, getDiffAnchors, getParentFolders, getRelativeDiffAnchor, @@ -26,6 +28,7 @@ import reducer, { getVersionFiles, getVersionInfo, initialState, + isCompareInfoLoading, isFileLoading, viewVersionFile, } from './versions'; @@ -383,17 +386,66 @@ describe(__filename, () => { expect(state.byAddonId[addonId]).toEqual(createVersionsMap(versions)); }); - it('sets the compare info to `null` on abortFetchDiff()', () => { - const versionsState = reducer(undefined, actions.abortFetchDiff()); + it('sets the compare info to `null` and the loading flag to `false` on abortFetchDiff()', () => { + const addonId = 1; + const baseVersionId = 2; + const headVersionId = 2; + + let versionsState = reducer( + undefined, + actions.beginFetchDiff({ + addonId, + baseVersionId, + headVersionId, + }), + ); + versionsState = reducer( + versionsState, + actions.abortFetchDiff({ + addonId, + baseVersionId, + headVersionId, + }), + ); - expect(versionsState.compareInfo).toEqual(null); + expect( + getCompareInfo(versionsState, addonId, baseVersionId, headVersionId), + ).toEqual(null); + expect( + isCompareInfoLoading( + versionsState, + addonId, + baseVersionId, + headVersionId, + ), + ).toEqual(false); }); - it('resets the compare info on beginFetchDiff()', () => { - let versionsState = reducer(undefined, actions.abortFetchDiff()); - versionsState = reducer(versionsState, actions.beginFetchDiff()); + it('resets the compare info and sets the loading flag to `true` on beginFetchDiff()', () => { + const addonId = 1; + const baseVersionId = 2; + const headVersionId = 2; + + let versionsState = reducer( + undefined, + actions.abortFetchDiff({ addonId, baseVersionId, headVersionId }), + ); + versionsState = reducer( + versionsState, + actions.beginFetchDiff({ addonId, baseVersionId, headVersionId }), + ); - expect(versionsState.compareInfo).toEqual(undefined); + expect( + getCompareInfo(versionsState, addonId, baseVersionId, headVersionId), + ).toEqual(undefined); + expect( + isCompareInfoLoading( + versionsState, + addonId, + baseVersionId, + headVersionId, + ), + ).toEqual(true); }); it('loads compare info', () => { @@ -433,7 +485,9 @@ describe(__filename, () => { }), ); - expect(versionsState.compareInfo).toEqual({ + expect( + getCompareInfo(versionsState, addonId, baseVersionId, headVersionId), + ).toEqual({ diffs: createInternalDiffs({ baseVersionId, headVersionId, @@ -441,6 +495,14 @@ describe(__filename, () => { }), mimeType, }); + expect( + isCompareInfoLoading( + versionsState, + addonId, + baseVersionId, + headVersionId, + ), + ).toEqual(false); }); it('throws an error when entry is missing on loadDiff()', () => { @@ -1389,10 +1451,24 @@ describe(__filename, () => { }; it('dispatches beginFetchDiff()', async () => { - const { dispatch, thunk } = _fetchDiff(); + const addonId = 1; + const baseVersionId = 2; + const headVersionId = 3; + + const { dispatch, thunk } = _fetchDiff({ + addonId, + baseVersionId, + headVersionId, + }); await thunk(); - expect(dispatch).toHaveBeenCalledWith(actions.beginFetchDiff()); + expect(dispatch).toHaveBeenCalledWith( + actions.beginFetchDiff({ + addonId, + baseVersionId, + headVersionId, + }), + ); }); it('calls getDiff()', async () => { @@ -1449,7 +1525,12 @@ describe(__filename, () => { const baseVersionId = 2; const headVersionId = version.id; - const { dispatch, thunk } = _fetchDiff({ version }); + const { dispatch, thunk } = _fetchDiff({ + addonId, + baseVersionId, + headVersionId, + version, + }); await thunk(); expect(dispatch).toHaveBeenCalledWith( @@ -1463,16 +1544,50 @@ describe(__filename, () => { }); it('dispatches abortFetchDiff() when API call has failed', async () => { + const addonId = 1; + const baseVersionId = 2; + const headVersionId = 2; const _getDiff = jest.fn().mockReturnValue( Promise.resolve({ error: new Error('Bad Request'), }), ); - const { dispatch, thunk } = _fetchDiff({ _getDiff }); + const { dispatch, thunk } = _fetchDiff({ + _getDiff, + addonId, + baseVersionId, + headVersionId, + }); + await thunk(); + + expect(dispatch).toHaveBeenCalledWith( + actions.abortFetchDiff({ + addonId, + baseVersionId, + headVersionId, + }), + ); + }); + + it('prevents itself to execute more than once for the same diff', async () => { + const addonId = 1; + const baseVersionId = 2; + const headVersionId = 3; + + const { dispatch, thunk, store } = _fetchDiff({ + addonId, + baseVersionId, + headVersionId, + }); + // This simulates another previous call to `fetchDiff()`. + store.dispatch( + actions.beginFetchDiff({ addonId, baseVersionId, headVersionId }), + ); + await thunk(); - expect(dispatch).toHaveBeenCalledWith(actions.abortFetchDiff()); + expect(dispatch).not.toHaveBeenCalled(); }); }); @@ -2034,4 +2149,63 @@ describe(__filename, () => { ).toEqual(null); }); }); + + describe('getCompareInfoKey', () => { + it('computes a key given an addonId, baseVersionId and headVersionId', () => { + const addonId = 123; + const baseVersionId = 1; + const headVersionId = 2; + + expect( + getCompareInfoKey({ addonId, baseVersionId, headVersionId }), + ).toEqual(`${addonId}/${baseVersionId}/${headVersionId}/`); + }); + + it('computes a key given an addonId, baseVersionId, headVersionId and path', () => { + const addonId = 123; + const baseVersionId = 1; + const headVersionId = 2; + const path = 'path'; + + expect( + getCompareInfoKey({ addonId, baseVersionId, headVersionId, path }), + ).toEqual(`${addonId}/${baseVersionId}/${headVersionId}/${path}`); + }); + }); + + describe('isCompareInfoLoading', () => { + it('returns false by default', () => { + const addonId = 123; + const baseVersionId = 1; + const headVersionId = 2; + + expect( + isCompareInfoLoading( + // Nothing has been loaded in this state. + initialState, + addonId, + baseVersionId, + headVersionId, + ), + ).toEqual(false); + }); + + it('returns true when loading compare info', () => { + const addonId = 123; + const baseVersionId = 1; + const headVersionId = 2; + const state = reducer( + undefined, + actions.beginFetchDiff({ + addonId, + baseVersionId, + headVersionId, + }), + ); + + expect( + isCompareInfoLoading(state, addonId, baseVersionId, headVersionId), + ).toEqual(true); + }); + }); }); diff --git a/src/reducers/versions.tsx b/src/reducers/versions.tsx index 6a4fe8ced..770ea9c82 100644 --- a/src/reducers/versions.tsx +++ b/src/reducers/versions.tsx @@ -235,11 +235,25 @@ export const actions = { return (payload: { addonId: number; versions: ExternalVersionsList }) => resolve(payload); }), - beginFetchDiff: createAction('BEGIN_FETCH_DIFF'), + beginFetchDiff: createAction('BEGIN_FETCH_DIFF', (resolve) => { + return (payload: { + addonId: number; + baseVersionId: number; + headVersionId: number; + path?: string; + }) => resolve(payload); + }), beginFetchVersionFile: createAction('BEGIN_FETCH_VERSION_FILE', (resolve) => { return (payload: { path: string; versionId: number }) => resolve(payload); }), - abortFetchDiff: createAction('ABORT_FETCH_DIFF'), + abortFetchDiff: createAction('ABORT_FETCH_DIFF', (resolve) => { + return (payload: { + addonId: number; + baseVersionId: number; + headVersionId: number; + path?: string; + }) => resolve(payload); + }), abortFetchVersionFile: createAction('ABORT_FETCH_VERSION_FILE', (resolve) => { return (payload: { path: string; versionId: number }) => resolve(payload); }), @@ -264,10 +278,15 @@ export type VersionsState = { byAddonId: { [addonId: number]: VersionsMap; }; - compareInfo: - | CompareInfo // data successfully loaded - | null // an error has occured - | undefined; // data not fetched yet + compareInfo: { + [compareInfoKey: string]: + | CompareInfo // data successfully loaded + | null // an error has occured + | undefined; // data not fetched yet + }; + compareInfoIsLoading: { + [compareInfoKey: string]: boolean; + }; versionInfo: { [versionId: number]: | Version // data successfully loaded @@ -291,7 +310,8 @@ export type VersionsState = { export const initialState: VersionsState = { byAddonId: {}, - compareInfo: undefined, + compareInfo: {}, + compareInfoIsLoading: {}, versionFiles: {}, versionFilesLoading: {}, versionInfo: {}, @@ -663,6 +683,56 @@ type FetchDiffParams = { path?: string; }; +type GetCompareInfoKeyParams = { + addonId: number; + baseVersionId: number; + headVersionId: number; + path?: string; +}; + +export const getCompareInfoKey = ({ + addonId, + baseVersionId, + headVersionId, + path, +}: GetCompareInfoKeyParams) => { + return [addonId, baseVersionId, headVersionId, path].join('/'); +}; + +export const getCompareInfo = ( + versions: VersionsState, + addonId: number, + baseVersionId: number, + headVersionId: number, + path?: string, +) => { + const compareInfoKey = getCompareInfoKey({ + addonId, + baseVersionId, + headVersionId, + path, + }); + + return versions.compareInfo[compareInfoKey]; +}; + +export const isCompareInfoLoading = ( + versions: VersionsState, + addonId: number, + baseVersionId: number, + headVersionId: number, + path?: string, +) => { + const compareInfoKey = getCompareInfoKey({ + addonId, + baseVersionId, + headVersionId, + path, + }); + + return versions.compareInfoIsLoading[compareInfoKey] || false; +}; + export const fetchDiff = ({ _getDiff = getDiff, addonId, @@ -672,8 +742,22 @@ export const fetchDiff = ({ }: FetchDiffParams): ThunkActionCreator => { return async (dispatch, getState) => { const { api: apiState, versions: versionsState } = getState(); + if ( + isCompareInfoLoading( + versionsState, + addonId, + baseVersionId, + headVersionId, + path, + ) + ) { + log.debug('Aborting because the diff is already being fetched'); + return; + } - dispatch(actions.beginFetchDiff()); + dispatch( + actions.beginFetchDiff({ addonId, baseVersionId, headVersionId, path }), + ); const response = await _getDiff({ addonId, @@ -684,8 +768,10 @@ export const fetchDiff = ({ }); if (isErrorResponse(response)) { + dispatch( + actions.abortFetchDiff({ addonId, baseVersionId, headVersionId, path }), + ); dispatch(errorsActions.addError({ error: response.error })); - dispatch(actions.abortFetchDiff()); } else { if (!getVersionInfo(versionsState, response.id)) { dispatch(actions.loadVersionInfo({ version: response })); @@ -939,19 +1025,50 @@ const reducer: Reducer> = ( }; } case getType(actions.beginFetchDiff): { + const key = getCompareInfoKey(action.payload); + return { ...state, - compareInfo: undefined, + compareInfo: { + ...state.compareInfo, + [key]: undefined, + }, + compareInfoIsLoading: { + ...state.compareInfoIsLoading, + [key]: true, + }, }; } case getType(actions.abortFetchDiff): { + const key = getCompareInfoKey(action.payload); + return { ...state, - compareInfo: null, + compareInfo: { + ...state.compareInfo, + [key]: null, + }, + compareInfoIsLoading: { + ...state.compareInfoIsLoading, + [key]: false, + }, }; } case getType(actions.loadDiff): { - const { baseVersionId, headVersionId, version } = action.payload; + const { + addonId, + baseVersionId, + headVersionId, + path, + version, + } = action.payload; + + const compareInfoKey = getCompareInfoKey({ + addonId, + baseVersionId, + headVersionId, + path, + }); const headVersion = getVersionInfo(state, headVersionId); if (!headVersion) { @@ -968,12 +1085,19 @@ const reducer: Reducer> = ( return { ...state, compareInfo: { - diffs: createInternalDiffs({ - baseVersionId, - headVersionId, - version, - }), - mimeType: entry.mimeType, + ...state.compareInfo, + [compareInfoKey]: { + diffs: createInternalDiffs({ + baseVersionId, + headVersionId, + version, + }), + mimeType: entry.mimeType, + }, + }, + compareInfoIsLoading: { + ...state.compareInfoIsLoading, + [compareInfoKey]: false, }, }; }