[Data View Field] Fix popularity score bugs

src/platform/packages/shared/kbn-unified-data-table/src/utils/popularize_field.test.ts

@@ -81,6 +81,31 @@ describe('Popularize field', () => {
     expect(field.count).toEqual(1);
   });
 
+  test('should increment', async () => {
+    const field = {
+      count: 5,
+    };
+    const dataView = {
+      id: 'id',
+      fields: {
+        getByName: () => field,
+      },
+      setFieldCount: jest.fn().mockImplementation((fieldName, count) => {
+        field.count = count;
+      }),
+      isPersisted: () => true,
+    } as unknown as DataView;
+    const fieldName = '@timestamp';
+    const updateSavedObjectMock = jest.fn();
+    const dataViewsService = {
+      updateSavedObject: updateSavedObjectMock,
+    } as unknown as DataViewsContract;
+    const result = await popularizeField(dataView, fieldName, dataViewsService, capabilities);
+    expect(result).toBeUndefined();
+    expect(updateSavedObjectMock).toHaveBeenCalled();
+    expect(field.count).toEqual(6);
+  });
+
   test('hides errors', async () => {
     const field = {
       count: 0,

src/platform/plugins/shared/data_view_field_editor/__jest__/client_integration/field_editor_flyout_content.test.ts

@@ -62,6 +62,7 @@ describe('<FieldEditorFlyoutContent />', () => {
       script: { source: 'emit(true)' },
       customLabel: 'cool demo test field',
       format: { id: 'boolean' },
+      popularity: 1,
     };
 
     const { find } = await setup({ fieldToCreate });
@@ -70,6 +71,7 @@ describe('<FieldEditorFlyoutContent />', () => {
     expect(find('nameField.input').props().value).toBe(fieldToCreate.name);
     expect(find('typeField').props().value).toBe(fieldToCreate.type);
     expect(find('scriptField').props().value).toBe(fieldToCreate.script.source);
+    expect(find('editorFieldCount').props().value).toBe(fieldToCreate.popularity.toString());
   });
 
   test('should accept an "onSave" prop', async () => {
@@ -172,6 +174,26 @@ describe('<FieldEditorFlyoutContent />', () => {
         script: { source: 'echo("hello")' },
         format: null,
       });
+
+      await toggleFormRow('popularity');
+      await fields.updatePopularity('5');
+
+      await waitForUpdates();
+
+      await act(async () => {
+        find('fieldSaveButton').simulate('click');
+        jest.advanceTimersByTime(0); // advance timers to allow the form to validate
+      });
+
+      fieldReturned = onSave.mock.calls[onSave.mock.calls.length - 1][0];
+
+      expect(fieldReturned).toEqual({
+        name: 'someName',
+        type: 'date',
+        script: { source: 'echo("hello")' },
+        format: null,
+        popularity: 5,
+      });
     });
 
     test('should not block validation if no documents could be fetched from server', async () => {

src/platform/plugins/shared/data_view_field_editor/__jest__/client_integration/helpers/common_actions.ts

@@ -42,7 +42,7 @@ export const waitForUpdates = async (testBed?: TestBed) => {
 
 export const getCommonActions = (testBed: TestBed) => {
   const toggleFormRow = async (
-    row: 'customLabel' | 'customDescription' | 'value' | 'format',
+    row: 'customLabel' | 'customDescription' | 'value' | 'format' | 'popularity',
     value: 'on' | 'off' = 'on'
   ) => {
     const testSubj = `${row}Row.toggle`;
@@ -102,6 +102,15 @@ export const getCommonActions = (testBed: TestBed) => {
     testBed.component.update();
   };
 
+  const updatePopularity = async (value: string) => {
+    await act(async () => {
+      testBed.form.setInputValue('editorFieldCount', value);
+      jest.advanceTimersByTime(0); // advance timers to allow the form to validate
+    });
+
+    testBed.component.update();
+  };
+
   const getScriptError = () => {
     const scriptError = testBed.component.find('#runtimeFieldScript-error-0');
 
@@ -123,6 +132,7 @@ export const getCommonActions = (testBed: TestBed) => {
       updateType,
       updateScript,
       updateFormat,
+      updatePopularity,
       getScriptError,
     },
   };

src/platform/plugins/shared/data_view_field_editor/public/components/field_editor/field_editor.tsx

@@ -43,9 +43,11 @@ export interface FieldEditorFormState {
   submit: FormHook<Field>['submit'];
 }
 
-export interface FieldFormInternal extends Omit<Field, 'type' | 'internalType' | 'fields'> {
+export interface FieldFormInternal
+  extends Omit<Field, 'type' | 'internalType' | 'fields' | 'popularity'> {
   fields?: Record<string, { type: RuntimePrimitiveTypes }>;
   type: TypeSelection;
+  popularity?: string;
   __meta__: {
     isCustomLabelVisible: boolean;
     isCustomDescriptionVisible: boolean;
@@ -84,6 +86,7 @@ const formDeserializer = (field: Field): FieldFormInternal => {
 
   return {
     ...field,
+    popularity: typeof field.popularity === 'number' ? String(field.popularity) : field.popularity,
     type: fieldType,
     format,
     __meta__: {
@@ -97,12 +100,15 @@ const formDeserializer = (field: Field): FieldFormInternal => {
 };
 
 const formSerializer = (field: FieldFormInternal): Field => {
-  const { __meta__, type, format, ...rest } = field;
+  const { __meta__, type, format, popularity, ...rest } = field;
+
   return {
     type: type && type[0].value!,
     // By passing "null" we are explicitly telling DataView to remove the
     // format if there is one defined for the field.
     format: format === undefined ? null : format,
+    // convert from the input string value into a number
+    popularity: typeof popularity === 'string' ? Number(popularity) || 0 : popularity,
     ...rest,
   };
 };

src/platform/plugins/shared/data_view_field_editor/public/open_editor.tsx

@@ -130,10 +130,17 @@ export const getFieldEditorOpener =
         };
       };
 
-      const dataViewLazy =
-        dataViewLazyOrNot instanceof DataViewLazy
-          ? dataViewLazyOrNot
-          : await dataViews.toDataViewLazy(dataViewLazyOrNot);
+      let dataViewLazy: DataViewLazy;
+
+      if (dataViewLazyOrNot instanceof DataViewLazy) {
+        dataViewLazy = dataViewLazyOrNot;
+      } else {
+        if (dataViewLazyOrNot.id) {
+          // force cache reset to have the latest field attributes
+          dataViews.clearDataViewLazyCache(dataViewLazyOrNot.id);
+        }
+        dataViewLazy = await dataViews.toDataViewLazy(dataViewLazyOrNot);
+      }
 
       const dataViewField = fieldNameToEdit
         ? (await dataViewLazy.getFieldByName(fieldNameToEdit, true)) ||

src/platform/plugins/shared/data_views/common/data_views/abstract_data_views.ts

@@ -365,8 +365,15 @@ export abstract class AbstractDataView {
   getAsSavedObjectBody(): DataViewAttributes {
     const stringifyOrUndefined = (obj: any) => (obj ? JSON.stringify(obj) : undefined);
 
+    const fieldAttrsWithValues: Record<string, FieldAttrSet> = {};
+    this.fieldAttrs.forEach((attrs, fieldName) => {
+      if (Object.keys(attrs).length) {
+        fieldAttrsWithValues[fieldName] = attrs;
+      }
+    });
+
     return {
-      fieldAttrs: stringifyOrUndefined(Object.fromEntries(this.fieldAttrs.entries())),
+      fieldAttrs: stringifyOrUndefined(fieldAttrsWithValues),
       title: this.getIndexPattern(),
       timeFieldName: this.timeFieldName,
       sourceFilters: stringifyOrUndefined(this.sourceFilters),
@@ -382,8 +389,10 @@ export abstract class AbstractDataView {
   }
 
   protected toSpecShared(includeFields = true): DataViewSpec {
+    // `cloneDeep` is added to make sure that the original fieldAttrs map is not modified with the following `delete` operation.
+    const fieldAttrs = cloneDeep(Object.fromEntries(this.fieldAttrs.entries()));
+
     // if fields aren't included, don't include count
-    const fieldAttrs = Object.fromEntries(this.fieldAttrs.entries());
     if (!includeFields) {
       Object.keys(fieldAttrs).forEach((key) => {
         delete fieldAttrs[key].count;

src/platform/plugins/shared/data_views/common/data_views/data_view.test.ts

@@ -396,6 +396,22 @@ describe('IndexPattern', () => {
       indexPattern.removeRuntimeField(newField);
     });
 
+    test('add and remove a popularity score from a runtime field', () => {
+      const newField = 'new_field_test';
+      indexPattern.addRuntimeField(newField, {
+        ...runtimeWithAttrs,
+        popularity: 10,
+      });
+      expect(indexPattern.getFieldByName(newField)?.count).toEqual(10);
+      indexPattern.setFieldCount(newField, 20);
+      expect(indexPattern.getFieldByName(newField)?.count).toEqual(20);
+      indexPattern.setFieldCount(newField, null);
+      expect(indexPattern.getFieldByName(newField)?.count).toEqual(0);
+      indexPattern.setFieldCount(newField, undefined);
+      expect(indexPattern.getFieldByName(newField)?.count).toEqual(0);
+      indexPattern.removeRuntimeField(newField);
+    });
+
     test('add and remove composite runtime field as new fields', () => {
       const fieldCount = indexPattern.fields.length;
       indexPattern.addRuntimeField('new_field', runtimeCompositeWithAttrs);
@@ -505,6 +521,19 @@ describe('IndexPattern', () => {
       const dataView2 = create('test2', spec);
       expect(dataView1.sourceFilters).not.toBe(dataView2.sourceFilters);
     });
+
+    test('getting spec without fields does not modify fieldAttrs', () => {
+      const fieldAttrs = { bytes: { count: 5, customLabel: 'test_bytes' }, agent: { count: 1 } };
+      const dataView = new DataView({
+        fieldFormats: fieldFormatsMock,
+        spec: {
+          fieldAttrs,
+        },
+      });
+      const spec = dataView.toSpec(false);
+      expect(spec.fieldAttrs).toEqual({ bytes: { customLabel: fieldAttrs.bytes.customLabel } });
+      expect(JSON.parse(dataView.getAsSavedObjectBody().fieldAttrs!)).toEqual(fieldAttrs);
+    });
   });
 
   describe('should initialize from spec with field attributes', () => {

src/platform/plugins/shared/data_views/common/data_views/data_view_lazy.test.ts

@@ -480,6 +480,23 @@ describe('DataViewLazy', () => {
       dataViewLazy.removeRuntimeField(newField);
     });
 
+    test('add and remove a popularity score from a runtime field', async () => {
+      const newField = 'new_field_test';
+      fieldCapsResponse = [];
+      dataViewLazy.addRuntimeField(newField, {
+        ...runtimeWithAttrs,
+        popularity: 10,
+      });
+      expect((await dataViewLazy.getFieldByName(newField))?.count).toEqual(10);
+      dataViewLazy.setFieldCount(newField, 20);
+      expect((await dataViewLazy.getFieldByName(newField))?.count).toEqual(20);
+      dataViewLazy.setFieldCount(newField, null);
+      expect((await dataViewLazy.getFieldByName(newField))?.count).toEqual(0);
+      dataViewLazy.setFieldCount(newField, undefined);
+      expect((await dataViewLazy.getFieldByName(newField))?.count).toEqual(0);
+      dataViewLazy.removeRuntimeField(newField);
+    });
+
     test('add and remove composite runtime field as new fields', async () => {
       const fieldMap = (await dataViewLazy.getFields({ fieldName: ['*'] })).getFieldMap();
       const fieldCount = Object.values(fieldMap).length;

src/platform/plugins/shared/data_views/common/data_views/data_views.test.ts

@@ -51,8 +51,9 @@ const savedObject = {
     typeMeta: '{}',
     type: '',
     runtimeFieldMap:
-      '{"aRuntimeField": { "type": "keyword", "script": {"source": "emit(\'hello\')"}}}',
-    fieldAttrs: '{"aRuntimeField": { "count": 5, "customLabel": "A Runtime Field"}}',
+      '{"aRuntimeField": { "type": "keyword", "script": {"source": "emit(\'hello\')"}}, "wrongCountType": { "type": "keyword", "script": {"source": "emit(\'test\')"}}}',
+    fieldAttrs:
+      '{"aRuntimeField": { "count": 5, "customLabel": "A Runtime Field" }, "wrongCountType": { "count": "50" }}',
   },
   type: 'index-pattern',
   references: [],
@@ -682,6 +683,23 @@ describe('IndexPatterns', () => {
     expect(attrs.fieldFormatMap).toMatchInlineSnapshot(`"{}"`);
   });
 
+  test('gets the correct field attrs', async () => {
+    const id = 'id';
+    setDocsourcePayload(id, savedObject);
+    const dataView = await indexPatterns.get(id);
+    expect(dataView.getFieldByName('aRuntimeField')).toEqual(
+      expect.objectContaining({
+        count: 5,
+        customLabel: 'A Runtime Field',
+      })
+    );
+    expect(dataView.getFieldByName('wrongCountType')).toEqual(
+      expect.objectContaining({
+        count: 50,
+      })
+    );
+  });
+
   describe('defaultDataViewExists', () => {
     beforeEach(() => {
       indexPatterns.clearCache();