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.

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

Modules: Allow filtering of active modules. #8465

Merged
merged 2 commits into from
Jan 30, 2018
Merged

Modules: Allow filtering of active modules. #8465

merged 2 commits into from
Jan 30, 2018

Conversation

philipjohn
Copy link
Contributor

This change introduces a filter (jetpack_active_modules) that allows theme and plugin developers to modify the list of active Jetpack modules.

For some projects, there will be multiple copies of a site in different environments (e.g. development, QA, staging as well as production) and maintaining consistency across environments is important for quality assurance. Manually enabling/disabling modules across multiple environments is time consuming, inefficient and susceptible to human error. This change allows active modules to be defined in code, under version control, helping to ensure consistency across environments.

@philipjohn
Modules: Allow filtering of active modules.
06f079c 
This change introduces a filter (`jetpack_active_modules`) that allows theme and plugin developers to modify the list of active Jetpack modules.

For some projects, there will be multiple copies of a site in different environments (e.g. development, QA, staging as well as production) and maintaining consistency across environments is important for quality assurance. Manually enabling/disabling modules across multiple environments is time consuming, inefficient and susceptible to human error. This change allows active modules to be defined in code, under version control, helping to ensure consistency across environments.
@philipjohn philipjohn requested a review from a team as a code owner January 5, 2018 12:56
@oskosk oskosk added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. General [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Jan 5, 2018
@oskosk oskosk added this to the 5.8 milestone Jan 5, 2018
@oskosk
Copy link
Contributor

oskosk commented Jan 5, 2018

@philipjohn I've just labeled this one as ready for review. Please relabel as in progress if it's not the case. Thanks!

@philipjohn philipjohn added [Status] Needs Testing We need to add this change to the testing call for this month's release and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Jan 5, 2018
@philipjohn
Copy link
Contributor Author

Thanks @oskosk - I've changed to Needs Testing because I've been reminded I haven't really tested it yet.

@rickybanister
Copy link

This is neat, thanks @philipjohn.

I have one question (which in no way should block the work here)—is it possible, once this is implemented, for us to export the active module definition file? I have hopes of adding an import/export config feature to the Jetpack Beta plugin that would start to let us rebrand the beta plugin as a developer tools plugin. Exporting/importing your config across sites via a file would be a nice addition to it.

@jeherve
Copy link
Member

jeherve commented Jan 5, 2018

Related: #6621

is it possible, once this is implemented, for us to export the active module definition file?

Related: #5656

jetpack_active_modules does indeed bring us even closer to that.

@rickybanister
Copy link

Awesome!

@oskosk
Copy link
Contributor

oskosk commented Jan 8, 2018

LGTM!

zinigor
zinigor reviewed Jan 29, 2018
View reviewed changes
Copy link
Member

@zinigor zinigor left a comment

Choose a reason for hiding this comment

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

While this generally looks good to me, do you think it may be better to filter the array just before returning it? This way protect and vaultpress will be also filterable.

@zinigor zinigor 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 Testing We need to add this change to the testing call for this month's release labels Jan 29, 2018
@philipjohn
Copy link
Contributor Author

I hadn't done that because in my mind I didn't want VIPs to disable either of those. But you're right, others might want to and we should allow that. We can use the filter ourselves on VIP Go to ensure the modules we want to be active always are.

@philipjohn
Modules: Move jetpack_active_modules filter
eb3a011 
Filter the list of active modules right before we return the list so that Vaultpress and Protect can also be filtered.
@philipjohn philipjohn 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 Jan 30, 2018
zinigor
zinigor approved these changes Jan 30, 2018
View reviewed changes
Copy link
Member

@zinigor zinigor left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for making that change!

@zinigor zinigor 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 Jan 30, 2018
philipjohn added a commit to Automattic/vip-go-mu-plugins that referenced this pull request Jan 30, 2018
@philipjohn
Jetpack: Enforce some modules.
0480f30 
The `jetpack_active_modules` filter allows developers to enable/disable some modules in code. We might need to ensure some modules are always enabled/disabled and so we hook in to that filter ourselves.

At the moment, this just forces Vaultpress to be enabled. It won't have any effect until Automattic/jetpack#8465 is merged and released.
@philipjohn philipjohn mentioned this pull request Jan 30, 2018
Closed
@dereksmart dereksmart merged commit e47a724 into master Jan 30, 2018
@dereksmart dereksmart deleted the feature/active-modules-filter branch January 30, 2018 17:49
jeherve added a commit that referenced this pull request Jan 30, 2018
@jeherve
Changelog: add #8465
f314bbb
zinigor pushed a commit that referenced this pull request Jan 30, 2018
@oskosk @zinigor
Changelog 5.8: running changelog (#8442)
1ee2c03 
* Changelog 5.8: create base for changelog.

* Update 5.8 release post link

* fix 5.8 release date

* Updates to plugin description

* Changelog: add #8499

* Changelog: add #8506

* Changelog: add #8509

* Changelog: add #8516

* Changelog: add #8517

* Changelog: add #8523

* Changelog: add #8547

* Changelog: add #8496

* Changelog: add #8584

* Changelog: add #8595

* Changelog: add #8445

* Changelog: add #8431

* Changelog: add #8284

* Changelog: add #8270

* Changelog: add #8124

* Changelog: add #8581

* Changelog: add #8463

* Changelog: add #8568 (#8646)

* Updates to testing list and changelog

* Changelog: add #8443

* Changelog: add #8459

* Changelog: add #8469

* Changelog: add #8464

* Changelog: add #8478 and #8479

* Changelog: add #8483

* Changelog: add #8488

* Changelog: add #8513

* Changelog: add #8555

* Changelog: add #8565

* Changelog: add #8601

* Changelog: add #8612

* Changelog: add first pass at Search items.

* Changelog: add more info to help test Search.

* Changelog: add #8144

* Changelog: add #8313

* Changelog: add #8419

* Changelog: add #8465

* Changelog: add #8515

* Changelog: add #8587

* Changelog: add #8591

* Changelog: add #8659

* Changelog: add #8661

* Changelog: add #8671

* Changelog: add 5.7.1 to archived changelog too.

* Reverted changes to readme, removed entry about backups.
@niranjan-uma-shankar niranjan-uma-shankar mentioned this pull request May 6, 2019
Merged
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oskosk oskosk oskosk approved these changes

@zinigor zinigor zinigor approved these changes

Assignees
No one assigned
Labels
General [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Milestone
5.8
Development

Successfully merging this pull request may close these issues.
8 participants
@philipjohn @oskosk @rickybanister @jeherve @zinigor @kraftbj @dereksmart @matticbot