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

Instagram Gallery Block: Stop automatically connecting blocks without access token #15792

Merged
merged 5 commits into from
May 18, 2020

Conversation

Copons
Copy link
Contributor

@Copons Copons commented May 14, 2020

Changes proposed in this Pull Request:

  • Only attempt to automatically connect to Instagram if the Instagram Gallery block has an access token.
Previously

On mount, IG blocks automatically attempted to connect to Instagram.
If they had a token stored, they validated it and, if valid, used it.
If they didn't have a token, they fetched it and, if existing, used it

This had the intended side effect of automatically connecting new blocks inserted by users with valid tokens.
But it also had the unintended side effect of... automatically connecting blocks inserted by other users and left "empty" (not connected, without access token) in a post.

Now...

On mount, IG blocks only automatically attempt to connect to Instagram if they have a token stored.
They will validate it and, if valid, use the token.

"Empty" blocks don't automatically connect anymore.
Instead, they will show the normal "Connect to Instagram" button.
But before sending the user over to Instagram to authorize the connection, we first check if they have a valid token.
If they do, the block will then, finally, automatically use the token, and the user won't be sent to Instagram anymore.
If they don't, they'll proceed to Instagram to create a new token as usual.

Testing instructions:

  • Insert an Instagram Gallery block, and connect it to Instagram.
  • If you have a valid Instagram token, make sure the block embeds the gallery straight away.
  • Otherwise, make sure you are redirected to Instagram to authorize the connection.
  • Insert another block, and make sure it shows the "Connect" button regardless.
  • Connect the second block too (again: no Instagram authorization should be needed).
  • Now disconnect one of these two blocks.
  • Make sure both blocks are disconnected.
  • Connect one of them: make sure you're sent to Instagram to authorize the connection.
  • Make sure the other block is not connected automatically.

Proposed changelog entry for your changes:

  • N/A

@Copons Copons added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Status] Needs Team Review [Block] Latest Instagram Posts labels May 14, 2020
@Copons Copons requested review from a team May 14, 2020 16:24
@Copons Copons self-assigned this May 14, 2020
@jeherve jeherve added this to the 8.6 milestone May 14, 2020
@Copons Copons changed the base branch from update/instagram-gallery-use-gallery-hook to master May 14, 2020 17:39
@Copons Copons force-pushed the fix/instagram-gallery-connection-flow branch from 4768ce3 to 8198364 Compare May 14, 2020 17:40
@jetpackbot
Copy link

jetpackbot commented May 14, 2020

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: June 2, 2020.
Scheduled code freeze: May 26, 2020

Generated by 🚫 dangerJS against 1a3860d

@Copons Copons force-pushed the fix/instagram-gallery-connection-flow branch from 8198364 to 5821494 Compare May 14, 2020 22:12
@Copons
Copy link
Contributor Author

Copons commented May 15, 2020

@scruffian apologize for having moved this back to WIP without notice!

In 8058f64 I've actually removed the auto-connect flow, which was kinda the whole point of the PR.
It wasn't used anymore and the execution shouldn't have really reached it.

I've only kept the "auto-disconnection", which is used when the user disconnects a block, to automatically disconnect all other blocks using the same token.

Copy link
Member

@scruffian scruffian left a comment

Choose a reason for hiding this comment

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

LGTM and works as advertised

@scruffian scruffian added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] Needs Team Review labels May 15, 2020
@jeherve jeherve removed the [Status] Needs Review To request a review from Crew. Label will be renamed soon. label May 15, 2020
@jeherve jeherve added the [Status] Ready to Merge Go ahead, you can push that green button! label May 15, 2020
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This works well in my tests.

@Copons Copons merged commit 0ad3079 into master May 18, 2020
@Copons Copons deleted the fix/instagram-gallery-connection-flow branch May 18, 2020 11:28
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels May 18, 2020
@Copons
Copy link
Contributor Author

Copons commented May 18, 2020

r207600-wpcom

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Latest Instagram Posts [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants