Skip to content

Issue/13326 fix threat found redirection#13941

Merged
malinajirka merged 6 commits intodevelopfrom
issue/13326-fix-threat-found-redirection
Feb 4, 2021
Merged

Issue/13326 fix threat found redirection#13941
malinajirka merged 6 commits intodevelopfrom
issue/13326-fix-threat-found-redirection

Conversation

@malinajirka
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka commented Feb 2, 2021

Parent issue #13326

With the introduction of the Scan screen, we can introduce support for redirection to Scan screen from the notifications. Previously, the app would redirect the user to a webview.

Merge instructions:

  1. Make sure FluxC's PR is merged first
  2. Update FluxC tag
  3. Remove "Not ready for merge label

To test:
Prerequisities:

  • disable Scan feature flag
  1. Go to Notifications
  2. Click on a "Jetpack Scan detected a possible security threat" item
  3. Click on "Site's Scan Results" span
  4. Notice a webview is shown

Prerequisities:

  • enable Scan feature flag
  1. Go to Notifications
  2. Click on a "Jetpack Scan detected a possible security threat" item
  3. Click on "Site's Scan Results" span
  4. Notice Scan screen is shown
  5. Run the test again, but make sure that you have selected a different site (SiteB) in the app than the one (SiteA) on which the threat was found -> make sure the Scan screen is shown for SiteA

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.

@malinajirka malinajirka added this to the 16.7 milestone Feb 2, 2021
@malinajirka malinajirka requested a review from a team February 2, 2021 13:52
@malinajirka malinajirka self-assigned this Feb 2, 2021
@malinajirka malinajirka requested review from ParaskP7 and removed request for a team February 2, 2021 13:52
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Feb 2, 2021

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

@malinajirka malinajirka mentioned this pull request Feb 2, 2021
67 tasks
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Feb 2, 2021

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

@ParaskP7 ParaskP7 self-assigned this Feb 3, 2021
Copy link
Copy Markdown
Contributor

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

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

👋 @malinajirka !

I have reviewed and tested this PR as per the instruction, good job! 🌟

I have left only one suggestion (💡 ) for you. I am going to approve this PR anyway, since none is blocking. I am NOT going to merge this PR yet to give you some time to apply my suggestion. However, feel free to ignore them and merge the PR yourself.

@malinajirka malinajirka requested a review from ParaskP7 February 3, 2021 09:40
@malinajirka
Copy link
Copy Markdown
Contributor Author

Thanks for the review @ParaskP7! ;) I've implemented the suggested improvement. It's ready for another round. Thanks!

Copy link
Copy Markdown
Contributor

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

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

LGTM!

@malinajirka malinajirka merged commit a4ebd83 into develop Feb 4, 2021
@malinajirka malinajirka deleted the issue/13326-fix-threat-found-redirection branch February 4, 2021 07:43
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