Skip to content

Conversation

@josephfusco
Copy link
Member

@josephfusco josephfusco commented Jan 29, 2024

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:

add_action( 'plugins_loaded', function() {

  if ( function_exists( 'register_graphql_admin_notice' ) ) {
    $config = [
      
      // required. string. The message to display in the notice. If no message, the notice won't display.
      'message' => __( 'Your notice message', 'your-textdomain' ), 
      
      // optional. boolean. whether the notice should be dismissable. False to be non-dismissable.
      'is_dismissable' => true, 
     
      // optional. string. The type of message. warning, success, error, info
      'type' => 'info', 
      
      // optional. callback. return true to display the notice (i.e. only display if another plugin is active)
      'conditions' => function() { 
         // if xx == true, return true
         // if xx == false, return false  
       }
    ];
    
    register_graphql_admin_notice( 'my-notice-slug', $config );
  }

} );

CleanShot 2024-02-02 at 16 58 50

Signed-off-by: Joe Fusco <[email protected]>
@coveralls
Copy link

coveralls commented Jan 29, 2024

Coverage Status

coverage: 84.275% (-0.5%) from 84.765%
when pulling e6cd74a on acf-deprecation-notice
into 121a691 on develop.

/**
* @return void
*/
public function maybe_display_notices(): void {

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.

@LondonCityMedia
Copy link

I tested this by replacing the Admin.php file in our live on our system yesterday, and the deprecation warnings still persist today.
Screenshot 2024-01-31 at 10 19 51

@justlevine
Copy link
Collaborator

justlevine commented Jan 31, 2024

@xbd63

I tested this by replacing the Admin.php file in our live on our system yesterday, and the deprecation warnings still persist today.

  1. This is a draft PR, so regardless it shouldn't be expected to work in its current state.

  2. The purpose this PR is to display an admin notice on the WordPress dashboard. It has nothing to do with PHP deprecated #2981 or any other PHP 8.3 compatibility work.

@LondonCityMedia
Copy link

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.

jasonbahl and others added 5 commits January 31, 2024 10:40
- replaced unset calls with $this->remove_admin_notice
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
@josephfusco josephfusco changed the title Add Admin Notice for WPGraphQL for ACF fix: Add Admin Notice for WPGraphQL for ACF Feb 2, 2024
/**
* Render the notices.
*/
protected function render_notices(): void {

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.

@jasonbahl jasonbahl changed the title fix: Add Admin Notice for WPGraphQL for ACF feat: Add Admin Notice for WPGraphQL for ACF Feb 2, 2024
@jasonbahl jasonbahl marked this pull request as ready for review February 2, 2024 23:59
@jasonbahl jasonbahl self-assigned this Feb 2, 2024
@jasonbahl jasonbahl requested a review from justlevine February 2, 2024 23:59
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.

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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* @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.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* @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();
Copy link
Collaborator

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 📱)

@jasonbahl jasonbahl removed their assignment Feb 5, 2024
Signed-off-by: Joe Fusco <[email protected]>
Signed-off-by: Joe Fusco <[email protected]>
@josephfusco josephfusco requested a review from jasonbahl February 6, 2024 20:22
@jasonbahl jasonbahl closed this Feb 8, 2024
@jasonbahl jasonbahl reopened this Feb 8, 2024
@qlty-cloud-legacy
Copy link

Code Climate has analyzed commit e6cd74a and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 3

View more on Code Climate.

@jasonbahl jasonbahl merged commit 28f0952 into develop Feb 8, 2024
This was referenced Feb 8, 2024
@justlevine justlevine deleted the acf-deprecation-notice branch August 3, 2024 11:57
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.

6 participants