WPQueryParams: sniff for exclude array key being set#589
Conversation
| 'type' => 'warning', | ||
| 'message' => 'Using `post__not_in` should be done with caution, see https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information.', | ||
| 'keys' => [ | ||
| 'post__not_in', |
There was a problem hiding this comment.
Since post__not_in and exclude are the same, should we punt it into one group?
There was a problem hiding this comment.
I considered that, but we'd need to have a careful think about the warning message in that case as using the existing message would be confusing (which is why they are in separate groups now).
There was a problem hiding this comment.
We can definitely revamp the warning message to something more ambiguous.
There was a problem hiding this comment.
I'll have a think about it. Suggestions welcome.
There was a problem hiding this comment.
Maybe something like...
"Using exclusionary parameters should be done with caution, see https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information."
We can even re-vamp the documentation a bit to account for it as well.
|
As noted in #369, I'd prefer the second commit to be punted to 3.0 - thanks for thinking ahead and splitting out the code into the two commits! |
This: * Switches the `WordPressVIPMinimum.Performance.WPQueryParams` sniff over to use the WordPressCS `AbstractArrayAssignmentRestrictionsSniff` as the parent sniff. * Adds sniffing for the `exclude` array key via the `AbstractArrayAssignmentRestrictionsSniff` sniff logic. Includes unit test.
|
@GaryJones I've removed the second commit from the PR (kept locally with a "wait/3.0" annotation). @rebeccahum I've changed the group key for Once this PR is merged, a new issue should be opened for 3.0 as a reminder that the rest of the sniff still needs to be switched over to the new abstract. |
jrfnl
force-pushed
the
fix/369-exclude-arg-in-get_posts
branch
from
3e009d0 to
20fdf8b
Compare
3 participants
Note: I've split this change into two commits to allow for moving the second commit to the 3.0.0 release if so desired.
WPQueryParams: sniff for
excludearray key being setThis:
WordPressVIPMinimum.Performance.WPQueryParamssniff over to use the WordPressCSAbstractArrayAssignmentRestrictionsSniffas the parent sniff.excludearray key via theAbstractArrayAssignmentRestrictionsSniffsniff logic.Includes unit test.
Fixes #369
WPQueryParams: defer to the parent sniff
This changes over the existing keys being sniffed for by this sniff to use the logic from the new
AbstractArrayAssignmentRestrictionsSniffparent instead of custom logic.Note: this is a BC-break as the error codes for the existing checks change!
WordPressVIPMinimum.Performance.WPQueryParams.PostNotInis nowWordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_post__not_inWordPressVIPMinimum.Performance.WPQueryParams.SuppressFiltersTrueis nowWordPressVIPMinimum.Performance.WPQueryParams.SuppressFilters_suppress_filters