-
Notifications
You must be signed in to change notification settings - Fork 466
feat: Add Admin Notice for WPGraphQL for ACF #3027
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
Conversation
Signed-off-by: Joe Fusco <[email protected]>
| /** | ||
| * @return void | ||
| */ | ||
| public function maybe_display_notices(): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method maybe_display_notices has 52 lines of code (exceeds 40 allowed). Consider refactoring.
|
@xbd63
|
|
Ok thanks for explaining that. On the WP GraphQL I do see a notice "Check out the new WPGraphQL for ACF". Please let me know if I need to do anything else. |
- replaced unset calls with $this->remove_admin_notice
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
| /** | ||
| * Render the notices. | ||
| */ | ||
| protected function render_notices(): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method render_notices has 55 lines of code (exceeds 40 allowed). Consider refactoring.
justlevine
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a quick pass, will review more in depth tomorrow and also probably make those array types more specific
| } | ||
|
|
||
| /** | ||
| * @param string $slug A unique slug to identify the admin notice by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * @param string $slug A unique slug to identify the admin notice by | |
| * Registers a configurable notice on the WordPress Dashboard for WPGraphQL-related notices. | |
| * | |
| * @param string $slug A unique slug to identify the admin notice by. |
|
|
||
| /** | ||
| * @param string $slug A unique slug to identify the admin notice by | ||
| * @param array<mixed> $config The config for the admin notice. Determines visibility, context, etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * @param array<mixed> $config The config for the admin notice. Determines visibility, context, etc. | |
| * @param array<string,mixed> $config The config for the admin notice. Determines visibility, context, etc. |
| $this->admin_enabled = apply_filters( 'graphql_show_admin', true ); | ||
| $this->graphiql_enabled = apply_filters( 'graphql_enable_graphiql', get_graphql_setting( 'graphiql_enabled', true ) ); | ||
|
|
||
| $admin_notices = new AdminNotices(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be wrapped in an is_admin() check. I don't think we're doing that earlier in the lifecycle but I might be wrong (reviewing via 📱)
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
- extend WPGraphQLTestCase instead of TestCase
|
Code Climate has analyzed commit e6cd74a and detected 3 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |

What does this implement/fix? Explain your changes.
This introduces a new
register_graphql_admin_notice()API that can be used to display an admin notice on the Plugins page, or on the WPGraphQL Settings or GraphiQL IDE page.This can be used by core WPGraphQL or for extensions to provide notices scoped to WPGraphQL pages.
It allows for notices to be permanent or dismissable and handles nonces for dismissing, storing a flag of dismissed notices in user_meta so that users won't be bugged again by the same notice after they've dismissed it.
Does this close any currently open issues?
no
Any other comments?
Documentation: https://www.wpgraphql.com/functions/register_graphql_admin_notice
example usage: