Add user preference api #1948

notnownikki · 2017-07-19T18:16:13Z

Simple API to get and set user preferences.

nylen · 2017-07-19T18:22:17Z

lib/api.php

+ * @param  string           $request The parameter key for the value being validated.
+ * @return bool             If the preference name is valid.
+ */
+function gutenberg_validate_preference_name( $preference_name, $request, $key ) {


Do we need to wrap all of this logic in functions? Why not just a single static class that also contains a register_routes method?

We don't need to, but merging these and the Gutenberg_User_Preferences would couple it to the API, as it would become dependent on the API request. But perhaps that's not a problem, I'm still getting used to our coding styles and usually I'd have a class that handled the preferences and could be reused outside of the API. I'll change this round.

nylen · 2017-07-19T18:22:46Z

lib/preferences.php

+	 * @return bool             If the preference name is valid preference.
+	 */
+	public static function is_valid_preference_name( $preference_name ) {
+		return in_array( $preference_name, self::VALID_PREFERENCES );


We can add a filter here to allow plugins to register their own user preferences. Not something that needs to be done for this PR.

nylen · 2017-07-19T18:24:34Z

lib/api.php

+function gutenburg_register_routes() {
+	register_rest_route( 'gutenburg/v1', '/user-preferences', array(
+		'methods'  => WP_REST_Server::READABLE,
+		'callback' => 'gutenburg_get_user_preferences',


We will want a permission_callback here and with other endpoints to make sure the user is authenticated (and has whatever capability we deem necessary to use the Gutenberg interface, probably edit_posts). We should be able to re-use the same permission callback for all of these routes..

nylen · 2017-07-19T18:25:48Z

lib/api.php

+				'validate_callback' => 'gutenberg_validate_preference_name',
+			),
+		),
+	) );


Let's combine these two endpoints using the same route ('/user-preferences/(?P<preference>[a-z_]+)') into a single register_rest_route call. See: https://core.trac.wordpress.org/browser/tags/4.8/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php#L90

Actually, now that I think about it, I don't think there are many use cases for getting a single preference. Setting a single preference and setting multiple preferences can be done the same way:

POST /gutenberg/v1/user-preferences { "pref1": "value1", "pref2: "value2" // if needed, and so on }

or the value can be set to null to delete them.

Agreed. Will change this.

notnownikki · 2017-07-20T14:46:38Z

Merged the api routes and functions into the preferences class, fixed a load of lint issues and issues with older PHP versions, and added API tests.

Will squash this up once we're happy.

swissspidy · 2017-07-26T15:18:33Z

Couldn't we just use the users/me endpoint when all that's done is setting some user meta...?

Instead of using the REST API for this, we should think about leveraging the user settings WordPress already has. See wp_user_settings(), get_all_user_settings(), getUserSetting(), setUserSetting(), etc.

The wp-settings-{time}-[UID] cookie is made to store user preferences like the number of list table items, etc. Seems like most recently used blocks would be a good fit for that as well.

We can simply update the cookie on the client side using setUserSetting(), and every time the cookie is sent to WordPress, it will update the user setting.

nylen · 2017-07-27T14:12:25Z

/users/me isn't a great fit here because of permissions: there's no reason to allow users to read each others' preferences, and AFAIK there's no way to support this kind of permission with register_meta.

The existing user settings functions may work fine; I don't think we knew about them, but we should try that first.

nylen · 2017-07-27T14:12:55Z

lib/class-gutenberg-user-preferences.php

+		$user_id = get_current_user_id();
+		$preferences = array();
+		foreach ( self::$valid_preferences as $preference_name ) {
+			$preferences[ $preference_name ] = get_user_meta( $user_id, 'gutenberg_' . $preference_name );


What happens when a preference is unset? It should be excluded from this array.

nylen · 2017-07-27T14:13:14Z

lib/class-gutenberg-user-preferences.php

+		}
+
+		foreach ( $params['preferences'] as $preference_name => $value ) {
+			update_user_meta( $user_id, 'gutenberg_' . $preference_name, $value );


If null is passed here, then the preference should be deleted instead.

nylen · 2017-07-27T14:14:27Z

This code looks nice and clean, the only thing I see that's missing is handling of unset preferences and deleting preferences.

Let's hold off on adding those, though, we may not need this at all. Sorry about the potential waste of effort 😞

notnownikki · 2017-08-21T07:53:28Z

In the end this wasn't required, but before I close and delete the branch, @nylen what was the alternative to this?

swissspidy · 2017-08-21T10:14:37Z

@notnownikki #2140 used the existing user settings functionality I mentioned earlier.

notnownikki · 2017-08-21T10:23:37Z

I'm not quite sure what this would look like using setUserSetting... we'll want to keep track of how often the user uses blocks, so on first load, the user sees their most frequently used blocks. It doesn't look like it's possible to have a user setting that contains more than simple values, so we'd have to have a setting per block type with the number of uses?

swissspidy · 2017-08-21T10:37:40Z

You can totally store arrays etc. with it. No need for per-block-type-settings

notnownikki · 2017-08-21T10:41:00Z

Ah brilliant! Thanks, docs on it were a bit difficult to find, codex. and developer. returned nothing when I searched for it.

notnownikki · 2017-08-22T10:39:53Z

@swissspidy so I've been trying this, and can't get it to work with anything but strings.

window.setUserSetting('test', { option: true } )

results in this:

window.getUserSetting('test')
"objectObject"

I still can't find docs on this, but reading the code, it converts the value to a string just by calling .toString which isn't going to be able to store objects or arrays, so I still can't see how it's a viable alternative to this PR. Can you show me how it should be used?

youknowriad · 2017-08-22T11:09:20Z

I had the same issue when using setUserSetting. I ended up using the userSetting global in the same way setUserSetting does #2462 (comment)

notnownikki · 2017-08-22T11:44:03Z

Closing in anticipation of @youknowriad's PR that will take care of serialization

notnownikki · 2017-08-22T13:27:20Z

Reopening.

Turns out we've been looking at this from different perspectives. There's a need for local storage that persists on the client side (e.g. recently used blocks, session preferences) and storing data that persists across session and browser (e.g. block usage stats so we can determine what blocks a user frequently uses, across all their posts).

This PR lets us fulfil the latter, and there is currently no way of doing this. So, reopening in case js-core discussions don't result in something usable for us.

codecov · 2017-08-22T13:33:07Z

Codecov Report

❗ No coverage uploaded for pull request base (master@659c64f). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff            @@
##             master   #1948   +/-   ##
========================================
  Coverage          ?   27.2%           
========================================
  Files             ?     160           
  Lines             ?    4914           
  Branches          ?     819           
========================================
  Hits              ?    1337           
  Misses            ?    3030           
  Partials          ?     547

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 659c64f...2e1259e. Read the comment docs.

BE-Webdesign · 2017-09-04T15:10:24Z

@notnownikki

I think we should use {action}_user_option over {action}_user_meta. The user options API in WordPress is multisite compatible, where user meta is not. Basically it is a wrapper for user meta but adds the blog id as part of the meta key. Going to check out the code more in depth so we can hopefully get this merged soon.

gziolo · 2018-01-27T11:40:49Z

It looks like #1455 is closed. I'm closing this one as it is very old. Feel free to reopen if this one is still necessary. Thanks :)

notnownikki added 3 commits July 19, 2017 19:13

User preferences API

38a6970

Simple API to get and set user preferences.

Add missing preferences.php

16f32af

docstring fix

226e221

nylen reviewed Jul 19, 2017

View reviewed changes

notnownikki added 4 commits July 20, 2017 11:27

Merge preferences class and API route methods, fix lint issues

deb859a

Lint cleanups, tests for API

0a41626

PHP 5.3 fix

9c63d3c

Docstring fix

2e1259e

notnownikki changed the title ~~[WIP] Add user preference api~~ Add user preference api Jul 24, 2017

nylen reviewed Jul 27, 2017

View reviewed changes

westonruter mentioned this pull request Jul 29, 2017

Add available page templates REST field and selector to sidebar #2086

Closed

2 tasks

nylen mentioned this pull request Aug 1, 2017

Add opt-in usage tracking for blocks added to post content #2140

Merged

notnownikki closed this Aug 22, 2017

notnownikki deleted the add/user-preference-api branch August 22, 2017 11:44

notnownikki restored the add/user-preference-api branch August 22, 2017 13:24

notnownikki reopened this Aug 22, 2017

notnownikki mentioned this pull request Aug 22, 2017

REST API: Custom settings endpoint #1455

Closed

gziolo closed this Jan 27, 2018

gziolo deleted the add/user-preference-api branch January 27, 2018 11:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add user preference api #1948

Add user preference api #1948

notnownikki commented Jul 19, 2017 •

edited by nylen

Loading

nylen Jul 19, 2017

notnownikki Jul 20, 2017

nylen Jul 19, 2017

nylen Jul 19, 2017

nylen Jul 19, 2017

nylen Jul 19, 2017

notnownikki Jul 20, 2017

notnownikki commented Jul 20, 2017

swissspidy commented Jul 26, 2017

nylen commented Jul 27, 2017

nylen Jul 27, 2017

nylen Jul 27, 2017

nylen commented Jul 27, 2017

notnownikki commented Aug 21, 2017

swissspidy commented Aug 21, 2017

notnownikki commented Aug 21, 2017

swissspidy commented Aug 21, 2017

notnownikki commented Aug 21, 2017

notnownikki commented Aug 22, 2017

youknowriad commented Aug 22, 2017

notnownikki commented Aug 22, 2017

notnownikki commented Aug 22, 2017

codecov bot commented Aug 22, 2017 •

edited

Loading

BE-Webdesign commented Sep 4, 2017

gziolo commented Jan 27, 2018

Add user preference api #1948

Add user preference api #1948

Conversation

notnownikki commented Jul 19, 2017 • edited by nylen 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

notnownikki commented Jul 20, 2017

swissspidy commented Jul 26, 2017

nylen commented Jul 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nylen commented Jul 27, 2017

notnownikki commented Aug 21, 2017

swissspidy commented Aug 21, 2017

notnownikki commented Aug 21, 2017

swissspidy commented Aug 21, 2017

notnownikki commented Aug 21, 2017

notnownikki commented Aug 22, 2017

youknowriad commented Aug 22, 2017

notnownikki commented Aug 22, 2017

notnownikki commented Aug 22, 2017

codecov bot commented Aug 22, 2017 • edited Loading

Codecov Report

BE-Webdesign commented Sep 4, 2017

gziolo commented Jan 27, 2018

notnownikki commented Jul 19, 2017 •

edited by nylen

Loading

codecov bot commented Aug 22, 2017 •

edited

Loading