[App Search] Curation: Add query management UX & organic documents li…

…st (#94488)

* Update CurationLogic to handle queries management

- updating the active query
- adding queriesLoading and organicDocumentsLoading state
- clean up / refactor updateCurations slightly - instead of taking a {queries} param, we should opt for a separate updateQueries action and storing a queries state
  - this allows us to more granularly hook into activeQuery when queries is updated without the current activeQuery and correct for that

* Add ActiveQuerySelect component

- used for switching the currently active query

* Add ManageQueriesModal component

- used for editing/adding/removing the queries a curation manages
- primarily a light wrapper around the existing reusable CurationQueries component (also used in the create new curation view)

* Add OrganicDocuments and CurationResult components

* Update Curation view with new components

+ update breadcrumb to pull from queries instead of curation.queries, mostly for consistency w/ other usages & slightly faster responsiveness when updating queries

* Fix unnecessary import

* Meeting feedback: organic documents title copy tweak

* PR feedback - test assertion

x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/constants.ts

@@ -32,3 +32,8 @@ export const SUCCESS_MESSAGE = i18n.translate(
   'xpack.enterpriseSearch.appSearch.engine.curations.deleteSuccessMessage',
   { defaultMessage: 'Successfully removed curation.' }
 );
+
+export const RESULT_ACTIONS_DIRECTIONS = i18n.translate(
+  'xpack.enterpriseSearch.appSearch.engine.curations.resultActionsDescription',
+  { defaultMessage: 'Promote results by clicking the star, hide them by clicking the eye.' }
+);

x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/curation.test.tsx

@@ -30,10 +30,7 @@ describe('Curation', () => {
   };
   const values = {
     dataLoading: false,
-    curation: {
-      id: 'cur-123456789',
-      queries: ['query A', 'query B'],
-    },
+    queries: ['query A', 'query B'],
   };
   const actions = {
     loadCuration: jest.fn(),

x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/curation.tsx

@@ -10,7 +10,7 @@ import { useParams } from 'react-router-dom';
 
 import { useValues, useActions } from 'kea';
 
-import { EuiPageHeader, EuiSpacer } from '@elastic/eui';
+import { EuiPageHeader, EuiSpacer, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
 
 import { FlashMessages } from '../../../../shared/flash_messages';
 import { SetAppSearchChrome as SetPageChrome } from '../../../../shared/kibana_chrome';
@@ -20,6 +20,8 @@ import { Loading } from '../../../../shared/loading';
 import { MANAGE_CURATION_TITLE } from '../constants';
 
 import { CurationLogic } from './curation_logic';
+import { OrganicDocuments } from './documents';
+import { ActiveQuerySelect, ManageQueriesModal } from './queries';
 
 interface Props {
   curationsBreadcrumb: BreadcrumbTrail;
@@ -28,7 +30,7 @@ interface Props {
 export const Curation: React.FC<Props> = ({ curationsBreadcrumb }) => {
   const { curationId } = useParams() as { curationId: string };
   const { loadCuration } = useActions(CurationLogic({ curationId }));
-  const { dataLoading, curation } = useValues(CurationLogic({ curationId }));
+  const { dataLoading, queries } = useValues(CurationLogic({ curationId }));
 
   useEffect(() => {
     loadCuration();
@@ -38,20 +40,27 @@ export const Curation: React.FC<Props> = ({ curationsBreadcrumb }) => {
 
   return (
     <>
-      <SetPageChrome trail={[...curationsBreadcrumb, curation.queries.join(', ')]} />
+      <SetPageChrome trail={[...curationsBreadcrumb, queries.join(', ')]} />
       <EuiPageHeader
         pageTitle={MANAGE_CURATION_TITLE}
         /* TODO: Restore defaults button */
         responsive={false}
       />
 
-      {/* TODO: Active query switcher / Manage queries modal */}
+      <EuiFlexGroup alignItems="flexEnd" gutterSize="xl" responsive={false}>
+        <EuiFlexItem>
+          <ActiveQuerySelect />
+        </EuiFlexItem>
+        <EuiFlexItem grow={false}>
+          <ManageQueriesModal />
+        </EuiFlexItem>
+      </EuiFlexGroup>
 
       <EuiSpacer size="xl" />
       <FlashMessages />
 
       {/* TODO: PromotedDocuments section */}
-      {/* TODO: OrganicDocuments section */}
+      <OrganicDocuments />
       {/* TODO: HiddenDocuments section */}
 
       {/* TODO: AddResult flyout */}

x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/curation_logic.test.ts

@@ -47,6 +47,10 @@ describe('CurationLogic', () => {
       organic: [],
       hidden: [],
     },
+    queries: [],
+    queriesLoading: false,
+    activeQuery: '',
+    organicDocumentsLoading: false,
   };
 
   beforeEach(() => {
@@ -60,15 +64,75 @@ describe('CurationLogic', () => {
 
   describe('actions', () => {
     describe('onCurationLoad', () => {
-      it('should set curation state & dataLoading to false', () => {
+      it('should set curation, queries, activeQuery, & all loading states to false', () => {
         mount();
 
         CurationLogic.actions.onCurationLoad(MOCK_CURATION_RESPONSE);
 
         expect(CurationLogic.values).toEqual({
           ...DEFAULT_VALUES,
           curation: MOCK_CURATION_RESPONSE,
+          queries: ['some search'],
+          activeQuery: 'some search',
           dataLoading: false,
+          queriesLoading: false,
+          organicDocumentsLoading: false,
+        });
+      });
+
+      it("should not override activeQuery once it's been set", () => {
+        mount({ activeQuery: 'test' });
+
+        CurationLogic.actions.onCurationLoad(MOCK_CURATION_RESPONSE);
+
+        expect(CurationLogic.values.activeQuery).toEqual('test');
+      });
+    });
+
+    describe('onCurationError', () => {
+      it('should set all loading states to false', () => {
+        mount({
+          dataLoading: true,
+          queriesLoading: true,
+          organicDocumentsLoading: true,
+        });
+
+        CurationLogic.actions.onCurationError();
+
+        expect(CurationLogic.values).toEqual({
+          ...DEFAULT_VALUES,
+          dataLoading: false,
+          queriesLoading: false,
+          organicDocumentsLoading: false,
+        });
+      });
+    });
+
+    describe('updateQueries', () => {
+      it('should set queries state & queriesLoading to true', () => {
+        const values = { ...DEFAULT_VALUES, queries: ['a', 'b'], activeQuery: 'a' };
+        mount(values);
+
+        CurationLogic.actions.updateQueries(['a', 'b', 'c']);
+
+        expect(CurationLogic.values).toEqual({
+          ...values,
+          queries: ['a', 'b', 'c'],
+          queriesLoading: true,
+        });
+      });
+    });
+
+    describe('setActiveQuery', () => {
+      it('should set activeQuery state & organicDocumentsLoading to true', () => {
+        mount();
+
+        CurationLogic.actions.setActiveQuery('some query');
+
+        expect(CurationLogic.values).toEqual({
+          ...DEFAULT_VALUES,
+          activeQuery: 'some query',
+          organicDocumentsLoading: true,
         });
       });
     });
@@ -119,35 +183,23 @@ describe('CurationLogic', () => {
 
       it('should make a PUT API call with queries and promoted/hidden IDs to update', async () => {
         http.put.mockReturnValueOnce(Promise.resolve(MOCK_CURATION_RESPONSE));
-        mount({}, { curationId: 'cur-123456789' });
-        jest.spyOn(CurationLogic.actions, 'onCurationLoad');
-
-        CurationLogic.actions.updateCuration();
-        jest.runAllTimers();
-        await nextTick();
-
-        expect(http.put).toHaveBeenCalledWith(
-          '/api/app_search/engines/some-engine/curations/cur-123456789',
+        mount(
           {
-            body: '{"queries":[],"query":"","promoted":[],"hidden":[]}', // Uses state currently in CurationLogic
-          }
+            queries: ['a', 'b', 'c'],
+            activeQuery: 'b',
+          },
+          { curationId: 'cur-123456789' }
         );
-        expect(CurationLogic.actions.onCurationLoad).toHaveBeenCalledWith(MOCK_CURATION_RESPONSE);
-      });
-
-      it('should allow passing a custom queries param', async () => {
-        http.put.mockReturnValueOnce(Promise.resolve(MOCK_CURATION_RESPONSE));
-        mount({}, { curationId: 'cur-123456789' });
         jest.spyOn(CurationLogic.actions, 'onCurationLoad');
 
-        CurationLogic.actions.updateCuration({ queries: ['hello', 'world'] });
+        CurationLogic.actions.updateCuration();
         jest.runAllTimers();
         await nextTick();
 
         expect(http.put).toHaveBeenCalledWith(
           '/api/app_search/engines/some-engine/curations/cur-123456789',
           {
-            body: '{"queries":["hello","world"],"query":"","promoted":[],"hidden":[]}',
+            body: '{"queries":["a","b","c"],"query":"b","promoted":[],"hidden":[]}', // Uses state currently in CurationLogic
           }
         );
         expect(CurationLogic.actions.onCurationLoad).toHaveBeenCalledWith(MOCK_CURATION_RESPONSE);
@@ -156,13 +208,46 @@ describe('CurationLogic', () => {
       it('handles errors', async () => {
         http.put.mockReturnValueOnce(Promise.reject('error'));
         mount({}, { curationId: 'cur-123456789' });
+        jest.spyOn(CurationLogic.actions, 'onCurationError');
 
         CurationLogic.actions.updateCuration();
         jest.runAllTimers();
         await nextTick();
 
         expect(clearFlashMessages).toHaveBeenCalled();
         expect(flashAPIErrors).toHaveBeenCalledWith('error');
+        expect(CurationLogic.actions.onCurationError).toHaveBeenCalled();
+      });
+    });
+
+    describe('listeners that call updateCuration as a side effect', () => {
+      beforeAll(() => {
+        mount();
+        jest.spyOn(CurationLogic.actions, 'updateCuration').mockImplementation(() => {});
+      });
+
+      afterAll(() => {
+        (CurationLogic.actions.updateCuration as jest.Mock).mockRestore();
+      });
+
+      afterEach(() => {
+        expect(CurationLogic.actions.updateCuration).toHaveBeenCalled();
+      });
+
+      describe('updateQueries', () => {
+        it('calls updateCuration', () => {
+          CurationLogic.actions.updateQueries(['hello', 'world']);
+        });
+
+        it('should also call setActiveQuery if the current activeQuery was deleted from queries', () => {
+          jest.spyOn(CurationLogic.actions, 'setActiveQuery');
+          CurationLogic.actions.updateQueries(['world']);
+          expect(CurationLogic.actions.setActiveQuery).toHaveBeenCalledWith('world');
+        });
+      });
+
+      it('setActiveQuery', () => {
+        CurationLogic.actions.setActiveQuery('test');
       });
     });
   });

x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/curation_logic.ts

@@ -18,12 +18,19 @@ import { Curation } from '../types';
 interface CurationValues {
   dataLoading: boolean;
   curation: Curation;
+  queries: Curation['queries'];
+  queriesLoading: boolean;
+  activeQuery: string;
+  organicDocumentsLoading: boolean;
 }
 
 interface CurationActions {
   loadCuration(): void;
   onCurationLoad(curation: Curation): { curation: Curation };
-  updateCuration(options?: { queries?: string[] }): { queries?: string[] };
+  updateCuration(): void;
+  onCurationError(): void;
+  updateQueries(queries: Curation['queries']): { queries: Curation['queries'] };
+  setActiveQuery(query: string): { query: string };
 }
 
 interface CurationProps {
@@ -35,14 +42,18 @@ export const CurationLogic = kea<MakeLogicType<CurationValues, CurationActions,
   actions: () => ({
     loadCuration: true,
     onCurationLoad: (curation) => ({ curation }),
-    updateCuration: ({ queries } = {}) => ({ queries }),
+    updateCuration: true,
+    onCurationError: true,
+    updateQueries: (queries) => ({ queries }),
+    setActiveQuery: (query) => ({ query }),
   }),
   reducers: () => ({
     dataLoading: [
       true,
       {
         loadCuration: () => true,
         onCurationLoad: () => false,
+        onCurationError: () => false,
       },
     ],
     curation: [
@@ -58,6 +69,36 @@ export const CurationLogic = kea<MakeLogicType<CurationValues, CurationActions,
         onCurationLoad: (_, { curation }) => curation,
       },
     ],
+    queries: [
+      [],
+      {
+        onCurationLoad: (_, { curation }) => curation.queries,
+        updateQueries: (_, { queries }) => queries,
+      },
+    ],
+    queriesLoading: [
+      false,
+      {
+        updateQueries: () => true,
+        onCurationLoad: () => false,
+        onCurationError: () => false,
+      },
+    ],
+    activeQuery: [
+      '',
+      {
+        setActiveQuery: (_, { query }) => query,
+        onCurationLoad: (activeQuery, { curation }) => activeQuery || curation.queries[0],
+      },
+    ],
+    organicDocumentsLoading: [
+      false,
+      {
+        setActiveQuery: () => true,
+        onCurationLoad: () => false,
+        onCurationError: () => false,
+      },
+    ],
   }),
   listeners: ({ actions, values, props }) => ({
     loadCuration: async () => {
@@ -76,7 +117,7 @@ export const CurationLogic = kea<MakeLogicType<CurationValues, CurationActions,
         navigateToUrl(generateEnginePath(ENGINE_CURATIONS_PATH));
       }
     },
-    updateCuration: async ({ queries }, breakpoint) => {
+    updateCuration: async (_, breakpoint) => {
       const { http } = HttpLogic.values;
       const { engineName } = EngineLogic.values;
 
@@ -88,8 +129,8 @@ export const CurationLogic = kea<MakeLogicType<CurationValues, CurationActions,
           `/api/app_search/engines/${engineName}/curations/${props.curationId}`,
           {
             body: JSON.stringify({
-              queries: queries || values.curation.queries,
-              query: '', // TODO: activeQuery state
+              queries: values.queries,
+              query: values.activeQuery,
               promoted: [], // TODO: promotedIds state
               hidden: [], // TODO: hiddenIds state
             }),
@@ -98,7 +139,15 @@ export const CurationLogic = kea<MakeLogicType<CurationValues, CurationActions,
         actions.onCurationLoad(response);
       } catch (e) {
         flashAPIErrors(e);
+        actions.onCurationError();
       }
     },
+    updateQueries: ({ queries }) => {
+      const activeQueryDeleted = !queries.includes(values.activeQuery);
+      if (activeQueryDeleted) actions.setActiveQuery(queries[0]);
+
+      actions.updateCuration();
+    },
+    setActiveQuery: () => actions.updateCuration(),
   }),
 });

x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/documents/index.ts

@@ -0,0 +1,8 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+export { OrganicDocuments } from './organic_documents';