Skip to content

Jetpack Mobile: Threat Details - list items builder#13703

Merged
malinajirka merged 11 commits intodevelopfrom
issue/13326-threat-details-list-items-builder
Jan 6, 2021
Merged

Jetpack Mobile: Threat Details - list items builder#13703
malinajirka merged 11 commits intodevelopfrom
issue/13326-threat-details-list-items-builder

Conversation

@ashiagr
Copy link
Copy Markdown
Contributor

@ashiagr ashiagr commented Jan 4, 2021 •
edited
Loading

Parent: #13326

This PR builds list items for threat details screen.
Custom views for context lines, file name are created in a child PR: #13704

To test

Prerequisite:

  1. Make sure both Scan feature flag and MySiteImprovements flag are set to on in the App settings.
  2. WP.com account with a site having scan capability and different threats (e.g. http://do.wpmt.co/jp-scan-daily/).
  • Login to the app using above WP.com account and select the site on the My Site tab
  • Notice that scan menu is displayed
  • Click scan menu item followed by threat list item
  • Notice that threat details list items are built correctly as per the table below.
    (Tests included in ThreatDetailsListItemsBuilderTest)

References:

  1. Calypso's threat description, fix details and buttons.
  2. Scan design iteration i5

Basic Details

Item Icon/Text
Icon ic_scan_idle_threats_found
ThreatHeader Same as ThreatItem Header (in red) #13646
ThreatSubHeader Same as ThreatItem SubHeader #13646
ProblemHeader What was the problem?
ProblemDescription {threatModel.baseThreatModel.description}

Technical Details

Item Title
Header The technical details
Item  Present in
Custom View for fileName CoreFileModificationThreatModel, FileThreatModel
Custom View for threatContext FileThreatModel
diff CoreFileModificationThreatModel

Fix Details

Item ThreatStatus Fixable? Text
Header FIXED   How did Jetpack fix it?
  CURRENT Yes How will we fix it?
    No Resolving the threat
  IGNORED, UNKNOWN   How will we fix it?
Item Fixable?  FixType Text
Description Yes REPLACE Jetpack Scan will replace the affected file or directory.
    DELETE Jetpack Scan will delete the affected file or directory.
    UPDATE If fixable.target presentJetpack Scan will update to a newer version {fixable.target}.ElseJetpack Scan will resolve the threat.
    EDIT Jetpack Scan will edit the affected file or directory.
    UNKNOWN Jetpack Scan will resolve the threat.
  No   Jetpack Scan cannot automatically fix this threat. We suggest that you resolve the threat manually: ensure that WordPress, your theme, and all of your plugins are up to date, and remove the offending code, theme, or plugin from your site. \n \n If you need more help to resolve this threat, we recommend Codeable, a trusted freelancer marketplace of highly vetted WordPress experts. They have identified a select group of security experts to help with these projects. Pricing ranges from $70–120/hour, and you can get a free estimate with no obligation to hire.

Buttons

ThreatStatus Fixable? Button
ThreatStatus != FIXED Yes Fix threat
  No Get a free estimate
CURRENT   Ignore threat

Notes

  1. Ignore all styling

Merge Instructions

  1. Make sure Threat Details: Skeleton with blank Content UiState #13675, child PR Jetpack Mobile: Threat Details - custom views #13704 are merged Merged
  2. Update FluxC hash with tag for ScanStore: Get threat details by id WordPress-FluxC-Android#1819 Done in 21e0436
  3. Remove the "Not Ready for Merge label" Removed
  4. Merge this PR

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 self-assigned this Jan 4, 2021
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jan 4, 2021 •
edited
Loading

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jan 4, 2021 •
edited
Loading

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

@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jan 4, 2021 •
edited
Loading

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

@ashiagr ashiagr mentioned this pull request Jan 4, 2021
Merged
3 tasks
@ashiagr ashiagr force-pushed the issue/13326-threat-details-list-items-builder branch from 724b4bd to 4b29446 Compare January 4, 2021 10:02
@ashiagr ashiagr marked this pull request as ready for review January 4, 2021 11:45
@ashiagr ashiagr requested a review from malinajirka January 4, 2021 11:47
@ashiagr ashiagr added this to the 16.5 milestone Jan 4, 2021
This was referenced Jan 4, 2021
Merged
Closed
@malinajirka malinajirka self-assigned this Jan 5, 2021
malinajirka
malinajirka reviewed Jan 5, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Really great job @ashiagr! Even though the logic is quite complex the code makes it very easy to follow.

I found one "styling" issue. The "Jetpack Scan cannot automatically fix this threat. We suggest ..." text gets cut off mid sentence and the view is not scrollable. (It also seems the SubHeader view is not hidden when it's empty, but I assumed that's not part of this PR)

@ashiagr @malinajirka
Update WordPress/src/main/res/values/strings.xml
25b0b0f 
Co-authored-by: malinajirka <malinajirka@gmail.com>
@ashiagr
Copy link
Copy Markdown
Contributor Author

ashiagr commented Jan 5, 2021

Thanks @malinajirka!

The "Jetpack Scan cannot automatically fix this threat. We suggest ..." text gets cut off mid sentence and the view is not scrollable.

It also seems the SubHeader view is not hidden when it's empty, but I assumed that's not part of this PR.

This is because common list items currently have a listPreferredItemHeight set on them. These will be fixed in the "Polish the UI" task.

Base automatically changed from issue/13326-threat-details-custom-views to develop January 5, 2021 13:25
@ashiagr ashiagr requested a review from malinajirka January 6, 2021 04:44
@malinajirka malinajirka merged commit b95cc21 into develop Jan 6, 2021
@malinajirka malinajirka deleted the issue/13326-threat-details-list-items-builder branch January 6, 2021 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malinajirka malinajirka Awaiting requested review from malinajirka

Assignees

@ashiagr ashiagr

@malinajirka malinajirka

Labels

[Type] Task

Projects

None yet

Milestone

16.5

Development

Successfully merging this pull request may close these issues.

2 participants

@ashiagr @malinajirka