Skip to content

Issue/13329 backup download notification#14075

Merged
zwarm merged 8 commits intodevelopfrom
issue/13329-backup-download-notification
Feb 16, 2021
Merged

Issue/13329 backup download notification#14075
zwarm merged 8 commits intodevelopfrom
issue/13329-backup-download-notification

Conversation

@zwarm
Copy link
Copy Markdown
Contributor

@zwarm zwarm commented Feb 15, 2021

Parent #13329

This PR adds temporary support for Backup Download ready notifications.

Currently the notifications endpoint returns type=stat which is turned into a Stats deep link. This is not the correct behavior for the download notification. In this PR we intercept the note when it is returned from the API and swap type=stat with type=rewind-download-ready before it gets loaded into the database.

This code MUST be replaced once the API properly formats the Note (see issue #14074)

Notes
When viewing the notification detail view, the User name appears to be a link; however tapping on it will not take you anywhere. The type for this clickable range is marked as User; however User links are treated as if they are reader comment links and rely on siteId to be present in the node. In the case of backup download (and restore), the user node does not include siteId, so we can't forward the user anywhere (And I don't believe we should forward to the reader). Either way, it feels like the entire body response needs to be customized at the API level for both of these features. And we should identify where each link should forward too. Adding hacky workarounds is a poor solution.

Merge Instructions:

  • Review FluxC PR merge and create a tag in FluxC develop
  • Replace FluxC hash with tag from develop in build.gradle
  • Remove the "Not Ready for Merge label"
  • Merge this PR

To test:

  • Launch the app
  • Navigate to the activity log
  • Tap on a item that supports backup and choose 'Download backup" from the menu
  • Kick off a backup & wait until in ends
  • Navigate back to the home screen
  • Tap the Notifications tab
  • Look for the "Your backup is ready for download" item
  • Tap on the item to launch the notification detail view
  • Tap on the site's Backups text (should be blue)
  • Note that the Backup view is launched

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.8 milestone Feb 15, 2021
@zwarm zwarm requested a review from malinajirka February 15, 2021 20:57
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Feb 15, 2021

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 Feb 15, 2021

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

@zwarm zwarm mentioned this pull request Feb 16, 2021
34 tasks
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka left a comment

Choose a reason for hiding this comment

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

Thanks @zwarm! It's the best solution we can implement considering the current constraints. Good job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants