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

Add the filter to User::is_private() so users can choose to bypass o… #1685

Conversation

zacscott
Copy link

…r modify this functionality (relates to issue #961)

Your checklist for this pull request

Thanks for sending a pull request! Please make sure you click the link above to view the contribution guidelines, then fill out the blanks below.

🚨Please review the guidelines for contributing to this repository.

  • Make sure you are making a pull request against the develop branch (left side). Also you should start your branch off our develop.
  • Make sure you are requesting to pull request from a topic/feature/bugfix branch (right side). Don't pull request from your master!

What does this implement/fix? Explain your changes.

This change adds a filter graphql_user_model_is_private to the User::is_private() method.

Importantly this filter is BEFORE the normal code of the method is executed and can be used to bypass the rest of the function.

Ordinarily this method does a very expensive query via the use of the count_user_posts() function. This is called multiple times for some GraphQL queries adding considerable load as documented in the issue - #961

This filter can be used to bypass this count_user_posts() at the developers discretion, like so:

add_filter( 'graphql_user_model_is_private', __NAMESPACE__ .'\\graphql_user_model_is_private' );

function graphql_user_model_is_private( $is_private ) {
	return false;
}

Does this close any currently open issues?

This addresses this issue - #961

Any relevant logs, error output, GraphiQL screenshots, etc?

N/A / everything is noted on the issue - #961

Any other comments?

This was based on this declined PR #962

Where has this been tested?

Operating System: Ubuntu 20.04

WordPress Version: 5.6

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.004%) to 79.662% when pulling 96c50e4 on zacscott:feature/#961-user-model-is-private-filter into 91e1249 on wp-graphql:develop.

* to see if the user has published posts.
* @param User $this Instance of the User Model
*/
$is_private = apply_filters( 'graphql_user_model_is_private', null, $this );
Copy link
Member

Choose a reason for hiding this comment

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

I think we should probably handle this in the Model abstract so all models get this. That's how I handled it here: #972.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've made updates to @CodeProKid original #972 PR to get it to pass tests, and this now allows the filter to run on all Models.

If 3rd party code uses that filter, it will be used. If not, it will fall back to the individual Model's is_private() method.

I'll get that merged and released.

Closing this PR in favor of that one.

@jasonbahl jasonbahl closed this Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants