Refactoring to support changed feature ids (#282)

src/ensembl/src/content/app/browser/Browser.test.tsx

@@ -28,6 +28,9 @@ import BrowserNavBar from './browser-nav/BrowserNavBar';
 
 import { createChrLocationValues } from 'tests/fixtures/browser';
 
+jest.mock('./hooks/useBrowserRouting', () => () => ({
+  changeGenomeId: jest.fn()
+}));
 jest.mock('./browser-bar/BrowserBar', () => () => <div>BrowserBar</div>);
 jest.mock('./browser-image/BrowserImage', () => () => <div>BrowserImage</div>);
 jest.mock('./browser-nav/BrowserNavBar', () => () => <div>BrowserNavBar</div>);
@@ -54,12 +57,6 @@ describe('<Browser />', () => {
   const defaultProps: BrowserProps = {
     activeGenomeId: faker.lorem.words(),
     activeEnsObjectId: faker.lorem.words(),
-    allActiveEnsObjectIds: {
-      [faker.lorem.words()]: faker.lorem.words()
-    },
-    allChrLocations: {
-      [faker.lorem.words()]: createChrLocationValues().tupleValue
-    },
     browserActivated: false,
     browserNavOpened: false,
     browserQueryParams: {},
@@ -67,15 +64,11 @@ describe('<Browser />', () => {
     isDrawerOpened: false,
     isTrackPanelOpened: false,
     exampleEnsObjects: [],
-    committedSpecies: [],
     changeBrowserLocation: jest.fn(),
-    changeFocusObject: jest.fn(),
     restoreBrowserTrackStates: jest.fn(),
     fetchGenomeData: jest.fn(),
-    replace: jest.fn(),
     toggleTrackPanel: jest.fn(),
     toggleDrawer: jest.fn(),
-    setDataFromUrlAndSave: jest.fn(),
     viewportWidth: BreakpointWidth.DESKTOP
   };
 

src/ensembl/src/content/app/browser/Browser.tsx

@@ -14,26 +14,28 @@
  * limitations under the License.
  */
 
-import React, { useEffect, useState, useCallback } from 'react';
-import { useLocation, useParams } from 'react-router-dom';
+import React, { useEffect, useState } from 'react';
+import { useParams } from 'react-router-dom';
 import { connect } from 'react-redux';
-import { replace, Replace } from 'connected-react-router';
 import { Link } from 'react-router-dom';
-import find from 'lodash/find';
-import isEqual from 'lodash/isEqual';
 import upperFirst from 'lodash/upperFirst';
 
+import useBrowserRouting from './hooks/useBrowserRouting';
+
 import analyticsTracking from 'src/services/analytics-service';
 import browserStorageService from './browser-storage-service';
 import * as urlFor from 'src/shared/helpers/urlHelper';
 import { BrowserTrackStates } from './track-panel/trackPanelConfig';
 import { BreakpointWidth } from 'src/global/globalConfig';
 
+import {
+  parseEnsObjectId,
+  buildFocusIdForUrl
+} from 'src/shared/state/ens-object/ensObjectHelpers';
+import { getChrLocationFromStr } from './browserHelper';
+
 import {
   changeBrowserLocation,
-  changeFocusObject,
-  setDataFromUrlAndSave,
-  ParsedUrlPayload,
   restoreBrowserTrackStates
 } from './browserActions';
 import { fetchGenomeData } from 'src/shared/state/genome/genomeActions';
@@ -47,13 +49,10 @@ import {
   getBrowserActiveGenomeId,
   getBrowserQueryParams,
   getBrowserActiveEnsObjectId,
-  getBrowserActiveEnsObjectIds,
-  getAllChrLocations
+  getBrowserActiveEnsObjectIds
 } from './browserSelectors';
 import { getIsTrackPanelOpened } from './track-panel/trackPanelSelectors';
-import { getChrLocationFromStr, getChrLocationStr } from './browserHelper';
 import { getIsDrawerOpened } from './drawer/drawerSelectors';
-import { getEnabledCommittedSpecies } from 'src/content/app/species-selector/state/speciesSelectorSelectors';
 import { getExampleEnsObjects } from 'src/shared/state/ens-object/ensObjectSelectors';
 import { getBreakpointWidth } from 'src/global/globalSelectors';
 
@@ -68,8 +67,7 @@ import Drawer from './drawer/Drawer';
 import { StandardAppLayout } from 'src/shared/components/layout';
 
 import { RootState } from 'src/store';
-import { ChrLocation, ChrLocations } from './browserState';
-import { CommittedItem } from 'src/content/app/species-selector/types/species-search';
+import { ChrLocation } from './browserState';
 import { EnsObject } from 'src/shared/state/ens-object/ensObjectTypes';
 
 import 'ensembl-genome-browser';
@@ -79,8 +77,6 @@ import styles from './Browser.scss';
 export type BrowserProps = {
   activeGenomeId: string | null;
   activeEnsObjectId: string | null;
-  allActiveEnsObjectIds: { [genomeId: string]: string };
-  allChrLocations: ChrLocations;
   browserActivated: boolean;
   browserNavOpened: boolean;
   browserQueryParams: { [key: string]: string };
@@ -88,116 +84,24 @@ export type BrowserProps = {
   isDrawerOpened: boolean;
   isTrackPanelOpened: boolean;
   exampleEnsObjects: EnsObject[];
-  committedSpecies: CommittedItem[];
   viewportWidth: BreakpointWidth;
   changeBrowserLocation: (locationData: {
     genomeId: string;
     ensObjectId: string | null;
     chrLocation: ChrLocation;
   }) => void;
-  changeFocusObject: (objectId: string) => void;
   restoreBrowserTrackStates: () => void;
   fetchGenomeData: (genomeId: string) => void;
-  replace: Replace;
   toggleTrackPanel: (isOpen: boolean) => void;
   toggleDrawer: (isDrawerOpened: boolean) => void;
-  setDataFromUrlAndSave: (payload: ParsedUrlPayload) => void;
 };
 
 export const Browser = (props: BrowserProps) => {
   const [, setTrackStatesFromStorage] = useState<BrowserTrackStates>({});
+  const { changeGenomeId } = useBrowserRouting();
 
   const { isDrawerOpened } = props;
   const params: { [key: string]: string } = useParams();
-  const location = useLocation();
-
-  const setDataFromUrl = () => {
-    const { genomeId } = params;
-    const { focus = null, location = null } = props.browserQueryParams;
-    const chrLocation = location ? getChrLocationFromStr(location) : null;
-
-    if (
-      !genomeId ||
-      (genomeId === props.activeGenomeId &&
-        focus === props.activeEnsObjectId &&
-        isEqual(chrLocation, props.chrLocation))
-    ) {
-      return;
-    }
-
-    const payload = {
-      activeGenomeId: genomeId,
-      activeEnsObjectId: focus || null,
-      chrLocation
-    };
-
-    if (focus && !chrLocation) {
-      /*
-       changeFocusObject needs to be called before setDataFromUrlAndSave
-       in order to prevent creating an previouslyViewedObject entry
-       for the focus object that is viewed first.
-       */
-      props.changeFocusObject(focus);
-    } else if (focus && chrLocation) {
-      props.changeFocusObject(focus);
-      props.changeBrowserLocation({
-        genomeId,
-        ensObjectId: focus,
-        chrLocation
-      });
-    } else if (chrLocation) {
-      props.changeBrowserLocation({
-        genomeId,
-        ensObjectId: focus,
-        chrLocation
-      });
-    }
-
-    props.setDataFromUrlAndSave(payload);
-  };
-
-  const changeSelectedSpecies = useCallback(
-    (genomeId: string) => {
-      const { allChrLocations, allActiveEnsObjectIds } = props;
-      const chrLocation = allChrLocations[genomeId];
-      const activeEnsObjectId = allActiveEnsObjectIds[genomeId];
-
-      const params = {
-        genomeId,
-        focus: activeEnsObjectId,
-        location: chrLocation ? getChrLocationStr(chrLocation) : null
-      };
-
-      props.replace(urlFor.browser(params));
-    },
-    [props.allChrLocations, props.allActiveEnsObjectIds]
-  );
-
-  // handle url changes
-  useEffect(() => {
-    // handle navigation to /app/browser
-    if (!params.genomeId) {
-      // select either the species that the user viewed during the previous visit,
-      // of the first selected species
-      const { activeGenomeId, committedSpecies } = props;
-      if (
-        activeGenomeId &&
-        find(
-          committedSpecies,
-          ({ genome_id }: CommittedItem) => genome_id === activeGenomeId
-        )
-      ) {
-        changeSelectedSpecies(activeGenomeId);
-      } else {
-        if (committedSpecies[0]) {
-          changeSelectedSpecies(committedSpecies[0].genome_id);
-        }
-      }
-    } else {
-      // handle navigation to /app/browser/:genomeId?focus=:focus&location=:location
-      setDataFromUrl();
-    }
-  }, [params.genomeId, location.search]);
 
   useEffect(() => {
     const { activeGenomeId, fetchGenomeData } = props;
@@ -249,7 +153,7 @@ export const Browser = (props: BrowserProps) => {
 
   return (
     <div className={styles.browserInnerWrapper}>
-      <BrowserAppBar onSpeciesSelect={changeSelectedSpecies} />
+      <BrowserAppBar onSpeciesSelect={changeGenomeId} />
       {props.browserQueryParams.focus ? (
         <StandardAppLayout
           mainContent={mainContent}
@@ -279,16 +183,18 @@ export const ExampleObjectLinks = (props: BrowserProps) => {
   }
 
   const links = props.exampleEnsObjects.map((exampleObject: EnsObject) => {
+    const parsedEnsObjectId = parseEnsObjectId(exampleObject.object_id);
+    const focusId = buildFocusIdForUrl(parsedEnsObjectId);
     const path = urlFor.browser({
       genomeId: activeGenomeId,
-      focus: exampleObject.object_id
+      focus: focusId
     });
 
     return (
       <div key={exampleObject.object_id} className={styles.exampleLink}>
         <Link to={path}>
           <span className={styles.objectType}>
-            {upperFirst(exampleObject.object_type)}
+            {upperFirst(exampleObject.type)}
           </span>
           <span className={styles.objectLabel}>{exampleObject.label}</span>
         </Link>
@@ -310,28 +216,21 @@ const mapStateToProps = (state: RootState) => {
     activeGenomeId,
     activeEnsObjectId: getBrowserActiveEnsObjectId(state),
     allActiveEnsObjectIds: getBrowserActiveEnsObjectIds(state),
-    allChrLocations: getAllChrLocations(state),
     browserActivated: getBrowserActivated(state),
     browserNavOpened: getBrowserNavOpened(state),
     browserQueryParams: getBrowserQueryParams(state),
     chrLocation: getChrLocation(state),
     isDrawerOpened: getIsDrawerOpened(state),
     isTrackPanelOpened: getIsTrackPanelOpened(state),
-    exampleEnsObjects: activeGenomeId
-      ? getExampleEnsObjects(state, activeGenomeId)
-      : [],
-    committedSpecies: getEnabledCommittedSpecies(state),
+    exampleEnsObjects: getExampleEnsObjects(state),
     viewportWidth: getBreakpointWidth(state)
   };
 };
 
 const mapDispatchToProps = {
   changeBrowserLocation,
-  changeFocusObject,
   fetchGenomeData,
-  replace,
   toggleDrawer,
-  setDataFromUrlAndSave,
   restoreBrowserTrackStates,
   toggleTrackPanel
 };

src/ensembl/src/content/app/browser/browser-image/BrowserImage.tsx

@@ -25,6 +25,8 @@ import { CircleLoader } from 'src/shared/components/loader/Loader';
 import Overlay from 'src/shared/components/overlay/Overlay';
 
 import browserMessagingService from 'src/content/app/browser/browser-messaging-service';
+import { parseFeatureId } from 'src/content/app/browser/browserHelper';
+import { buildEnsObjectId } from 'src/shared/state/ens-object/ensObjectHelpers';
 import {
   getBrowserCogTrackList,
   getBrowserNavOpened,
@@ -106,7 +108,8 @@ export const BrowserImage = (props: BrowserImageProps) => {
     }
 
     if (ensObjectId) {
-      props.updateBrowserActiveEnsObject(ensObjectId);
+      const parsedId = parseFeatureId(ensObjectId);
+      props.updateBrowserActiveEnsObject(buildEnsObjectId(parsedId));
     }
 
     if (messageCount) {

src/ensembl/src/content/app/browser/browser-reset/BrowserReset.test.tsx

@@ -16,19 +16,18 @@
 
 import React from 'react';
 import { mount } from 'enzyme';
+import faker from 'faker';
 
 import { BrowserReset, BrowserResetProps } from './BrowserReset';
 import ImageButton from 'src/shared/components/image-button/ImageButton';
 
-import { createEnsObject } from 'tests/fixtures/ens-object';
-
 describe('<BrowserReset />', () => {
   afterEach(() => {
     jest.resetAllMocks();
   });
 
   const defaultProps: BrowserResetProps = {
-    focusObject: createEnsObject(),
+    focusObjectId: `${faker.lorem.word()}:gene:${faker.lorem.word()}`,
     changeFocusObject: jest.fn(),
     isActive: true
   };
@@ -41,7 +40,7 @@ describe('<BrowserReset />', () => {
 
     test('renders nothing when focus feature does not exist', () => {
       const wrapper = mount(
-        <BrowserReset {...defaultProps} focusObject={null} />
+        <BrowserReset {...defaultProps} focusObjectId={null} />
       );
       expect(wrapper.html()).toBe(null);
     });

src/ensembl/src/content/app/browser/browser-reset/BrowserReset.tsx

@@ -18,7 +18,7 @@ import React, { FunctionComponent } from 'react';
 import { connect } from 'react-redux';
 
 import {
-  getBrowserActiveEnsObject,
+  getBrowserActiveEnsObjectId,
   isFocusObjectPositionDefault
 } from '../browserSelectors';
 import { getIsDrawerOpened } from '../drawer/drawerSelectors';
@@ -29,12 +29,11 @@ import ImageButton from 'src/shared/components/image-button/ImageButton';
 import styles from './BrowserReset.scss';
 import { ReactComponent as resetIcon } from 'static/img/browser/track-reset.svg';
 
-import { EnsObject } from 'src/shared/state/ens-object/ensObjectTypes';
 import { Status } from 'src/shared/types/status';
 import { RootState } from 'src/store';
 
 export type BrowserResetProps = {
-  focusObject: EnsObject | null;
+  focusObjectId: string | null;
   changeFocusObject: (objectId: string) => void;
   isActive: boolean;
 };
@@ -42,8 +41,8 @@ export type BrowserResetProps = {
 export const BrowserReset: FunctionComponent<BrowserResetProps> = (
   props: BrowserResetProps
 ) => {
-  const { focusObject } = props;
-  if (!focusObject) {
+  const { focusObjectId } = props;
+  if (!focusObjectId) {
     return null;
   }
 
@@ -52,7 +51,7 @@ export const BrowserReset: FunctionComponent<BrowserResetProps> = (
   };
 
   const handleClick = () => {
-    props.changeFocusObject(focusObject.object_id);
+    props.changeFocusObject(focusObjectId);
   };
 
   return (
@@ -71,7 +70,7 @@ const mapStateToProps = (state: RootState) => {
   const isFocusObjectInDefaultPosition = isFocusObjectPositionDefault(state);
   const isDrawerOpened = getIsDrawerOpened(state);
   return {
-    focusObject: getBrowserActiveEnsObject(state),
+    focusObjectId: getBrowserActiveEnsObjectId(state),
     isActive: !isFocusObjectInDefaultPosition && !isDrawerOpened
   };
 };

src/ensembl/src/content/app/browser/browserActions.ts

@@ -24,6 +24,7 @@ import get from 'lodash/get';
 import config from 'config';
 import * as urlFor from 'src/shared/helpers/urlHelper';
 import { getChrLocationStr } from './browserHelper';
+import { buildFocusIdForUrl } from 'src/shared/state/ens-object/ensObjectHelpers';
 
 import browserMessagingService from 'src/content/app/browser/browser-messaging-service';
 import browserStorageService from './browser-storage-service';
@@ -125,20 +126,18 @@ export const fetchDataForLastVisitedObjects: ActionCreator<ThunkAction<
 >> = () => async (dispatch, getState: () => RootState) => {
   const state = getState();
   const activeEnsObjectIdsMap = getBrowserActiveEnsObjectIds(state);
-  const activeEnsObjectIds = Object.values(activeEnsObjectIdsMap);
-  activeEnsObjectIds.forEach((id) => dispatch(fetchEnsObject(id)));
+  Object.values(activeEnsObjectIdsMap).forEach((objectId) =>
+    dispatch(fetchEnsObject(objectId))
+  );
 };
 
 export const updateBrowserActiveEnsObjectIds = createAction(
   'browser/update-active-ens-object-ids'
 )<{ [objectId: string]: string }>();
 
-export const updateBrowserActiveEnsObjectIdsAndSave: ActionCreator<ThunkAction<
-  void,
-  any,
-  null,
-  Action<string>
->> = (activeEnsObjectId: string) => {
+export const updateBrowserActiveEnsObjectIdsAndSave = (
+  activeEnsObjectId: string
+): ThunkAction<void, any, null, Action<string>> => {
   return (dispatch, getState: () => RootState) => {
     const state = getState();
     const activeGenomeId = getBrowserActiveGenomeId(state);
@@ -182,7 +181,7 @@ export const restoreBrowserTrackStates: ActionCreator<ThunkAction<
   any,
   null,
   Action<string>
->> = () => (dispatch, getState: () => RootState) => {
+>> = () => (_, getState: () => RootState) => {
   const state = getState();
   const activeGenomeId = getBrowserActiveGenomeId(state);
   const activeEnsObjectId = getBrowserActiveEnsObjectId(state);
@@ -292,7 +291,7 @@ export const setChrLocation: ActionCreator<ThunkAction<
     const activeGenomeId = getBrowserActiveGenomeId(state);
     const activeEnsObjectId = getBrowserActiveEnsObjectId(state);
     const savedChrLocation = getChrLocation(state);
-    if (!activeGenomeId) {
+    if (!activeGenomeId || !activeEnsObjectId) {
       return;
     }
     const payload = {
@@ -305,7 +304,7 @@ export const setChrLocation: ActionCreator<ThunkAction<
     if (!isEqual(chrLocation, savedChrLocation)) {
       const newUrl = urlFor.browser({
         genomeId: activeGenomeId,
-        focus: activeEnsObjectId,
+        focus: buildFocusIdForUrl(activeEnsObjectId),
         location: getChrLocationStr(chrLocation)
       });
       dispatch(replace(newUrl));
@@ -327,7 +326,7 @@ export const changeBrowserLocation: ActionCreator<ThunkAction<
   ensObjectId: string | null;
   chrLocation: ChrLocation;
 }) => {
-  return (dispatch, getState: () => RootState) => {
+  return (_, getState: () => RootState) => {
     const state = getState();
     const [chrCode, startBp, endBp] = locationData.chrLocation;
 
@@ -335,11 +334,10 @@ export const changeBrowserLocation: ActionCreator<ThunkAction<
       locationData.ensObjectId || getBrowserActiveEnsObjectId(state);
 
     const messageCount = getBrowserMessageCount(state);
-    const focusInstruction = activeEnsObjectId
-      ? {
-          focus: activeEnsObjectId
-        }
-      : {};
+    const focusInstruction: { focus?: string } = {};
+    if (activeEnsObjectId) {
+      focusInstruction.focus = activeEnsObjectId;
+    }
 
     browserMessagingService.send('bpane', {
       stick: `${locationData.genomeId}:${chrCode}`,
@@ -350,23 +348,26 @@ export const changeBrowserLocation: ActionCreator<ThunkAction<
   };
 };
 
-export const changeFocusObject: ActionCreator<ThunkAction<
-  any,
-  any,
-  null,
-  Action<string>
->> = (objectId) => {
-  return (dispatch, getState: () => RootState) => {
-    const state = getState();
-    const messageCount = getBrowserMessageCount(state);
+export const changeFocusObject = (
+  objectId: string
+): ThunkAction<any, any, null, Action<string>> => (
+  dispatch,
+  getState: () => RootState
+) => {
+  const state = getState();
+  const messageCount = getBrowserMessageCount(state);
+  const activeGenomeId = getBrowserActiveGenomeId(state);
+
+  if (!activeGenomeId) {
+    return;
+  }
 
-    dispatch(updatePreviouslyViewedObjectsAndSave());
+  dispatch(updatePreviouslyViewedObjectsAndSave());
 
-    browserMessagingService.send('bpane', {
-      focus: objectId,
-      'message-counter': messageCount
-    });
-  };
+  browserMessagingService.send('bpane', {
+    focus: objectId,
+    'message-counter': messageCount
+  });
 };
 
 export const updateCogList = createAction('browser/update-cog-list')<number>();

src/ensembl/src/content/app/browser/browserHelper.ts

@@ -17,8 +17,16 @@
 import noop from 'lodash/noop';
 import apiService from 'src/services/api-service';
 
-import { ChrLocation } from './browserState';
 import { getNumberWithoutCommas } from 'src/shared/helpers/formatters/numberFormatter';
+import { parseEnsObjectId, buildEnsObjectId } from 'src/shared/state/ens-object/ensObjectHelpers';
+
+import { ChrLocation } from './browserState';
+
+type GenomeBrowserFocusIdConstituents = {
+  genomeId: string;
+  type: string;
+  objectId: string;
+}
 
 export function getChrLocationFromStr(chrLocationStr: string): ChrLocation {
   const [chrCode, chrRegion] = chrLocationStr.split(':');
@@ -39,6 +47,13 @@ export function getChrLocationStr(
   return `${chrCode}:${startBp}-${endBp}`;
 }
 
+export const stringifyGenomeBrowserFocusId = (params: GenomeBrowserFocusIdConstituents) => 
+  buildEnsObjectId(params);
+
+// Genome browser sends focus feature id in the format <genome_id>:<feature_type>:<feature_id>.
+export const parseFeatureId = (id: string): GenomeBrowserFocusIdConstituents =>
+  parseEnsObjectId(id);
+
 export type RegionValidationErrors = {
   genomeIdError: string | null;
   regionParamError: string | null;
@@ -159,6 +174,8 @@ export const validateRegion = async (params: {
     try {
       const url = `/api/genome/region/validate?genome_id=${genomeId}&region=${regionInput}`;
       const response: RegionValidationResponse = await apiService.fetch(url);
+      const regionId = buildEnsObjectId({ genomeId, type: 'region', objectId: regionInput });
+      response.region_id = regionId;
 
       processValidationMessages(
         getRegionValidationMessages(response),

src/ensembl/src/content/app/browser/drawer/Drawer.tsx

@@ -33,7 +33,10 @@ import DrawerBookmarks from './drawer-views/DrawerBookmarks';
 import styles from './Drawer.scss';
 import SnpIndels from './drawer-views/SnpIndels';
 
-import { EnsObject } from 'src/shared/state/ens-object/ensObjectTypes';
+import {
+  EnsObject,
+  EnsObjectGene
+} from 'src/shared/state/ens-object/ensObjectTypes';
 
 export type DrawerProps = {
   drawerView: string;
@@ -50,9 +53,9 @@ export const Drawer = (props: DrawerProps) => {
   const getDrawerViewComponent = () => {
     switch (drawerView) {
       case 'track:gene-feat':
-        return <DrawerGene ensObject={ensObject} />;
+        return <DrawerGene ensObject={ensObject as EnsObjectGene} />;
       case 'track:gene-feat-1':
-        return <DrawerTranscript ensObject={ensObject} />;
+        return <DrawerTranscript ensObject={ensObject as EnsObjectGene} />;
       case 'track:gene-pc-fwd':
         return <ProteinCodingGenes forwardStrand={true} />;
       case 'track:gene-other-fwd':

src/ensembl/src/content/app/browser/drawer/drawer-views/DrawerGene.tsx

@@ -18,12 +18,12 @@ import React, { FunctionComponent } from 'react';
 
 import { getDisplayStableId } from 'src/shared/state/ens-object/ensObjectHelpers';
 
-import { EnsObject } from 'src/shared/state/ens-object/ensObjectTypes';
+import { EnsObjectGene } from 'src/shared/state/ens-object/ensObjectTypes';