Conversation
|
You can trigger an installable build for these changes by visiting CircleCI here. |
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
momo-ozawa
left a comment
There was a problem hiding this comment.
Tap on the 'Fix all threats' button
It will appear as if nothing happened
Refresh after a minute and there should be no fixable threats
I tested on the Digital Ocean Jetpack test site, and wasn't able to get rid of the fixable threats after refreshing.
Also, I found it a bit confusing how the message says "5 potential threats", but the alert shows "3 active threats". It would be nice to match the web and display "Jetpack can autofix 3 of 5 found threats". Not sure if you were already planning on addressing this in a different PR, but just in case!
|
|
||
| private struct Strings { | ||
| static let fixAllAlertTitleFormat = NSLocalizedString("Please confirm you want to fix all %1$d active threats", comment: "Confirmation title presented before fixing all the threats, displays the number of threats to be fixed") | ||
| static let fixAllSingleAlertTitle = NSLocalizedString("Please confirm you want to fix all %1$d active threat", comment: "Confirmation title presented before fixing a single threat") |
There was a problem hiding this comment.
Single threat string 👀
| static let fixAllSingleAlertTitle = NSLocalizedString("Please confirm you want to fix all %1$d active threat", comment: "Confirmation title presented before fixing a single threat") | |
| static let fixSingleAlertTitle = NSLocalizedString("Please confirm you want to fix this threat", comment: "Confirmation title presented before fixing a single threat") |
| static let fixAllAlertTitleFormat = NSLocalizedString("Please confirm you want to fix all %1$d active threats", comment: "Confirmation title presented before fixing all the threats, displays the number of threats to be fixed") | ||
| static let fixAllSingleAlertTitle = NSLocalizedString("Please confirm you want to fix all %1$d active threat", comment: "Confirmation title presented before fixing a single threat") | ||
| static let fixAllAlertTitleMessage = NSLocalizedString("Jetpack will be fixing all the detected active threats.", comment: "Confirmation message presented before fixing all the threats, displays the number of threats to be fixed") | ||
| static let fixAllSingleAlertMessage = NSLocalizedString("Jetpack will be fixing all the detected active threat.", comment: "Confirmation message presented before fixing a single threat") |
There was a problem hiding this comment.
Single threat string 👀
| static let fixAllSingleAlertMessage = NSLocalizedString("Jetpack will be fixing all the detected active threat.", comment: "Confirmation message presented before fixing a single threat") | |
| static let fixSingleAlertMessage = NSLocalizedString("Jetpack will be fixing the detected active threat.", comment: "Confirmation message presented before fixing a single threat") |
There was a problem hiding this comment.
I'd been staring at strings for too long 😆 Fixed in: f67b820
That's odd, do these threats get fixed when trigger on the web? |
The alert confirmation uses the wording from the web and only displays the number of threats that can be fixed. I can address this in a later PR when we do design review. |
leandroalonso
left a comment
There was a problem hiding this comment.
@momo-ozawa Did a small exploration here and the fix is not working because the SFTP user doesn't have write permission. On the web there's a notice stating that "not all threats were fixed".
@emilylaguna is this part of handling errors and it will be addressed in another PR?
Besides that, tested the instructions on another site and it worked just fine.
Yep! I have a whole task for error handling for the section. |
Project: #15190
Related PR: wordpress-mobile/WordPressKit-iOS#329
To test:
PR submission checklist:
RELEASE-NOTES.txtif necessary.