Skip to content
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

Update backup status views: no backup, failed backup, scheduled backup #40932

Merged
merged 4 commits into from
Apr 10, 2020

Conversation

cleacos
Copy link
Contributor

@cleacos cleacos commented Apr 8, 2020

Changes proposed in this Pull Request

Update the backup status views when:

  • We don't have a backup on the selected date
  • The backup from today is scheduled (it calculates the next expected time plus 3 hours of safety margin)
  • Some styles applied.

Backup scheduled:
image

Failed backup (copy updated):
image

No Backup:
image

Testing instructions

  • Apply these changes
  • Check the copies for the different views: Backup Failed, No Backup, Scheduled Backup
  • Check the expected time (the remaining hours) if you have a scheduled backup for today

You need to have a Jetpack site with a backup subscription (or a plan with it). To have a:

  • Failed Backup scenario: disconnect the site for one, so you'll get a failed backup. Also, you can modify the props of the component, change rewind__backup_complete_full by rewind__backup_error
    image

  • No Backup scenario: cancel your subscription and after that, the system will not trigger any backup. The other case is when your backup is delayed to the next day, but it's out of your hands. Directly you can modify the props of DailyBackupStatus component, add in new propfield backup: null

image

  • Scheduled Backup scenario: If the backup for today already happened, move your time zone until reach the next day. Also, you can manipulate the props as before, select today and set the backup props to null

- Updated the copies for: failed backup, no backup.
- Added a new status backup view for scheduled backup for today.
- Applied some styles
@cleacos cleacos added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Jetpack Cloud Anything related to the Jetpack Cloud (cloud.jetpack.com) labels Apr 8, 2020
@cleacos cleacos requested review from cbauerman and rcoll April 8, 2020 20:09
@cleacos cleacos self-assigned this Apr 8, 2020
@matticbot
Copy link
Contributor

<Gridicon icon="sync" />
<>
<Gridicon icon="cloud-upload" className="daily-backup-status__gridicon-no-backup" />
<div className="daily-backup-status__title">{ translate( 'No backup' ) }</div>
Copy link

Choose a reason for hiding this comment

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

ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 21 times:
translate( 'No backups' ) ES Score: 7

const yesterday = today.subtract( 1, 'days' );

const lastBackupDay = lastBackupDate.isSame( yesterday, 'day' )
? translate( 'Yesterday ' )
Copy link

Choose a reason for hiding this comment

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

ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 54 times:
translate( 'yesterday' ) ES Score: 15
See 1 additional suggestions in the PR translation status page

<>
<Gridicon className="daily-backup-status__gridicon-backup-scheduled" icon="cloud-upload" />
<div className="daily-backup-status__static-title">
{ translate( 'Backup Scheduled:' ) }
Copy link

Choose a reason for hiding this comment

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

ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 9 times:
translate( 'Backup Scheduled' ) ES Score: 11

@enejb
Copy link
Member

enejb commented Apr 9, 2020

Can you explain in the testing instructions How to get into the different state in order to test this PR?

@enejb
Copy link
Member

enejb commented Apr 9, 2020

Why do we say more "Backups from today" (under the contact Support button). From the second screenshot that part shouldn't be there or am I missing something. :)

@enejb
Copy link
Member

enejb commented Apr 9, 2020

I wasn't able to test this PR but the code so far looks good and it was easy to follow. :)

@cleacos cleacos requested a review from enejb April 9, 2020 09:57
@cleacos
Copy link
Contributor Author

cleacos commented Apr 9, 2020

@enejb

Can you explain in the testing instructions How to get into the different state in order to test this PR?

Updated

Why do we say more "Backups from today" (under the contact Support button). From the second screenshot that part shouldn't be there or am I missing something. :)

Yes, that is, the text shouldn't be there, but it's part of another component. In this PR: #40752 that issue is fixed.

@cleacos cleacos closed this Apr 9, 2020
@cleacos cleacos reopened this Apr 9, 2020
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 9, 2020
@cleacos cleacos added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 9, 2020
const backupDate = applySiteOffset( backup.activityTs, { timezone, gmtOffset } );

const displayDate = backupDate.format( 'L' );
const displayTime = backupDate.format( 'LT' );

return (
<Card className="daily-backup-status__failed">
<>
Copy link
Member

Choose a reason for hiding this comment

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

Neat. Had to look this up. Shorthand for Fragment. Love it.

Copy link
Member

@rcoll rcoll left a comment

Choose a reason for hiding this comment

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

Looks good, tests good. Shipping.

@rcoll rcoll merged commit eefde73 into master Apr 10, 2020
@rcoll rcoll deleted the update/jetpack-cloud-backup-status-add-no-backup-view branch April 10, 2020 14:28
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Jetpack Cloud Anything related to the Jetpack Cloud (cloud.jetpack.com)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants