From c7f2c1656d8cd36abdee6ea267518bf9b34be59e Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Fri, 28 Oct 2022 13:54:46 +0100 Subject: [PATCH 1/5] Embed user data in REST API for post lock. --- includes/REST_API/Stories_Lock_Controller.php | 65 +++++++++++++++++-- packages/wp-story-editor/src/api/storyLock.js | 6 +- .../src/components/postLock/postLock.js | 24 ++++--- .../REST_API/Stories_Lock_Controller.php | 43 +++++++++++- 4 files changed, 118 insertions(+), 20 deletions(-) diff --git a/includes/REST_API/Stories_Lock_Controller.php b/includes/REST_API/Stories_Lock_Controller.php index 0e27bc4886fb..03a40b9f3537 100644 --- a/includes/REST_API/Stories_Lock_Controller.php +++ b/includes/REST_API/Stories_Lock_Controller.php @@ -303,10 +303,17 @@ public function prepare_item_for_response( $item, $request ) { $lock_data = [ 'locked' => false, 'time' => '', - 'user' => 0, + 'user' => [ + 'name' => '', + 'id' => 0, + ], 'nonce' => $nonce, ]; + if ( get_option( 'show_avatars' ) ) { + $lock_data['user']['avatar'] = []; + } + if ( ! empty( $item ) ) { /** This filter is documented in wp-admin/includes/ajax-actions.php */ $time_window = apply_filters( 'wp_check_post_lock_window', 150 ); @@ -318,6 +325,19 @@ public function prepare_item_for_response( $item, $request ) { 'user' => isset( $item['user'] ) ? (int) $item['user'] : 0, 'nonce' => $nonce, ]; + if ( isset( $item['user'] ) ) { + $user = get_user_by( 'id', $item['user'] ); + if ( $user ) { + $lock_data['user'] = [ + 'name' => $user->display_name, + 'id' => $item['user'], + ]; + + if ( get_option( 'show_avatars' ) ) { + $lock_data['user']['avatar'] = rest_get_avatar_urls( $user ); + } + } + } } } @@ -433,13 +453,50 @@ public function get_item_schema(): array { 'context' => [ 'view', 'edit', 'embed' ], ], 'user' => [ - 'description' => __( 'The ID for the author of the lock.', 'web-stories' ), - 'type' => 'integer', - 'context' => [ 'view', 'edit', 'embed' ], + 'description' => __( 'User', 'web-stories' ), + 'type' => 'object', + 'properties' => [ + 'id' => [ + 'description' => __( 'The ID for the author of the lock.', 'web-stories' ), + 'type' => 'integer', + 'readonly' => true, + 'context' => [ 'view', 'edit', 'embed' ], + ], + 'name' => [ + 'description' => __( 'Display name for the user.', 'web-stories' ), + 'type' => 'string', + 'readonly' => true, + 'context' => [ 'embed', 'view', 'edit' ], + ], + ], ], ], ]; + if ( get_option( 'show_avatars' ) ) { + $avatar_properties = []; + + $avatar_sizes = rest_get_avatar_sizes(); + + foreach ( $avatar_sizes as $size ) { + $avatar_properties[ $size ] = [ + /* translators: %d: Avatar image size in pixels. */ + 'description' => sprintf( __( 'Avatar URL with image size of %d pixels.', 'web-stories' ), $size ), + 'type' => 'string', + 'format' => 'uri', + 'context' => [ 'embed', 'view', 'edit' ], + ]; + } + + $schema['properties']['user']['properties']['avatar'] = [ + 'description' => __( 'Avatar URLs for the user.', 'web-stories' ), + 'type' => 'object', + 'context' => [ 'embed', 'view', 'edit' ], + 'readonly' => true, + 'properties' => $avatar_properties, + ]; + } + $this->schema = $schema; return $this->add_additional_fields_schema( $this->schema ); diff --git a/packages/wp-story-editor/src/api/storyLock.js b/packages/wp-story-editor/src/api/storyLock.js index 254a26adad52..2cad8791d18d 100644 --- a/packages/wp-story-editor/src/api/storyLock.js +++ b/packages/wp-story-editor/src/api/storyLock.js @@ -24,11 +24,7 @@ import { addQueryArgs } from '@googleforcreators/url'; import apiFetch from '@wordpress/api-fetch'; export function getStoryLockById(storyId, stories) { - const path = addQueryArgs(`${stories}${storyId}/lock/`, { - _embed: 'author', - }); - - return apiFetch({ path }); + return apiFetch({ path: `${stories}${storyId}/lock/` }); } export function setStoryLockById(storyId, stories) { diff --git a/packages/wp-story-editor/src/components/postLock/postLock.js b/packages/wp-story-editor/src/components/postLock/postLock.js index 8a5568dee40f..462167fb6109 100644 --- a/packages/wp-story-editor/src/components/postLock/postLock.js +++ b/packages/wp-story-editor/src/components/postLock/postLock.js @@ -96,13 +96,17 @@ function PostLock() { // When async call only if dialog is true, current user is loaded and post locking is enabled. const doGetStoryLock = useCallback(() => { - if (showLockedDialog && currentUserLoaded) { - getStoryLockById(storyId, stories) - .then(({ locked, nonce: newNonce, _embedded }) => { + (async () => { + if (showLockedDialog && currentUserLoaded) { + try { + const { + locked, + nonce: newNonce, + user, + } = await getStoryLockById(storyId, stories); const lockAuthor = { - id: _embedded?.author?.[0]?.id || 0, - name: _embedded?.author?.[0]?.name || '', - avatar: _embedded?.author?.[0]?.avatar_urls?.['96'] || '', + ...user, + avatar: user?.avatar?.['96'] || '', }; if (locked && initialOwner === null) { setInitialOwner(lockAuthor); @@ -114,11 +118,11 @@ function PostLock() { } // Refresh nonce on every request. setNonce(newNonce); - }) - .catch((err) => { + } catch (err) { trackError('post_lock', err.message); - }); - } + } + } + })(); }, [ setCurrentOwner, storyId, diff --git a/tests/phpunit/integration/tests/REST_API/Stories_Lock_Controller.php b/tests/phpunit/integration/tests/REST_API/Stories_Lock_Controller.php index f13098562ed2..8184686efcd0 100644 --- a/tests/phpunit/integration/tests/REST_API/Stories_Lock_Controller.php +++ b/tests/phpunit/integration/tests/REST_API/Stories_Lock_Controller.php @@ -69,7 +69,7 @@ public function test_register(): void { $routes = rest_get_server()->get_routes(); $this->assertArrayHasKey( '/web-stories/v1/web-story/(?P[\d]+)/lock', $routes ); - $this->assertCount( 3, $routes['/web-stories/v1/web-story/(?P[\d]+)/lock'] ); + $this->assertCount( 6, $routes['/web-stories/v1/web-story/(?P[\d]+)/lock'] ); } /** @@ -195,6 +195,47 @@ public function test_get_item_with_lock(): void { $data = $response->get_data(); $links = $response->get_links(); $this->assertArrayHasKey( 'locked', $data ); + $this->assertArrayHasKey( 'user', $data ); + $this->assertArrayHasKey( 'id', $data['user'] ); + $this->assertArrayHasKey( 'name', $data['user'] ); + $this->assertArrayHasKey( 'avatar', $data['user'] ); + $this->assertSame( self::$author_id, $data['user']['id'] ); + $this->assertTrue( $data['locked'] ); + + $this->assertArrayHasKey( 'self', $links ); + $this->assertArrayHasKey( 'author', $links ); + } + + /** + * @covers ::get_item + * @covers ::prepare_item_for_response + * @covers ::prepare_links + * @covers ::get_item_permissions_check + */ + public function test_get_item_with_lock_disabled_avatar(): void { + update_option( 'show_avatars', false ); + $this->controller->register(); + + wp_set_current_user( self::$author_id ); + $story = self::factory()->post->create( + [ + 'post_type' => \Google\Web_Stories\Story_Post_Type::POST_TYPE_SLUG, + 'post_status' => 'draft', + 'post_author' => self::$author_id, + ] + ); + $new_lock = ( time() - 100 ) . ':' . self::$author_id; + update_post_meta( $story, '_edit_lock', $new_lock ); + $request = new WP_REST_Request( \WP_REST_Server::READABLE, '/web-stories/v1/web-story/' . $story . '/lock' ); + $response = rest_get_server()->dispatch( $request ); + $data = $response->get_data(); + $links = $response->get_links(); + $this->assertArrayHasKey( 'locked', $data ); + $this->assertArrayHasKey( 'user', $data ); + $this->assertArrayHasKey( 'id', $data['user'] ); + $this->assertArrayHasKey( 'name', $data['user'] ); + $this->assertArrayNotHasKey( 'avatar', $data['user'] ); + $this->assertSame( self::$author_id, $data['user']['id'] ); $this->assertTrue( $data['locked'] ); $this->assertArrayHasKey( 'self', $links ); From 4a64c174215e5da03f2f626c24bf153db7c70f32 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Mon, 31 Oct 2022 10:41:34 +0000 Subject: [PATCH 2/5] Apply suggestions from code review --- .../integration/tests/REST_API/Stories_Lock_Controller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/phpunit/integration/tests/REST_API/Stories_Lock_Controller.php b/tests/phpunit/integration/tests/REST_API/Stories_Lock_Controller.php index 8184686efcd0..783453e1c505 100644 --- a/tests/phpunit/integration/tests/REST_API/Stories_Lock_Controller.php +++ b/tests/phpunit/integration/tests/REST_API/Stories_Lock_Controller.php @@ -69,7 +69,7 @@ public function test_register(): void { $routes = rest_get_server()->get_routes(); $this->assertArrayHasKey( '/web-stories/v1/web-story/(?P[\d]+)/lock', $routes ); - $this->assertCount( 6, $routes['/web-stories/v1/web-story/(?P[\d]+)/lock'] ); + $this->assertCount( 3, $routes['/web-stories/v1/web-story/(?P[\d]+)/lock'] ); } /** From c7b72c0b8e0f7a4f22b3ee1868e655fd4bbd99cd Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Wed, 2 Nov 2022 10:52:32 +0000 Subject: [PATCH 3/5] Use new fields in dashboard as well. --- .../src/api/utils/reshapeStoryObject.js | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/wp-dashboard/src/api/utils/reshapeStoryObject.js b/packages/wp-dashboard/src/api/utils/reshapeStoryObject.js index 48cfadbfde8b..8457db177209 100644 --- a/packages/wp-dashboard/src/api/utils/reshapeStoryObject.js +++ b/packages/wp-dashboard/src/api/utils/reshapeStoryObject.js @@ -32,16 +32,16 @@ export default function reshapeStoryObject(originalStoryData) { preview_link: previewLink, edit_link: editStoryLink, story_poster: storyPoster, - _embedded: { - author = [{ name: '', id: 0 }], - 'wp:lock': lock = [{ locked: false }], - 'wp:lockuser': lockUser = [{ id: 0, name: '' }], - } = {}, + _embedded: { author, 'wp:lock': lock } = {}, _links: links = {}, } = originalStoryData; if (!id) { return null; } + + const { locked = false, user: lockUser = { id: 0, name: '' } } = + lock?.[0] || {}; + const storyAuthor = author?.[0] || { id: 0, name: '' }; const capabilities = { hasEditAction: Object.prototype.hasOwnProperty.call(links, REST_LINKS.EDIT), hasDeleteAction: Object.prototype.hasOwnProperty.call( @@ -59,14 +59,13 @@ export default function reshapeStoryObject(originalStoryData) { modified, modifiedGmt: `${modified_gmt}Z`, author: { - name: author[0].name, - id: author[0].id, + id: storyAuthor.id, + name: storyAuthor.name, }, - locked: lock[0]?.locked, + locked, lockUser: { - id: lockUser[0].id, - name: lockUser[0].name, - avatar: lockUser[0]?.avatar_urls?.['96'] || null, + ...lockUser, + avatar: lockUser?.avatar?.['96'] || null, }, bottomTargetAction: editStoryLink, featuredMediaUrl: storyPoster?.url, From 5fef560007f1b98e874baebfb365a339f4f15b99 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Wed, 2 Nov 2022 11:37:14 +0000 Subject: [PATCH 4/5] Remove all references to wp:lockuser. --- .../wp-dashboard/src/api/constants/index.js | 2 +- .../src/api/utils/test/reshapeStoryObject.js | 36 +++++++++---------- .../src/api/constants/index.js | 2 +- .../src/api/utils/transformStoryResponse.js | 6 ++-- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/packages/wp-dashboard/src/api/constants/index.js b/packages/wp-dashboard/src/api/constants/index.js index 38ca210a4722..e8da92c6b56b 100644 --- a/packages/wp-dashboard/src/api/constants/index.js +++ b/packages/wp-dashboard/src/api/constants/index.js @@ -35,7 +35,7 @@ export const STORY_FIELDS = [ export const SEARCH_PAGES_FIELDS = ['id', 'title']; export const GET_PAGE_FIELDS = ['title', 'link']; -export const STORY_EMBED = 'wp:lock,wp:lockuser,author'; +export const STORY_EMBED = 'wp:lock,author'; export const REST_LINKS = { EDIT: 'wp:action-edit', diff --git a/packages/wp-dashboard/src/api/utils/test/reshapeStoryObject.js b/packages/wp-dashboard/src/api/utils/test/reshapeStoryObject.js index 89050973a3b5..5a75686dbb7f 100644 --- a/packages/wp-dashboard/src/api/utils/test/reshapeStoryObject.js +++ b/packages/wp-dashboard/src/api/utils/test/reshapeStoryObject.js @@ -51,18 +51,18 @@ describe('reshapeStoryObject', () => { }, _embedded: { author: [{ id: 1, name: 'admin' }], - 'wp:lock': [{ locked: true, time: '1628506372', user: 1 }], - 'wp:lockuser': [ + 'wp:lock': [ { - id: 1, - name: 'admin', - url: 'http://localhost:8899', - description: '', - link: 'http://localhost:8899/author/admin', - avatar_urls: { - 24: 'http://2.gravatar.com/avatar/b642b4217b34b1e8d3bd915fc65c4452?s=24&d=mm&r=g', - 48: 'http://2.gravatar.com/avatar/b642b4217b34b1e8d3bd915fc65c4452?s=48&d=mm&r=g', - 96: 'http://2.gravatar.com/avatar/b642b4217b34b1e8d3bd915fc65c4452?s=96&d=mm&r=g', + locked: true, + time: '1628506372', + user: { + id: 1, + name: 'admin', + avatar: { + 24: 'http://2.gravatar.com/avatar/b642b4217b34b1e8d3bd915fc65c4452?s=24&d=mm&r=g', + 48: 'http://2.gravatar.com/avatar/b642b4217b34b1e8d3bd915fc65c4452?s=48&d=mm&r=g', + 96: 'http://2.gravatar.com/avatar/b642b4217b34b1e8d3bd915fc65c4452?s=96&d=mm&r=g', + }, }, }, ], @@ -127,14 +127,14 @@ describe('reshapeStoryObject', () => { }, _embedded: { author: [{ id: 1, name: 'admin' }], - 'wp:lock': [{ locked: true, time: '1628506372', user: 1 }], - 'wp:lockuser': [ + 'wp:lock': [ { - id: 1, - name: 'admin', - url: 'http://localhost:8899', - description: '', - link: 'http://localhost:8899/author/admin', + locked: true, + time: '1628506372', + user: { + id: 1, + name: 'admin', + }, }, ], }, diff --git a/packages/wp-story-editor/src/api/constants/index.js b/packages/wp-story-editor/src/api/constants/index.js index 6d52c2a10de7..d405104f3aab 100644 --- a/packages/wp-story-editor/src/api/constants/index.js +++ b/packages/wp-story-editor/src/api/constants/index.js @@ -35,7 +35,7 @@ export const STORY_FIELDS = [ '_links', ].join(','); -export const STORY_EMBED = 'wp:lockuser,author,wp:publisherlogo,wp:term'; +export const STORY_EMBED = 'wp:lock,author,wp:publisherlogo,wp:term'; export const MEDIA_FIELDS = [ 'id', diff --git a/packages/wp-story-editor/src/api/utils/transformStoryResponse.js b/packages/wp-story-editor/src/api/utils/transformStoryResponse.js index e2c516db4733..3d374c449607 100644 --- a/packages/wp-story-editor/src/api/utils/transformStoryResponse.js +++ b/packages/wp-story-editor/src/api/utils/transformStoryResponse.js @@ -36,9 +36,9 @@ function transformStoryResponse(post) { capabilities: {}, extras: { lockUser: { - id: embedded?.['wp:lockuser']?.[0].id || 0, - name: embedded?.['wp:lockuser']?.[0].name || '', - avatar: embedded?.['wp:lockuser']?.[0].avatar_urls?.['96'] || '', + id: embedded?.['wp:lock']?.[0].user.id || 0, + name: embedded?.['wp:lock']?.[0].user.name || '', + avatar: embedded?.['wp:lock']?.[0].user.avatar?.['96'] || '', }, }, featuredMedia: storyPoster From bb234cb202b5d2e712709f6fb140f35dc4b05a35 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Wed, 2 Nov 2022 12:26:19 +0000 Subject: [PATCH 5/5] Remove more wp:lockuser. --- includes/Admin/Dashboard.php | 2 +- includes/templates/admin/edit-story.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/Admin/Dashboard.php b/includes/Admin/Dashboard.php index 46eb643cac89..8e7a08386007 100644 --- a/includes/Admin/Dashboard.php +++ b/includes/Admin/Dashboard.php @@ -333,7 +333,7 @@ public function load_stories_dashboard(): void { '_embed' => rawurlencode( implode( ',', - [ 'wp:lock', 'wp:lockuser', 'author' ] + [ 'wp:lock', 'author' ] ) ), 'context' => 'edit', diff --git a/includes/templates/admin/edit-story.php b/includes/templates/admin/edit-story.php index 4a8616e9edc5..eb69165a59f7 100644 --- a/includes/templates/admin/edit-story.php +++ b/includes/templates/admin/edit-story.php @@ -95,7 +95,7 @@ '_embed' => rawurlencode( implode( ',', - [ 'wp:lockuser', 'author', 'wp:publisherlogo', 'wp:term' ] + [ 'wp:lock', 'author', 'wp:publisherlogo', 'wp:term' ] ) ), 'context' => 'edit',