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

Preferences persistence - mark modified property as not readonly in schema #40637

Merged
merged 2 commits into from
May 2, 2022

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Apr 27, 2022

What?

Changes a user meta _modified schema property (used for persisting preferences) to not be readonly.

Why?

As mentioned here, it shouldn't be readonly - #39795 (comment)

The property is updated in a PUT request, so that means it shouldn't be read only.

This doesn't seem to change anything in terms of the actual user experience. Possibly this schema is only used for docs and the readonly aspect has to be implemented separately.

Testing Instructions

Test that persisted preferences still work:

  1. Login to WordPress using the same user in two different browsers (e.g. chrome and firefox)
  2. Edit a post in each browser
  3. Change a preference in one browser (e.g. enable Top Toolbar)
  4. Reload the page on the other browser

Expected - the preference change should be reflected (e.g. top toolbar should be enabled)

@talldan talldan added the [Type] Code Quality Issues or PRs that relate to code quality label Apr 27, 2022
@talldan talldan self-assigned this Apr 27, 2022
@talldan talldan requested a review from noisysocks April 27, 2022 06:10
@noisysocks
Copy link
Member

This doesn't seem to change anything in terms of the actual user experience. Possibly this schema is only used for docs and the readonly aspect has to be implemented separately.

I think that's true cc. @TimothyBJacobs

@TimothyBJacobs
Copy link
Member

We should also remove context from the _modified property since the same value is defined for the entire schema.

This doesn't seem to change anything in terms of the actual user experience. Possibly this schema is only used for docs and the readonly aspect has to be implemented separately.

I think that's true cc. @TimothyBJacobs

More or less. Top level readonly properties get removed from the args description, but it isn't really a complete api.

https://github.com/WordPress/wordpress-develop/blob/d47a44ab68c8afc413b7f7b63ffd89498d213031/src/wp-includes/rest-api.php#L3245

@talldan
Copy link
Contributor Author

talldan commented Apr 28, 2022

We should also remove context from the _modified property since the same value is defined for the entire schema.

Thanks for spotting that, I've pushed a commit to remove it 👍

@talldan talldan merged commit d0b600f into trunk May 2, 2022
@talldan talldan deleted the fix/meta-property-incorrectly-marked-as-readonly branch May 2, 2022 01:00
@github-actions github-actions bot added this to the Gutenberg 13.2 milestone May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants