Skip to content

Jetpack Section: Adds Scan item to the My Site menu#15427

Merged
emilylaguna merged 9 commits intodevelopfrom
task/mysite-scan-section
Dec 3, 2020
Merged

Jetpack Section: Adds Scan item to the My Site menu#15427
emilylaguna merged 9 commits intodevelopfrom
task/mysite-scan-section

Conversation

@emilylaguna
Copy link
Copy Markdown
Contributor

Part of #15190
Related WordPressKit PR: wordpress-mobile/WordPressKit-iOS#309

This PR adds the logic to display the Scan menu item for sites that support it. This does not include an icon or the correct action when tapping on the item yet.

Screenshots

To test:

Scan item appears

  1. Launch the app
  2. Tap on the My Site tab
  3. Tap on a blog that has the Jetpack Scan feature enabled

Expectation: the 'Scan' menu item appears under the 'Activity Log'

Scan doesn't appear for non-Jetpack Scan sites

  1. Launch the app
  2. Tap on the My Site tab
  3. Tap on a blog that does not support Jetpack scan (wp.com site, or a Jetpack site that doesn't have the feature enabled)

Expectation: the 'Scan' menu item DOES NOT appear

PR submission checklist:

  • I have considered adding unit tests where possible.
  • 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.

@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Dec 3, 2020

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

@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Dec 3, 2020

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

Copy link
Copy Markdown
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.

@emilylaguna it all works as described. But I think we're missing the Feature Flag for the Scan option?

service.getScanAvailableForSite(siteID, success: success, failure: failure)
}

private func defaultApi() -> WordPressComRestApi {
Copy link
Copy Markdown
Contributor

@leandroalonso leandroalonso Dec 3, 2020

Choose a reason for hiding this comment

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

We have this in so many places that I wonder if we should extend WordPressComRestApi and add that as a static func.

We don't need to change the other places where this is used but it would be good to have a centralized place. Up to you anyway!

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.

Done in: 7282ac7

}]];
}

if ([self.blog supports:BlogFeatureJetpackScan]) {
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.

Since we're merging that to develop we should create a Feature Flag for it, no?

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.

Oops 🤦‍♀️, done in: 540bc33

@emilylaguna
Copy link
Copy Markdown
Contributor Author

@leandroalonso Back to you!

@emilylaguna emilylaguna merged commit 80939fb into develop Dec 3, 2020
@emilylaguna emilylaguna deleted the task/mysite-scan-section branch December 3, 2020 20:38
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.

2 participants