Skip to content

[ML] Check for the sort field type in Anomalies table before updating the URL state.#98270

Merged
darnautov merged 2 commits intoelastic:masterfrom
darnautov:ML-98117-fix-table-pagination
Apr 26, 2021
Merged

[ML] Check for the sort field type in Anomalies table before updating the URL state.#98270
darnautov merged 2 commits intoelastic:masterfrom
darnautov:ML-98117-fix-table-pagination

Conversation

@darnautov
Copy link
Copy Markdown
Contributor

@darnautov darnautov commented Apr 26, 2021 •
edited
Loading

Summary

Fixes #98117

The regression has been introduced in #97350.

The column name is defined as a tooltip, which resulted in the onChange callback to receive it as a field value instead of the string definition.
image

I added a check for a string type, but I assume the actual issue is on the EUI side, as it should check for the field property instead of the name when fires the onChange callback.

Important observation: When I perform sorting by severity by clicking on a column header, the onChange callback receives a correct payload.
image

Checklist

@darnautov darnautov added :ml v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.14.0 v7.13.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Apr 26, 2021
@darnautov darnautov requested a review from a team as a code owner April 26, 2021 11:07
@darnautov darnautov self-assigned this Apr 26, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Apr 26, 2021

Pinging @elastic/ml-ui (:ml)

@darnautov darnautov changed the title [ML] check sort field type [ML] Check for the sort field type in Anomalies table before updating the URL state. Apr 26, 2021
@darnautov darnautov mentioned this pull request Apr 26, 2021
Closed
1 task
@darnautov darnautov requested a review from pheyos April 26, 2021 13:13
@peteharverson peteharverson mentioned this pull request Apr 26, 2021
Closed
15 tasks
peteharverson
peteharverson approved these changes Apr 26, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and LGTM

pheyos
pheyos approved these changes Apr 26, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and LGTM

alvarezmelissa87
alvarezmelissa87 approved these changes Apr 26, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ⚡

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Apr 26, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
ml 5.9MB 5.9MB +29.0B

History

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

cc @darnautov

@darnautov darnautov merged commit 0cf3399 into elastic:master Apr 26, 2021
@darnautov darnautov deleted the ML-98117-fix-table-pagination branch April 26, 2021 15:13
@kibanamachine kibanamachine mentioned this pull request Apr 26, 2021
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 26, 2021
@darnautov @kibanamachine
[ML] Check for the sort field type in Anomalies table before updating…
036f3eb 
… the URL state. (elastic#98270)

* [ML] check sort field type

* [ML] functional tests for anomalies table pagination
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 26, 2021
@darnautov @kibanamachine
[ML] Check for the sort field type in Anomalies table before updating…
b1dd0b1 
… the URL state. (elastic#98270)

* [ML] check sort field type

* [ML] functional tests for anomalies table pagination
@kibanamachine kibanamachine mentioned this pull request Apr 26, 2021
Merged
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Apr 26, 2021

💚 Backport successful

Status Branch Result
✅ 7.13
✅ 7.x

The backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Apr 26, 2021
@kibanamachine @darnautov
[ML] Check for the sort field type in Anomalies table before updating…
c595f16 
… the URL state. (#98270) (#98313)

* [ML] check sort field type

* [ML] functional tests for anomalies table pagination

Co-authored-by: Dima Arnautov <dmitrii.arnautov@elastic.co>
kibanamachine added a commit that referenced this pull request Apr 26, 2021
@kibanamachine @darnautov
[ML] Check for the sort field type in Anomalies table before updating…
4f62dcc 
… the URL state. (#98270) (#98314)

* [ML] check sort field type

* [ML] functional tests for anomalies table pagination

Co-authored-by: Dima Arnautov <dmitrii.arnautov@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pheyos pheyos pheyos approved these changes

@alvarezmelissa87 alvarezmelissa87 alvarezmelissa87 approved these changes

@peteharverson peteharverson peteharverson approved these changes

Assignees

@darnautov darnautov

Labels

auto-backport Deprecated - use backport:version if exact versions are needed :ml release_note:skip Skip the PR/issue when compiling release notes v7.13.0 v7.14.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[ML] Cannot change number of rows in the anomalies table

6 participants

@darnautov @elasticmachine @kibanamachine @pheyos @alvarezmelissa87 @peteharverson