Skip to content

Issue/13328 restore details#13773

Merged
zwarm merged 11 commits intodevelopfrom
issue/13328-restore-details
Jan 15, 2021
Merged

Issue/13328 restore details#13773
zwarm merged 11 commits intodevelopfrom
issue/13328-restore-details

Conversation

@zwarm
Copy link
Copy Markdown
Contributor

@zwarm zwarm commented Jan 14, 2021

Parent #13328

This PR implements the "details" view (a.k.a. the select options) for the restore request.

Although RestoreDetailsViewModel was cloned off of BackupDownloadDetailsViewModel, the logic deviates because of the introduction of the Warning step in restore.

Cloned Classes
RestoreListItemState
RestoreDetailsAdapter
RestoreDetailsFragment
RestoreSubHeaderViewHolder
RestoreDetailsStateListItemBuilder
restore_list_subheader_item.xml

Notes:

  • Ignore styling - this is being handled separately
  • Ignore unit test placement in packages - this is being handled separately
  • RestoreActivity & RestoreViewModel (+unit tests) will continue to get updated as I move throughout the steps of the wizard

To test:

  • Launch the app with RestoreFeatureConfig flag on and the BackupFeatureFlag on
  • Navigate to ActivityLog
  • Tap the more menu
  • Select the restore option
  • Note (1) that the detail view is shown (2) the checkboxes can be selected/unselected, tapping on X or back button closes the view

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.

@zwarm zwarm added this to the 16.6 milestone Jan 14, 2021
@zwarm zwarm requested a review from ParaskP7 January 14, 2021 15:53
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jan 14, 2021

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

Generated by 🚫 dangerJS

@zwarm zwarm mentioned this pull request Jan 14, 2021
Closed
30 tasks
@peril-wordpress-mobile
Copy link
Copy Markdown

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

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

This was referenced Jan 14, 2021
Merged
Merged
Merged
ParaskP7
ParaskP7 approved these changes Jan 15, 2021
View reviewed changes
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.

👋 @zwarm !

I have reviewed and tested this PR as per the instruction, great job! Also, kudos for the tests and the given/when/then pattern! 🌟 🌟

I have left only minor (🔍) comments and one question (❓), which I would like to discuss. I am going to approve this PR anyway, since none is blocking. Also, I am not going to merge this PR yet to give you some time to apply any of my suggestions. However, feel free to ignore them and merge the PR yourself.

@ParaskP7 ParaskP7 self-assigned this Jan 15, 2021
@peril-wordpress-mobile
Copy link
Copy Markdown

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

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

@zwarm
Copy link
Copy Markdown
Contributor Author

zwarm commented Jan 15, 2021

@ParaskP7 Thanks for the review. I addressed all issues and answered your question. Once CI completes, I'll merge. 🙇

@ParaskP7
Copy link
Copy Markdown
Contributor

ParaskP7 commented Jan 15, 2021 •
edited
Loading

@ParaskP7 Thanks for the review. I addressed all issues and answered your question. Once CI completes, I'll merge. 🙇

Brilliant @zwarm , thank you so much! I have reviewed the changes and resolved all my comments. Feel free to merge when CI completed. 🌟

@zwarm zwarm merged commit 8fdef6b into develop Jan 15, 2021
@zwarm zwarm deleted the issue/13328-restore-details branch January 15, 2021 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ParaskP7 ParaskP7 ParaskP7 approved these changes

Assignees

@ParaskP7 ParaskP7

Labels

None yet

Projects

None yet

Milestone

16.6

Development

Successfully merging this pull request may close these issues.

2 participants

@zwarm @ParaskP7