Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix type errors in formula.test.tsx #176343

Closed
nickofthyme opened this issue Feb 6, 2024 · 1 comment · Fixed by #178507
Closed

Fix type errors in formula.test.tsx #176343

nickofthyme opened this issue Feb 6, 2024 · 1 comment · Fixed by #178507
Assignees
Labels
Feature:Lens impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:Visualizations Visualization editors, elastic-charts and infrastructure technical debt Improvement of the software architecture and operational architecture

Comments

@nickofthyme
Copy link
Contributor

Fix typescript type errors from #175178 removing @ts-expect-error comments.

x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/formula.test.tsx

This error involves types that are a bit complex, a general approach in described below. See #176114 (review) for more details.

Here is a commit a20a96c with a start to the fix but is missing the correct params type.


The mock function of createOperationDefinitionMock is trying to conform to multiple type definitions in a single function.

I reworked the types to pass a generic type based on the input type, with it defaulting to the field type.

This works for almost all cases, haven't checked other files 🤞🏼, except for used with the label: 'moving_average', see next comment 👇🏼 .

When defining the moving average definition like here...

moving_average: createOperationDefinitionMock('moving_average', {
input: 'fullReference',
operationParams: [{ name: 'window', type: 'number', required: true }],
filterable: true,
getErrorMessage: jest.fn(() => ['mock error']),
buildColumn: ({ referenceIds }, columnsParams) => ({
label: 'moving_average',
dataType: 'number',
operationType: 'moving_average',
isBucketed: false,
scale: 'ratio',
timeScale: undefined,
params: { window: 5 },
references: referenceIds,
filter: getFilter(undefined, columnsParams),
}),
}),

The type of params: { window: 5 } is pointing to this type here thus why adding the window prop fixes the error.

But I think window type should be pulling from MovingAverageIndexPatternColumn instead which is here...

export type MovingAverageIndexPatternColumn = FormattedIndexPatternColumn &
ReferenceBasedIndexPatternColumn & {
operationType: typeof MOVING_AVERAGE_ID;
params: {
window: number;
};
};

@nickofthyme nickofthyme added technical debt Improvement of the software architecture and operational architecture Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Feb 6, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

@stratoula stratoula added impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Feature:Lens labels Feb 7, 2024
@dej611 dej611 self-assigned this Mar 12, 2024
dej611 added a commit that referenced this issue Mar 13, 2024
## Summary

Fix #176343 


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Lens impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:Visualizations Visualization editors, elastic-charts and infrastructure technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants