Changes for platform team review feedbck

docs/api/saved-objects/find.asciidoc

@@ -44,7 +44,12 @@ experimental[] Retrieve a paginated set of {kib} saved objects by various condit
   (Optional, array|string) The fields to return in the `attributes` key of the response.
 
 `sort_field`::
-  (Optional, string) The field that sorts the response.
+  (Optional, string) The field that sorts the response. There are two kinds of fields: "root" fields that exist for all saved objects (such
+  as "updated_at"), and "type" fields that are specific to a given object type (e.g. those fields that are returned in the `attributes` key
+  of the response).
+    * If a single type is defined in the `type` parameter, both "type" fields and "root" fields are allowed, and validity checks are made in
+    that order.
+    * If multiple types are defined in the `type` parameter, only "root" fields are allowed.
 
 `has_reference`::
   (Optional, object) Filters to objects that have a relationship with the type and ID combination.

src/core/server/saved_objects/import/import_saved_objects.test.ts

@@ -57,7 +57,7 @@ describe('#importSavedObjectsFromStream', () => {
       importIdMap: new Map(),
     });
     getMockFn(regenerateIds).mockReturnValue(new Map());
-    getMockFn(validateReferences).mockResolvedValue({ errors: [] });
+    getMockFn(validateReferences).mockResolvedValue([]);
     getMockFn(checkConflicts).mockResolvedValue({
       errors: [],
       filteredObjects: [],
@@ -228,9 +228,7 @@ describe('#importSavedObjectsFromStream', () => {
             ['baz', {}],
           ]),
         });
-        getMockFn(validateReferences).mockResolvedValue({
-          errors: [errors[1]],
-        });
+        getMockFn(validateReferences).mockResolvedValue([errors[1]]);
         getMockFn(checkConflicts).mockResolvedValue({
           errors: [errors[2]],
           filteredObjects,
@@ -277,7 +275,7 @@ describe('#importSavedObjectsFromStream', () => {
 
       test('does not check conflicts or check origin conflicts', async () => {
         const options = setupOptions(true);
-        getMockFn(validateReferences).mockResolvedValue({ errors: [] });
+        getMockFn(validateReferences).mockResolvedValue([]);
 
         await importSavedObjectsFromStream(options);
         expect(checkConflicts).not.toHaveBeenCalled();
@@ -296,7 +294,7 @@ describe('#importSavedObjectsFromStream', () => {
             ['bar', {}],
           ]),
         });
-        getMockFn(validateReferences).mockResolvedValue({ errors: [errors[1]] });
+        getMockFn(validateReferences).mockResolvedValue([errors[1]]);
         // this importIdMap is not composed with the one obtained from `collectSavedObjects`
         const importIdMap = new Map().set(`id1`, { id: `newId1` });
         getMockFn(regenerateIds).mockReturnValue(importIdMap);
@@ -429,7 +427,7 @@ describe('#importSavedObjectsFromStream', () => {
         collectedObjects: [],
         importIdMap: new Map(), // doesn't matter
       });
-      getMockFn(validateReferences).mockResolvedValue({ errors: [errors[1]] });
+      getMockFn(validateReferences).mockResolvedValue([errors[1]]);
       getMockFn(checkConflicts).mockResolvedValue({
         errors: [errors[2]],
         filteredObjects: [],

src/core/server/saved_objects/import/import_saved_objects.ts

@@ -64,7 +64,7 @@ export async function importSavedObjectsFromStream({
     savedObjectsClient,
     namespace
   );
-  errorAccumulator = [...errorAccumulator, ...validateReferencesResult.errors];
+  errorAccumulator = [...errorAccumulator, ...validateReferencesResult];
 
   if (createNewCopies) {
     importIdMap = regenerateIds(collectSavedObjectsResult.collectedObjects);

src/core/server/saved_objects/import/resolve_import_errors.test.ts

@@ -66,7 +66,7 @@ describe('#importSavedObjectsFromStream', () => {
       importIdMap: new Map(),
     });
     getMockFn(regenerateIds).mockReturnValue(new Map());
-    getMockFn(validateReferences).mockResolvedValue({ errors: [] });
+    getMockFn(validateReferences).mockResolvedValue([]);
     getMockFn(checkConflicts).mockResolvedValue({
       errors: [],
       filteredObjects: [],
@@ -303,9 +303,7 @@ describe('#importSavedObjectsFromStream', () => {
           collectedObjects: [], // doesn't matter
           importIdMap: new Map(), // doesn't matter
         });
-        getMockFn(validateReferences).mockResolvedValue({
-          errors: [errors[1]],
-        });
+        getMockFn(validateReferences).mockResolvedValue([errors[1]]);
         getMockFn(checkConflicts).mockResolvedValue({
           errors: [errors[2]],
           filteredObjects: [],
@@ -371,9 +369,7 @@ describe('#importSavedObjectsFromStream', () => {
           collectedObjects: [], // doesn't matter
           importIdMap: new Map(), // doesn't matter
         });
-        getMockFn(validateReferences).mockResolvedValue({
-          errors: [errors[1]],
-        });
+        getMockFn(validateReferences).mockResolvedValue([errors[1]]);
         getMockFn(regenerateIds).mockReturnValue(
           new Map([
             ['foo', { id: 'randomId1' }],
@@ -503,7 +499,7 @@ describe('#importSavedObjectsFromStream', () => {
         collectedObjects: [],
         importIdMap: new Map(), // doesn't matter
       });
-      getMockFn(validateReferences).mockResolvedValue({ errors: [errors[1]] });
+      getMockFn(validateReferences).mockResolvedValue([errors[1]]);
       getMockFn(createSavedObjects).mockResolvedValueOnce({
         errors: [errors[2]],
         createdObjects: [],

src/core/server/saved_objects/import/resolve_import_errors.ts

@@ -99,7 +99,7 @@ export async function resolveSavedObjectsImportErrors({
     namespace,
     retries
   );
-  errorAccumulator = [...errorAccumulator, ...validateReferencesResult.errors];
+  errorAccumulator = [...errorAccumulator, ...validateReferencesResult];
 
   if (createNewCopies) {
     // In case any missing reference errors were resolved, ensure that we regenerate those object IDs as well

src/core/server/saved_objects/import/validate_references.test.ts

@@ -254,11 +254,7 @@ describe('validateReferences()', () => {
 
   test('returns empty when no objects are passed in', async () => {
     const result = await validateReferences([], savedObjectsClient);
-    expect(result).toMatchInlineSnapshot(`
-      Object {
-        "errors": Array [],
-      }
-    `);
+    expect(result).toEqual([]);
     expect(savedObjectsClient.bulkGet).toHaveBeenCalledTimes(0);
   });
 
@@ -357,52 +353,50 @@ describe('validateReferences()', () => {
     ];
     const result = await validateReferences(savedObjects, savedObjectsClient);
     expect(result).toMatchInlineSnapshot(`
-      Object {
-        "errors": Array [
-          Object {
-            "error": Object {
-              "references": Array [
-                Object {
-                  "id": "3",
-                  "type": "index-pattern",
-                },
-              ],
-              "type": "missing_references",
-            },
-            "id": "2",
-            "meta": Object {
-              "title": "My Visualization 2",
-            },
+      Array [
+        Object {
+          "error": Object {
+            "references": Array [
+              Object {
+                "id": "3",
+                "type": "index-pattern",
+              },
+            ],
+            "type": "missing_references",
+          },
+          "id": "2",
+          "meta": Object {
             "title": "My Visualization 2",
-            "type": "visualization",
           },
-          Object {
-            "error": Object {
-              "references": Array [
-                Object {
-                  "id": "5",
-                  "type": "index-pattern",
-                },
-                Object {
-                  "id": "6",
-                  "type": "index-pattern",
-                },
-                Object {
-                  "id": "7",
-                  "type": "search",
-                },
-              ],
-              "type": "missing_references",
-            },
-            "id": "4",
-            "meta": Object {
-              "title": "My Visualization 4",
-            },
+          "title": "My Visualization 2",
+          "type": "visualization",
+        },
+        Object {
+          "error": Object {
+            "references": Array [
+              Object {
+                "id": "5",
+                "type": "index-pattern",
+              },
+              Object {
+                "id": "6",
+                "type": "index-pattern",
+              },
+              Object {
+                "id": "7",
+                "type": "search",
+              },
+            ],
+            "type": "missing_references",
+          },
+          "id": "4",
+          "meta": Object {
             "title": "My Visualization 4",
-            "type": "visualization",
           },
-        ],
-      }
+          "title": "My Visualization 4",
+          "type": "visualization",
+        },
+      ]
     `);
     expect(savedObjectsClient.bulkGet).toMatchInlineSnapshot(`
       [MockFunction] {
@@ -479,7 +473,7 @@ describe('validateReferences()', () => {
       },
     ];
     const result = await validateReferences(savedObjects, savedObjectsClient, undefined, retries);
-    expect(result.errors).toHaveLength(0);
+    expect(result).toEqual([]);
     expect(savedObjectsClient.bulkGet).not.toHaveBeenCalled();
   });
 
@@ -509,11 +503,7 @@ describe('validateReferences()', () => {
       },
     ];
     const result = await validateReferences(savedObjects, savedObjectsClient);
-    expect(result).toMatchInlineSnapshot(`
-      Object {
-        "errors": Array [],
-      }
-    `);
+    expect(result).toEqual([]);
     expect(savedObjectsClient.bulkGet).toHaveBeenCalledTimes(1);
   });
 
@@ -539,11 +529,7 @@ describe('validateReferences()', () => {
       },
     ];
     const result = await validateReferences(savedObjects, savedObjectsClient);
-    expect(result).toMatchInlineSnapshot(`
-      Object {
-        "errors": Array [],
-      }
-    `);
+    expect(result).toEqual([]);
     expect(savedObjectsClient.bulkGet).toHaveBeenCalledTimes(0);
   });
 
@@ -568,11 +554,7 @@ describe('validateReferences()', () => {
       },
     ];
     const result = await validateReferences(savedObjects, savedObjectsClient);
-    expect(result).toMatchInlineSnapshot(`
-      Object {
-        "errors": Array [],
-      }
-    `);
+    expect(result).toEqual([]);
     expect(savedObjectsClient.bulkGet).toHaveBeenCalledTimes(0);
   });
 

src/core/server/saved_objects/import/validate_references.ts

@@ -130,7 +130,5 @@ export async function validateReferences(
     };
   });
 
-  return {
-    errors: Object.values(errorMap),
-  };
+  return Object.values(errorMap);
 }

src/plugins/saved_objects_management/public/lib/find_objects.ts

@@ -47,9 +47,7 @@ export async function findObject(
   type: string,
   id: string
 ): Promise<SavedObjectWithMetadata> {
-  const response = await http.get<Record<string, any>>(
+  return await http.get<SavedObjectWithMetadata>(
     `/api/kibana/management/saved_objects/${encodeURIComponent(type)}/${encodeURIComponent(id)}`
   );
-
-  return response as SavedObjectWithMetadata;
 }

src/plugins/saved_objects_management/public/lib/import_file.ts

@@ -18,6 +18,7 @@
  */
 
 import { HttpStart, SavedObjectsImportError } from 'src/core/public';
+import { ImportMode } from '../management_section/objects_table/components/import_mode_control';
 
 interface ImportResponse {
   success: boolean;
@@ -28,8 +29,7 @@ interface ImportResponse {
 export async function importFile(
   http: HttpStart,
   file: File,
-  createNewCopies: boolean,
-  overwrite: boolean
+  { createNewCopies, overwrite }: ImportMode
 ) {
   const formData = new FormData();
   formData.append('file', file);

src/plugins/saved_objects_management/public/lib/process_import_response.ts

@@ -29,7 +29,7 @@ import {
 } from 'src/core/public';
 
 export interface FailedImport {
-  obj: Pick<SavedObjectsImportError, 'id' | 'type' | 'title' | 'meta' | 'overwrite'>;
+  obj: Omit<SavedObjectsImportError, 'error'>;
   error:
     | SavedObjectsImportConflictError
     | SavedObjectsImportAmbiguousConflictError

src/plugins/saved_objects_management/public/management_section/objects_table/components/flyout.test.tsx

@@ -178,7 +178,10 @@ describe('Flyout', () => {
       component.setState({ file: mockFile, isLegacyFile: false });
       await component.instance().import();
 
-      expect(importFileMock).toHaveBeenCalledWith(defaultProps.http, mockFile, false, true);
+      expect(importFileMock).toHaveBeenCalledWith(defaultProps.http, mockFile, {
+        createNewCopies: false,
+        overwrite: true,
+      });
       expect(component.state()).toMatchObject({
         conflictedIndexPatterns: undefined,
         conflictedSavedObjectsLinkedToSavedSearches: undefined,

src/plugins/saved_objects_management/public/management_section/objects_table/components/flyout.tsx

@@ -160,12 +160,11 @@ export class Flyout extends Component<FlyoutProps, FlyoutState> {
   import = async () => {
     const { http } = this.props;
     const { file, importMode } = this.state;
-    const { createNewCopies, overwrite } = importMode;
     this.setState({ status: 'loading', error: undefined });
 
     // Import the file
     try {
-      const response = await importFile(http, file!, createNewCopies, overwrite);
+      const response = await importFile(http, file!, importMode);
       this.setState(processImportResponse(response), () => {
         // Resolve import errors right away if there's no index patterns to match
         // This will ask about overwriting each object, etc
@@ -912,9 +911,9 @@ export class Flyout extends Component<FlyoutProps, FlyoutState> {
     const { conflictingRecord } = this.state;
     if (conflictingRecord) {
       const { conflict } = conflictingRecord;
-      const finish = (overwrite: boolean, destinationId?: string) =>
+      const onFinish = (overwrite: boolean, destinationId?: string) =>
         conflictingRecord.done([overwrite, destinationId]);
-      confirmOverwriteModal = <OverwriteModal {...{ conflict, finish }} />;
+      confirmOverwriteModal = <OverwriteModal {...{ conflict, onFinish }} />;
     }
 
     return (