Refactor attribute service (#78414)

* Making saveMethod mandatory in attribute service * Making unwrap method mandatory * Making book embeddable respect new attribute service * Remove savedObjectsClient from attribute service * Add checkForDuplicateTitle method to book embeddable * Make options mandatory on attribute service * Changing Lens attribute service * Somw more typescript fixes * Fixing attribute service typescript and tests * Fixing typescript errors * Unsetting feature flag Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
elastic · Oct 6, 2020 · 989e9c9 · 989e9c9
1 parent ffc1caa
commit 989e9c9
Show file tree

Hide file tree

Showing 14 changed files with 261 additions and 153 deletions.
diff --git a/examples/embeddable_examples/kibana.json b/examples/embeddable_examples/kibana.json
@@ -4,7 +4,7 @@
   "kibanaVersion": "kibana",
   "server": true,
   "ui": true,
-  "requiredPlugins": ["embeddable", "uiActions", "dashboard"],
+  "requiredPlugins": ["embeddable", "uiActions", "dashboard", "savedObjects"],
   "optionalPlugins": [],
   "extraPublicDirs": ["public/todo", "public/hello_world", "public/todo/todo_ref_embeddable"],
   "requiredBundles": ["kibanaReact"]

diff --git a/examples/embeddable_examples/public/book/book_embeddable_factory.tsx b/examples/embeddable_examples/public/book/book_embeddable_factory.tsx
@@ -33,12 +33,19 @@ import {
   BookEmbeddableOutput,
 } from './book_embeddable';
 import { CreateEditBookComponent } from './create_edit_book_component';
-import { OverlayStart } from '../../../../src/core/public';
+import {
+  OverlayStart,
+  SavedObjectsClientContract,
+  SimpleSavedObject,
+} from '../../../../src/core/public';
 import { DashboardStart, AttributeService } from '../../../../src/plugins/dashboard/public';
+import { checkForDuplicateTitle, OnSaveProps } from '../../../../src/plugins/saved_objects/public';
 
 interface StartServices {
   getAttributeService: DashboardStart['getAttributeService'];
   openModal: OverlayStart['openModal'];
+  savedObjectsClient: SavedObjectsClientContract;
+  overlays: OverlayStart;
 }
 
 export type BookEmbeddableFactory = EmbeddableFactory<
@@ -117,11 +124,55 @@ export class BookEmbeddableFactoryDefinition
     });
   }
 
+  private async unwrapMethod(savedObjectId: string): Promise<BookSavedObjectAttributes> {
+    const { savedObjectsClient } = await this.getStartServices();
+    const savedObject: SimpleSavedObject<BookSavedObjectAttributes> = await savedObjectsClient.get<
+      BookSavedObjectAttributes
+    >(this.type, savedObjectId);
+    return { ...savedObject.attributes };
+  }
+
+  private async saveMethod(
+    type: string,
+    attributes: BookSavedObjectAttributes,
+    savedObjectId?: string
+  ) {
+    const { savedObjectsClient } = await this.getStartServices();
+    if (savedObjectId) {
+      return savedObjectsClient.update(type, savedObjectId, attributes);
+    }
+    return savedObjectsClient.create(type, attributes);
+  }
+
+  private async checkForDuplicateTitleMethod(props: OnSaveProps): Promise<true> {
+    const start = await this.getStartServices();
+    const { savedObjectsClient, overlays } = start;
+    return checkForDuplicateTitle(
+      {
+        title: props.newTitle,
+        copyOnSave: false,
+        lastSavedTitle: '',
+        getEsType: () => this.type,
+        getDisplayName: this.getDisplayName || (() => this.type),
+      },
+      props.isTitleDuplicateConfirmed,
+      props.onTitleDuplicate,
+      {
+        savedObjectsClient,
+        overlays,
+      }
+    );
+  }
+
   private async getAttributeService() {
     if (!this.attributeService) {
-      this.attributeService = await (await this.getStartServices()).getAttributeService<
+      this.attributeService = (await this.getStartServices()).getAttributeService<
         BookSavedObjectAttributes
-      >(this.type);
+      >(this.type, {
+        saveMethod: this.saveMethod.bind(this),
+        unwrapMethod: this.unwrapMethod.bind(this),
+        checkForDuplicateTitle: this.checkForDuplicateTitleMethod.bind(this),
+      });
     }
     return this.attributeService!;
   }

diff --git a/examples/embeddable_examples/public/book/edit_book_action.tsx b/examples/embeddable_examples/public/book/edit_book_action.tsx
@@ -31,10 +31,13 @@ import {
 } from './book_embeddable';
 import { CreateEditBookComponent } from './create_edit_book_component';
 import { DashboardStart } from '../../../../src/plugins/dashboard/public';
+import { OnSaveProps } from '../../../../src/plugins/saved_objects/public';
+import { SavedObjectsClientContract } from '../../../../src/core/target/types/public/saved_objects';
 
 interface StartServices {
   openModal: OverlayStart['openModal'];
   getAttributeService: DashboardStart['getAttributeService'];
+  savedObjectsClient: SavedObjectsClientContract;
 }
 
 interface ActionContext {
@@ -56,8 +59,24 @@ export const createEditBookAction = (getStartServices: () => Promise<StartServic
       );
     },
     execute: async ({ embeddable }: ActionContext) => {
-      const { openModal, getAttributeService } = await getStartServices();
-      const attributeService = getAttributeService<BookSavedObjectAttributes>(BOOK_SAVED_OBJECT);
+      const { openModal, getAttributeService, savedObjectsClient } = await getStartServices();
+      const attributeService = getAttributeService<BookSavedObjectAttributes>(BOOK_SAVED_OBJECT, {
+        saveMethod: async (
+          type: string,
+          attributes: BookSavedObjectAttributes,
+          savedObjectId?: string
+        ) => {
+          if (savedObjectId) {
+            return savedObjectsClient.update(type, savedObjectId, attributes);
+          }
+          return savedObjectsClient.create(type, attributes);
+        },
+        checkForDuplicateTitle: (props: OnSaveProps) => {
+          return new Promise(() => {
+            return true;
+          });
+        },
+      });
       const onSave = async (attributes: BookSavedObjectAttributes, useRefType: boolean) => {
         const newInput = await attributeService.wrapAttributes(
           attributes,

diff --git a/examples/embeddable_examples/public/plugin.ts b/examples/embeddable_examples/public/plugin.ts
@@ -22,7 +22,7 @@ import {
   EmbeddableStart,
   CONTEXT_MENU_TRIGGER,
 } from '../../../src/plugins/embeddable/public';
-import { Plugin, CoreSetup, CoreStart } from '../../../src/core/public';
+import { Plugin, CoreSetup, CoreStart, SavedObjectsClient } from '../../../src/core/public';
 import {
   HelloWorldEmbeddableFactory,
   HELLO_WORLD_EMBEDDABLE,
@@ -76,6 +76,7 @@ export interface EmbeddableExamplesSetupDependencies {
 export interface EmbeddableExamplesStartDependencies {
   embeddable: EmbeddableStart;
   dashboard: DashboardStart;
+  savedObjectsClient: SavedObjectsClient;
 }
 
 interface ExampleEmbeddableFactories {
@@ -158,12 +159,15 @@ export class EmbeddableExamplesPlugin
       new BookEmbeddableFactoryDefinition(async () => ({
         getAttributeService: (await core.getStartServices())[1].dashboard.getAttributeService,
         openModal: (await core.getStartServices())[0].overlays.openModal,
+        savedObjectsClient: (await core.getStartServices())[0].savedObjects.client,
+        overlays: (await core.getStartServices())[0].overlays,
       }))
     );
 
     const editBookAction = createEditBookAction(async () => ({
       getAttributeService: (await core.getStartServices())[1].dashboard.getAttributeService,
       openModal: (await core.getStartServices())[0].overlays.openModal,
+      savedObjectsClient: (await core.getStartServices())[0].savedObjects.client,
     }));
     deps.uiActions.registerAction(editBookAction);
     deps.uiActions.attachAction(CONTEXT_MENU_TRIGGER, editBookAction.id);

diff --git a/src/plugins/dashboard/public/attribute_service/attribute_service.mock.tsx b/src/plugins/dashboard/public/attribute_service/attribute_service.mock.tsx
@@ -31,19 +31,16 @@ export const mockAttributeService = <
   R extends SavedObjectEmbeddableInput = SavedObjectEmbeddableInput
 >(
   type: string,
-  options?: AttributeServiceOptions<A>,
+  options: AttributeServiceOptions<A>,
   customCore?: jest.Mocked<CoreStart>
 ): AttributeService<A, V, R> => {
   const core = customCore ? customCore : coreMock.createStart();
-  const service = new AttributeService<A, V, R>(
+  return new AttributeService<A, V, R>(
     type,
     jest.fn(),
-    core.savedObjects.client,
-    core.overlays,
     core.i18n.Context,
     core.notifications.toasts,
-    jest.fn().mockReturnValue(() => ({ getDisplayName: () => type })),
-    options
+    options,
+    jest.fn().mockReturnValue(() => ({ getDisplayName: () => type }))
   );
-  return service;
 };
diff --git a/src/plugins/dashboard/public/attribute_service/attribute_service.test.ts b/src/plugins/dashboard/public/attribute_service/attribute_service.test.ts
@@ -20,6 +20,7 @@
 import { ATTRIBUTE_SERVICE_KEY } from './attribute_service';
 import { mockAttributeService } from './attribute_service.mock';
 import { coreMock } from '../../../../core/public/mocks';
+import { OnSaveProps } from '../../../saved_objects/public/save_modal';
 
 interface TestAttributes {
   title: string;
@@ -37,6 +38,30 @@ describe('attributeService', () => {
   let attributes: TestAttributes;
   let byValueInput: TestByValueInput;
   let byReferenceInput: { id: string; savedObjectId: string };
+  const defaultSaveMethod = (
+    type: string,
+    testAttributes: TestAttributes,
+    savedObjectId?: string
+  ): Promise<{ id: string }> => {
+    return new Promise(() => {
+      return { id: '123' };
+    });
+  };
+  const defaultUnwrapMethod = (savedObjectId: string): Promise<TestAttributes> => {
+    return new Promise(() => {
+      return { ...attributes };
+    });
+  };
+  const defaultCheckForDuplicateTitle = (props: OnSaveProps): Promise<true> => {
+    return new Promise(() => {
+      return true;
+    });
+  };
+  const options = {
+    saveMethod: defaultSaveMethod,
+    unwrapMethod: defaultUnwrapMethod,
+    checkForDuplicateTitle: defaultCheckForDuplicateTitle,
+  };
 
   beforeEach(() => {
     attributes = {
@@ -55,9 +80,10 @@ describe('attributeService', () => {
   });
 
   describe('determining input type', () => {
-    const defaultAttributeService = mockAttributeService<TestAttributes>(defaultTestType);
+    const defaultAttributeService = mockAttributeService<TestAttributes>(defaultTestType, options);
     const customAttributeService = mockAttributeService<TestAttributes, TestByValueInput>(
-      defaultTestType
+      defaultTestType,
+      options
     );
 
     it('can determine input type given default types', () => {
@@ -85,39 +111,32 @@ describe('attributeService', () => {
   });
 
   describe('unwrapping attributes', () => {
-    it('can unwrap all default attributes when given reference type input', async () => {
-      const core = coreMock.createStart();
-      core.savedObjects.client.get = jest.fn().mockResolvedValueOnce({
-        attributes,
+    it('does not throw error when given reference type input with no unwrap method', async () => {
+      const attributeService = mockAttributeService<TestAttributes>(defaultTestType, {
+        saveMethod: defaultSaveMethod,
+        checkForDuplicateTitle: jest.fn(),
       });
-      const attributeService = mockAttributeService<TestAttributes>(
-        defaultTestType,
-        undefined,
-        core
-      );
-      expect(await attributeService.unwrapAttributes(byReferenceInput)).toEqual(attributes);
+      expect(await attributeService.unwrapAttributes(byReferenceInput)).toEqual(byReferenceInput);
     });
 
     it('returns attributes when when given value type input', async () => {
-      const attributeService = mockAttributeService<TestAttributes>(defaultTestType);
+      const attributeService = mockAttributeService<TestAttributes>(defaultTestType, options);
       expect(await attributeService.unwrapAttributes(byValueInput)).toEqual(attributes);
     });
 
     it('runs attributes through a custom unwrap method', async () => {
-      const core = coreMock.createStart();
-      core.savedObjects.client.get = jest.fn().mockResolvedValueOnce({
-        attributes,
-      });
-      const attributeService = mockAttributeService<TestAttributes>(
-        defaultTestType,
-        {
-          customUnwrapMethod: (savedObject) => ({
-            ...savedObject.attributes,
-            testAttr2: { array: [1, 2, 3, 4, 5], testAttr3: 'kibanana' },
-          }),
+      const attributeService = mockAttributeService<TestAttributes>(defaultTestType, {
+        saveMethod: defaultSaveMethod,
+        unwrapMethod: (savedObjectId) => {
+          return new Promise((resolve) => {
+            return resolve({
+              ...attributes,
+              testAttr2: { array: [1, 2, 3, 4, 5], testAttr3: 'kibanana' },
+            });
+          });
         },
-        core
-      );
+        checkForDuplicateTitle: jest.fn(),
+      });
       expect(await attributeService.unwrapAttributes(byReferenceInput)).toEqual({
         ...attributes,
         testAttr2: { array: [1, 2, 3, 4, 5], testAttr3: 'kibanana' },
@@ -127,67 +146,57 @@ describe('attributeService', () => {
 
   describe('wrapping attributes', () => {
     it('returns given attributes when use ref type is false', async () => {
-      const attributeService = mockAttributeService<TestAttributes>(defaultTestType);
+      const attributeService = mockAttributeService<TestAttributes>(defaultTestType, options);
       expect(await attributeService.wrapAttributes(attributes, false)).toEqual({ attributes });
     });
 
-    it('updates existing saved object with new attributes when given id', async () => {
+    it('calls saveMethod with appropriate parameters', async () => {
       const core = coreMock.createStart();
+      const saveMethod = jest.fn();
+      saveMethod.mockReturnValueOnce({});
       const attributeService = mockAttributeService<TestAttributes>(
         defaultTestType,
-        undefined,
+        {
+          saveMethod,
+          unwrapMethod: defaultUnwrapMethod,
+          checkForDuplicateTitle: defaultCheckForDuplicateTitle,
+        },
         core
       );
       expect(await attributeService.wrapAttributes(attributes, true, byReferenceInput)).toEqual(
         byReferenceInput
       );
-      expect(core.savedObjects.client.update).toHaveBeenCalledWith(
-        defaultTestType,
-        '123',
-        attributes
-      );
-    });
-
-    it('creates new saved object with attributes when given no id', async () => {
-      const core = coreMock.createStart();
-      core.savedObjects.client.create = jest.fn().mockResolvedValueOnce({
-        id: '678',
-      });
-      const attributeService = mockAttributeService<TestAttributes>(
-        defaultTestType,
-        undefined,
-        core
-      );
-      expect(await attributeService.wrapAttributes(attributes, true)).toEqual({
-        savedObjectId: '678',
-      });
-      expect(core.savedObjects.client.create).toHaveBeenCalledWith(defaultTestType, attributes);
+      expect(saveMethod).toHaveBeenCalledWith(defaultTestType, attributes, '123');
     });
 
     it('uses custom save method when given an id', async () => {
-      const customSaveMethod = jest.fn().mockReturnValue({ id: '123' });
+      const saveMethod = jest.fn().mockReturnValue({ id: '123' });
       const attributeService = mockAttributeService<TestAttributes>(defaultTestType, {
-        customSaveMethod,
+        saveMethod,
+        unwrapMethod: defaultUnwrapMethod,
+        checkForDuplicateTitle: defaultCheckForDuplicateTitle,
       });
       expect(await attributeService.wrapAttributes(attributes, true, byReferenceInput)).toEqual(
         byReferenceInput
       );
-      expect(customSaveMethod).toHaveBeenCalledWith(
+      expect(saveMethod).toHaveBeenCalledWith(
         defaultTestType,
         attributes,
         byReferenceInput.savedObjectId
       );
     });
 
     it('uses custom save method given no id', async () => {
-      const customSaveMethod = jest.fn().mockReturnValue({ id: '678' });
+      const saveMethod = jest.fn().mockReturnValue({ id: '678' });
       const attributeService = mockAttributeService<TestAttributes>(defaultTestType, {
-        customSaveMethod,
+        saveMethod,
+        unwrapMethod: defaultUnwrapMethod,
+        checkForDuplicateTitle: defaultCheckForDuplicateTitle,
       });
       expect(await attributeService.wrapAttributes(attributes, true)).toEqual({
         savedObjectId: '678',
       });
-      expect(customSaveMethod).toHaveBeenCalledWith(defaultTestType, attributes, undefined);
+      expect(saveMethod).toHaveBeenCalledWith(defaultTestType, attributes, undefined);
     });
   });
 });