Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map Block: Fix missing Mapbox access token on WPCOM #14440

Closed
wants to merge 1 commit into from

Conversation

Copons
Copy link
Contributor

@Copons Copons commented Jan 23, 2020

Changes proposed in this Pull Request:

In #14307, we let WPCOM users use the Map block without providing a Mapbox access token.
For some reasons, the WPCOM_REST_API_V2_Endpoint_Service_API_Keys class is not instantiated on WPCOM when called directly, from the front end.

This PR changes how we access that endpoint: instead of calling it directly, we call it remotely as intended, leaving to the endpoint all the needed checks.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • Bug fix

Testing instructions:

Basically repeat the #14307 tests:

Prerequisites

  • Have a Simple, Atomic, and Jetpack test sites ready.
  • Make sure none of them has a Mapbox token saved. In that case, before applying this PR, please remove it in the Mapbox Access Token field in the Map block sidebar.

Jetpack

  • Open the editor and insert a Map block.
  • Make sure the Map block loads in the placeholder state, and asks for a token.
  • Provide a token, and make sure the map loads correctly.
  • Make sure it's possible to change and remove the token in the Mapbox Access Token field in the block sidebar.
  • Save the post and check the front end.
  • Make sure the Map block is rendered correctly.
  • Update the token in the Mapbox Access Token field in the block sidebar with a random incorrect string.
  • Wait a bit for the API call to complete and then make sure the block reverted to the placeholder state, with an "incorrect token" error notice.

Simple

  • Apply D38065-code and sandbox the API.
  • Open the editor and insert a Map block.
  • Make sure the Map block loads without asking for an access token.
  • Make sure there is no Mapbox Access Token field in the block sidebar.
  • Save the post and check the front end.
  • Make sure the Map block is rendered correctly.

Atomic

  • Install and activate the Jetpack Beta plugin.
  • In wp-admin, navigate to Jetpack -> Jetpack Beta; search for the fix/map-block-missing-api-key-wpcom feature branch, and activate it.
  • Repeat the same testing steps for a Simple site (skip the first step).

Proposed changelog entry for your changes:

No changelog entry needed: this is a hotfix for a still unreleased feature.

@Copons Copons added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Block] Map labels Jan 23, 2020
@Copons Copons requested review from jeherve and a team January 23, 2020 15:52
@Copons Copons self-assigned this Jan 23, 2020
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello Copons! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D38065-code before merging this PR. Thank you!

@jetpackbot
Copy link

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: February 11, 2020.
Scheduled code freeze: February 4, 2020

Generated by 🚫 dangerJS against aa6cb5f

@jeherve
Copy link
Member

jeherve commented Jan 23, 2020

Closing in favour of #14442, this wasn't working on Atomic (the endpoint isn't accessible to non-wpcom sites).

@jeherve jeherve closed this Jan 23, 2020
@Copons Copons deleted the fix/map-block-missing-api-key-wpcom branch January 23, 2020 16:49
jeherve added a commit that referenced this pull request Jan 23, 2020
@kraftbj kraftbj removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Map Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants