[Lens] Add suffix formatter

[flash1293]

Fixes #76714

This PR adds a new "suffix" formatter which works similar to the range formatter. It's taking a nested serialized field format object and uses it to format the value itself, then adds the suffix in the back. As a param it takes a unit which can be passed to the time_scale function as well.

This PR doesn't handle the label change - I imagine this to be part of the PR piecing all the parts together (UI to enable this, setting the label, setting the outer formatter, writing the time_scale expression function)

Testing

To test, replace the buildColumn function in x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx by this:

    buildColumn: ({ suggestedPriority, field, previousColumn }) => ({
      label: ofName(field.displayName),
      dataType: 'number',
      operationType: type,
      suggestedPriority,
      sourceField: field.name,
      isBucketed: false,
      scale: 'ratio',
      params:
        // TODO just for testing
        previousColumn && previousColumn.dataType === 'number'
          ? previousColumn.params
          : {
              parentFormat: {
                id: 'suffix',
                params: {
                  unit: 'h',
                },
              },
            },
    }),

Then configure a chart using average/min/max/median operations.

Related changes

In x-pack/plugins/lens/public/indexpattern_datasource/format_column.ts, I had to make sure the id of the outer format is used correctly. This wasn't an issue for range because it only replaced params on an already existing outer format.

[elasticmachine]

Pinging @elastic/kibana-app (Team:KibanaApp)

[wylieconlon]

It looks like the field-specific default formatter is not being respected in all cases. Steps to reproduce:

1. Apply the code change you suggested
2. Drag a numeric field into the workspace
3. Hover over the tooltip

Other than this issue the behavior LGTM

[flash1293]

@elasticmachine merge upstream

[flash1293]

@wylieconlon Thanks for the review, the nested params were not passed along the right way. I fixed it by explicitly handling the separate cases.

format_column definitely needs unit tests, I will add them on a separate PR if that's OK.

[dej611]

Not sure I proposed an improvement over the current one, but I got lost parsing all the branchings in the format_column body

[dej611]

I find this bit a bit hard to wrap my head around.
I would propose something alternative:

const isNested = isNestedFormat(col.meta.params);
const innerParams = isNested ? {id: col.meta.params?.id, params: col.meta.params?.params } : col.meta.params?.params;

const formatId = isNested ? col.meta.params.id : parentFormatId;

return withParams(col, {
    ...col.meta.params,
    id: formatId,
    params: {
      ...innerParams,
      ...parentFormatParams
    }
});

[flash1293]

@dej611 I refactored the code like this, however there is a bug in there - the first ternary condition calculating the innerParams is flipped (it should be the nested params if the original formatter was nested, not the other way around)

[mbondyra]

Functionality works great and code looks good to me. The only thing that doesn't work so well is the test case provided (try switching from count of records or cardinality to average - the suffix won't be applied). The right solution would be in this case to add suffix in both cases:

buildColumn: ({ field, previousColumn }) => ({
      label: ofName(field.displayName),
      dataType: 'number',
      operationType: type,
      sourceField: field.name,
      isBucketed: false,
      scale: 'ratio',
      params:
        // TODO just for testing
        previousColumn && previousColumn.dataType === 'number'
          ?{
             ...previousColumn.params,
              parentFormat: {
                id: 'suffix',
                params: {
                  unit: 'h',
                },
              },
            } 
          : {
              parentFormat: {
                id: 'suffix',
                params: {
                  unit: 'h',
                },
              },
            },
    }),

It doesn't influence the working code though so PR approved :)

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 025011e

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
lens	432	433	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
lens	911.0KB	913.3KB	+2.3KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
lens	50.3KB	50.5KB	+151.0B

History

• 💚 Build #87136 succeeded 83f576c
• 💚 Build #86645 succeeded f34e06b
• 💚 Build #86448 succeeded eb55c77
• 💔 Build #86439 failed 63823c2

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[wylieconlon]

Haven't tested it or seriously reviewed again, but I don't want to block it any more.

Changes were made