Skip to content

Jetpack Section: Scan: Push to the scan view from the Notifications#15779

Merged
emilylaguna merged 4 commits intodevelopfrom
task/15190-scan-notifications
Feb 4, 2021
Merged

Jetpack Section: Scan: Push to the scan view from the Notifications#15779
emilylaguna merged 4 commits intodevelopfrom
task/15190-scan-notifications

Conversation

@emilylaguna
Copy link
Copy Markdown
Contributor

Project: #15190

Tapping on the threat notification will now push to the scan results view.

To test:

  1. Launch the app
  2. Tap the Notifications tab
  3. Locate a "Jetpack Scan" notification
  4. Tap on the "sites's scan Results" link
  5. You should be pushed to the correct sites, scan results views

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 Feb 3, 2021

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

@emilylaguna emilylaguna mentioned this pull request Feb 3, 2021
55 tasks
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Feb 3, 2021

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

@emilylaguna emilylaguna changed the title Jetpack Section: Scan: Add support to the notification view Jetpack Section: Scan: Push to the scan view from the Notifications Feb 3, 2021
Copy link
Copy Markdown
Contributor

@momo-ozawa momo-ozawa left a comment

Choose a reason for hiding this comment

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

Works as described!
LGTM with one question.

Comment on lines +88 to +90
case .scan:
try coordinator.displayScanWithSiteID(range.siteID)

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.

❓ Do we need to use a feature flag for this?

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.

🤦 I totally forgot to do that, I went back and added the checking if scan is available on the site with the intention of feature flagging it too but nope forgot..

Done in: a27feae

Copy link
Copy Markdown
Contributor

@momo-ozawa momo-ozawa left a comment

Choose a reason for hiding this comment

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

@emilylaguna emilylaguna merged commit 8573d00 into develop Feb 4, 2021
@emilylaguna emilylaguna deleted the task/15190-scan-notifications branch February 4, 2021 14:42
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