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

[Content management] Add "Last updated" metadata to TableListView #132321

Merged
merged 21 commits into from
May 19, 2022

Conversation

sebelga
Copy link
Contributor

@sebelga sebelga commented May 17, 2022

In this PR I've updated the implementation of the <TableListView /> component (exposed by the kibana_react plugin) to render a "Last updated" column if the items of the table contain the updatedAt metadata.

I have then updated the consuming apps to include the updatedAt from their saved objects when fetching the table items.

Note on implementation

I've decided to render relative date ("a minute ago", "yesterday", "2 days ago") for up to 7 days ago. If an object has been updated more than 7 days ago we then render this format: "May 17, 2022"

Less than 7 days More than 7 days
Screenshot 2022-05-18 at 15 31 45 Screenshot 2022-05-18 at 15 32 19

When there is no data for a specific saved object then a dash "-" is displayed with a tooltip

Screenshot 2022-05-18 at 14 56 17

How to test

  • Launch Kibana and load sample data
  • Navigate to the different list views (Dashboard, visualization, maps, graph) and verify that there is a "Last updated" column.
  • Verify that the default sorting is "DESC" on the "Last updated" column
  • Edit one of the object and save it. Navigate back to the listing to verify that the updated at is indeed updated

Release note

The list view for Dashboards, Visualizations, Maps and Graph has a new "Last updated" column to easily access content that has been recently modified.

Updated apps

Screenshot 2022-05-17 at 15 22 18

Screenshot 2022-05-17 at 15 22 03

Screenshot 2022-05-17 at 15 21 42

Screenshot 2022-05-17 at 15 21 20

Fixes #132134

@sebelga sebelga force-pushed the content-management/surface-updated-at branch from 69d8be7 to 88c203d Compare May 17, 2022 13:55
@sebelga sebelga marked this pull request as ready for review May 17, 2022 13:57
@sebelga sebelga requested a review from a team May 17, 2022 13:57
@sebelga sebelga requested review from a team as code owners May 17, 2022 13:57
@sebelga sebelga added ci:deploy-cloud Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.3.0 Feature:Content Management User generated content (saved objects) management labels May 17, 2022
@sebelga sebelga requested a review from majagrubic May 17, 2022 13:58
Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

Data Discovery changes LGTM 👍

Copy link
Contributor

@Heenawter Heenawter left a comment

Choose a reason for hiding this comment

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

Works great for Dashboard! Filed an issue to make use of this in Canvas as well, since we are currently implementing something similar without using the TableListView component. LGTM 👍

@cchaos cchaos self-requested a review May 17, 2022 15:47
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

VisEditors changes LGTM

@sebelga sebelga requested review from majagrubic and cchaos May 18, 2022 13:39
@sebelga
Copy link
Contributor Author

sebelga commented May 18, 2022

@nreese I got the same failure on a different build. Any idea why this test is now failing with the changes made here?

@cchaos
Copy link
Contributor

cchaos commented May 18, 2022

@KOTungseth Can you check this UI copy? Mainly the new elements are:

  • Column header: Last updated. Someone mentioned though that this could be confused with data having been updated. Should it be something more like Last edited? And should it be Last vs Recently?
  • Unknown tooltip: Updated date unknown. There's a very rare chance this date may not exist so we've opted for a simple - in the cell contents with this text in the tooltip.

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-gis changes LGTM
code review

@nreese
Copy link
Contributor

nreese commented May 18, 2022

@sebelga

I am getting the following API integration test fail, it does not seem related to the changes in this PR. Apparently 2 vectors got swapped in the response.

See #132368. A change in the elasticsearch snapshot changed the order of the geometry getting returned. This caused these tests to fail. They have been skipped in main until we can get a PR out to update the tests.

@sebelga
Copy link
Contributor Author

sebelga commented May 19, 2022

@elasticmachine merge upstream

@sebelga
Copy link
Contributor Author

sebelga commented May 19, 2022

Perfect thanks @nreese 👍

@KOTungseth
Copy link
Contributor

KOTungseth commented May 19, 2022

  • Column header: Last updated. Someone mentioned though that this could be confused with data having been updated. Should it be something more like Last edited? And should it be Last vs Recently?

Let's stay with Last updated.

  • Unknown tooltip: Updated date unknown. There's a very rare chance this date may not exist so we've opted for a simple - in the cell contents with this text in the tooltip.

For consistency, let's change this tooltip to Last updated unknown.

@teresaalvarezsoler
Copy link

@cchaos If we think it can be confusing, we could add a tooltip when the user hovers to explain what we mean by "Last updated"

@sebelga
Copy link
Contributor Author

sebelga commented May 19, 2022

Thanks for the review @KOTungseth ! 👍

@sebelga sebelga enabled auto-merge (squash) May 19, 2022 15:26
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Great improvement, thanks @sebelga!

@teresaalvarezsoler I had a similar tooltip idea. We'll need to workshop the language on that for a bit, so I won't hold up this PR and we can add later.

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #29 / rules security and spaces enabled: basic ruleRegistryAlertsSearchStrategy logs should return alerts from log rules

Metrics [docs]

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
kibanaReact 204 212 +8

Async chunks

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

id before after diff
graph 474.8KB 474.8KB +24.0B
kibanaReact 209.6KB 209.6KB +2.0B
maps 2.6MB 2.6MB +22.0B
total +48.0B

Page load bundle

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

id before after diff
dashboard 65.9KB 65.9KB +26.0B
kibanaReact 59.7KB 61.0KB +1.3KB
lens 32.4KB 32.5KB +24.0B
maps 78.2KB 78.3KB +24.0B
visualizations 46.5KB 46.5KB +24.0B
total +1.4KB
Unknown metric groups

API count

id before after diff
kibanaReact 240 248 +8

History

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

@sebelga sebelga merged commit a80bfb7 into elastic:main May 19, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label May 19, 2022
@sebelga sebelga deleted the content-management/surface-updated-at branch May 20, 2022 06:59
@tylersmalley tylersmalley added ci:cloud-deploy Create or update a Cloud deployment and removed ci:deploy-cloud labels Aug 17, 2022
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 ci:cloud-deploy Create or update a Cloud deployment Feature:Content Management User generated content (saved objects) management release_note:enhancement Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) ui-copy Review of UI copy with docs team is recommended v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Content management] Surface "updated_at" in TableListView