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

Module Overrides: Add support to disable backups UI #10004

Merged
merged 6 commits into from
Aug 22, 2018

Conversation

ebinnion
Copy link
Contributor

@ebinnion ebinnion commented Aug 8, 2018

Currently, our module overrides functionality does not allow disabling UI relating to VaultPress or Backups. This is an issue for some of our partnerships where hosting partners need the ability to disable that UI.

Changes proposed in this Pull Request:

  • Add ability to disable backups related UI.

Testing instructions:

  • Checkout branch
  • In an integration plugin, add add_filter( 'jetpack_show_backups', '__return_false' );
  • Load Jetpack admin page
  • Ensure that backups and scanning are not mentioned in UI
  • Install and activate VaultPress
  • Ensure that backups and scanning are mentioned

Proposed changelog entry for your changes:

  • Adds ability to disable backups UI by filter when VaultPress is not activated.

Screenshots

After

after_dashboard
after_plan
after_security_settings
after_backups_search

Before

before_dashboard
before_plan
before_security_settings
before_backups_search

@ebinnion ebinnion added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it General [Status] In Progress Admin Page React-powered dashboard under the Jetpack menu labels Aug 8, 2018
@ebinnion ebinnion added this to the 6.5 milestone Aug 8, 2018
@ebinnion ebinnion self-assigned this Aug 8, 2018
@ebinnion ebinnion requested a review from a team as a code owner August 8, 2018 22:34
@ebinnion ebinnion force-pushed the update/module-overrides-vaultpress branch from 729afe7 to 36dcdaa Compare August 8, 2018 23:30
@ebinnion ebinnion changed the title Module Overrides: Add support to disable simulated backups module Module Overrides: Add support to disable backups UI Aug 8, 2018
@ebinnion ebinnion force-pushed the update/module-overrides-vaultpress branch from 36dcdaa to 5e53261 Compare August 9, 2018 19:26
@ebinnion ebinnion requested a review from eliorivero August 9, 2018 20:05
@ebinnion ebinnion added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Aug 9, 2018
Copy link
Contributor

@eliorivero eliorivero 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 and just needs a couple of updates.

*
* @param bool $show_backups Should UI for backups be displayed? True by default.
*/
'showBackups' => is_plugin_active( 'vaultpress/vaultpress.php' ) || apply_filters( 'jetpack_show_backups', true ),
Copy link
Contributor

Choose a reason for hiding this comment

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

Use Jetpack::is_plugin_active() instead of is_plugin_active since that one gets plugins that were single or network activated.

const hideBackupFeature = (
! this.props.showBackups &&
item &&
-1 !== backupFeatureIds.indexOf( item.id )
Copy link
Contributor

Choose a reason for hiding this comment

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

Lodash's includes function is already imported in this file so you can use it here:

includes( backupFeatureIds, item.id )

@eliorivero eliorivero added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Aug 9, 2018
@ebinnion ebinnion dismissed eliorivero’s stale review August 9, 2018 23:05

Feedback addressed.

@ebinnion
Copy link
Contributor Author

ebinnion commented Aug 9, 2018

Thanks for the feedback! I've addressed it in the latest commit.

@abidhahmed abidhahmed added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Aug 10, 2018
@eliorivero
Copy link
Contributor

Code is good now but found a rogue reference

captura de pantalla 2018-08-10 a la s 12 48 33

@ebinnion
Copy link
Contributor Author

Thanks. I've addressed that as well as a couple of others I found. 🙇

@eliorivero
Copy link
Contributor

Sorry, when Jetpack is disconnected, there's one more reference in the screen in WP Admin > Jetpack:

captura de pantalla 2018-08-10 a la s 19 52 50

and another in any WP Admin screen in the connection banner

captura de pantalla 2018-08-10 a la s 19 53 34

@ebinnion ebinnion force-pushed the update/module-overrides-vaultpress branch from 6f68aa8 to 9a4218c Compare August 15, 2018 15:21
@jetpackbot
Copy link

This is automated (and not very smart btw) check which relies on PULL_REQUEST_TEMPLATE.We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS

@ebinnion
Copy link
Contributor Author

Thanks for the review @eliorivero. I pushed up two commits to address.

@eliorivero eliorivero added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Aug 21, 2018
Copy link
Contributor

@eliorivero eliorivero left a comment

Choose a reason for hiding this comment

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

Looks great now!

@ebinnion ebinnion merged commit e61d65f into master Aug 22, 2018
@ebinnion ebinnion deleted the update/module-overrides-vaultpress branch August 22, 2018 17:32
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Aug 22, 2018
@@ -7263,6 +7263,7 @@ public static function handle_post_authorization_actions( $activate_sso = false,
}

/**
<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

LOL ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin Page React-powered dashboard under the Jetpack menu General [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants