Skip to content

Issue/13326 fix threat from threat details#13825

Merged
malinajirka merged 12 commits intodevelopfrom
issue/13326-fix-threat
Jan 20, 2021
Merged

Issue/13326 fix threat from threat details#13825
malinajirka merged 12 commits intodevelopfrom
issue/13326-fix-threat

Conversation

@ashiagr
Copy link
Copy Markdown
Contributor

@ashiagr ashiagr commented Jan 20, 2021

Parent #13326

This PR

  • Reuses FixThreatsUsecase for fix threat action on the threat details screen.
  • When fix request succeeds, navigates back to the scan state screen and fetches fix status by reusing existing logic for fix threats status.
  • Moves ignore threat success message to the scan state screen as per Issue/13326 ignore threat #13768 (comment).
fix_threat_web_success.mov
fix_threat_from_threat_details_mobile_success.mov

To test:

Prerequisite:

  • Make sure both Scan feature flag and MySiteImprovements flag are set to on in the App settings.
  • You have access to WP.com account with a site having scan capability and threats (e.g. https://pressable-jetpack-daily-scan.mystagingwebsite.com). Server credentials should be set on the site.
  • A threat is added to the site. In case you run out of threats, re-install bad calendar plugin from here and re-run scan.
  1. Login to the app using above WP.com account and select the site on the My Site tab
  2. Click scan menu item on My Site
  3. Click a threat on scan state screen
  4. Click fix threat button on the threat details screen
  5. Notice that a confirmation dialog is shown (see video)
  6. Click ok in the confirmation dialog
  7. Notice that app navigates back to scan state screen and fix status is shown

Repeat above steps and enable airplane mode after step 6. Notice that an error message is displayed and actions buttons are re-enabled.

Also test that when ignore threat action succeeds, ignore threat success message is shown on the scan state screen.

(Tests included in ThreatDetailsViewModelTest , ScanViewModelTest)

Notes:

  1. Ignore all styling
  2. Server credentials check is not handled
  3. Fix status fine-tuning will be done separately
  4. Task for pluralisation of messages (threat vs threats based on threat count) is added to backlog

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.

@ashiagr ashiagr added this to the 16.6 milestone Jan 20, 2021
@ashiagr ashiagr self-assigned this Jan 20, 2021
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jan 20, 2021

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

@ashiagr ashiagr requested a review from malinajirka January 20, 2021 11:48
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jan 20, 2021

You can test the changes on this Pull Request by downloading the APK here.

@ashiagr ashiagr mentioned this pull request Jan 20, 2021
67 tasks
@ashiagr ashiagr changed the title Issue/13326 fix threat Issue/13326 fix threat from threat details Jan 20, 2021
@malinajirka malinajirka self-assigned this Jan 20, 2021
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka left a comment

Choose a reason for hiding this comment

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

Thanks @ashiagr! It looks good to me. Feel free to merge it after you consider the comment.

@ashiagr ashiagr requested a review from malinajirka January 20, 2021 13:30
@ashiagr
Copy link
Copy Markdown
Contributor Author

ashiagr commented Jan 20, 2021

Thanks for the review @malinajirka! I've addressed the review comment.
Ready for another look.

@malinajirka malinajirka merged commit aca866a into develop Jan 20, 2021
@malinajirka malinajirka deleted the issue/13326-fix-threat branch January 20, 2021 14:08
@ashiagr ashiagr mentioned this pull request Jan 21, 2021
3 tasks
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