Skip to content

Jetpack Section: Restore Status and Restore Complete#15640

Merged
momo-ozawa merged 10 commits intodevelopfrom
task/15191-restore-status
Jan 18, 2021
Merged

Jetpack Section: Restore Status and Restore Complete#15640
momo-ozawa merged 10 commits intodevelopfrom
task/15191-restore-status

Conversation

@momo-ozawa
Copy link
Copy Markdown
Contributor

@momo-ozawa momo-ozawa commented Jan 14, 2021 •
edited
Loading

Part of #15191

  • Hooked up restore site APIs
    • Restore site
    • Get rewind status (poll for status every 5 seconds)
  • Added a new Restore Complete screen

Merge directions:

Notes:

  • Styling isn't 100% complete
  • The Visit site button on the Restore Complete screen isn't hooked up yet
  • Error handling will be addressed in a separate PR

To test:

  1. My Site > Activity Log > rewindable activity cell
  2. Tap on ellipsis icon
  3. Tap on Restore
  4. Tap on Restore to this point
  5. Tap on Confirm
    • ✅ The progress view should update
    • ✅ Once the site has been restored, the app should transition to the Restore Complete screen (Note that it might take a little while to transition after it hits 100%)
30% (.running) 100% (.running) Complete (.finished)
Screen Shot 2021-01-14 at 19 38 26 Screen Shot 2021-01-14 at 19 38 34 Screen Shot 2021-01-14 at 19 38 55

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.

@momo-ozawa momo-ozawa added this to the 16.6 milestone Jan 14, 2021
@momo-ozawa momo-ozawa self-assigned this Jan 14, 2021
@peril-wordpress-mobile
Copy link
Copy Markdown

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

You can trigger an installable build for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link
Copy Markdown

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

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

leandroalonso
leandroalonso reviewed Jan 14, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@leandroalonso leandroalonso left a comment

Choose a reason for hiding this comment

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

@momo-ozawa works as described!

One question though, the confirmation message says:

Screen Shot 2021-01-14 at 12 56 29

However, this might be not true: what if the user selected just WordPress Themes?

cc @osullivanchris

...wRelated/Jetpack/Jetpack Restore/Restore Complete/JetpackRestoreCompleteViewController.swift Outdated
title: NSLocalizedString("Restore", comment: "Title for Jetpack Restore Complete screen"),
iconImage: .gridicon(.history),
messageTitle: NSLocalizedString("Your site has been restored", comment: "Title for the Jetpack Restore Complete message."),
messageDescription: NSLocalizedString("All of your selected itmes are now restored back to %1$@.", comment: "Description for the Jetpack Backup Restore message. %1$@ is a placeholder for the selected date."),
Copy link
Copy Markdown
Contributor

@leandroalonso leandroalonso Jan 14, 2021

Choose a reason for hiding this comment

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

Suggested change
messageDescription: NSLocalizedString("All of your selected itmes are now restored back to %1$@.", comment: "Description for the Jetpack Backup Restore message. %1$@ is a placeholder for the selected date."),
messageDescription: NSLocalizedString("All of your selected items are now restored back to %1$@.", comment: "Description for the Jetpack Backup Restore message. %1$@ is a placeholder for the selected date."),

Typo: itmes

Copy link
Copy Markdown
Contributor Author

@momo-ozawa momo-ozawa Jan 15, 2021

Choose a reason for hiding this comment

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

Fixed! 24c9a6e

WordPress/Classes/Services/JetpackRestoreService.swift Outdated
@objc class JetpackRestoreService: LocalCoreDataService {

private lazy var serviceV1: ActivityServiceRemote_ApiVersion1_0 = {
let api = WordPressComRestApi.defaultApi(in: managedObjectContext,
Copy link
Copy Markdown
Contributor

@leandroalonso leandroalonso Jan 14, 2021

Choose a reason for hiding this comment

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

You can omit the localeKey

Copy link
Copy Markdown
Contributor Author

@momo-ozawa momo-ozawa Jan 15, 2021

Choose a reason for hiding this comment

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

Fixed! 10cd307

Base automatically changed from task/15191-backup-status to develop January 15, 2021 01:27
@momo-ozawa
Copy link
Copy Markdown
Contributor Author

momo-ozawa commented Jan 15, 2021

@leandroalonso @osullivanchris

However, this might be not true: what if the user selected just WordPress Themes?

Hmm, true. This is what Wordpress.com looks like.

Screen Shot 2021-01-15 at 10 35 02

@osullivanchris
Copy link
Copy Markdown

osullivanchris commented Jan 15, 2021

However, this might be not true: what if the user selected just WordPress Themes?

Good spot. I pulled the message from something already existing on web. But maybe I didn't grab the right one.

@momo-ozawa in that screenshot, does the message change depending on what you select?

Either way, my intention was to follow what we are doing on web. So if we have something like this already, that's great, let's use it. If we don't, then we can come up with our own alternative.

@osullivanchris osullivanchris mentioned this pull request Jan 15, 2021
Merged
3 tasks
@osullivanchris
Copy link
Copy Markdown

osullivanchris commented Jan 15, 2021

Discussed this on Android over here. After discussion with @ashiagr - realised that the word 'All' is really the only one causing issue.

If we remove the word 'all', the sentence still works grammatically, but it doesn't conclusively say that we are removing everything when the user has only selected some things. That should resolve the issue I think.

cc @leandroalonso

@momo-ozawa
Copy link
Copy Markdown
Contributor Author

momo-ozawa commented Jan 16, 2021 •
edited
Loading

@leandroalonso (cc: @osullivanchris)

  • Removed the word "all"
  • Restore action is disabled if no items are selected
Updated restore warning copy No items selected At least 1 item selected
Simulator Screen Shot - iPhone 12 Pro - 2021-01-16 at 10 39 45 Simulator Screen Shot - iPhone 12 Pro - 2021-01-16 at 10 39 56 Simulator Screen Shot - iPhone 12 Pro - 2021-01-16 at 10 40 01

@momo-ozawa momo-ozawa mentioned this pull request Jan 18, 2021
Merged
3 tasks
@momo-ozawa momo-ozawa merged commit 56b3f6f into develop Jan 18, 2021
@momo-ozawa momo-ozawa deleted the task/15191-restore-status branch January 18, 2021 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leandroalonso leandroalonso leandroalonso approved these changes

@emilylaguna emilylaguna Awaiting requested review from emilylaguna

Assignees

@momo-ozawa momo-ozawa

Labels

Jetpack Mobile [Type] Task

Projects

None yet

Milestone

16.6

Development

Successfully merging this pull request may close these issues.

3 participants

@momo-ozawa @osullivanchris @leandroalonso