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

Jetpack App: Show all sites and allow site creation #15920

Conversation

ashiagr
Copy link
Contributor

@ashiagr ashiagr commented Feb 9, 2022

Closes #15916
Depends on wordpress-mobile/WordPress-Login-Flow-Android#79

This PR

  • Enables showing all sites (no Jetpack filtering).
  • Enables the ability to create a new site.
  • Disables adding self-hosted site option while creating a new site.

To test:

  1. Install and login to the Jetpack app.
  2. Notice that all sites are shown on the Login Epilogue screen.
  3. Click Done button.
  4. Go to Site Picker from My Site tab.
  5. Notice that all sites are shown on the Site Picker screen.
  6. Notice that + button exists on top right of the Site Picker screen.
  7. Click + button.
  8. Notice that option to add self-hosted sites does not exists and wpcom site creation flow starts with "Choose a design" option.

TBD - Added to main task

  • Remove Quick Start flow after site creation?

Merging Notes:

  • Targets a feature branch that will include similar smaller PRs to enable Jetpack app features.
  • Depends on a draft login lib PR removing Jetpack app sites filtering that cannot be merged with trunk right now.
  • Don't merge the PR yet, I'll take care of the merging.

Regression Notes

  1. Potential unintended areas of impact
    Affected logic in WordPress Android app

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

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

PR submission checklist:

  • I have completed the Regression Notes.
  • 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.

@ashiagr ashiagr self-assigned this Feb 9, 2022
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Feb 9, 2022

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

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Feb 9, 2022

You can test the changes on this Pull Request by downloading the APKs:

Copy link
Contributor

@zwarm zwarm left a comment

Choose a reason for hiding this comment

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

🔆 Nice work. I can log into the Jetpack app with any wp.com account and see all sites. As expected, I can't log into the Jetpack app with a self-hosted site unless I enter WP.com credentials.

@ashiagr
Copy link
Contributor Author

ashiagr commented Feb 10, 2022

Thanks for a review, @zwarm!

I felt jittery in removing these conditions all over the code, so I introduced BuildConfig constants that will allow us to enable/ disable features in the Jetpack app in the future as follows:

  • ENABLE_SITE_CREATION - Controls

    • "+" menu visibility in the Site Picker screen. 200208f
    • New Login Epilogue screen with "Create a new site" button (I enabled this screen to match with iOS). 861ad82
  • ENABLE_ADD_SELF_HOSTED_SITE - Controls "Add self-hosted site" option via "+" menu in the Site Picker screen. 4519984

Note: I haven't introduced any constant for filtering Jetpack sites as it needs to be passed to the Login lib and felt a bit tricky to be controlled with LoginMode and requiring changes to WooCommerce app etc. At least we'll have constants for enabling other features for now and we can revisit it later.

Can you give it another pass? Also, check that the new Login Epilogue screen is shown after login. 🙇‍♀️

@ashiagr ashiagr requested a review from zwarm February 11, 2022 13:33
@ashiagr ashiagr merged commit 08c3a64 into feature/15915-enable-jetpack-app-features Feb 11, 2022
@ashiagr ashiagr deleted the issue/15916-enable-all-sites-jetpack-app branch February 11, 2022 13:54
@ashiagr
Copy link
Contributor Author

ashiagr commented Feb 14, 2022

Reviewed as part of feature branch PR #15946.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants