Skip to content

[Lens] Version-aware sorting to data table#125361

Merged
flash1293 merged 2 commits intoelastic:mainfrom
flash1293:lens-table-version-sorting
Feb 11, 2022
Merged

[Lens] Version-aware sorting to data table#125361
flash1293 merged 2 commits intoelastic:mainfrom
flash1293:lens-table-version-sorting

Conversation

@flash1293
Copy link
Copy Markdown
Contributor

@flash1293 flash1293 commented Feb 11, 2022

Fixes #124015

This PR adds version-aware sorting to the client side data table sorting in Lens:
Screenshot 2022-02-11 at 12 06 21

The information about whether a column contains version number wasn't passed to the table so far. This PR introduces a sortingHint to the datasource operation meta data (as the datasource is aware of index patterns, it knows whether an underlying field has the version type) which is read by the datatable toExpression function and passed to the datatable function as an additional argument. In there, already existing semver and compare-versions packages are used to check for valid versions and sort accordingly.

@flash1293 flash1293 added release_note:enhancement Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// Feature:Lens backport:skip This PR does not require backporting v8.2.0 labels Feb 11, 2022
@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Feb 11, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lens 736 744 +8

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
lens 315 316 +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 1.0MB 1.0MB +9.8KB

Page load bundle

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

id before after diff
lens 43.3KB 43.4KB +39.0B
Unknown metric groups

API count

id before after diff
lens 364 365 +1

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

@flash1293 flash1293 marked this pull request as ready for review February 11, 2022 12:52
@flash1293 flash1293 requested a review from a team as a code owner February 11, 2022 12:52
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Feb 11, 2022

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

mbondyra
mbondyra approved these changes Feb 11, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@mbondyra mbondyra left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tested, works great.
Screenshot 2022-02-11 at 14 18 48

Not related to this task, but should we at some point add version as a field type? Right now it's interpreted as a string in the data panel.
Screenshot 2022-02-11 at 14 19 38

testing data if someone wants to use it 
  PUT _ingest/pipeline/add_timestamp
{
  "description": "Adds timestamp",
  "processors": [
    {
      "set": {
        "field": "_source.timestamp",
        "value": "{{_ingest.timestamp}}"
      }
    }
  ]
}


PUT my-index-000001
{
  "mappings": {
    "properties": {
      "my_version": {
        "type": "version"
      }
    }
  }
}

PUT my-index-000001/_settings
{
  "index": {
    "default_pipeline": "add_timestamp"
  }
}

POST _bulk
{ "index" : { "_index" : "my-index-000001"} }
{ "my_version" : "1.112.0"}
{ "index" : { "_index" : "my-index-000001"} }
{ "my_version" : "1.21.0"}
{ "index" : { "_index" : "my-index-000001"} }
{ "my_version" : "1.1.0"}
{ "index" : { "_index" : "my-index-000001"} }
{ "my_version" : "8.1"}

@flash1293
Copy link
Copy Markdown
Contributor Author

flash1293 commented Feb 11, 2022 •
edited
Loading

Not related to this task, but should we at some point add version as a field type? Right now it's interpreted as a string in the data panel.

yeah, it's rolled up into string in all the places (there's a many-to-one relationship between ES field types and Kibana field types). Breaking it out into its own type is much more complex because it has to be done for all the places currently handling string fields in all the apps which is probably not worth it because it's very close to string in most regards.

@flash1293 flash1293 merged commit efa89e6 into elastic:main Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mbondyra mbondyra mbondyra approved these changes

Assignees

No one assigned

Labels

backport:skip This PR does not require backporting Feature:Lens release_note:enhancement Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v8.2.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Lens] Wrong table sorting for fields of type version

4 participants

@flash1293 @kibana-ci @elasticmachine @mbondyra