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

[BUGS-5161] Update deprecated filter call #544

Merged
merged 22 commits into from
Sep 29, 2022

Conversation

jazzsequence
Copy link
Contributor

@jazzsequence jazzsequence commented Sep 27, 2022

This PR resolves the issue reported in BUGS-5161 as well as making a number of other minor updates that improve the overall quality of the Solr Power plugin.

  • The commit that resolves the actual bug is 9e259f8
    • The whitelist_options filter name was deprecated in WP 5.5 in favor of the more inclusive allowed_options. The resolution is simply using the new filter name.
  • Unrelatedly, my first quality-of-life improvement was to replace sanity_check with environment_check for similar reasons e296885
  • When using the plugin locally, if SOLR_PATH is not defined, on activation, the plugin assumes you want to upload a schema. This is likely not the case, but the error message does not tell you what's wrong. This PR updates the error message to indicate that SOLR_PATH is not defined, so if that's the problem, you might know how to fix it. 8a5cbc9
  • If the plugin is installed on a WordPress multisite, the docs indicate that you should Network Activate the plugin, but that is not enforced. 3f53c37 changes this to wp_die if you activate on a single site of a multisite and links to the Network Admin Plugins page to network activate there.
  • Grunt actions were not running because of changes to grunt-sass. This PR updates the package.json to require node-sass and implements this in the Gruntfile.
  • Changes were necessary in the Gruntfile which was calling a parameter that was not (or no longer) supported, resulting in errors when Grunt was run.
  • Some changes were made to the PHPCS rules to not flag short array syntax ([]) or warn about not aligning equals signs.
    • Short array syntax was added in PHP 5.4, so there's no reason to require arrays to be written in long form.
    • Aligning equals signs for variables require additional time (that could be better used elsewhere on more important tasks) to align and re-align variable assignments when new variables are added or renamed.

fixes #541
related #538

@jazzsequence jazzsequence force-pushed the bugs-5161-save-facet-options branch from 6750fb8 to e296885 Compare September 27, 2022 18:58
On local, if SOLR_PATH is missing, you get a schema upload error when you maybe don't even really want to upload a schema. In that case, it would be helpful to know that the reason you can't activate the plugin is because the SOLR_PATH constant is missing.
Adds a die if you attempt to activate the plugin in a single site inside a multisite instance.
@jazzsequence jazzsequence self-assigned this Sep 27, 2022
@jazzsequence jazzsequence changed the title [BUGS-5161] wip [BUGS-5161] Update deprecated filter call Sep 28, 2022
@jazzsequence jazzsequence marked this pull request as ready for review September 28, 2022 18:59
@jazzsequence jazzsequence requested a review from a team as a code owner September 28, 2022 18:59
@jazzsequence jazzsequence enabled auto-merge (squash) September 28, 2022 21:26
Copy link
Contributor

@kporras07 kporras07 left a comment

Choose a reason for hiding this comment

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

Removing the mangling exception for jQuery makes me a bit nervous but I looked at the code and wasn't able to find a reason why it was originally added so I'd say LGTM

@jazzsequence jazzsequence merged commit cdc22a5 into master Sep 29, 2022
@jazzsequence jazzsequence deleted the bugs-5161-save-facet-options branch September 29, 2022 21:16
@jazzsequence
Copy link
Contributor Author

@kporras07 If it had had any adverse effects when I tried to run the build step, I would have tried to add it back in but nothing bad seemed to happen and everything still worked as expected so ¯_(ツ)_/¯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Site Network, cannot apply facet option changes.
2 participants