-
Notifications
You must be signed in to change notification settings - Fork 798
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Protect: Separate scanner and history views via React Router and UI adjustments #38325
Protect: Separate scanner and history views via React Router and UI adjustments #38325
Conversation
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. 🔴 Action required: Please add missing changelog entries for the following projects: Use the Jetpack CLI tool to generate changelog entries by running the following command: Once your PR is ready for review, check one last time that all required checks appearing at the bottom of this PR are passing or skipped. Protect plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
4c6d3e7
to
3a02de9
Compare
c3be7f8
to
d4dd0c4
Compare
3a02de9
to
e2c9aee
Compare
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
ca6908d
to
c8a08d5
Compare
c8a08d5
to
27c6505
Compare
d4dd0c4
to
6f77288
Compare
27c6505
to
7094605
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shaping up really nicely! I believe a few of the points I made in my review of the initial enhancements still require attention or further discussion:
- I think it would still be nice to have the ability to flip back and forth from the Scanner and History page from the various states of the page (eg. currently when scanning or in scan error state we cannot access History)
- Manually unignored threats (via the VPMC tool) retain an
ignored
status and therefor display theScanHistoryDetails
component even when in the current threats list. I think this might actually also be an issue in Calypso so perhaps the full and proper solution is ensuring that status gets set correctly when it gets unignored.- UPDATE: This will be fixed in https://github.com/Automattic/jetpack-scan-team/issues/1281
- Could we now display what filter is being applied in the
EmptyList
(eg.So far, there are no **fixed** threats in your scan history...
, instead of...selected filters
)?
A few new things I came across:
- The display of the error state in the History page could maybe use some love (eg. conditionally removing the sub-header message, perhaps structuring/styling it more like scan error with the in-progress image):
- There is now an odd animation/styling issue when the onboarding popover displays next to the
Scan now
button:
- The History page is still accessible and empty (missing data) when a user does not have an upgrade. I see we've removed the toggles to get there easily, but anyone can still visit
/admin.php?page=jetpack-protect#/scan/history
to access it. - Now that some of our classes and models have been moved to dedicated packages (
packages/protect-status
, andpackages/protect-models
), should we consider also movingclass-scan-history
? - When viewing History, and selecting a specific plugin, if you toggle the ignored or fixed filter and that plugin is not there, or if you do not have any of either fixed or ignored archived threats, the
EmptyList
component is displayed and you are no longer able to access the filter toggles:
- Separately, but as you'll see in the above screen capture, for some reason, WordPress threats show up in the display of
all
threats, but then are missing when we filter byignored
orfixed
. - Now that the secondary
Scan now
button has been moved to theEmptyList
component, we appear to be missing the onboarding popover for that flow:
@@ -39,9 +40,10 @@ const AdminPage = ( { children } ) => { | |||
setTimeout( () => { | |||
refreshPlan(); | |||
refreshStatus( true ); | |||
refreshScanHistory(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why we shouldn't treat this like refreshStatus
with a hard refresh?
@@ -176,9 +182,7 @@ const PaidList = ( { list } ) => { | |||
diff, | |||
filename, | |||
firstDetected, // todo: still needs a proper fix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove this todo now since I think we've addressed this by camelizing the scanHistory
initial state variable in the reducer.
6f77288
to
998b3b5
Compare
8a424f5
to
674d3a9
Compare
998b3b5
to
abdc2dd
Compare
674d3a9
to
71f1191
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice enhancements! Let's go ahead and merge this into the project branch and address any additional items in dedicated followup tasks.
5856382
into
add/protect-threat-history
* Add Scan_History class * changelog * Update changelog * Merge trunk, fix versions * Add initial UI for scan history * Add firstDetected and fixedOn details to threat cards * Fix ignored message * Fix console error * Adjust wording and styles * Optimize * Fix firstDetected and fixedOn property retrieval issues, add todos * Remove unneeded ref * Consolidate scan history UI to existing components * Remove todos * Return false for scanHistory when upgrade is missing * Manage ScanPage states when viewing history * Move source data logic to useProtectData hook * Improve Summary component Button logic * ScanPage refactoring * Reapply missing scanIsUnavailable check * Add comments * Hide ignore when viewing threats, and clear history cache when updating or fixing * Add unignore actions and conditional handling to the UI * Add null coalescing to core threat prop assignment * Fix core threat normalization * Optimizations and refactoring * Revert standarization, and use conditionals * Add dummy arg to avoid bad translation minification * Changlog entry * Move viewingScanHistory out of initial state * Use threat status over lack of fixedOn to display unignore button * Add foundation for unignore request * Remove need for signature_id * Add TypeScript support to Jetpack Protect * Downgrade typescript to 5.0.4, to match other monorepo projects * Add TypeScript support to Jetpack Protect * Init feature branch * Protect: Add Scan_History class (#37903) * Protect: Add Scan History UI (#37988) * Init feature branch * Init feature branch * Init feature branch * Use react router for threat history, separate scan and history root components, minor UI adjustments * changelog * Add dummy args to avoid bad minification * Hide scan section navigation when user has no plan * Wrap ScanButton with forwardRef * Add onboarding popover to Scan Now button in empty state * Fix filtering of core threat history * Redirect to /scan from /scan/history when user has no plan * Fix animation glitch in onboarding popover * Remove unnecessary additions * Further removals after failed base merge * More removals * Add todos * Use status to conditional render unignore action button * Move ignore/unignore actions to dedicated modals * Add TypeScript support to Jetpack Protect * Downgrade typescript to 5.0.4, to match other monorepo projects * Add TypeScript support to Jetpack Protect * Init feature branch * Protect: Add Scan_History class (#37903) * Protect: Add Scan History UI (#37988) * changelog * Protect: Separate scanner and history views via React Router and UI adjustments (#38325) * Run ./tools/fixup-project-versions.sh * Restore project versions * Run ./tools/fixup-project-versions.sh * Remove todo comment (#38628) * Remove changelog * Display fix threat button only for current threats * Further unification * Refresh scan history after unignore * Update projects/plugins/protect/src/js/components/unignore-threat-modal/index.jsx Co-authored-by: Nate Weller <nate.weller@automattic.com> * Fix lint errors --------- Co-authored-by: Nate Weller <hello@nateweller.com> Co-authored-by: Nate Weller <nate.weller@automattic.com>
* Add Scan_History class * changelog * Update changelog * Merge trunk, fix versions * Add initial UI for scan history * Add firstDetected and fixedOn details to threat cards * Fix ignored message * Fix console error * Adjust wording and styles * Optimize * Fix firstDetected and fixedOn property retrieval issues, add todos * Remove unneeded ref * Consolidate scan history UI to existing components * Remove todos * Return false for scanHistory when upgrade is missing * Manage ScanPage states when viewing history * Move source data logic to useProtectData hook * Improve Summary component Button logic * ScanPage refactoring * Reapply missing scanIsUnavailable check * Add comments * Hide ignore when viewing threats, and clear history cache when updating or fixing * Add unignore actions and conditional handling to the UI * Add null coalescing to core threat prop assignment * Fix core threat normalization * Optimizations and refactoring * Revert standarization, and use conditionals * Add dummy arg to avoid bad translation minification * Changlog entry * Move viewingScanHistory out of initial state * Use threat status over lack of fixedOn to display unignore button * Add foundation for unignore request * Remove need for signature_id * Add TypeScript support to Jetpack Protect * Downgrade typescript to 5.0.4, to match other monorepo projects * Add TypeScript support to Jetpack Protect * Init feature branch * Protect: Add Scan_History class (#37903) * Protect: Add Scan History UI (#37988) * Init feature branch * Init feature branch * Init feature branch * Use react router for threat history, separate scan and history root components, minor UI adjustments * changelog * Add dummy args to avoid bad minification * Hide scan section navigation when user has no plan * Wrap ScanButton with forwardRef * Add onboarding popover to Scan Now button in empty state * Fix filtering of core threat history * Redirect to /scan from /scan/history when user has no plan * Fix animation glitch in onboarding popover * Remove unnecessary additions * Further removals after failed base merge * More removals * Add todos * Use status to conditional render unignore action button * Move ignore/unignore actions to dedicated modals * Add TypeScript support to Jetpack Protect * Downgrade typescript to 5.0.4, to match other monorepo projects * Add TypeScript support to Jetpack Protect * Init feature branch * Protect: Add Scan_History class (#37903) * Protect: Add Scan History UI (#37988) * changelog * Protect: Separate scanner and history views via React Router and UI adjustments (#38325) * Run ./tools/fixup-project-versions.sh * Restore project versions * Run ./tools/fixup-project-versions.sh * Remove todo comment (#38628) * Remove changelog * Display fix threat button only for current threats * Further unification * Refresh scan history after unignore * Update projects/plugins/protect/src/js/components/unignore-threat-modal/index.jsx Co-authored-by: Nate Weller <nate.weller@automattic.com> * Fix lint errors --------- Co-authored-by: Nate Weller <hello@nateweller.com> Co-authored-by: Nate Weller <nate.weller@automattic.com>
* Add Scan_History class * changelog * Update changelog * Merge trunk, fix versions * Add initial UI for scan history * Add firstDetected and fixedOn details to threat cards * Fix ignored message * Fix console error * Adjust wording and styles * Optimize * Fix firstDetected and fixedOn property retrieval issues, add todos * Remove unneeded ref * Consolidate scan history UI to existing components * Remove todos * Return false for scanHistory when upgrade is missing * Manage ScanPage states when viewing history * Move source data logic to useProtectData hook * Improve Summary component Button logic * ScanPage refactoring * Reapply missing scanIsUnavailable check * Add comments * Hide ignore when viewing threats, and clear history cache when updating or fixing * Add unignore actions and conditional handling to the UI * Add null coalescing to core threat prop assignment * Fix core threat normalization * Optimizations and refactoring * Revert standarization, and use conditionals * Add dummy arg to avoid bad translation minification * Changlog entry * Move viewingScanHistory out of initial state * Use threat status over lack of fixedOn to display unignore button * Add foundation for unignore request * Remove need for signature_id * Add TypeScript support to Jetpack Protect * Downgrade typescript to 5.0.4, to match other monorepo projects * Add TypeScript support to Jetpack Protect * Init feature branch * Protect: Add Scan_History class (#37903) * Protect: Add Scan History UI (#37988) * Init feature branch * Init feature branch * Init feature branch * Use react router for threat history, separate scan and history root components, minor UI adjustments * changelog * Add dummy args to avoid bad minification * Hide scan section navigation when user has no plan * Wrap ScanButton with forwardRef * Add onboarding popover to Scan Now button in empty state * Fix filtering of core threat history * Redirect to /scan from /scan/history when user has no plan * Fix animation glitch in onboarding popover * Remove unnecessary additions * Further removals after failed base merge * More removals * Add todos * Use status to conditional render unignore action button * Move ignore/unignore actions to dedicated modals * Add TypeScript support to Jetpack Protect * Downgrade typescript to 5.0.4, to match other monorepo projects * Add TypeScript support to Jetpack Protect * Init feature branch * Protect: Add Scan_History class (#37903) * Protect: Add Scan History UI (#37988) * changelog * Protect: Separate scanner and history views via React Router and UI adjustments (#38325) * Run ./tools/fixup-project-versions.sh * Restore project versions * Run ./tools/fixup-project-versions.sh * Remove todo comment (#38628) * Remove changelog * Display fix threat button only for current threats * Further unification * Refresh scan history after unignore * Update projects/plugins/protect/src/js/components/unignore-threat-modal/index.jsx Co-authored-by: Nate Weller <nate.weller@automattic.com> * Fix lint errors --------- Co-authored-by: Nate Weller <hello@nateweller.com> Co-authored-by: Nate Weller <nate.weller@automattic.com>
* Add Scan_History class * changelog * Update changelog * Merge trunk, fix versions * Add initial UI for scan history * Add firstDetected and fixedOn details to threat cards * Fix ignored message * Fix console error * Adjust wording and styles * Optimize * Fix firstDetected and fixedOn property retrieval issues, add todos * Remove unneeded ref * Consolidate scan history UI to existing components * Remove todos * Return false for scanHistory when upgrade is missing * Manage ScanPage states when viewing history * Move source data logic to useProtectData hook * Improve Summary component Button logic * ScanPage refactoring * Reapply missing scanIsUnavailable check * Add comments * Hide ignore when viewing threats, and clear history cache when updating or fixing * Add unignore actions and conditional handling to the UI * Add null coalescing to core threat prop assignment * Fix core threat normalization * Optimizations and refactoring * Revert standarization, and use conditionals * Add dummy arg to avoid bad translation minification * Changlog entry * Move viewingScanHistory out of initial state * Use threat status over lack of fixedOn to display unignore button * Add foundation for unignore request * Remove need for signature_id * Add TypeScript support to Jetpack Protect * Downgrade typescript to 5.0.4, to match other monorepo projects * Add TypeScript support to Jetpack Protect * Init feature branch * Protect: Add Scan_History class (#37903) * Protect: Add Scan History UI (#37988) * Init feature branch * Init feature branch * Init feature branch * Use react router for threat history, separate scan and history root components, minor UI adjustments * changelog * Add dummy args to avoid bad minification * Hide scan section navigation when user has no plan * Wrap ScanButton with forwardRef * Add onboarding popover to Scan Now button in empty state * Fix filtering of core threat history * Redirect to /scan from /scan/history when user has no plan * Fix animation glitch in onboarding popover * Remove unnecessary additions * Further removals after failed base merge * More removals * Add todos * Use status to conditional render unignore action button * Move ignore/unignore actions to dedicated modals * Add TypeScript support to Jetpack Protect * Downgrade typescript to 5.0.4, to match other monorepo projects * Add TypeScript support to Jetpack Protect * Init feature branch * Protect: Add Scan_History class (#37903) * Protect: Add Scan History UI (#37988) * changelog * Protect: Separate scanner and history views via React Router and UI adjustments (#38325) * Run ./tools/fixup-project-versions.sh * Restore project versions * Run ./tools/fixup-project-versions.sh * Remove todo comment (#38628) * Remove changelog * Display fix threat button only for current threats * Further unification * Refresh scan history after unignore * Update projects/plugins/protect/src/js/components/unignore-threat-modal/index.jsx Co-authored-by: Nate Weller <nate.weller@automattic.com> * Fix lint errors --------- Co-authored-by: Nate Weller <hello@nateweller.com> Co-authored-by: Nate Weller <nate.weller@automattic.com>
* Add Scan_History class * changelog * Update changelog * Merge trunk, fix versions * Add initial UI for scan history * Add firstDetected and fixedOn details to threat cards * Fix ignored message * Fix console error * Adjust wording and styles * Optimize * Fix firstDetected and fixedOn property retrieval issues, add todos * Remove unneeded ref * Consolidate scan history UI to existing components * Remove todos * Return false for scanHistory when upgrade is missing * Manage ScanPage states when viewing history * Move source data logic to useProtectData hook * Improve Summary component Button logic * ScanPage refactoring * Reapply missing scanIsUnavailable check * Add comments * Hide ignore when viewing threats, and clear history cache when updating or fixing * Add unignore actions and conditional handling to the UI * Add null coalescing to core threat prop assignment * Fix core threat normalization * Optimizations and refactoring * Revert standarization, and use conditionals * Add dummy arg to avoid bad translation minification * Changlog entry * Move viewingScanHistory out of initial state * Use threat status over lack of fixedOn to display unignore button * Add foundation for unignore request * Remove need for signature_id * Add TypeScript support to Jetpack Protect * Downgrade typescript to 5.0.4, to match other monorepo projects * Add TypeScript support to Jetpack Protect * Init feature branch * Protect: Add Scan_History class (#37903) * Protect: Add Scan History UI (#37988) * Init feature branch * Init feature branch * Init feature branch * Use react router for threat history, separate scan and history root components, minor UI adjustments * changelog * Add dummy args to avoid bad minification * Hide scan section navigation when user has no plan * Wrap ScanButton with forwardRef * Add onboarding popover to Scan Now button in empty state * Fix filtering of core threat history * Redirect to /scan from /scan/history when user has no plan * Fix animation glitch in onboarding popover * Remove unnecessary additions * Further removals after failed base merge * More removals * Add todos * Use status to conditional render unignore action button * Move ignore/unignore actions to dedicated modals * Add TypeScript support to Jetpack Protect * Downgrade typescript to 5.0.4, to match other monorepo projects * Add TypeScript support to Jetpack Protect * Init feature branch * Protect: Add Scan_History class (#37903) * Protect: Add Scan History UI (#37988) * changelog * Protect: Separate scanner and history views via React Router and UI adjustments (#38325) * Run ./tools/fixup-project-versions.sh * Restore project versions * Run ./tools/fixup-project-versions.sh * Remove todo comment (#38628) * Remove changelog * Display fix threat button only for current threats * Further unification * Refresh scan history after unignore * Update projects/plugins/protect/src/js/components/unignore-threat-modal/index.jsx Co-authored-by: Nate Weller <nate.weller@automattic.com> * Fix lint errors --------- Co-authored-by: Nate Weller <hello@nateweller.com> Co-authored-by: Nate Weller <nate.weller@automattic.com>
Related to #38144
Proposed changes:
/scan/history
and/scan/history/:filter
routes./scan
route and default redirect from/
->/scan
.useThreatHistory()
hook and uses separate routes/root components for the scanner and history views.Other information:
Jetpack product discussion
#38144
Does this pull request change what data or activity we track or use?
No
Testing instructions:
Screenshots