Migrate logs table to DataViews #99056

oandregal · 2025-01-29T19:33:09Z

WORK IN PROGRESS: NOT TO MERGE.

Depends on chore(deps): update wordpress monorepo #96470 which needs to land first.
Fixes Migrate the Logs table to DataViews #96793

Proposed Changes

Updates the existing log table list to use DataViews

Why are these changes being made?

We want to consolidate the listing interactions across our products into a single experience powered by DataViews.

Testing Instructions

yarn && yarn start

Visit http://calypso.localhost:3000/sites.
Select a site to open the preview panel.
Select a site that has the "Logs" tab active (e.g.: atomic).
Verify that the table listing is using DataViews and works as expected.

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
- For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

github-actions · 2025-01-29T19:33:27Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-132456

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-132456&env=jetpack

Automattic for Agencies live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-132456&env=a8c-for-agencies

matticbot · 2025-01-29T19:39:48Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~2557 bytes added 📈 [gzipped])

name        parsed_size           gzip_size
site-logs       +6654 B  (+0.5%)    +1288 B  (+0.3%)
site-tools      +6626 B  (+0.4%)    +1269 B  (+0.3%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

ntsekouras · 2025-01-30T12:48:52Z

client/sites/tools/logs/index.tsx

+	// - DataViews: address the "show more" interaction.
+	// - DataViews: spacing left/right.
+
+	const { __ } = useI18n(); // Can we use translate instead?


How about importing from @wordpress/i18n? What the difference between importing, using translate or useI18n? 🤔

No idea why we use one or the other (there's also a translate call 🤷 ). Right now, I'm just trying to make things work following what the existing component chain does. Later, I'll dig into the details.

@wordpress/i18n is non-Reactive. It works in most cases, but if you change the locale or download language files after rendering, without refreshing the page, the UI won't refresh. Whereas translate = useTranslate() from calypso-i18n and @wordpress/react-i18n are both Reactive and will rerender the consumer when anything related to translations changes.

As discussed with @tyxla and others, It's seems now there's more pros to using @wordpress/i18n than calypso-i18n. Should we be doing a P2 post on this to get everyone on the same page and understand the long term plan / migration...

Just to cnfirm, as @alshakero mentioned, In React context, we should be using { useI18n } from '@wordpress/react-i18n' instead of @wordpress/i18n to benefit from the changes in locale or async loaded translation chunks. @wordpress/i18n is preferred only in non-React context. I did talk about it here.

As discussed with @tyxla and others, It's seems now there's more pros to using @wordpress/i18n than calypso-i18n. Should we be doing a P2 post on this to get everyone on the same page and understand the long term plan / migration...

Interesting. Can you point me to where these discussions took place. I was/am under a different impression, also judging from the sentiment I got in a similar discussion pxLjZ-9Eg-p2

I've recently dealt with dozens of translation issues primarily stemming from devs using a variety of i18n formatters, either not having the same locale or even the right locale internally. It explicitly concerned numbers, but numbers are just as important. Having all these packages available for similar (but not identical) purposes isn't serving the goal.

As far as numbers go, I am unifying everything behind a single method: pxLjZ-9Ji-p2 And that method currently lives in i18n-calypso. I am not even sure if I will pull it out right now, as the alternatives aren't exactly better. @wordpress/i18n isn't reactive, @wordpress/react-18n is an extremely minimal package, @automattic/i18n-utils doesn't deal with internal state/locale definition, etc.

I would 100% urge everyone, for now, to stick with i18n-calypso in Calypso until we work out the replacement (if we ever do). Just "using another" without proper migration isn't helping the code, and, judging from what I've seen the past couple of weeks, it's not helping the user either (and it slipped from everyone's eyes for a long time).

I may be missing something, but I don't understand why we need to be bound to i18n-calypso for number formatting. Yes, that's where the old utility function was before, but is that a good enough reason? What prevents us from moving closer to step 4 of your original proposal, and instead of having to bind number formatting to i18n-calypso, build it separately, and just allow it to take a locale in addition to the desired format? If we can make number formatting independent, then we would be able to use it with both i18n-calypso and @wordpress/i18n and @wordpress/react-i18n?

Well we are a couple of dozens of bugs ahead of my original proposal, all/most stemming from locale issues. Some similar situation exists (but not issues per se) with the JP formatter.

In my ideal scenario right now, numberFormat works as translate, with internal locale state. My fear with the alternative, of passing a browser-safe locale, is it won't be effective. People will insist on passing ant locale to it.

So internal locale state handling + optional locale param to encompass JP is what I'm shooting for. Make (keep) it stupid easy for most cases, expansive for others.

Got it. In any case, separating the browser-safe locale retrieval from i18n-calypso is something we can solve in the future. It's not a blocker for this PR, since all the works happens in Calypso's client and using i18n-calypso is still fine.

Yes, we'll come back to it. Not meaning this as a blocker. It just caught my attention, fighting with similar things.

A separate package imported into i18n-calypso (to wrap the calls with its locale state) might be right. wp-calypso will continue using it from there, never import it separately. There's also a lot going on around setLocale, not sure what it would mean to abstract it out somehow instead.

ntsekouras · 2025-01-30T12:54:26Z

client/sites/tools/logs/index.tsx

+		setView( ( oldView ) => ( {
+			...oldView,
+			...newView,
+			type: 'table' as const,


Do we need this?

Yes, if you don't add as const, TS will view type as a mutable string and dataviews expect strictly table, grid and two other values I can't recall.

alshakero

Hi everyone! Just a drive by review trying to help and learn about dataviews.

alshakero · 2025-01-31T09:37:39Z

client/sites/tools/logs/index.tsx

+	// - DataViews: address the "show more" interaction.
+	// - DataViews: spacing left/right.
+
+	const { __ } = useI18n(); // Can we use translate instead?


@wordpress/i18n is non-Reactive. It works in most cases, but if you change the locale or download language files after rendering, without refreshing the page, the UI won't refresh. Whereas translate = useTranslate() from calypso-i18n and @wordpress/react-i18n are both Reactive and will rerender the consumer when anything related to translations changes.

alshakero · 2025-01-31T09:41:04Z

client/sites/tools/logs/index.tsx

+	const moment = useLocalizedMoment();
+	const getLatestDateRange = useCallback( () => {
+		const startTime = moment().subtract( 7, 'd' );
+		const endTime = moment();
+		return { startTime, endTime };
+	}, [ moment ] );


I don't want to make your life harder. But@wordpress/dataviews is 340KB gzipped and moment is 73KB gzipped. It would be amazing if we follow up with some performance considerations (eg async loading, not using moment, etc..).

alshakero · 2025-01-31T09:42:21Z

client/sites/tools/logs/index.tsx

+		setView( ( oldView ) => ( {
+			...oldView,
+			...newView,
+			type: 'table' as const,


Yes, if you don't add as const, TS will view type as a mutable string and dataviews expect strictly table, grid and two other values I can't recall.

alshakero · 2025-01-31T09:45:04Z

client/sites/tools/logs/index.tsx

+	useEffect( () => {
+		setView( ( view ) => ( {
+			...view,
+			sort: {
+				field: getSortFieldForLogType( logType ),
+			},
+			fields: getVisibleFieldsForLogType( logType ),
+		} ) );
+	}, [ logType ] );


From what I see here, having view as a state is making your life harder. Maybe make all its dependencies states but the view itself can be a constructed in the end before returning.

const view = { type: 'table' as const, sort: { field: getSortFieldForLogType( logType ) }, fields: getVisibleFieldsForLogType( logType ), }

alshakero · 2025-01-31T09:45:49Z

client/sites/tools/logs/index.tsx

 import { sprintf } from '@wordpress/i18n';
 import { useI18n } from '@wordpress/react-i18n';
+import { translate } from 'i18n-calypso';


I would use useTranslate instead. See why here.

alshakero · 2025-01-31T09:47:46Z

client/sites/tools/logs/index.tsx

+						<span>{ translate( 'From' ) }</span>
+						<DateTimePicker
+							className="site-logs-toolbar__datepicker"
+							id="from"


Do we need the id if it's wrapped by the label?

alshakero · 2025-01-31T09:48:05Z

client/sites/tools/logs/index.tsx

+						<span>{ translate( 'To' ) }</span>
+						<DateTimePicker
+							className="site-logs-toolbar__datepicker"
+							id="to"


alshakero · 2025-01-31T09:49:02Z

client/sites/tools/logs/index.tsx

@@ -251,3 +263,257 @@ export const SiteLogs = ( {
 		</div>
 	);
 };
+
+const EMPTY_ARRAY: Array< any > = [];
+const useDataLogs = ( {


I would put this in a separate file.

oandregal · 2025-01-31T13:26:19Z

Cross-posting status from p5j4vm-51h-p2#comment-7495.

Current status as of January 31st: all basic interactions are working.

The only major interaction missing is “expand row to read more”. The DataViews native approach would be implementing it via a modal action.
It needs style tweaks and fixes (spacing, date header, empty states for filters, etc.).
In terms of data filters & sorting, there’s parity with the existing implementation. The current endpoint doesn’t support sorting by fields other than the timestamp, searching by text, filtering by multiple values (e.g., “severity is any of: …”), or adding more filters (e.g., filter by kind or name) — some of these would be valuable, but I consider them follow-up PRs.
Code-wise, I just made it work. I’ve reused as many pieces as I could from the current implementation, but it’s not ready — I’ll address this next while design feedback comes.

matt-west · 2025-02-04T13:50:28Z

Nice work so far @oandregal!

Spacing

It sounds like you’re already aware of this, but in the interest of being comprehensive, there are a couple spacing issues that need fixing:

The DataViews table should span the entire width of the content page. There’s currently some left + right padding.
The padding above the Logs title in the header is also inconsistent with other pages.

Auto-refresh

The auto-refresh option is missing. Is it possible to place this in the toolbar above the table?

Entry details behaviour

I’ve explored a few different ways we can show the details for a specific log entry. I suspect the modal approach will be the easiest to implement short-term as it doesn’t require changes to DataViews. I’m quite taken by the drawer approach though. What do you think?

Modal

We could add an action icon to the row that triggers a modal for viewing the full details related to a log entry.

Drawer

In this approach clicking a log entry would pop-up a drawer at the bottom of the content pane with the full details. Users could resize and close this drawer as they wish. (Sort of like browser dev tools)

Adding the drawer starts to make the content pane feel really crowded. Some of this will be relieved with the upcoming changes that simplify the site header and hide the sites list though.

Dropdown

This approach restores the current behaviour where selecting a log entry expands the row to display the full details. A nice advantage of this approach is that you can expand and compare multiple entries at the same time.

Educational tip

The filter icon above the table can be easily missed, so I think it warrants spotlighting that some of the filters have moved. We’ve used a similar component before so hopefully there's something you can reuse.

Cleaning up filters

We current have filters spread over multiple levels which is not ideal. I think this is a limitation of current DataViews capabilities, but just wanted to call it out.

In the future it would be nice if we could integrate the date pickers into the toolbar to clean up the filters. I don't think DataViews supports this at the moment though. (Please correct me if I’m wrong!)

matticbot · 2025-02-06T22:27:00Z

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

blaze-dashboard
notifications
odyssey-stats
wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug update/logs-to-dataviews on your sandbox.

…sary All value

oandregal requested review from a team as code owners January 29, 2025 19:33

oandregal requested a review from southp January 29, 2025 19:33

matticbot added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jan 29, 2025

oandregal marked this pull request as draft January 29, 2025 19:33

oandregal added Sites [Status] In Progress labels Jan 29, 2025

oandregal changed the base branch from trunk to renovate/wordpress-monorepo January 29, 2025 19:35

oandregal changed the title ~~Update/logs to dataviews~~ Migrate logs table to DataViews Jan 29, 2025

sirbrillig removed the request for review from a team January 29, 2025 20:51

ntsekouras reviewed Jan 30, 2025

View reviewed changes

aneeshd16 removed request for a team and southp January 30, 2025 20:59

alshakero reviewed Jan 31, 2025

View reviewed changes

oandregal force-pushed the renovate/wordpress-monorepo branch from 65149e6 to a1297b3 Compare February 4, 2025 09:53

Base automatically changed from renovate/wordpress-monorepo to trunk February 4, 2025 11:21

oandregal force-pushed the update/logs-to-dataviews branch 2 times, most recently from 99ff7c1 to 8b02a4d Compare February 6, 2025 22:19

oandregal force-pushed the update/logs-to-dataviews branch from 8b02a4d to 34fbdd3 Compare February 7, 2025 12:01

oandregal added 29 commits February 7, 2025 15:30

Pagination

a321a50

Total items cannot be undefined

d9aa3a4

Disable search

9428d28

Fields: names and sorting

6f17ef0

Filter by severity

8f2f239

Filter by status

24917c5

Filter by request_type

c1e0aac

Field labels: translate

7128121

Labels

96708dd

Severity: add badge

7673bf1

Request type: add badge

9e2236f

Style badges properly

0e4dc3c

Fields: format dates

575dd25

Control cell overflow

6736b04

Add download logs button

132aa4c

TypeScript

559eb10

Type fields

2063da7

Extract useFields and types

3a5b696

Rename SiteLogsTab type to LogType

f21a91a

Move types to existing place: use-site-logs-query

07d9dd2

Extract useView

7f81ffb

Extract useData

5161878

Fix field types

6845303

Provide a getItemId function

2dc8b35

Clean up

34932aa

Remove local state for date and use URL query instead

a0033d1

Add support for multiple values

c229849

Filters: multiple values for request_type and status + remove unneces…

1618c45

…sary All value

ServerLog: format date

d163e3f

oandregal force-pushed the update/logs-to-dataviews branch from 34fbdd3 to d163e3f Compare February 7, 2025 14:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate logs table to DataViews #99056

Migrate logs table to DataViews #99056

oandregal commented Jan 29, 2025 •

edited

Loading

github-actions bot commented Jan 29, 2025 •

edited by matticbot

Loading

matticbot commented Jan 29, 2025 •

edited

Loading

ntsekouras Jan 30, 2025 •

edited

Loading

oandregal Jan 30, 2025 •

edited

Loading

alshakero Jan 31, 2025 •

edited

Loading

youknowriad Feb 6, 2025

tyxla Feb 6, 2025 •

edited

Loading

chriskmnds Feb 7, 2025

tyxla Feb 7, 2025

chriskmnds Feb 7, 2025 •

edited

Loading

tyxla Feb 7, 2025

chriskmnds Feb 7, 2025

ntsekouras Jan 30, 2025

alshakero Jan 31, 2025

alshakero left a comment •

edited

Loading

alshakero Jan 31, 2025 •

edited

Loading

alshakero Jan 31, 2025

alshakero Jan 31, 2025

alshakero Jan 31, 2025

alshakero Jan 31, 2025 •

edited

Loading

alshakero Jan 31, 2025

alshakero Jan 31, 2025

alshakero Jan 31, 2025

oandregal commented Jan 31, 2025

matt-west commented Feb 4, 2025

matticbot commented Feb 6, 2025 •

edited

Loading

Migrate logs table to DataViews #99056

Are you sure you want to change the base?

Migrate logs table to DataViews #99056

Conversation

oandregal commented Jan 29, 2025 • edited Loading

Proposed Changes

Why are these changes being made?

Testing Instructions

Pre-merge Checklist

github-actions bot commented Jan 29, 2025 • edited by matticbot Loading

matticbot commented Jan 29, 2025 • edited Loading

ntsekouras Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

oandregal Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

alshakero Jan 31, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tyxla Feb 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriskmnds Feb 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alshakero left a comment • edited Loading

Choose a reason for hiding this comment

alshakero Jan 31, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alshakero Jan 31, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal commented Jan 31, 2025

matt-west commented Feb 4, 2025

Spacing

Auto-refresh

Entry details behaviour

Modal

Drawer

Dropdown

Educational tip

Cleaning up filters

matticbot commented Feb 6, 2025 • edited Loading

oandregal commented Jan 29, 2025 •

edited

Loading

github-actions bot commented Jan 29, 2025 •

edited by matticbot

Loading

matticbot commented Jan 29, 2025 •

edited

Loading

ntsekouras Jan 30, 2025 •

edited

Loading

oandregal Jan 30, 2025 •

edited

Loading

alshakero Jan 31, 2025 •

edited

Loading

tyxla Feb 6, 2025 •

edited

Loading

chriskmnds Feb 7, 2025 •

edited

Loading

alshakero left a comment •

edited

Loading

alshakero Jan 31, 2025 •

edited

Loading

alshakero Jan 31, 2025 •

edited

Loading

matticbot commented Feb 6, 2025 •

edited

Loading