[WS] Improve Source 404 UX

- Add NotFound to SourceRouter + add breadcrumbs for organization views - When an actual source ID 404s, fix blanket redirect to a dashboard aware redirect - personal dashboard 404s should send the user back to personal sources, not organization sources + add a flash message error (similar to how App Search behaves for engine 404s) + harden error status checks (gracefully allow for non-http errors to fall back flashAPIErrors
elastic · Jun 24, 2021 · 90c7fc4 · 90c7fc4
1 parent 4089bb1
commit 90c7fc4
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 43 deletions.
diff --git a/...se_search/public/applications/workplace_search/views/content_sources/source_logic.test.ts b/...se_search/public/applications/workplace_search/views/content_sources/source_logic.test.ts
@@ -22,8 +22,6 @@ jest.mock('../../app_logic', () => ({
 }));
 import { AppLogic } from '../../app_logic';
 
-import { NOT_FOUND_PATH } from '../../routes';
-
 import { SourceLogic } from './source_logic';
 
 describe('SourceLogic', () => {
@@ -176,47 +174,55 @@ describe('SourceLogic', () => {
         expect(initializeFederatedSummarySpy).toHaveBeenCalledWith(contentSource.id);
       });
 
-      it('handles error', async () => {
-        const error = {
-          response: {
-            error: 'this is an error',
-            status: 400,
-          },
-        };
-        const promise = Promise.reject(error);
-        http.get.mockReturnValue(promise);
-        SourceLogic.actions.initializeSource(contentSource.id);
-        await expectedAsyncError(promise);
+      describe('errors', () => {
+        it('handles generic errors', async () => {
+          const mockError = Promise.reject('error');
+          http.get.mockReturnValue(mockError);
 
-        expect(flashAPIErrors).toHaveBeenCalledWith(error);
-      });
+          SourceLogic.actions.initializeSource(contentSource.id);
+          await expectedAsyncError(mockError);
 
-      it('handles not found state', async () => {
-        const error = {
-          response: {
-            error: 'this is an error',
-            status: 404,
-          },
-        };
-        const promise = Promise.reject(error);
-        http.get.mockReturnValue(promise);
-        SourceLogic.actions.initializeSource(contentSource.id);
-        await expectedAsyncError(promise);
+          expect(flashAPIErrors).toHaveBeenCalledWith('error');
+        });
 
-        expect(navigateToUrl).toHaveBeenCalledWith(NOT_FOUND_PATH);
-      });
+        describe('404s', () => {
+          const mock404 = Promise.reject({ response: { status: 404 } });
 
-      it('renders error messages passed in success response from server', async () => {
-        const errors = ['ERROR'];
-        const promise = Promise.resolve({
-          ...contentSource,
-          errors,
+          it('redirects to the organization sources page on organization views', async () => {
+            AppLogic.values.isOrganization = true;
+            http.get.mockReturnValue(mock404);
+
+            SourceLogic.actions.initializeSource('404ing_org_source');
+            await expectedAsyncError(mock404);
+
+            expect(navigateToUrl).toHaveBeenCalledWith('/sources');
+            expect(setErrorMessage).toHaveBeenCalledWith('Source not found.');
+          });
+
+          it('redirects to the personal dashboard sources page on personal views', async () => {
+            AppLogic.values.isOrganization = false;
+            http.get.mockReturnValue(mock404);
+
+            SourceLogic.actions.initializeSource('404ing_personal_source');
+            await expectedAsyncError(mock404);
+
+            expect(navigateToUrl).toHaveBeenCalledWith('/p/sources');
+            expect(setErrorMessage).toHaveBeenCalledWith('Source not found.');
+          });
         });
-        http.get.mockReturnValue(promise);
-        SourceLogic.actions.initializeSource(contentSource.id);
-        await promise;
 
-        expect(setErrorMessage).toHaveBeenCalledWith(errors);
+        it('renders error messages passed in success response from server', async () => {
+          const errors = ['ERROR'];
+          const promise = Promise.resolve({
+            ...contentSource,
+            errors,
+          });
+          http.get.mockReturnValue(promise);
+          SourceLogic.actions.initializeSource(contentSource.id);
+          await promise;
+
+          expect(setErrorMessage).toHaveBeenCalledWith(errors);
+        });
       });
     });
 

diff --git a/...erprise_search/public/applications/workplace_search/views/content_sources/source_logic.ts b/...erprise_search/public/applications/workplace_search/views/content_sources/source_logic.ts
@@ -20,7 +20,7 @@ import {
 import { HttpLogic } from '../../../shared/http';
 import { KibanaLogic } from '../../../shared/kibana';
 import { AppLogic } from '../../app_logic';
-import { NOT_FOUND_PATH, SOURCES_PATH, getSourcesPath } from '../../routes';
+import { PERSONAL_SOURCES_PATH, SOURCES_PATH, getSourcesPath } from '../../routes';
 import { ContentSourceFullData, Meta, DocumentSummaryItem, SourceContentItem } from '../../types';
 
 export interface SourceActions {
@@ -155,8 +155,14 @@ export const SourceLogic = kea<MakeLogicType<SourceValues, SourceActions>>({
           clearFlashMessages();
         }
       } catch (e) {
-        if (e.response.status === 404) {
-          KibanaLogic.values.navigateToUrl(NOT_FOUND_PATH);
+        if (e?.response?.status === 404) {
+          const redirect = isOrganization ? SOURCES_PATH : PERSONAL_SOURCES_PATH;
+          KibanaLogic.values.navigateToUrl(redirect);
+          setErrorMessage(
+            i18n.translate('xpack.enterpriseSearch.workplaceSearch.sources.notFoundErrorMessage', {
+              defaultMessage: 'Source not found.',
+            })
+          );
         } else {
           flashAPIErrors(e);
         }

diff --git a/..._search/public/applications/workplace_search/views/content_sources/source_router.test.tsx b/..._search/public/applications/workplace_search/views/content_sources/source_router.test.tsx
@@ -90,7 +90,7 @@ describe('SourceRouter', () => {
     expect(wrapper.find(Overview)).toHaveLength(1);
     expect(wrapper.find(SourceSettings)).toHaveLength(1);
     expect(wrapper.find(SourceContent)).toHaveLength(1);
-    expect(wrapper.find(Route)).toHaveLength(3);
+    expect(wrapper.find(Route)).toHaveLength(4);
   });
 
   it('renders source routes (custom)', () => {
@@ -100,6 +100,6 @@ describe('SourceRouter', () => {
     expect(wrapper.find(DisplaySettingsRouter)).toHaveLength(1);
     expect(wrapper.find(Schema)).toHaveLength(1);
     expect(wrapper.find(SchemaChangeErrors)).toHaveLength(1);
-    expect(wrapper.find(Route)).toHaveLength(6);
+    expect(wrapper.find(Route)).toHaveLength(7);
   });
 });
diff --git a/...prise_search/public/applications/workplace_search/views/content_sources/source_router.tsx b/...prise_search/public/applications/workplace_search/views/content_sources/source_router.tsx
@@ -13,7 +13,7 @@ import { useActions, useValues } from 'kea';
 
 import { AppLogic } from '../../app_logic';
 import { WorkplaceSearchPageTemplate, PersonalDashboardLayout } from '../../components/layout';
-import { CUSTOM_SERVICE_TYPE } from '../../constants';
+import { NAV, CUSTOM_SERVICE_TYPE } from '../../constants';
 import {
   REINDEX_JOB_PATH,
   SOURCE_DETAILS_PATH,
@@ -24,6 +24,7 @@ import {
   getContentSourcePath as sourcePath,
   getSourcesPath,
 } from '../../routes';
+import { NotFound } from '../../views/not_found';
 
 import { DisplaySettingsRouter } from './components/display_settings';
 import { Overview } from './components/overview';
@@ -85,6 +86,9 @@ export const SourceRouter: React.FC = () => {
       <Route exact path={sourcePath(SOURCE_SETTINGS_PATH, sourceId, isOrganization)}>
         <SourceSettings />
       </Route>
+      <Route>
+        <NotFound isOrganization={isOrganization} pageChrome={[NAV.SOURCES]} />
+      </Route>
     </Switch>
   );
 };