Skip to content

Jetpack Section: Fix/Ignore Threat#15740

Merged
momo-ozawa merged 12 commits intodevelopfrom
task/15190-threat-detail-buttons
Feb 2, 2021
Merged

Jetpack Section: Fix/Ignore Threat#15740
momo-ozawa merged 12 commits intodevelopfrom
task/15190-threat-detail-buttons

Conversation

@momo-ozawa
Copy link
Copy Markdown
Contributor

@momo-ozawa momo-ozawa commented Jan 29, 2021

Part of #15190
WPKit PR: wordpress-mobile/WordPressKit-iOS#339

Decription:

  • Update fix threat button to trigger fix threat request
  • Update ignore threat button to trigger ignore threat request

To test:

Fix threat

  1. Launch the app, tap on the My Site, tap on a site with Jetpack Scan and also has Threats
  2. Tap on a threat cell
  3. Tap Fix threat button
  4. ✅ Fix threat alert is displayed
  5. Tap Cancel button on alert
  6. ✅ Alert is dismissed
  7. Tap Fix threat button, then tap OK button on alert
  8. ✅ Should navigate back to the Scan screen
  9. After about a minute has passed, pull to refresh
  10. ✅ The fixed threat no longer shows up
  11. Tap on History > Fixed tab
  12. ✅ The fixed threat shows up in the Fixed tab

Ignore threat

  1. Launch the app, tap on the My Site, tap on a site with Jetpack Scan and also has Threats
  2. Tap on a threat cell
  3. Tap Ignore threat button
  4. ✅ Ignore threat alert is displayed
  5. Tap Cancel button on alert
  6. ✅ Alert is dismissed
  7. Tap Ignore threat button, then tap OK button on alert
  8. ✅ Should navigate back to the Scan screen
  9. After about a minute has passed, pull to refresh
  10. ✅ The ignored threat no longer shows up
  11. Tap on History > Ignored tab
  12. ✅ The ignored threat shows up in the Ignored tab

Example screenshots:

Fix threat Ignore threat
Simulator Screen Shot - iPhone 12 Pro - 2021-01-29 at 18 56 02 Simulator Screen Shot - iPhone 12 Pro - 2021-01-29 at 18 56 08

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 Jan 29, 2021

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 Jan 29, 2021

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

Copy link
Copy Markdown
Contributor

@emilylaguna emilylaguna left a comment

Choose a reason for hiding this comment

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

Nice, works well. Added a comment below.


// MARK: - Public

public func fixThreat() {
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.

Hmm, I'm wondering if we could use a delegate attached to the JetpackScanViewController that triggers the fix / ignore methods since that view does a lot of heavy lifting for the fixing / ignoring.

Thoughts?

Copy link
Copy Markdown
Contributor Author

@momo-ozawa momo-ozawa Feb 1, 2021

Choose a reason for hiding this comment

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

Ahh, so that's why there was an ignoreThreat method already defined in JetpackScanCoordinator!
Makes sense to consolidate the fixing / ignoring in JetpackScanCoordinator.

Fixed! 2462d66

Copy link
Copy Markdown
Contributor

@emilylaguna emilylaguna left a comment

Choose a reason for hiding this comment

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

Awesome, looks good! 🚢 👍

# Conflicts:
#	Podfile
#	Podfile.lock
@momo-ozawa momo-ozawa merged commit 14a6776 into develop Feb 2, 2021
@momo-ozawa momo-ozawa deleted the task/15190-threat-detail-buttons branch February 2, 2021 01:23
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