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.

Sign up for GitHub

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

Jump to bottom

[ML] Transforms list: persist pagination through refresh interval #76786

Merged
merged 2 commits into from
Sep 9, 2020
Merged

[ML] Transforms list: persist pagination through refresh interval #76786

merged 2 commits into from
Sep 9, 2020

Conversation

alvarezmelissa87
Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 commented Sep 4, 2020 •
edited
Loading

Summary

Related issue #75828
Persists pagination for transforms list. The functionality is now consistent with anomaly detection and analytics jobs list.

This PR

  • switches the EuiInMemoryTable used in the transforms list to the EuiBasicTable in order to be able to externally control pagination

  • creates a useTableSettings custom hook to control pagination/sort for the basic table

  • adds a SearchBar component which handles filtering of the table

image

Checklist

Delete any items that are not applicable to this PR.

@alvarezmelissa87 alvarezmelissa87 added release_note:skip Skip the PR/issue when compiling release notes Feature:Transforms ML transforms v7.10.0 labels Sep 4, 2020
@alvarezmelissa87 alvarezmelissa87 requested a review from a team as a code owner September 4, 2020 15:57
@alvarezmelissa87 alvarezmelissa87 self-assigned this Sep 4, 2020
@alvarezmelissa87 alvarezmelissa87 mentioned this pull request Sep 4, 2020
Closed
3 tasks
qn895
qn895 reviewed Sep 4, 2020
View reviewed changes
...nsform/public/app/sections/transform_management/components/transform_list/transform_list.tsx
const filterList = () => {
if (searchQueryText !== '') {
const query = EuiSearchBar.Query.parse(searchQueryText);
let clauses: any = [];
Copy link
Member

@qn895 qn895 Sep 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Suggestion to use Array<TermClause | FieldClause> here if that's what filterAnalytics accepts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call - I think this also might be part of the EUI type issue I ran into but I will timebox some further digging time 👌

qn895
qn895 reviewed Sep 4, 2020
View reviewed changes
.../public/app/sections/transform_management/components/transform_list/transform_search_bar.tsx Outdated

const filters = [
{
type: 'field_value_selection' as const,
Copy link
Member

Choose a reason for hiding this comment

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

nit: not sure if you need the as const here and for the ones below. Maybe we can set const filters: SearchFilterConfig[] and TS will know to cast those fields correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use SearchFilterConfig in 9c2d7d1

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

peteharverson
peteharverson approved these changes Sep 7, 2020
View reviewed changes
Copy link
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

walterra
walterra reviewed Sep 7, 2020
View reviewed changes
...orm/public/app/sections/transform_management/components/transform_list/use_table_settings.ts Outdated
});

const getPageOfItems = (
list: any[],
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be TransformListRow[]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated useTableSettings type to be generic and accept item type - 9c2d7d1

@alvarezmelissa87 alvarezmelissa87 force-pushed the ml-transforms-page-persist branch from d3e967b to c0fa084 Compare September 8, 2020 15:01
@alvarezmelissa87
Copy link
Contributor Author

This has been updated and is ready for a final look when you get a chance 🙏 cc @peteharverson, @walterra, @qn895

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
transform 458 +138 320

async chunks size

id value diff baseline
transform 759.3KB +56.8KB 702.5KB

History

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

walterra
walterra approved these changes Sep 9, 2020
View reviewed changes
Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

Latest changes LGTM

@alvarezmelissa87 alvarezmelissa87 merged commit e985f1a into elastic:master Sep 9, 2020
@alvarezmelissa87 alvarezmelissa87 mentioned this pull request Sep 9, 2020
Merged
alvarezmelissa87 added a commit to alvarezmelissa87/kibana that referenced this pull request Sep 9, 2020
@alvarezmelissa87
[ML] Transforms list: persist pagination through refresh interval (el…
5a40d4d 
…astic#76786)

* use basic table in transforms to persist pagination

* simplify state and update types
@alvarezmelissa87 alvarezmelissa87 deleted the ml-transforms-page-persist branch September 9, 2020 14:08
alvarezmelissa87 added a commit that referenced this pull request Sep 9, 2020
@alvarezmelissa87
[ML] Transforms list: persist pagination through refresh interval (#7…
f37b01b 
…6786) (#77061)

* use basic table in transforms to persist pagination

* simplify state and update types
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 9, 2020
@gmmorris
Merge branch 'master' into alerting-exempt-rbac
1ef7d8b 
* master: (68 commits)
  a11y tests on spaces home page including feature control  (elastic#76515)
  [ML] Transforms list: persist pagination through refresh interval (elastic#76786)
  [ML] Replace all use of date_histogram interval with fixed_interval (elastic#76876)
  [Timelion] Update timelion deprecation links (elastic#77008)
  [Security Solution] Refactor Network Details to use Search Strategy (elastic#76928)
  Upgrade elastic charts to 21.1.2 (elastic#76939)
  [Alerting][Connectors] Refactor Jira: Generic Implementation (phase one) (elastic#73778)
  [Snapshot & Restore] fix pre existing policy with no existing repository (elastic#76861)
  Update saved object management UI text (elastic#76826)
  [Form lib] Add validations prop to UseArray and expose "moveItem" handler (elastic#76949)
  [Logs UI] Use fields api in log stream (elastic#76919)
  [UI Metrics] Support multi-colon keys (elastic#76913)
  [APM] Script for creating functional test archive (elastic#76926)
  [ENDPOINT] First version of the trusted apps list. (elastic#76304)
  Correct field for rum page url (elastic#76916)
  [Security Solution] Fix redirect properly old SIEM App routes (elastic#76868)
  Bump http-proxy from 1.17.0 to 1.18.1 (elastic#76924)
  [RUM Dashboard] Visitor breakdown usability (elastic#76834)
  [Search] Add a new advanced setting searchTimeout (elastic#75728)
  [DOCS] Adds timelion deprecation to new visualize docs structure (elastic#76959)
  ...
rylnd added a commit to rylnd/kibana that referenced this pull request Sep 10, 2020
@rylnd
Merge branch 'master' into eql_rules_creation
5094b04 
* master: (38 commits)
  Reporting/Test: unskip non-screenshot tests (elastic#77088)
  Move metrics to setup and add cgroup metrics (elastic#76730)
  [Enterprise Search] Add Overview landing page/plugin (elastic#76734)
  First pass. Change TS type. Update OpenAPI (elastic#76434)
  [CI] Balance xpack ci groups a bit (elastic#77068)
  [Security_solution][Detections] Refactor signal ancestry to allow multiple parents (elastic#76531)
  [Maps] convert MetricsEditor to TS (elastic#76727)
  IndexMigrator: fix non blocking migration wrapper promise rejection (elastic#77018)
  [Enterprise Search] Update config data endpoint to v2 (elastic#76970)
  [ML] Add decision path charts to exploration results table (elastic#73561)
  Bump eventemitter3 from 4.0.0 to 4.0.7 (elastic#77016)
  [Ingest Pipelines] Add descriptions for ingest processors K-S (elastic#76981)
  [Metrics UI] Replace Snapshot API with Metrics API (elastic#76253)
  legacy utils cleanup (elastic#76608)
  [ML] Account for "properties" layer in find_file_structure mappings (elastic#77035)
  fixed typo
  Upgrade to Kea 2.2 (elastic#77047)
  a11y tests on spaces home page including feature control  (elastic#76515)
  [ML] Transforms list: persist pagination through refresh interval (elastic#76786)
  [ML] Replace all use of date_histogram interval with fixed_interval (elastic#76876)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qn895 qn895 qn895 left review comments

@peteharverson peteharverson peteharverson approved these changes

@walterra walterra walterra approved these changes

Assignees

@alvarezmelissa87 alvarezmelissa87

Labels
Feature:Transforms ML transforms :ml release_note:skip Skip the PR/issue when compiling release notes v7.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@alvarezmelissa87 @elasticmachine @kibanamachine @walterra @peteharverson @qn895