Allow /sites/${site}/external-media/copy/pexels to insert post meta data

[mreishus]

Changes proposed in this Pull Request:

• Allow /sites/${site}/external-media/copy/pexels to insert post meta data

Does this pull request change what data or activity we track or use?

No

Testing instructions (Jetpack):

• N/A

Testing Instructions (Unit Tests):

• jetpack docker phpunit -- --filter=Endpoint_External_Media

Testing instructions (WPCOM):

• Apply D69692-code to sandbox
• Visit internal vertical-images tool (See: D69577-code for full instructions)
• Choose a vertical
• Enter a search term (example: "building") and press enter or click on magnifying glass
• See photos
• Click on at least on image to select it
• Click on the "Add 1 photos to VERTICAL" button
• see no feedback but use devtools to see that the POST happened :)
• sandbox PduBjF-5-p2 (See: D69577-code for full instructions)
• Visit https:///wp-admin/upload.php (See: D69577-code for full instructions)
• click on the photo you uploaded
• click on edit more details
• On sandbox, edit wp-admin/post.php and add

$meta = get_post_meta( $post_id );
l(compact('meta'));

around line 37, inside the if ( $post_id ) { block

• F5 the url (should look like https://<vertical site>/wp-admin/post.php?post=49&action=edit) and tail -f /tmp/php-errors
• Expect to see the metadata

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello mreishus! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D69692-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ⚠️ All commits were linted before commit.
  
• ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

---

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

---

Jetpack plugin:

• Next scheduled release: December 7, 2021.
• Scheduled code freeze: November 30, 2021.

[obenland]

I love the unit test you added!

[obenland]

It's too bad additionalProperties doesn't cover that. I wonder if it doesn't work recursively but only in a top-level object?

[mreishus]

I believe that's the case.

[obenland]

It doesn't look like string values are actually sanitized unless we add a format. Should we do that?

[mreishus]

I added this, but after sending a vertical_id of test_id_with_spaces_at_end (there are multiple spaces at the end of this that github is filtering out), it's saved that way to the database, which makes me think it's not working. (Expected to see: Multiple spaces collapsed to one)

[obenland]

Maybe all of these sanitations don't work nested?

[obenland]

I'm not sure if defining an enum parameter would help with validation here?

[mreishus]

I added this, but it doesn't actually seem to work. Probably because of the same nested level thing. Maybe I should keep it anyway just to be more explicit?

[obenland]

Ah that is really too bad

[mreishus]

Updated w/ additional validation.

[obenland]

@mreishus New idea: Let's sanitize the meta part separately:

public $meta_schema    => array(
 				'type'                 => 'object',
 				'additionalProperties' => false,
 				'properties'           => array(
 					'vertical_id'   => array(
 						'type'   => 'string',
 						'format' => 'text-field',
 					),
 					'pexels_object' => array(
 						'type' => 'object',
 					),
 					'orientations'  => array(
 						'type' => 'array',
 						'enum' => array( 'landscape', 'portrait', 'square' ),
 					),
 				),
 			);

In sanitize_media:

public function sanitize_media( $param ) {
		$param.        = $this->prepare_media_param( $param );
		$param['meta'] = rest_sanitize_value_from_schema( $param['meta'], $this->meta_schema );
		
		return rest_sanitize_value_from_schema( $param, $this->media_schema );
	}

Does that work?

[mreishus]

@mreishus New idea: Let's sanitize the meta part separately:
Does that work?

No, it doesn't work. The closest I found was this:

	public function validate_media( $param ) {
		$param = $this->prepare_media_param( $param );
		foreach ( array_keys($param) as $i ) {
			$param[$i]['meta'] = rest_validate_value_from_schema( $param[$i]['meta'], $this->meta_schema );
		}

		return rest_validate_value_from_schema( $param, $this->media_schema, 'media' );
	}

That ends up replacing one of the sub-parts of $param with a WP_Error object:

WP_Error Object                                                               
(                           
    [errors] => Array
        (                  
            [rest_not_in_enum] => Array 
                (           
                    [0] => [orientations] is not one of portrait and square.                                                                                
                )                                                             
                                                                              
        )                                                                     
                                       
    [error_data] => Array 
        (        
        )                                                                                                                                                   
                                                                                                                                                            
    [additional_data:protected] => Array                                                                                                                    
        (                                                                                                                                                   
        )                                                                                                                                                   
                                                                                                                                                            
)                                                                                                                                                           

However, whatever is downstream/upstream of this and looking for errors doesn't find it, and the invalid orientations are saved anyway, and no error is passed to the user.

For sanitization, neither

public function sanitize_media( $param ) {
		$param        = $this->prepare_media_param( $param );
		$param['meta'] = rest_sanitize_value_from_schema( $param['meta'], $this->meta_schema );
		
		return rest_sanitize_value_from_schema( $param, $this->media_schema );
	}

nor

	public function sanitize_media( $param ) {
		$param = $this->prepare_media_param( $param );
		foreach ( array_keys($param) as $i ) {
			$param[$i]['meta'] = rest_sanitize_value_from_schema( $param[$i]['meta'], $this->meta_schema );
		}
		return rest_sanitize_value_from_schema( $param, $this->media_schema );
	}

changes vertical_id=test_id_with_spaces_at_end (multiple spaces) when given the definition

	public $meta_schema = array(
		'type'                 => 'object',
		'additionalProperties' => false,
		'properties'           => array(
			'vertical_id'   => array(
				'type'   => 'string',
				'format' => 'text-field',
			),
			'pexels_object' => array(
				'type' => 'object',
			),
			'orientations'  => array(
				'type' => 'array',
				'enum' => array( 'portrait', 'square' ),
			),
		),
	);

Additionally, I cannot find any instances of format.*text-field in jetpack or wpcom (except for minified files with very long lines).

[mreishus]

We figured out a way to get the schema validation working in slack chat; updated.

[mreishus]

Reworked with adding a missing array layer and removing workarounds

[sixhours]

I'm seeing the correct metadata come through:

[mreishus]

Rebased, only need crew review I believe

[sixhours]

Still working after rebase! 👍

[mreishus]

@jeherve I can't seem to merge, only auto-merge. I see the required check Tests / PHP tests: PHP 8.0 WP latest (pull_request) Failing after 3m — PHP tests: PHP 8.0 WP latest is failing. Here's the message, I'm not sure if this is a new failure or an expected one:

[jeherve]

Here's the message, I'm not sure if this is a new failure or an expected one:

I think that was a one-off. I've relaunched the test, it should be okay this time. 🤞

[github-actions]

Great news! One last step: head over to your WordPress.com diff, D69692-code, and commit it.
Once you've done so, come back to this PR and add a comment with your changeset ID.

Thank you!

[mreishus]

r234959