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

Stats: Fix enable stats module flow #16873

Merged
merged 1 commit into from
Jul 15, 2021

Conversation

momo-ozawa
Copy link
Contributor

Fixes #16867

Description

This fixes a bug introduced in #16822, where self-hosted sites not connected to Jetpack weren’t able to enter the Jetpack Install / Connect flow.

What caused the bug / How I fixed it

I was checking if the stats module was disabled in the wrong place. As a result, self-hosted sites that weren't yet connected to Jetpack would only see the Enable Site Stats screen and never have a chance to go through the Jetpack Install / Connect flow.

To fix this, I'm now checking if the stats module is disabled if and only if we know the site is connected to Jetpack (i.e. the blog object has an account associated with it).

How to test

  1. Create a self-hosted site via Jurassic Ninja
  2. Don't connect Jetpack yet
  3. On the app, add the self-hosted site you just created
  4. Go to the Stats screen
  5. ✅ The Jetpack Install screen should be displayed
  6. Since the Jetpack connection flow is a bit flaky on the app, let's connect Jetpack through a browser (Make sure you activate Jetpack AND connect/authorize your account)
  7. On the app, go back to the Stats screen and go through the Jetpack Login screen
  8. Once you're logged in, go to the Stats screen
  9. ✅ The Stats screen should be displayed

Regression Notes

  1. Potential unintended areas of impact
    Jetpack Install / Enable stats module flows

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Tested the above steps

  3. What automated tests I added (or what prevented me from doing so)
    N/A

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

This fixes a bug where self-hosted sites that weren’t connected to Jetpack weren’t able to enter the Jetpack Install/Connect flow.
@peril-wordpress-mobile
Copy link

You can trigger an installable build for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jul 15, 2021

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@momo-ozawa
Copy link
Contributor Author

@thehenrybyrd thank you for spotting this, and @ScoutHarris thanks for the ping!

@mokagio Just a heads up about this PR, it might be good to release a new beta once this PR lands since this also fixes a Stats related bug. Sorry for this bug! 🙇‍♀️

Copy link
Contributor

@leandroalonso leandroalonso left a comment

Choose a reason for hiding this comment

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

:shipit:

@leandroalonso
Copy link
Contributor

@momo-ozawa just noticed you're targeting develop, I think we should target release/17.8

@momo-ozawa momo-ozawa changed the base branch from develop to release/17.8 July 15, 2021 14:24
@momo-ozawa momo-ozawa merged commit 712c74f into release/17.8 Jul 15, 2021
@momo-ozawa momo-ozawa deleted the fix/16867-jetpack-install-flow branch July 15, 2021 14:31
@mokagio mokagio mentioned this pull request Jul 16, 2021
4 tasks
@mokagio
Copy link
Contributor

mokagio commented Jul 16, 2021

@momo-ozawa this has been bundled as part of 17.8 beta 2 (17.8.0.2).

Thanks for your work 🙌

@momo-ozawa momo-ozawa restored the fix/16867-jetpack-install-flow branch August 2, 2021 14:19
@momo-ozawa momo-ozawa deleted the fix/16867-jetpack-install-flow branch October 13, 2021 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't enter Jetpack Install / Connect flow from Stats
3 participants