Story Locking: Ensure author users and below correctly see information (

#12559)
GoogleForCreators · Nov 3, 2022 · b4003ff · b4003ff
1 parent 80e2e2e
commit b4003ff
Show file tree

Hide file tree

Showing 11 changed files with 152 additions and 55 deletions.
diff --git 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/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/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',

diff --git 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/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,

diff --git 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
@@ -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/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/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

diff --git 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
@@ -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 );