Skip to content

Conversation

jasonbahl
Copy link
Collaborator

What does this implement/fix? Explain your changes.

This determines whether the DisableIntrospection rule is enabled before instantiating the rule.

Does this close any currently open issues?

related: #3247 (comment)

… to class instead of being re-defined within the class

- update the DisableIntrospection error message to provide more clarity
-
@jasonbahl jasonbahl requested a review from justlevine February 4, 2025 20:14
@jasonbahl jasonbahl self-assigned this Feb 4, 2025
@jasonbahl jasonbahl mentioned this pull request Feb 4, 2025
@coveralls
Copy link

coveralls commented Feb 4, 2025

Coverage Status

coverage: 83.099% (+0.002%) from 83.097%
when pulling e074b20 on jasonbahl:fix/update-disable-introspection-instantiation
into fe91f89 on wp-graphql:release/v2.0.0-beta.1.

src/Request.php Outdated
Comment on lines 205 to 210
// By default, introspection is disabled for non-logged-in users and when debug is off
$disable_introspection_rule_enabled = 0;
if ( ! get_current_user_id() && ! \WPGraphQL::debug() && 'off' === get_graphql_setting( 'public_introspection_enabled', 'off' ) ) {
$disable_introspection_rule_enabled = 1;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we do this in the DisableIntrospection constructor instead of dirtying up request?

(Ref: I need a way to overload with this logic

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@justlevine pushed up some changes that handle it in the constructor and should allow for overloading like your example.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry my github is acting wonky: #3290 (comment)

Copy link
Collaborator

@justlevine justlevine left a comment

Choose a reason for hiding this comment

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

😍

@jasonbahl jasonbahl merged commit 64a375d into wp-graphql:release/v2.0.0-beta.1 Feb 5, 2025
31 checks passed
@jasonbahl jasonbahl changed the title fix: update DisableIntrospection rule to pass the enabled/disabled state to class instead of being re-defined within the class fix: [2.0] update DisableIntrospection rule to pass the enabled/disabled state to class instead of being re-defined within the class Feb 11, 2025
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.

3 participants