Select versions in VersionChooser

src/components/VersionChooser/index.spec.tsx

@@ -1,5 +1,6 @@
 import * as React from 'react';
 import { Store } from 'redux';
+import { History } from 'history';
 
 import configureStore from '../../configureStore';
 import VersionSelect from '../VersionSelect';
@@ -9,33 +10,66 @@ import {
   actions as versionActions,
 } from '../../reducers/versions';
 import {
+  createContextWithFakeRouter,
+  createFakeHistory,
   createFakeThunk,
   fakeVersionsList,
   fakeVersionsListItem,
   shallowUntilTarget,
   spyOn,
 } from '../../test-helpers';
+import styles from './styles.module.scss';
 
-import VersionChooser, { VersionChooserBase, PublicProps } from '.';
+import VersionChooser, {
+  DefaultProps,
+  PropsFromRouter,
+  PublicProps,
+  VersionChooserBase,
+} from '.';
 
 describe(__filename, () => {
-  type RenderParams = Partial<PublicProps> & { store?: Store };
+  const lang = 'en-US';
+
+  type RenderParams = Partial<PublicProps> &
+    Partial<PropsFromRouter> &
+    Partial<DefaultProps> & {
+      store?: Store;
+      history?: History;
+    };
 
   const render = ({
     _fetchVersionsList,
-    addonId = 123,
+    addonId = '123',
+    baseVersionId = '1',
+    headVersionId = '2',
+    history = createFakeHistory(),
     store = configureStore(),
   }: RenderParams = {}) => {
-    const props = { addonId, _fetchVersionsList };
+    const contextWithRouter = createContextWithFakeRouter({
+      history,
+      match: {
+        params: {
+          addonId,
+          baseVersionId,
+          headVersionId,
+          lang,
+        },
+      },
+    });
+    const context = {
+      ...contextWithRouter,
+      context: {
+        ...contextWithRouter.context,
+        store,
+      },
+    };
+
+    const props = { addonId: parseInt(addonId, 10), _fetchVersionsList };
 
     return shallowUntilTarget(
       <VersionChooser {...props} />,
       VersionChooserBase,
-      {
-        shallowOptions: {
-          context: { store },
-        },
-      },
+      { shallowOptions: { ...context } },
     );
   };
 
@@ -59,7 +93,7 @@ describe(__filename, () => {
     const store = configureStore();
     _loadVersionsList(store, addonId, fakeVersionsList);
 
-    const root = render({ addonId, store });
+    const root = render({ addonId: String(addonId), store });
 
     expect(root.find(VersionSelect)).toHaveLength(2);
     expect(root.find(VersionSelect).at(0)).toHaveProp(
@@ -93,7 +127,7 @@ describe(__filename, () => {
     const store = configureStore();
     _loadVersionsList(store, addonId, [...listedVersions, ...unlistedVersions]);
 
-    const root = render({ addonId, store });
+    const root = render({ addonId: String(addonId), store });
 
     root.find(VersionSelect).forEach((versionSelect) => {
       expect(versionSelect).toHaveProp('listedVersions', listedVersions);
@@ -108,7 +142,7 @@ describe(__filename, () => {
     const fakeThunk = createFakeThunk();
     const _fetchVersionsList = fakeThunk.createThunk;
 
-    render({ _fetchVersionsList, addonId, store });
+    render({ _fetchVersionsList, addonId: String(addonId), store });
 
     expect(dispatch).toHaveBeenCalledWith(fakeThunk.thunk);
     expect(_fetchVersionsList).toHaveBeenCalledWith({ addonId });
@@ -122,7 +156,11 @@ describe(__filename, () => {
     const dispatch = spyOn(store, 'dispatch');
     const fakeThunk = createFakeThunk();
 
-    render({ _fetchVersionsList: fakeThunk.createThunk, addonId, store });
+    render({
+      _fetchVersionsList: fakeThunk.createThunk,
+      addonId: String(addonId),
+      store,
+    });
 
     expect(dispatch).not.toHaveBeenCalled();
   });
@@ -138,9 +176,97 @@ describe(__filename, () => {
 
     const secondAddonId = addonId + 123;
 
-    render({ _fetchVersionsList, addonId: secondAddonId, store });
+    render({ _fetchVersionsList, addonId: String(secondAddonId), store });
 
     expect(dispatch).toHaveBeenCalledWith(fakeThunk.thunk);
     expect(_fetchVersionsList).toHaveBeenCalledWith({ addonId: secondAddonId });
   });
+
+  it('pushes a new URL when the old version changes', () => {
+    const addonId = '999';
+    const baseVersionId = '3';
+    const headVersionId = '4';
+
+    const store = configureStore();
+    _loadVersionsList(store, parseInt(addonId, 10), fakeVersionsList);
+
+    const history = createFakeHistory();
+    const selectedVersion = '2';
+
+    const root = render({
+      addonId,
+      baseVersionId,
+      headVersionId,
+      history,
+      store,
+    });
+
+    const onChange = root
+      // Retrieve the `VersionSelect` component with this `className`.
+      .find({ className: styles.baseVersionSelect })
+      .prop('onChange');
+    onChange(selectedVersion);
+
+    expect(history.push).toHaveBeenCalledWith(
+      `/${lang}/compare/${addonId}/versions/${selectedVersion}...${headVersionId}/`,
+    );
+  });
+
+  it('pushes a new URL when the new version changes', () => {
+    const addonId = '999';
+    const baseVersionId = '3';
+    const headVersionId = '4';
+
+    const store = configureStore();
+    _loadVersionsList(store, parseInt(addonId, 10), fakeVersionsList);
+
+    const history = createFakeHistory();
+    const selectedVersion = '5';
+
+    const root = render({
+      addonId,
+      baseVersionId,
+      headVersionId,
+      history,
+      store,
+    });
+
+    const onChange = root
+      .find({ className: styles.headVersionSelect })
+      .prop('onChange');
+    onChange(selectedVersion);
+
+    expect(history.push).toHaveBeenCalledWith(
+      `/${lang}/compare/${addonId}/versions/${baseVersionId}...${selectedVersion}/`,
+    );
+  });
+
+  it('ensures the order of the versions is respected (old < new version)', () => {
+    const addonId = '999';
+    const baseVersionId = '4';
+    const headVersionId = '5';
+
+    const store = configureStore();
+    _loadVersionsList(store, parseInt(addonId, 10), fakeVersionsList);
+
+    const history = createFakeHistory();
+    const selectedVersion = '3'; // The value is lower than `baseVersionId`
+
+    const root = render({
+      addonId,
+      baseVersionId,
+      headVersionId,
+      history,
+      store,
+    });
+
+    const onChange = root
+      .find({ className: styles.headVersionSelect })
+      .prop('onChange');
+    onChange(selectedVersion);
+
+    expect(history.push).toHaveBeenCalledWith(
+      `/${lang}/compare/${addonId}/versions/${selectedVersion}...${baseVersionId}/`,
+    );
+  });
 });

src/components/VersionChooser/index.tsx

@@ -1,6 +1,7 @@
 import * as React from 'react';
 import { Col, Form, Row } from 'react-bootstrap';
 import { connect } from 'react-redux';
+import { withRouter, RouteComponentProps } from 'react-router-dom';
 
 import Loading from '../Loading';
 import { ApplicationState, ConnectedReduxProps } from '../../configureStore';
@@ -9,32 +10,85 @@ import { VersionsMap, fetchVersionsList } from '../../reducers/versions';
 import { gettext } from '../../utils';
 import styles from './styles.module.scss';
 
-export type PublicProps = {
+export type PublicProps = {};
+
+export type DefaultProps = {
   _fetchVersionsList: typeof fetchVersionsList;
-  addonId: number;
 };
 
 type PropsFromState = {
   versionsMap: VersionsMap;
 };
 
-type Props = PropsFromState & PublicProps & ConnectedReduxProps;
+export type PropsFromRouter = {
+  addonId: string;
+  baseVersionId: string;
+  headVersionId: string;
+  lang: string;
+};
+
+type RouterProps = RouteComponentProps<PropsFromRouter>;
+
+type Props = PublicProps &
+  ConnectedReduxProps &
+  DefaultProps &
+  PropsFromState &
+  RouterProps;
 
 export class VersionChooserBase extends React.Component<Props> {
-  static defaultProps = {
+  static defaultProps: DefaultProps = {
     _fetchVersionsList: fetchVersionsList,
   };
 
   componentDidMount() {
-    const { _fetchVersionsList, addonId, dispatch, versionsMap } = this.props;
+    const { _fetchVersionsList, dispatch, match, versionsMap } = this.props;
+    const { addonId } = match.params;
 
     if (!versionsMap) {
-      dispatch(_fetchVersionsList({ addonId }));
+      dispatch(_fetchVersionsList({ addonId: parseInt(addonId, 10) }));
     }
   }
 
+  onVersionChange = ({
+    baseVersionId,
+    headVersionId,
+  }: {
+    baseVersionId: string;
+    headVersionId: string;
+  }) => {
+    const { history, match } = this.props;
+    const { addonId, lang } = match.params;
+
+    const oldVersionId = parseInt(baseVersionId, 10);
+    const newVersionId = parseInt(headVersionId, 10);
+
+    // We make sure old version is older than the new version when user changes
+    // any of the two versions.
+    if (oldVersionId > newVersionId) {
+      history.push(
+        `/${lang}/compare/${addonId}/versions/${headVersionId}...${baseVersionId}/`,
+      );
+      return;
+    }
+
+    history.push(
+      `/${lang}/compare/${addonId}/versions/${baseVersionId}...${headVersionId}/`,
+    );
+  };
+
+  onNewVersionChange = (versionId: string) => {
+    const { baseVersionId } = this.props.match.params;
+    this.onVersionChange({ baseVersionId, headVersionId: versionId });
+  };
+
+  onOldVersionChange = (versionId: string) => {
+    const { headVersionId } = this.props.match.params;
+    this.onVersionChange({ baseVersionId: versionId, headVersionId });
+  };
+
   render() {
-    const { versionsMap } = this.props;
+    const { match, versionsMap } = this.props;
+    const { baseVersionId, headVersionId } = match.params;
 
     return (
       <div className={styles.VersionChooser}>
@@ -48,15 +102,21 @@ export class VersionChooserBase extends React.Component<Props> {
           {versionsMap ? (
             <Form.Row>
               <VersionSelect
+                className={styles.baseVersionSelect}
                 label={gettext('Choose an old version')}
                 listedVersions={versionsMap.listed}
+                onChange={this.onOldVersionChange}
                 unlistedVersions={versionsMap.unlisted}
+                value={baseVersionId}
               />
 
               <VersionSelect
+                className={styles.headVersionSelect}
                 label={gettext('Choose a new version')}
                 listedVersions={versionsMap.listed}
+                onChange={this.onNewVersionChange}
                 unlistedVersions={versionsMap.unlisted}
+                value={headVersionId}
                 withLeftArrow
               />
             </Form.Row>
@@ -73,13 +133,16 @@ export class VersionChooserBase extends React.Component<Props> {
 
 const mapStateToProps = (
   state: ApplicationState,
-  ownProps: PublicProps,
+  ownProps: RouterProps,
 ): PropsFromState => {
-  const versionsMap = state.versions.byAddonId[ownProps.addonId];
+  const { byAddonId } = state.versions;
+  const { addonId } = ownProps.match.params;
 
   return {
-    versionsMap,
+    versionsMap: byAddonId[parseInt(addonId, 10)],
   };
 };
 
-export default connect(mapStateToProps)(VersionChooserBase);
+export default withRouter<PublicProps & Partial<DefaultProps> & RouterProps>(
+  connect(mapStateToProps)(VersionChooserBase),
+);