Add data formatter by igorschoester · Pull Request #22020 · Yoast/wordpress-seo

igorschoester · 2025-02-05T15:51:13Z

Context

Create a layer that formats the data for presentation

Summary

This PR can be summarized in the following changelog entry:

Adds a data formatter and implement it in the top pages widget.

Relevant technical choices:

Using the browser APIs for formatting: URL and Intl.NumberFormat
Retrieving the locale from the HTML lang instead of the WP user locale as this seems easier. No script data transfer needed and the WP user locale uses an underscore instead of a dash.
Using an Object for context for flexibility, more can be added as needed (opposed to the string in the issue)

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

Enable the site kit feature flag
Ensure you get Site kit data and have it connected
Go to the dashboard
Verify you see the widget for "Top 5 most popular content"
Verify the format of the landing page/URL: just the pathname
For the numbers, the formatting should adapt to the user locale (retrieved from html tag' lang). For example, in Dutch the separators are reversed (1,234.56 vs 1.234,56 in US)
- Verify the format of the clicks and impressions: always non-fractional numbers
- Verify the format of the CTR: as percentage and with 2 fractions, will be zero padded
- Verify the format of the average position: 2 fractions, will be zero padded
- Verify the SEO score is still there, will be notAnalyzed if fed anything else
Change your locale to something with different number formatting and verify the above formats
Devs: you could feed some random data to test more thoroughly. However, you could also check the tests and see if they cover all you think they should.

Screenshot

Without feature flag

Disable the site kit feature flag
Refresh the dashboard page
Ensure there is no errors and no top 5 pages widget

Relevant test scenarios

Changes should be tested with the browser console open
Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
Changes should be tested on different editors (Default Block/Gutenberg/Classic/Elementor/other)
Changes should be tested on different browsers
Changes should be tested on multisite

Test instructions for QA when the code is in the RC

QA should use the same steps as above.

QA can test this PR by following these steps:

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

The dashboard?

UI changes

This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Other environments

This PR also affects Shopify. I have added a changelog entry starting with [shopify-seo], added test instructions for Shopify and attached the Shopify label to this PR.

Documentation

I have written documentation for this change. For example, comments in the Relevant technical choices, comments in the code, documentation on Confluence / shared Google Drive / Yoast developer portal, or other.

Quality assurance

I have tested this code to the best of my abilities.
During testing, I had activated all plugins that Yoast SEO provides integrations for.
I have added unit tests to verify the code works as intended.
If any part of the code is behind a feature flag, my test instructions also cover cases where the feature flag is switched off.
I have written this PR in accordance with my team's definition of done.
I have checked that the base branch is correctly set.

Innovation

No innovation project is applicable for this PR.
This PR falls under an innovation project. I have attached the innovation label.
I have added my hours to the WBSO document.

Fixes https://github.com/Yoast/reserved-tasks/issues/421

igorschoester · 2025-02-06T09:39:09Z

+	try {
+		return numberFormat.format( data );
+	} catch ( e ) {
+		return data.toString( 10 );
+	}


There can be errors if the setup is wrong, but currently this can not error (at least AFAIK).
But it seems nice to keep this in, unrelated to the numberFormat we get. Just that currently the catch has no test coverage.

igorschoester · 2025-02-06T09:40:20Z

+ * @param {import("../services/data-formatter")} dataFormatter The data formatter.
+ * @returns {function(?TopPageData[]): TopPageData[]} Function to format the top pages data.
+ */
+export const createTopPageFormatter = ( dataFormatter ) => ( data = [] ) => data.map( ( item ) => ( {


Exporting this just for easy use in the test 😅

github-actions · 2025-02-07T15:15:54Z

A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch.

* get the locale from the HTML tag * pass the data formatter in the widget factory * use the data formatter int he top pages widget * add tests # Conflicts: # packages/js/src/dashboard/services/widget-factory.js # packages/js/src/general/initialize.js

* ignore complexity in setup, don't think that will be nicer if spread out over more functions * add doc block in mock * decrease allowed warnings to be the same as the current amount

coveralls · 2025-02-10T09:29:59Z

Pull Request Test Coverage Report for Build 23527d3e85b95e18e96cfffbdce3864bc740d719

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

29 of 34 (85.29%) changed or added relevant lines in 4 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.02%) to 54.597%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
packages/js/src/dashboard/services/data-formatter.js	22	23	95.65%
packages/js/src/general/initialize.js	0	4	0.0%

Totals
Change from base Build 2d39f817ad47444fce9230e581a5710e9e3958b5:	0.02%
Covered Lines:	30341
Relevant Lines:	55995

💛 - Coveralls

vraja-pro · 2025-02-10T13:52:18Z

CR & AC

igorschoester added the changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog label Feb 5, 2025

igorschoester force-pushed the 421-create-frontend-data-formatter-and-the-wp-implementation branch from 87f2330 to 8ef1153 Compare February 6, 2025 08:10

igorschoester marked this pull request as ready for review February 6, 2025 09:34

igorschoester commented Feb 6, 2025

View reviewed changes

igorschoester force-pushed the 421-create-frontend-data-formatter-and-the-wp-implementation branch from a381ce9 to e38fd01 Compare February 6, 2025 13:59

github-actions Bot added the merge conflict label Feb 7, 2025

Base automatically changed from 420-create-frontend-data-provider-and-wp-implementation to trunk February 10, 2025 07:40

igorschoester added 3 commits February 10, 2025 10:10

Add data formatter

80eee5e

* get the locale from the HTML tag * pass the data formatter in the widget factory * use the data formatter int he top pages widget * add tests # Conflicts: # packages/js/src/dashboard/services/widget-factory.js # packages/js/src/general/initialize.js

Fix linting

9358fcf

* ignore complexity in setup, don't think that will be nicer if spread out over more functions * add doc block in mock * decrease allowed warnings to be the same as the current amount

Add siteKitSetup as widget type

790b323

igorschoester force-pushed the 421-create-frontend-data-formatter-and-the-wp-implementation branch from e38fd01 to 790b323 Compare February 10, 2025 09:18

github-actions Bot removed the merge conflict label Feb 10, 2025

Remove URL hostname

48f6e22

vraja-pro added this to the 24.6 milestone Feb 10, 2025

vraja-pro merged commit 7b6105e into trunk Feb 10, 2025

vraja-pro deleted the 421-create-frontend-data-formatter-and-the-wp-implementation branch February 10, 2025 13:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add data formatter#22020

Add data formatter#22020
vraja-pro merged 4 commits intotrunkfrom
421-create-frontend-data-formatter-and-the-wp-implementation

igorschoester commented Feb 5, 2025 •

edited

Loading

Uh oh!

igorschoester Feb 6, 2025

Uh oh!

igorschoester Feb 6, 2025

Uh oh!

github-actions Bot commented Feb 7, 2025

Uh oh!

coveralls commented Feb 10, 2025 •

edited

Loading

Uh oh!

vraja-pro commented Feb 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

igorschoester commented Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Summary

Relevant technical choices:

Test instructions

Test instructions for the acceptance test before the PR gets merged

Without feature flag

Relevant test scenarios

Test instructions for QA when the code is in the RC

Impact check

UI changes

Other environments

Documentation

Quality assurance

Innovation

Uh oh!

igorschoester Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

igorschoester Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Feb 7, 2025

Uh oh!

coveralls commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 23527d3e85b95e18e96cfffbdce3864bc740d719

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

vraja-pro commented Feb 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

igorschoester commented Feb 5, 2025 •

edited

Loading

coveralls commented Feb 10, 2025 •

edited

Loading