Skip to content

Add URL Details endpoint to REST API to allow retrieval of info about a remote URL#18042

Merged
getdave merged 57 commits intomasterfrom
try/add-url-preview-endpoint-to-rest-api
Feb 1, 2021
Merged

Add URL Details endpoint to REST API to allow retrieval of info about a remote URL#18042
getdave merged 57 commits intomasterfrom
try/add-url-preview-endpoint-to-rest-api

Conversation

@getdave
Copy link
Copy Markdown
Contributor

@getdave getdave commented Oct 21, 2019

Closes #18047.

Adds a new REST API endpoint to grab details from a external URL. So now if I want to get the contents of the title tag on https://wordpress.org I can send a request to this API endpoint and it will return the value in the response for use in the Gutenberg UI.

Why might this be useful? See #19387.


As part of #17846 we need to display the contents of the <title> tag from the url the user has typed into the input field. This has now long since shipped. Nonetheless, it might be a nice enhancement to be able to show details about the remote URL that has been selected.

This PR addresses this by adding a new experimental custom Gutenberg REST API endpoint. This endpoint:

  • makes a request for a remote URL.
  • parses the response in a DOMDocument we should investigate this in future (see here)
  • retrieves the title tag contents.

To prevent abuse, the endpoint is authenticated and requires the user to have the edit_posts capability to be able to make a request.

How has this been tested?

  • Manual testing (see below)
  • Automated tests

Automated Testing Instructions

npm run test-php should run the unit tests which reside in phpunit/class-wp-rest-url-details-controller-test.php.

Manual Testing Instructions

  • Check out this PR.
  • Open the Gutenberg local testing env at http://localhost:8889/wp-admin/ and login.
  • Create a new Post.
  • Choose one of the following options:

Option 1

  • Open devtools and in the console enter:
wp.apiFetch( { path: '/__experimental/url-details/?url=https://wordpress.org' } ).then( data => {
    console.log( data );
} );

You'll see the response returned there.

Option 2

  • Open devtools and in the console enter wpApiSettings.nonce. You should see a valid nonce returned as a string.
  • Copy the nonce.
  • In your browser goto: http://localhost:8889/wp-json/__experimental/url-details/?url=https://wordpress.org&_wpnonce=%YOUR_NONCE_HERE% - be sure to replace the nonce value with that copied from the previous step.
  • You should see a valid REST API response containing the contents of the title tag from wordpress.org (ie: "Blog Tool, Publishing Platform, and CMS \u2014 WordPress.org").

Types of changes

New feature (non-breaking change which adds functionality).

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@getdave getdave self-assigned this Oct 21, 2019
@getdave getdave added the REST API Interaction Related to REST API label Oct 21, 2019
@getdave getdave added the Needs Technical Feedback Needs testing from a developer perspective. label Oct 21, 2019
@obenland obenland self-requested a review October 31, 2019 22:10
@obenland obenland marked this pull request as ready for review October 31, 2019 22:10
@obenland obenland self-requested a review October 31, 2019 22:11
Copy link
Copy Markdown
Contributor Author

@getdave getdave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@obenland Thanks for the changes! Really appreciate you taking this on and making it much better.

Left some small comments. Interested in what you think.

We need to wrangle some more feedback from the REST API group in Core.

We also need a rebase to make the playground pass (it's happened on all PRs recently).

Lastly, what do you think about this comment on the Issue?

@obenland obenland force-pushed the try/add-url-preview-endpoint-to-rest-api branch from 2722e34 to 4c330d2 Compare November 1, 2019 22:10
@TimothyBJacobs
Copy link
Copy Markdown
Member

Has @mnelson4's comment been addressed?

@getdave
Copy link
Copy Markdown
Contributor Author

getdave commented Nov 18, 2019

@kadamwhite We'd really appreciate your expertise and input on this if at all possible.

@getdave
Copy link
Copy Markdown
Contributor Author

getdave commented Dec 3, 2019

@obenland Shall we try and move this forward in our next 20% time? What should we do to prepare for that so we can make progress at that time?

@obenland
Copy link
Copy Markdown
Member

obenland commented Dec 3, 2019

Sure. I think double-checking #18042 (comment) would be the next step

@getdave
Copy link
Copy Markdown
Contributor Author

getdave commented Jan 2, 2020

Sure. I think double-checking #18042 (comment) would be the next step

I've addressed the issue raised here.

@getdave getdave force-pushed the try/add-url-preview-endpoint-to-rest-api branch from 4c330d2 to a5d9c25 Compare January 2, 2020 14:46
@getdave getdave mentioned this pull request Jan 2, 2020
6 tasks
@getdave getdave requested review from aduth and draganescu January 16, 2020 10:26
@getdave
Copy link
Copy Markdown
Contributor Author

getdave commented Jan 16, 2020

Waiting on further feedback here. The outstanding issue blocking this from merging is the concern about exposing data about websites behind a firewall.

Oh...and it looks like we have some PHPUnit test warnings @obenland.

@mnelson4
Copy link
Copy Markdown

From what I could test, this doesn't expose data local data. Although I don't have a network behind a firewall etc, so I can't really comment about that.

I tested this out and it worked fine. It did seem a bit odd the response wasn't actually JSON, just a string of the site's title. (I suppose returning JSON leaves the door open to returning other metadata about the site.) I wasn't aware of anywhere else in the REST API that just does that, but it's not the end of the world.

@getdave
Copy link
Copy Markdown
Contributor Author

getdave commented Feb 1, 2021

Merged 🎉

I will raise followups to add extra functionality.

Thanks to everyone for your help in getting this over the line (particularly @TimothyBJacobs and @swissspidy).

@obenland
Copy link
Copy Markdown
Member

obenland commented Feb 1, 2021

Congrats @getdave! Nice work

@getdave
Copy link
Copy Markdown
Contributor Author

getdave commented Feb 5, 2021

@TimothyBJacobs At some point if this endpoint is accepted would I need to follow up with this in WordPess Core?

@TimothyBJacobs
Copy link
Copy Markdown
Member

Whenever the JS feature that uses this feature is slated to be included in Core, we'll create a Core ticket for merging the URL details endpoint. I can take care of merging it into Core.

@getdave
Copy link
Copy Markdown
Contributor Author

getdave commented Feb 6, 2021

Whenever the JS feature that uses this feature is slated to be included in Core, we'll create a Core ticket for merging the URL details endpoint. I can take care of merging it into Core.

That's great thanks. Any chance I could get involved there? Only Ive not committed to WP Core and this seems like a good opportunity.

@TimothyBJacobs
Copy link
Copy Markdown
Member

That's great thanks. Any chance I could get involved there? Only Ive not committed to WP Core and this seems like a good opportunity.

Yeah, you can absolutely take the lead on that!

@getdave
Copy link
Copy Markdown
Contributor Author

getdave commented Apr 22, 2021

Here is a follow up #28791

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Block] Navigation Affects the Navigation Block REST API Interaction Related to REST API [Type] Enhancement A suggestion for improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

REST API endpoint to retrieve information about a remote URL.