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

[Discover] Remove the legacy table #201254

Merged
merged 27 commits into from
Dec 3, 2024

Conversation

jughosta
Copy link
Contributor

@jughosta jughosta commented Nov 21, 2024

Summary

This PR removes the code related to the legacy doc table and 2 Advanced Settings: doc_table:legacy and truncate:maxHeight.

The legacy table in Discover was replaced by the new data grid in v8.3. The doc_table:legacy Advanced Setting was added to let users switch back to the legacy table if necessary. The removal of the setting and the legacy table entirely would allow us to reduce bundle size, maintenance burden, and code complexity.

Also the legacy table does not support many new features which were added to the grid only (e.g. comparing selected documents, context-aware UI based on current solution project, column resizing, bulk row selection, copy actions, new doc viewer flyout, and more).

Since v8.15 doc_table:legacy is marked as deprecated on Advanced Settings page via #179899

Since v8.16 truncate:maxHeight is marked as deprecated too via #183736

The removal of these 2 settings and the associated code is planned for v9.

Checklist

  • Documentation was added for features that require explanation or tutorials
  • Unit or functional tests were updated or added to match the most common scenarios
  • This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.

@jughosta jughosta added release_note:deprecation backport:skip This commit does not require backporting Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. labels Nov 21, 2024
@jughosta jughosta self-assigned this Nov 21, 2024
Copy link
Contributor

A documentation preview will be available soon.

Request a new doc build by commenting
  • Rebuild this PR: run docs-build
  • Rebuild this PR and all Elastic docs: run docs-build rebuild

run docs-build is much faster than run docs-build rebuild. A rebuild should only be needed in rare situations.

If your PR continues to fail for an unknown reason, the doc build pipeline may be broken. Elastic employees can check the pipeline status here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it safe to remove them from the schema?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it safe to remove them from this types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it safe to remove them from this schema?

Copy link
Member

Choose a reason for hiding this comment

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

Yes... this schema is for informational purposes, and it doesn't directly apply to mappings.

The idea was that the telemetry dictionary would tell us when it was removed. But we never managed to fully build that internal feature 😅

@jughosta jughosta requested review from a team as code owners November 26, 2024 10:01
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

Copy link
Member

@sabarasaba sabarasaba left a comment

Choose a reason for hiding this comment

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

kibana management changes lgtm

Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

packages/kbn-management/settings/setting_ids/index.ts lgtm! :)

@kertal kertal added the Feature:Discover Discover Application label Nov 26, 2024
Copy link
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.

Functional test service changes for data_grid and doc_table LGTM

Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

.buildkite/ftr_platform_stateful_configs.yml

Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

ML changes LGTM

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

kibana-presentation changes LGTM
code review only

Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

Core changes LGTM

Copy link
Member

Choose a reason for hiding this comment

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

Yes... this schema is for informational purposes, and it doesn't directly apply to mappings.

The idea was that the telemetry dictionary would tell us when it was removed. But we never managed to fully build that internal feature 😅

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

It's beautiful 🥹
CleanShot 2024-11-27 at 22 48 03@2x

I tested locally and it's working as expected -- no more legacy doc table and no noticeable differences in Discover. Thanks for removing this and LGTM, it's so nice to be able to drop all this tech debt 🤘

@kertal kertal requested a review from l-suarez November 28, 2024 07:42
@kertal
Copy link
Member

kertal commented Dec 2, 2024

@elasticmachine merge upstream

Copy link

@l-suarez l-suarez left a comment

Choose a reason for hiding this comment

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

Looks as expected. Thanks!

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #61 / Observability Rules Synthetics SyntheticsCustomStatusRule "after all" hook in "SyntheticsCustomStatusRule"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1923 1922 -1
canvas 1293 1292 -1
cloudSecurityPosture 661 660 -1
discover 983 941 -42
esqlDataGrid 370 369 -1
eventAnnotationListing 593 592 -1
lens 1519 1518 -1
logsExplorer 570 569 -1
logsShared 363 362 -1
observability 1074 1073 -1
searchPlayground 271 270 -1
securitySolution 6310 6309 -1
slo 857 856 -1
unifiedDocViewer 225 216 -9
unifiedHistogram 193 192 -1
total -64

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
@kbn/discover-utils 234 227 -7
@kbn/management-settings-ids 139 137 -2
total -9

Async chunks

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

id before after diff
aiops 613.9KB 613.9KB -41.0B
discover 812.9KB 784.8KB -28.1KB
unifiedDocViewer 126.0KB 115.8KB -10.3KB
total -38.4KB

Page load bundle

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

id before after diff
discover 51.1KB 44.1KB -7.1KB
unifiedDocViewer 12.3KB 11.8KB -482.0B
total -7.5KB
Unknown metric groups

API count

id before after diff
@kbn/discover-utils 284 277 -7
@kbn/management-settings-ids 140 138 -2
total -9

async chunk count

id before after diff
unifiedDocViewer 10 8 -2

ESLint disabled line counts

id before after diff
discover 24 18 -6
unifiedDocViewer 9 8 -1
total -7

References to deprecated APIs

id before after diff
discover 9 8 -1

Total ESLint disabled count

id before after diff
discover 26 20 -6
unifiedDocViewer 9 8 -1
total -7

History

cc @jughosta

@jughosta jughosta merged commit 14bdd8d into elastic:main Dec 3, 2024
9 checks passed
hop-dev pushed a commit to hop-dev/kibana that referenced this pull request Dec 5, 2024
- Closes elastic#167582

## Summary

This PR removes the code related to the legacy doc table and 2 Advanced
Settings: `doc_table:legacy` and `truncate:maxHeight`.

The legacy table in Discover was replaced by the new data grid in v8.3.
The `doc_table:legacy` Advanced Setting was added to let users switch
back to the legacy table if necessary. The removal of the setting and
the legacy table entirely would allow us to reduce bundle size,
maintenance burden, and code complexity.

Also the legacy table does not support many new features which were
added to the grid only (e.g. comparing selected documents, context-aware
UI based on current solution project, column resizing, bulk row
selection, copy actions, new doc viewer flyout, and more).

Since v8.15 `doc_table:legacy` is marked as deprecated on Advanced
Settings page via elastic#179899

Since v8.16 `truncate:maxHeight` is marked as deprecated too via
elastic#183736

The removal of these 2 settings and the associated code is planned for
v9.

### Checklist

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [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
- [x] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 9, 2024
- Closes elastic#167582

## Summary

This PR removes the code related to the legacy doc table and 2 Advanced
Settings: `doc_table:legacy` and `truncate:maxHeight`.

The legacy table in Discover was replaced by the new data grid in v8.3.
The `doc_table:legacy` Advanced Setting was added to let users switch
back to the legacy table if necessary. The removal of the setting and
the legacy table entirely would allow us to reduce bundle size,
maintenance burden, and code complexity.

Also the legacy table does not support many new features which were
added to the grid only (e.g. comparing selected documents, context-aware
UI based on current solution project, column resizing, bulk row
selection, copy actions, new doc viewer flyout, and more).

Since v8.15 `doc_table:legacy` is marked as deprecated on Advanced
Settings page via elastic#179899

Since v8.16 `truncate:maxHeight` is marked as deprecated too via
elastic#183736

The removal of these 2 settings and the associated code is planned for
v9.

### Checklist

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [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
- [x] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Discover Discover Application release_note:breaking Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discover] Remove legacy/classic table