Skip to content

Feature Hints: disable when plugins cannot be installed on site.#14108

Merged
jeherve merged 2 commits into
masterfrom
disable/psh-no-plugininstall
Nov 25, 2019
Merged

Feature Hints: disable when plugins cannot be installed on site.#14108
jeherve merged 2 commits into
masterfrom
disable/psh-no-plugininstall

Conversation

@jeherve
Copy link
Copy Markdown
Member

@jeherve jeherve commented Nov 22, 2019 •
edited
Loading

Changes proposed in this Pull Request:

  • When one cannot install plugins, we do not have to do anything with feature hints.

Internal reference: p1571740982220600-slack-jetpack-crew

Testing instructions:

I am not quite sure to be honest. @mjangda Any idea here?

  • Add define( 'DISALLOW_FILE_MODS', true ); to your site's wp-config.php file.
  • From that point on, you cannot access wp-admin/plugin-install.php on your site so Feature hints should not be triggered.
  • Remove the constant, head over to Plugins > Add New, and type "cdn" in the search field.
  • If the Asset CDN feature is not already enabled on your site, you should see a prompt to enable it.

Proposed changelog entry for your changes:

  • Feature Hints: disable when plugins cannot be installed on site.

@jeherve
Feature Hints: disable when plugins cannot be installed on site.
d56a04c 
When one cannot install plugins, we do not have to do anything with feature hints.
@jeherve jeherve added Bug When a feature is broken and / or not performing as intended [Status] Needs Review This PR is ready for review. [Pri] Low Plugin Search aka Feature Hints labels Nov 22, 2019
@jeherve jeherve requested review from a team and mjangda November 22, 2019 20:14
@jeherve jeherve self-assigned this Nov 22, 2019
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Nov 22, 2019 •
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: December 3, 2019.
Scheduled code freeze: November 26, 2019

Generated by 🚫 dangerJS against 983d9e8

@jeherve jeherve mentioned this pull request Nov 22, 2019
Closed
mjangda
mjangda reviewed Nov 22, 2019
View reviewed changes
Comment thread modules/plugin-search.php
@mjangda
Copy link
Copy Markdown
Member

mjangda commented Nov 22, 2019

I am not quite sure to be honest.

Yeah, kind of hard to test, but one way to verify would be to check Jetpack_Plugin_Search::$instance === null when the constant is enabled (via wp shell, for example).

@mjangda
Copy link
Copy Markdown
Member

mjangda commented Nov 22, 2019

You also obviously want to test the reverse: that plugin search hints are still active when the constant is not set or false :)

@mjangda mjangda mentioned this pull request Nov 22, 2019
Closed
4 tasks
@jeherve
Copy link
Copy Markdown
Member Author

jeherve commented Nov 22, 2019

You also obviously want to test the reverse

You're right, I should probably mention that :)

@jeherve jeherve added this to the 8.0 milestone Nov 22, 2019
dereksmart
dereksmart approved these changes Nov 25, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@dereksmart dereksmart left a comment

Choose a reason for hiding this comment

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

Works well. Tested that plugins hints still work when the constant is missing as well as declared false

@dereksmart dereksmart 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 Nov 25, 2019
@jeherve jeherve merged commit 5f3a9b5 into master Nov 25, 2019
@jeherve jeherve deleted the disable/psh-no-plugininstall branch November 25, 2019 16:19
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Nov 25, 2019
jeherve added a commit that referenced this pull request Nov 25, 2019
@jeherve
Changelog: add #14108
c760986
jeherve added a commit that referenced this pull request Nov 25, 2019
@jeherve
8.0 Release: running changelog (#13964)
807ab89 
* 8.0 Release: running changelog

* Changelog: add #13921

* Changelog: add #13980

* Changelog: add #13905

* Changelog: add #13971

* Changelog: add #13984

* Changelog: add #14009

* Changelog: add #13620

* Remove things that will ship in 7.9.1

* Changelog: add 7.9.1 release (#14044)

* Changelog: add base for 7.9.1 release

* Update release date and post link

* Changelog: add #14066

* Update changelog for 7.9.1

* Changelog: add #13405

* Changelog: add #13841

* Changelog: add #13924

* Changelog: add #13986

* Changelog: add #14010, #14028, #14053, #14055.

* Changelog: add #14054

* Changelog: add #14031

* Changelog: add #14039

* Changelog: add #14050

* Changelog: add #14070

* Changelog: add #14082

* Changelog: add #14084

* Changelog: add #14111

* Changelog: add #13961

* Changelog: add #14047

* Changelog: add #14091

* Changelog: add #14108

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

Reviewers

@mjangda mjangda mjangda left review comments

@dereksmart dereksmart dereksmart approved these changes

Assignees

@jeherve jeherve

Labels

Bug When a feature is broken and / or not performing as intended Plugin Search aka Feature Hints [Pri] Low

Projects

None yet

Milestone

8.0

Development

Successfully merging this pull request may close these issues.

5 participants

@jeherve @jetpackbot @mjangda @dereksmart @matticbot