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

Global styles revisions: remove human time diff and custom author fields from the API response #50234

Merged
merged 3 commits into from
May 2, 2023

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented May 2, 2023

What?

Like it says in the title, this PR removes the human friendly time diff ('date_display') and custom author fields ('author_display_name' and 'author_avatar_url')

Why, for the love of all that is pure and innocent?!

To make the response appear a little closer to the regular post revisions object.

The assumption is that the consumer can format the raw dates and grab user information as they wish.

Pursuant to the last point, there's a humanTimeDiff JS function in the works that will take care of this. and, furthermore, we can match revision authors on the frontend using the response from getUsers.

See #50089 for context

How?

Liberal use of my keyboard's delete key, which now has dried pasta sauce on it because I didn't wipe my fingers properly after lunch. I am not a monster!

Testing Instructions

Run the tests!

npm run test:unit:php -- --filter Gutenberg_REST_Global_Styles_Revisions_Controller_Test

You can also check the response in the console. Make sure you're in the site editor and your global styles have a few revisions already.

await wp.apiFetch( { url: `/index.php?rest_route=%2Fwp%2Fv2%2Fglobal-styles%2F${ wp.data.select('core').__experimentalGetCurrentGlobalStylesId() }%2Frevisions&_locale=user` } );

date_display, author_display_name and author_avatar_url should no longer appear in the response objects

Screenshot 2023-05-02 at 3 09 06 pm

ramonjd added 2 commits May 2, 2023 13:26
- removes the human friendly time diff to make the response appear a little closer to the regular post revisions object
- the assumption is that the consumer can format the raw dates as they wish
- updates tests
- removes the specific author/user properties from the rest controller
- updates tests
@ramonjd ramonjd changed the title Global styles: remove human time diff from the API response Global styles: remove human time diff and custom author fields from the API response May 2, 2023
@ramonjd ramonjd self-assigned this May 2, 2023
@ramonjd ramonjd added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label May 2, 2023
@ramonjd ramonjd marked this pull request as ready for review May 2, 2023 05:09
@ramonjd ramonjd requested a review from spacedmonkey as a code owner May 2, 2023 05:09
@ramonjd ramonjd mentioned this pull request May 2, 2023
4 tasks
@andrewserong andrewserong added the [Type] Enhancement A suggestion for improvement. label May 2, 2023
Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

Thanks for the update, this makes for a much leaner response when there's heaps of revisions, too 👍

✅ Response has the expected fields, with the no longer needed ones removed
✅ An OPTIONS request returns the correct fields in the schema, tested by running:

await wp.apiFetch( { method: 'OPTIONS', url: `/index.php?rest_route=%2Fwp%2Fv2%2Fglobal-styles%2F${ wp.data.select('core').__experimentalGetCurrentGlobalStylesId() }%2Frevisions&_locale=user` } );

See #49050 for context

Do you mean #50089 😄

Liberal use of my keyboard's delete key, which now has dried pasta sauce on it because I didn't wipe my fingers properly after lunch. I am not a monster!

That's okay, my keyboard has pumpkin on it now because I also forgot to wash my hands after chopping veggies for the soup I made. Lesson learnt!

LGTM ✨

@andrewserong andrewserong changed the title Global styles: remove human time diff and custom author fields from the API response Global styles revisions: remove human time diff and custom author fields from the API response May 2, 2023
@ramonjd
Copy link
Member Author

ramonjd commented May 2, 2023

Do you mean #50089 😄

Hah, I can't even manage my clipboard history. I'll update the description to save me embarrassment in case anyone comes back to this.

Thank you for testing!

@ramonjd ramonjd merged commit f9df0da into trunk May 2, 2023
@ramonjd ramonjd deleted the update/global-styles-revisions-controller branch May 2, 2023 06:08
@github-actions github-actions bot added this to the Gutenberg 15.8 milestone May 2, 2023
@noisysocks
Copy link
Member

Were there any backwards compatibility concerns here?

@andrewserong
Copy link
Contributor

Were there any backwards compatibility concerns here?

No, I don't think so. The endpoint was only added last week in #49974 and isn't being used anywhere yet.

@ramonjd
Copy link
Member Author

ramonjd commented May 2, 2023

Were there any backwards compatibility concerns here?

Thanks for raising it. What @andrewserong said.

That's part of the reason why I pared the API response back so much in this PR - there wasn't a very strong use case for these properties and I thought it best to remove them to avoid backwards compatibility issues later. Or laterz. Whichever you prefer 🛹

@noisysocks
Copy link
Member

All good just double checking 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants