Skip to content

Blogging Prompts: Hide self-hosted sites in the site picker from the prompts intro view - #18829

Merged
wargcm merged 2 commits into
trunkfrom
task/18429-filter-self-hosted-sites-intro
Jun 11, 2022
Merged

Blogging Prompts: Hide self-hosted sites in the site picker from the prompts intro view#18829
wargcm merged 2 commits into
trunkfrom
task/18429-filter-self-hosted-sites-intro

Conversation

@wargcm

@wargcm wargcm commented Jun 3, 2022

Copy link
Copy Markdown
Contributor

See: #18429

Description

Filters out self-hosted sites from the site picker when tapping on 'Try it now' from the prompts feature introduction view.

Testing

Note: I don't have any self-hosted sites, so I manually changed the return value from isAccessibleThroughWPCom. It might be easier doing it this way unless you have a self-hosted site.

To test:

  • Enable bloggingPrompts feature flag in FeatureFlag.swift
  • Log in if necessary and navigate to the dashboard
  • When the prompts feature introduction screen appears, tap on 'Try it now'
  • Verify no self-hosted sites display

Regression Notes

  1. Potential unintended areas of impact
    N/A

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    N/A

  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.

@wpmobilebot

wpmobilebot commented Jun 3, 2022

Copy link
Copy Markdown
Contributor
You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr18829-e932d8e on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot

wpmobilebot commented Jun 3, 2022

Copy link
Copy Markdown
Contributor
You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr18829-e932d8e on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@alpavanoglu alpavanoglu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oops I forgot to publish my review :-] It isn't major but I also think it could be misleading later if we skip it now

@@ -284,9 +294,12 @@ private extension BlogListDataSource {
}

var allBlogs: [Blog] {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The name of this variable should be updated I think to something like filteredBlogs as now it won't be allBlogs all times.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

True, it's a bit misleading at the moment. I updated this variable with your suggestion and the allBlogsCount to blogsCount in e932d8e. Thanks!

@ScoutHarris
ScoutHarris self-requested a review June 10, 2022 22:47

@ScoutHarris ScoutHarris left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Created a self-hosted site via jurassic.ninja and verified it didn't show in the list.

:shipit:

@wargcm

wargcm commented Jun 11, 2022

Copy link
Copy Markdown
Contributor Author

Thank you @ScoutHarris!

@wargcm
wargcm merged commit 542b39f into trunk Jun 11, 2022
@wargcm
wargcm deleted the task/18429-filter-self-hosted-sites-intro branch June 11, 2022 14:59
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.

4 participants