Skip to content

Adds notice in wp-admin if there's a conflict with backups and plan #14309

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.

Sign up for GitHub

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

Jump to bottom
Merged
ChaosExAnima merged 4 commits into from
Jan 9, 2020
Merged

Adds notice in wp-admin if there's a conflict with backups and plan #14309

ChaosExAnima merged 4 commits into from
Jan 9, 2020

Conversation

@ChaosExAnima
Copy link
Contributor

@ChaosExAnima ChaosExAnima commented Jan 8, 2020

Changes proposed in this Pull Request:

  • This adds a new component that displays a warning if there's a conflict between a site and backup.
  • The component is included manually in the JetpackNotices component, as the global notice state wasn't the desired space for this.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • 1143508703416848-as-1153206214866137

Testing instructions:

  • On a Jetpack site, purchase a backup plan and site plan.
  • Navigate to Jetpack -> Plans
  • Observe warning message.

Screenshot:
Screen Shot 2020-01-08 at 12 54 21 PM

Proposed changelog entry for your changes:

  • Adds warning for unnecessary backup plans.

@ChaosExAnima ChaosExAnima requested review from a team January 8, 2020 17:57
@ChaosExAnima ChaosExAnima self-assigned this Jan 8, 2020
@ChaosExAnima ChaosExAnima added [Status] Needs Review This PR is ready for review. Admin Page React-powered dashboard under the Jetpack menu labels Jan 8, 2020
@jetpackbot
Copy link
Collaborator

jetpackbot commented Jan 8, 2020 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: January 14, 2020.
Scheduled code freeze: January 7, 2020

Generated by 🚫 dangerJS against ce2839b

jeherve
jeherve requested changes Jan 9, 2020
View reviewed changes
_inc/client/components/jetpack-notices/plan-conflict-warning.jsx Outdated
return null;
}

let featureName = 'daily backups';
Copy link
Member

@jeherve jeherve Jan 9, 2020

Choose a reason for hiding this comment

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

It would be nice if this (and below) were translatable.

Copy link
Contributor Author

@ChaosExAnima ChaosExAnima Jan 9, 2020

Choose a reason for hiding this comment

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

Resolved in ce2839b, thanks!

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Jan 9, 2020
@jeherve jeherve added this to the 8.2 milestone Jan 9, 2020
@enejb
Copy link
Member

enejb commented Jan 9, 2020

Thanks for working on this Echo and thanks for adding the tests.
I can see that its working as expected in my testing and thanks for adding tests for this as well!

Besides the translations changes that were requested I think this PR is ready.

@ChaosExAnima ChaosExAnima added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Jan 9, 2020
@ChaosExAnima ChaosExAnima requested a review from jeherve January 9, 2020 16:07
jeherve
jeherve approved these changes Jan 9, 2020
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This looks good to me. 👍

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Jan 9, 2020
@ChaosExAnima ChaosExAnima merged commit 4ada7fa into master Jan 9, 2020
@ChaosExAnima ChaosExAnima deleted the update/backup-conflict-notice branch January 9, 2020 17:38
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Jan 9, 2020
jeherve added a commit that referenced this pull request Jan 17, 2020
@jeherve
Changelog: add #14309
b313473
jeherve added a commit that referenced this pull request Jan 28, 2020
@jeherve
Changelog: 8.2 release (#14332)
0589a49 
* [not verified] Remove empty readme section

* Initial changelog for 8.2

* Changelog: add #14220

* Changelog: add #14252

* Changelog: add #14291

* Changelog: add #14309

* Changelog: add #14304

* Changelog: add general connection log.

* Changelog: add #14275

* Changelog: add #14313

* Changelog: add #14213

* Changelog: add #14357

* Add sync testing instructions

* Add 8.1.1 changelog back

See eeaafab and 61757eb

* Changelog: add #14371

* Changelog: add #14386

* Changelog: add #14471

* Changelog: add #14325

* Changelog: add #14194

* Changelog: add #14340

* Changelog: add #14418

* Changelog: add #14417

* Changelog: add #14075

* Changelog: add #14467

* Changelog: add #14307

* Changelog: add #14326
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeherve jeherve jeherve approved these changes

Assignees

@ChaosExAnima ChaosExAnima

Labels

Admin Page React-powered dashboard under the Jetpack menu [Feature] Backups

Projects

None yet

Milestone

8.2

Development

Successfully merging this pull request may close these issues.

6 participants

@ChaosExAnima @jetpackbot @enejb @jeherve @matticbot