Routing Improvements

- invalid subpaths (tabs) result in a 404 page - 404 page is more consistent with regular error page
cBioPortal · Dec 13, 2019 · 6434643 · 6434643
1 parent 5f7168e
commit 6434643
Show file tree

Hide file tree

Showing 16 changed files with 116 additions and 54 deletions.
diff --git a/src/pages/groupComparison/GroupComparisonPage.tsx b/src/pages/groupComparison/GroupComparisonPage.tsx
@@ -13,7 +13,7 @@ import {MakeMobxView} from "../../shared/components/MobxView";
 import LoadingIndicator from "../../shared/components/loadingIndicator/LoadingIndicator";
 import ErrorMessage from "../../shared/components/ErrorMessage";
 import GroupSelector from "./groupSelector/GroupSelector";
-import {getTabId, GroupComparisonTab} from "./GroupComparisonUtils";
+import { GroupComparisonTab } from "./GroupComparisonTabs"
 import styles from "./styles.module.scss";
 import {StudyLink} from "shared/components/StudyLink/StudyLink";
 import {action, computed, IReactionDisposer, observable, reaction} from "mobx";

diff --git a/src/pages/groupComparison/GroupComparisonStore.ts b/src/pages/groupComparison/GroupComparisonStore.ts
@@ -8,12 +8,12 @@ import {
     getOverlapComputations,
     getSampleIdentifiers,
     getStudyIds,
-    GroupComparisonTab,
     IOverlapComputations,
     isGroupEmpty,
     partitionCasesByGroupMembership,
-    getNumSamples
+    getNumSamples,
 } from "./GroupComparisonUtils";
+import { GroupComparisonTab } from "./GroupComparisonTabs"
 import {remoteData} from "../../public-lib/api/remoteData";
 import {
     CancerStudy,

diff --git a/src/pages/groupComparison/GroupComparisonTabs.ts b/src/pages/groupComparison/GroupComparisonTabs.ts
@@ -0,0 +1,9 @@
+export enum GroupComparisonTab {
+    OVERLAP = "overlap",
+    MUTATIONS = "mutations",
+    CNA = "cna",
+    MRNA = "mrna",
+    PROTEIN = "protein",
+    SURVIVAL = "survival",
+    CLINICAL = "clinical"
+}
diff --git a/src/pages/groupComparison/GroupComparisonURLWrapper.ts b/src/pages/groupComparison/GroupComparisonURLWrapper.ts
@@ -1,7 +1,8 @@
 import URLWrapper from "../../shared/lib/URLWrapper";
 import ExtendedRouterStore from "../../shared/lib/ExtendedRouterStore";
 import {computed} from "mobx";
-import {getTabId, GroupComparisonTab} from "./GroupComparisonUtils";
+import {getTabId} from "./GroupComparisonUtils";
+import { GroupComparisonTab } from "./GroupComparisonTabs"
 import autobind from "autobind-decorator";
 import {OverlapStrategy} from "./GroupComparisonStore";
 

diff --git a/src/pages/groupComparison/GroupComparisonUtils.tsx b/src/pages/groupComparison/GroupComparisonUtils.tsx
@@ -20,16 +20,7 @@ import OverlapExclusionIndicator from "./OverlapExclusionIndicator";
 import Loader from "../../shared/components/loadingIndicator/LoadingIndicator";
 import ErrorMessage from "../../shared/components/ErrorMessage";
 import {stringListToIndexSet} from "public-lib/lib/StringUtils";
-
-export enum GroupComparisonTab {
-    OVERLAP = "overlap",
-    MUTATIONS = "mutations",
-    CNA = "cna",
-    MRNA = "mrna",
-    PROTEIN = "protein",
-    SURVIVAL = "survival",
-    CLINICAL = "clinical"
-}
+import { GroupComparisonTab } from "./GroupComparisonTabs"
 
 type Omit<T, K> = Pick<T, Exclude<keyof T, K>>;
 

diff --git a/src/pages/patientView/PatientViewPage.tsx b/src/pages/patientView/PatientViewPage.tsx
@@ -59,6 +59,7 @@ import { PagePath } from "shared/enums/PagePaths";
 import { GeneFilterOption } from "./mutation/GeneFilterMenu";
 import { checkNonProfiledGenesExist } from "./PatientViewPageUtils";
 import PatientViewGenePanelModal from "./PatientViewGenePanelModal/PatientViewGenePanelModal";
+import { PatientViewPageTabs } from "./PatientViewPageTabs";
 
 export interface IPatientViewPageProps {
     params: any; // react route
@@ -269,15 +270,15 @@ export default class PatientViewPage extends React.Component<IPatientViewPagePro
                                             this.patientViewPageStore.sampleToDiscreteGenePanelId.result,
                                             this.patientViewPageStore.genePanelIdToEntrezGeneIds.result);
     }
-    
+
     @autobind
     @action toggleGenePanelModal(genePanelId?:string|undefined) {
         this.genePanelModal = {
             isOpen: !this.genePanelModal.isOpen,
             genePanelId: genePanelId || ''
         };
     }
-    
+
     @computed get modalSelectedGenePanel() {
         return this.patientViewPageStore.genePanelIdToPanel.result[this.genePanelModal.genePanelId];
     }
@@ -403,8 +404,8 @@ export default class PatientViewPage extends React.Component<IPatientViewPagePro
                     )
                 }
                 <div className="patientViewPage">
-                    {this.genePanelModal.isOpen && 
-                        <PatientViewGenePanelModal 
+                    {this.genePanelModal.isOpen &&
+                        <PatientViewGenePanelModal
                             genePanel={this.modalSelectedGenePanel}
                             show={this.genePanelModal.isOpen}
                             onHide={this.toggleGenePanelModal}
@@ -448,7 +449,7 @@ export default class PatientViewPage extends React.Component<IPatientViewPagePro
                                 onTabClick={(id:string)=>this.urlWrapper.setTab(id)}
                                 className="mainTabs"
                             >
-                                <MSKTab key={0} id="summary" linkText="Summary">
+                                <MSKTab key={0} id={PatientViewPageTabs.Summary} linkText="Summary">
 
                                     <LoadingIndicator isLoading={this.patientViewPageStore.clinicalEvents.isPending} />
 
@@ -599,7 +600,7 @@ export default class PatientViewPage extends React.Component<IPatientViewPagePro
                                     }
                                 </MSKTab>
 
-                    <MSKTab key={2} id="clinicalData" linkText="Clinical Data">
+                    <MSKTab key={2} id={PatientViewPageTabs.ClinicalData} linkText="Clinical Data">
 
                         <div className="clearfix">
                             <FeatureTitle title="Patient"
@@ -626,15 +627,15 @@ export default class PatientViewPage extends React.Component<IPatientViewPagePro
                     </MSKTab>
 
 
-                    <MSKTab key={3} id="pathologyReport" linkText="Pathology Report"
+                    <MSKTab key={3} id={PatientViewPageTabs.PathologyReport} linkText="Pathology Report"
                             hide={!this.shouldShowPathologyReport(this.patientViewPageStore)}
                     >
                         <div>
                             <PathologyReport iframeHeight={WindowStore.size.height - 220} pdfs={this.patientViewPageStore.pathologyReport.result} />
                         </div>
                     </MSKTab>
 
-                    <MSKTab key={5} id="tissueImage" linkText="Tissue Image"
+                    <MSKTab key={5} id={PatientViewPageTabs.TissueImage} linkText="Tissue Image"
                             hide={this.hideTissueImageTab()}
                     >
                         <div>
@@ -643,7 +644,7 @@ export default class PatientViewPage extends React.Component<IPatientViewPagePro
                     </MSKTab>
 
                     {(this.patientViewPageStore.studyId === "mskimpact" && this.wholeSlideViewerUrl.result) && (
-                    <MSKTab key={6} id="MSKTissueImage" linkText="Tissue Image"
+                    <MSKTab key={6} id={PatientViewPageTabs.MSKTissueImage} linkText="Tissue Image"
                             unmountOnHide = {false}
                     >
                         <div>
@@ -654,7 +655,7 @@ export default class PatientViewPage extends React.Component<IPatientViewPagePro
 
                     {
                         this.shouldShowTrialMatch(this.patientViewPageStore) && (
-                            <MSKTab key={7} id="trialMatchTab" linkText="Matched Trials">
+                            <MSKTab key={7} id={PatientViewPageTabs.TrialMatchTab} linkText="Matched Trials">
                                 <TrialMatchTable
                                     sampleManager={sampleManager}
                                     detailedTrialMatches={this.patientViewPageStore.detailedTrialMatches.result}
@@ -664,7 +665,7 @@ export default class PatientViewPage extends React.Component<IPatientViewPagePro
                         )
                     }
 
-                    {/*<MSKTab key={5} id="mutationalSignatures" linkText="Mutational Signature Data" hide={true}>*/}
+                    {/*<MSKTab key={5} id={{PatientViewPageTabs.MutationalSignatures}} linkText="Mutational Signature Data" hide={true}>*/}
                         {/*<div className="clearfix">*/}
                             {/*<FeatureTitle title="Mutational Signatures" isLoading={ this.patientViewPageStore.clinicalDataGroupedBySample.isPending } className="pull-left" />*/}
                             {/*<LoadingIndicator isLoading={this.patientViewPageStore.mutationalSignatureData.isPending}/>*/}

diff --git a/src/pages/patientView/PatientViewPageTabs.ts b/src/pages/patientView/PatientViewPageTabs.ts
@@ -0,0 +1,9 @@
+export enum PatientViewPageTabs {
+    Summary = "summary",
+    ClinicalData = "clinicalData",
+    PathologyReport = "pathologyReport",
+    TissueImage = "tissueImage",
+    MSKTissueImage = "MSKTissueImage",
+    TrialMatchTab = "trialMatchTab",
+    MutationalSignatures = "mutationalSignatures",
+}
diff --git a/src/pages/resultsView/ErrorPage.tsx b/src/pages/resultsView/ErrorPage.tsx
@@ -0,0 +1,31 @@
+import * as React from 'react';
+import { inject } from 'mobx-react';
+import { AppStore } from 'AppStore';
+import PortalHeader from 'appShell/App/PortalHeader';
+import PortalFooter from 'appShell/App/PortalFooter';
+import UserMessager from 'shared/components/userMessager/UserMessage';
+import ErrorScreen from 'shared/components/errorScreen/ErrorScreen';
+import { formatErrorTitle } from 'shared/lib/errorFormatter';
+import { buildCBioPortalPageUrl } from 'shared/api/urls';
+
+@inject("appStore")
+export default class ErrorPage extends React.Component<{appStore?: AppStore}, {}> {
+    render () {
+        return (
+            <div>
+                <div className="pageTopContainer">
+                    <UserMessager />
+                    <div className="contentWidth">
+                        <PortalHeader appStore={this.props.appStore as AppStore}/>
+                    </div>
+                </div>
+                <div className="contentWrapper">
+                    <ErrorScreen
+                        title={"Sorry, this page doesn't exist."}
+                        body={<a href={buildCBioPortalPageUrl("/")}>Return to homepage</a>}
+                    />
+                </div>
+            </div>
+        )
+    }
+}
diff --git a/src/pages/studyView/StudyViewPage.tsx b/src/pages/studyView/StudyViewPage.tsx
@@ -7,9 +7,9 @@ import {
     CustomChart,
     StudyViewPageStore,
     StudyViewPageTabDescriptions,
-    StudyViewPageTabKeyEnum,
     StudyViewURLQuery,
 } from "pages/studyView/StudyViewPageStore";
+import { StudyViewPageTabKeyEnum } from "pages/studyView/StudyViewPageTabs";
 import LoadingIndicator from "shared/components/loadingIndicator/LoadingIndicator";
 import { ClinicalDataTab } from "./tabs/ClinicalDataTab";
 import getBrowserWindow from "../../public-lib/lib/getBrowserWindow";

diff --git a/src/pages/studyView/StudyViewPageStore.ts b/src/pages/studyView/StudyViewPageStore.ts
@@ -134,6 +134,7 @@ import {
 } from "pages/studyView/TableUtils";
 import { GeneTableRow } from './table/GeneTable';
 import { getSelectedGroups, getGroupParameters } from '../groupComparison/comparisonGroupManager/ComparisonGroupManagerUtils';
+import { StudyViewPageTabKeyEnum } from "pages/studyView/StudyViewPageTabs";
 
 export type ChartUserSetting = {
     id: string,
@@ -157,13 +158,6 @@ export type StudyPageSettings = {
     origin:string[]
 }
 
-export enum StudyViewPageTabKeyEnum {
-    SUMMARY = 'summary',
-    CLINICAL_DATA = 'clinicalData',
-    HEATMAPS = 'heatmaps',
-    CN_SEGMENTS = 'cnSegments'
-}
-
 export type StudyViewPageTabKey =
     StudyViewPageTabKeyEnum.CLINICAL_DATA | StudyViewPageTabKeyEnum.SUMMARY | StudyViewPageTabKeyEnum.HEATMAPS |
     StudyViewPageTabKeyEnum.CN_SEGMENTS;

diff --git a/src/pages/studyView/StudyViewPageTabs.ts b/src/pages/studyView/StudyViewPageTabs.ts
@@ -0,0 +1,6 @@
+export enum StudyViewPageTabKeyEnum {
+    SUMMARY = 'summary',
+    CLINICAL_DATA = 'clinicalData',
+    HEATMAPS = 'heatmaps',
+    CN_SEGMENTS = 'cnSegments',
+}
diff --git a/src/pages/studyView/StudyViewUtils.spec.ts b/src/pages/studyView/StudyViewUtils.spec.ts
@@ -61,9 +61,7 @@ import {
     StudyViewFilter
 } from 'shared/api/generated/CBioPortalAPIInternal';
 import {CancerStudy, ClinicalAttribute, Gene} from 'shared/api/generated/CBioPortalAPI';
-import {
-    StudyViewPageTabKeyEnum
-} from "./StudyViewPageStore";
+import { StudyViewPageTabKeyEnum } from "pages/studyView/StudyViewPageTabs";
 import {
     UniqueKey
 } from "./StudyViewUtils";

diff --git a/src/pages/studyView/StudyViewUtils.tsx b/src/pages/studyView/StudyViewUtils.tsx
@@ -14,9 +14,9 @@ import * as React from "react";
 import {buildCBioPortalPageUrl} from "../../shared/api/urls";
 import {IStudyViewScatterPlotData} from "./charts/scatterPlot/StudyViewScatterPlot";
 import {BarDatum} from "./charts/barChart/BarChart";
-import {
-    StudyViewPageTabKeyEnum, ChartUserSetting, CustomChart
-} from "./StudyViewPageStore";
+import { ChartUserSetting, CustomChart } from "./StudyViewPageStore";
+import { StudyViewPageTabKeyEnum } from "pages/studyView/StudyViewPageTabs";
+
 import {Layout} from 'react-grid-layout';
 import internalClient from "shared/api/cbioportalInternalClientInstance";
 import {VirtualStudy} from "shared/model/VirtualStudy";

diff --git a/src/pages/studyView/addChartButton/AddChartButton.tsx b/src/pages/studyView/addChartButton/AddChartButton.tsx
@@ -5,9 +5,10 @@ import {ChildButton, MainButton, Menu} from 'react-mfb';
 import 'react-mfb/mfb.css';
 import {
     CustomChart,
-    StudyViewPageStore, StudyViewPageTabKey,
-    StudyViewPageTabKeyEnum
+    StudyViewPageStore,
+    StudyViewPageTabKey,
 } from "../StudyViewPageStore";
+import { StudyViewPageTabKeyEnum } from "pages/studyView/StudyViewPageTabs";
 import autobind from 'autobind-decorator';
 import * as _ from 'lodash';
 import AddChartByType from "./addChartByType/AddChartByType";

diff --git a/src/pages/studyView/tabs/CNSegments.tsx b/src/pages/studyView/tabs/CNSegments.tsx
@@ -12,7 +12,8 @@ import LoadingIndicator from "shared/components/loadingIndicator/LoadingIndicato
 import CNSegmentsDownloader from "shared/components/cnSegments/CNSegmentsDownloader";
 import WindowStore from "shared/components/window/WindowStore";
 
-import {StudyViewPageStore, StudyViewPageTabKeyEnum} from "../StudyViewPageStore";
+import { StudyViewPageTabKeyEnum } from "pages/studyView/StudyViewPageTabs";
+import {StudyViewPageStore} from "../StudyViewPageStore";
 
 @observer
 export default class CNSegments extends React.Component<{ store: StudyViewPageStore }, {}> {

diff --git a/src/routes.jsx b/src/routes.jsx
@@ -14,6 +14,8 @@ import PageNotFound from './shared/components/pageNotFound/PageNotFound';
 /* HOW TO ADD A NEW ROUTE
  * 1. Import the "page" component using the bundle-loader directives as seen in imports below
  * 2. Add a Route element with getComponent set to the result the lazyLoadComponent function passed your new component
+ * If your route includes tabs, include `null, tabParamValidator(YourPageTabEnum) in the lazyLoadComponent call.
+ * This ensures that invalid sub routes 404 correctly
  */
 
 // import page components here
@@ -39,6 +41,7 @@ import News from 'bundle-loader?lazy!babel-loader!./pages/staticPages/news/News'
 import FAQ from 'bundle-loader?lazy!babel-loader!./pages/staticPages/faq/FAQ';
 import OQL from 'bundle-loader?lazy!babel-loader!./pages/staticPages/oql/OQL';
 import GroupComparisonPage from 'bundle-loader?lazy!babel-loader!./pages/groupComparison/GroupComparisonPage';
+import ErrorPage from 'bundle-loader?lazy!babel-loader!./pages/resultsView/ErrorPage';
 
 import AppConfig from 'appConfig';
 
@@ -49,17 +52,33 @@ import getBrowserWindow from 'public-lib/lib/getBrowserWindow';
 import { seekUrlHash } from 'shared/lib/seekUrlHash';
 import queryString from 'query-string';
 import { PagePath } from 'shared/enums/PagePaths';
+import { ResultsViewTab } from 'pages/resultsView/ResultsViewPageHelpers'
+import { StudyViewPageTabKeyEnum } from 'pages/studyView/StudyViewPageTabs'
+import { PatientViewPageTabs } from 'pages/patientView/PatientViewPageTabs'
+import { GroupComparisonTab } from 'pages/groupComparison/GroupComparisonTabs'
+
+/**
+ * Validates that the parameters either do not have
+ * a tab parameter, or have a parameter that matches a
+ * value in `tabEnum`
+ * @param tabEnum a TypeScript string enum
+ */
+function tabParamValidator(tabEnum) {
+    return function(params) {
+        return !params.tab || Object.values(tabEnum).indexOf(params.tab) > -1;
+    }
+}
 
 // accepts bundle-loader's deferred loader function and defers execution of route's render
 // until chunk is loaded
-function lazyLoadComponent(loader, loadingCallback) {
+function lazyLoadComponent(loader, loadingCallback, validator = (_) => {return true;}) {
     return (location, cb) => {
+        if (location && !validator(location.params)) {
+            loader = ErrorPage;
+        }
         loader(module => {
-            if (cb) cb(null, module.default);
-            if (loadingCallback) loadingCallback();
-            // if (typeof window.onReactAppReady === 'function') {
-            //     window.onReactAppReady();
-            // }
+            if (cb) { cb(null, module.default); }
+            if (loadingCallback) { loadingCallback(); }
         });
     };
 }
@@ -123,28 +142,29 @@ export const makeRoutes = routing => {
             />
             <Route
                 path="/results(/:tab)"
-                getComponent={lazyLoadComponent(ResultsViewPage)}
+                onEnter={() => {}}
+                getComponent={lazyLoadComponent(ResultsViewPage, null, tabParamValidator(ResultsViewTab))}
             />
             <Route
                 path={'/' + PagePath.Patient + '(/:tab)'}
                 onEnter={() => {
                     $(document).scrollTop(0);
                 }}
-                getComponent={lazyLoadComponent(PatientViewPage)}
+                getComponent={lazyLoadComponent(PatientViewPage, null, tabParamValidator(PatientViewPageTabs))}
             />
             <Route
-                path="/study(/:tab)"
+                path={"/" + PagePath.Study + "(/:tab)" }
                 onEnter={() => {
                     $(document).scrollTop(0);
                 }}
-                getComponent={lazyLoadComponent(StudyViewPage)}
+                getComponent={lazyLoadComponent(StudyViewPage, null, tabParamValidator(StudyViewPageTabKeyEnum))}
             />
             <Route
                 path="/comparison(/:tab)"
                 onEnter={() => {
                     $(document).scrollTop(0);
                 }}
-                getComponent={lazyLoadComponent(GroupComparisonPage)}
+                getComponent={lazyLoadComponent(GroupComparisonPage, null, tabParamValidator(GroupComparisonTab))}
             />
 
             <Route
@@ -256,7 +276,7 @@ export const makeRoutes = routing => {
                 onEnter={() => {
                     $(document).scrollTop(0);
                 }}
-                component={() => <PageNotFound />}
+                component={() => <ErrorPage />}
             />
         </Route>
     );