Add GraphQL API #51

ingeniumed · 2023-11-14T03:48:02Z

Description

This PR is meant add a GraphQL API to the Block Data API. It assumes that you have WP-GraphQL installed, and uses a filter to tweak the output of the ContentParser to generate the GraphQL response.

In addition, it uses the wp_unique_id function to generate ids for the blocks so that the blocks can be correlated together. The inner_blocks are therefore able to be flattened entirely, thereby bypassing the nested depth problem in graphQL.

This is in the draft status to get initial feedback, as well as to add some tests and guard rails around the GraphQL API.

Resolves #49

Steps to Test

Download, install and activate WP-GraphQL
Activate Block Data API with this branch
Modify the initial post to add some nested blocks
Run the following GraphQL query

query NewQuery {
  post(id: "1", idType: DATABASE_ID) {
    blocksData {
      blocks {
        id
        name
        attributes {
          name
          value
        }
        innerBlocks {
          name
          parentId
          id
          attributes {
            name
            value
          }
        }
      }
    }
  }
}

Ensure the output matches the post as well as the rest API

…e a blank graphQL class

…Block filter for this

src/graphql/graphql-api.php

src/parser/content-parser.php

src/graphql/graphql-api.php

…t parser

alecgeatches · 2023-11-27T19:52:49Z

@ingeniumed I noticed some errors in a response while testing today. In a post, I have this content:

<!-- wp:paragraph -->
<p>Unnested paragraph.</p>
<!-- /wp:paragraph -->

<!-- wp:columns -->
<div class="wp-block-columns"><!-- wp:column -->
<div class="wp-block-column"><!-- wp:paragraph -->
<p>Column 1</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column -->

<!-- wp:column -->
<div class="wp-block-column"><!-- wp:paragraph -->
<p>Column 2</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column -->

<!-- wp:column -->
<div class="wp-block-column"><!-- wp:paragraph -->
<p>Column 3</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->

And then I query for the blocks/innerBlocks of that post:

query NewQuery {
  post(id: 419, idType: DATABASE_ID) {
    blocksData {
      blocks {
        attributes {
          name
          value
        }
        id
        name
        innerBlocks {
          attributes {
            name
            value
          }
          id
          name
          parentId
        }
      }
    }
  }
}

The result appears to hold the correct blocks, but also a bunch of Internal server error in an errors array:

Click to expand

{
  "errors": [
    {
      "message": "Internal server error",
      "extensions": {
        "category": "internal"
      },
      "locations": [
        {
          "line": 12,
          "column": 11
        }
      ],
      "path": [
        "post",
        "blocksData",
        "blocks",
        1,
        "innerBlocks",
        0,
        "attributes"
      ]
    },
    {
      "message": "Internal server error",
      "extensions": {
        "category": "internal"
      },
      "locations": [
        {
          "line": 12,
          "column": 11
        }
      ],
      "path": [
        "post",
        "blocksData",
        "blocks",
        1,
        "innerBlocks",
        2,
        "attributes"
      ]
    },
    {
      "message": "Internal server error",
      "extensions": {
        "category": "internal"
      },
      "locations": [
        {
          "line": 12,
          "column": 11
        }
      ],
      "path": [
        "post",
        "blocksData",
        "blocks",
        1,
        "innerBlocks",
        4,
        "attributes"
      ]
    }
  ],
  "data": {
    "post": {
      "blocksData": {
        "blocks": [
          {
            "attributes": [
              {
                "name": "content",
                "value": "Unnested paragraph."
              },
              {
                "name": "dropCap",
                "value": ""
              }
            ],
            "id": "1",
            "name": "core/paragraph",
            "innerBlocks": null
          },
          {
            "attributes": [
              {
                "name": "isStackedOnMobile",
                "value": "1"
              }
            ],
            "id": "2",
            "name": "core/columns",
            "innerBlocks": [
              {
                "attributes": null,
                "id": "3",
                "name": "core/column",
                "parentId": "2"
              },
              {
                "attributes": [
                  {
                    "name": "content",
                    "value": "Column 1"
                  },
                  {
                    "name": "dropCap",
                    "value": ""
                  }
                ],
                "id": "4",
                "name": "core/paragraph",
                "parentId": "3"
              },
              {
                "attributes": null,
                "id": "5",
                "name": "core/column",
                "parentId": "2"
              },
              {
                "attributes": [
                  {
                    "name": "content",
                    "value": "Column 2"
                  },
                  {
                    "name": "dropCap",
                    "value": ""
                  }
                ],
                "id": "6",
                "name": "core/paragraph",
                "parentId": "5"
              },
              {
                "attributes": null,
                "id": "7",
                "name": "core/column",
                "parentId": "2"
              },
              {
                "attributes": [
                  {
                    "name": "content",
                    "value": "Column 3"
                  },
                  {
                    "name": "dropCap",
                    "value": ""
                  }
                ],
                "id": "8",
                "name": "core/paragraph",
                "parentId": "7"
              }
            ]
          }
        ]
      }
    }
  },
  "extensions": {
    "debug": [
      {
        "type": "DEBUG_LOGS_INACTIVE",
        "message": "GraphQL Debug logging is not active. To see debug logs, GRAPHQL_DEBUG must be enabled."
      }
    ]
  }
}

chriszarate · 2023-12-08T17:05:23Z

README.md

+```php
+/**
+	* Filter to enable/disable the graphQL API. By default, it is enabled.
+	* 
+	* @param bool $is_graphql_to_be_enabled Whether the graphQL API should be enabled or not.
+*/
+apply_filters( 'vip_block_data_api__is_graphql_enabled', true );
+```
+


It's a little odd to include the apply_filters definition here, which isn't relevant to usage. I would remove it

Suggested change

```php

/**

* Filter to enable/disable the graphQL API. By default, it is enabled.

*

* @param bool $is_graphql_to_be_enabled Whether the graphQL API should be enabled or not.

*/

apply_filters( 'vip_block_data_api__is_graphql_enabled', true );

```

Good point, removed here: https://github.com/Automattic/vip-block-data-api/blob/fac6ebf59e071e5db10234cabe951f7db1d4894f/README.md#graphql-1.

chriszarate · 2023-12-08T17:06:11Z

README.md

+add_filter( 'vip_block_data_api__is_graphql_enabled', function( ) {
+    return false;
+}, 10, 1);


Suggested change

add_filter( 'vip_block_data_api__is_graphql_enabled', function( ) {

return false;

}, 10, 1);

add_filter( 'vip_block_data_api__is_graphql_enabled', '__return_false' );

chriszarate · 2023-12-08T17:10:25Z

src/graphql/graphql-api.php

+			return new \Exception( $parser_results->get_error_message() );
+		}
+
+		$parser_results['blocks'] = array_map(function ( $block ) {


is there no phpcs on this repo?

Suggested change

$parser_results['blocks'] = array_map(function ( $block ) {

$parser_results['blocks'] = array_map( function ( $block ) {

Huh, weird! We do have phpcs configured, and it seems to spot other problems in this file just fine, but it's not enforcing parentheses spacing. Our phpcs configuration is a close relation to vip-go-mu-plugins's config which appears to be roughly the same. Looking into what sniff might be missing here.

Figured it out. I noticed if I add a missing space a couple of lines up, PHPCS will report it:

$block['id'] = Relay::toGlobalId('ID', wp_unique_id() ); // ^ missing space

FILE: src/graphql/graphql-api.php -------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------- 73 | ERROR | [x] Expected 1 spaces after opening parenthesis; 0 found | | (PEAR.Functions.FunctionCallSignature.SpaceAfterOpenBracket)

However, that same PEAR.Functions.FunctionCallSignature.SpaceAfterOpenBracket sniff doesn't kick in for array_map(). After looking at our configuration, I noticed we have some sniffs disabled related to anonymous functions and function calls. If I remove those, we'll now get multiple errors for the multi-line function call:

72 | ERROR | [x] Opening parenthesis of a multi-line function call must be the last content on the line | | (PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket) 74 | ERROR | [x] Only one argument is allowed per line in a multi-line function call (PEAR.Functions.FunctionCallSignature.MultipleArguments) 74 | ERROR | [x] Closing parenthesis of a multi-line function call must be on a line by itself (PEAR.Functions.FunctionCallSignature.CloseBracketLine)

So, in short, if we want to disable those sniffs to use some standard PHP array_map( function( ... ) { practices, we also lose the sniff that tells us about weird spacing after array_map().

Fixed manually.

chriszarate · 2023-12-08T17:11:49Z

src/graphql/graphql-api.php

+	/**
+	 * Transform the block's format to the format expected by the graphQL API.
+	 *
+	 * @param array $block An associative array of parsed block data with keys 'name' and 'attribute'.


+1 for type declarations instead of hand-written descriptions

Suggested change

* @param array $block An associative array of parsed block data with keys 'name' and 'attribute'.

* @param array $block An associative array of parsed block data with keys 'name' and 'attributes'.

chriszarate · 2023-12-08T17:13:07Z

src/graphql/graphql-api.php

+	/**
+	 * Convert the attributes to be in the name-value format that the schema expects.
+	 *
+	 * @param array $block An associative array of parsed block data with keys 'name' and 'attribute'.


Suggested change

* @param array $block An associative array of parsed block data with keys 'name' and 'attribute'.

* @param array $block An associative array of parsed block data with keys 'name' and 'attributes'.

chriszarate · 2023-12-08T17:13:33Z

src/graphql/graphql-api.php

+		// check if type of attributes is stdClass and unset it as that's not supported by graphQL.
+		if ( isset( $block['attributes'] ) && is_object( $block['attributes'] ) ) {
+			unset( $block['attributes'] );


how / why would this happen?

@ingeniumed Can you explain a bit more about the stdClass comment here? I'm not sure where that comes from either.

So this is because in the contentParser we set the actual attributes to be (object)[] which allows it to be given back as empty json in the rest api rather than as an empty array. Due to that, the internal data type comes back as stdClass rather than an array. This workaround is due to that.

I experimented with switching the (object)[] to a new ArrayObject but in that case it adds an empty array with key storage under the attributes which requires an even worse workaround. That's why it's been done this way.

I tried to search for a better solution but couldn't quite find one and I didn't want to break the rest response or the way the public function works because there are customers already using it.

This is the line I'm referencing btw in the contentParser.

Hopefully that explains it properly

@ingeniumed That sounds familiar now! As a side-effect of returning data in JSON via the REST API, if we want to return attributes: {} in a response, we need to cast [] into an object to avoid returning an array instead. This is just dealing with that unusual data in the GraphQL layer.

chriszarate · 2023-12-08T17:16:19Z

src/graphql/graphql-api.php

+				function ( $name, $value ) {
+					return [
+						'name'  => $name,
+						'value' => $value,


I guess WPGraphQL is doing the casting to string? You might want to do it here in code just to be explicit and test it.

I guess WPGraphQL is doing the casting to string?

I think you're right! I tested with a block delimiter value like {"test": 123}, which is returned as an integer from the parser and REST API. However, once we return it in a BlockAttribute, it is converted to a string. Added explicit conversion in 8cdc1e2 anyway.

chriszarate · 2023-12-08T17:16:56Z

src/graphql/graphql-api.php

+				// Set the parentId to be the ID of the parent block whose inner blocks are being flattened.
+				$inner_block['parentId'] = $parent_id;
+
+				// This block doesnt have any inner blocks, so just add it to the flattened blocks. Ensure the parentId is set.


indenting is off here

chriszarate · 2023-12-08T17:20:48Z

src/graphql/graphql-api.php

+					],
+					'parentId'    => [
+						'type'        => 'ID',
+						'description' => 'ID of the parent for this inner block, if it is an inner block. Otherwise, it will not be set. If it set, this will match the ID of the block',


last part confusing / redundant

Suggested change

'description' => 'ID of the parent for this inner block, if it is an inner block. Otherwise, it will not be set. If it set, this will match the ID of the block',

'description' => 'ID of the parent for this inner block, if it is an inner block. Otherwise, it will be null.',

chriszarate · 2023-12-08T17:24:05Z

src/graphql/graphql-api.php

+						'args'        => [
+							'flatten' => [
+								'type'        => 'Boolean',
+								'description' => 'Collate the inner blocks under each root block into a single list with a parent-child relationship. This is set to true by default, and setting it to false will reverse that to preserve the original block hierarchy, at the expense of knowing the exact depth when querying the inner blocks. Default: true',


Suggested change

'description' => 'Collate the inner blocks under each root block into a single list with a parent-child relationship. This is set to true by default, and setting it to false will reverse that to preserve the original block hierarchy, at the expense of knowing the exact depth when querying the inner blocks. Default: true',

'description' => 'Collate the inner blocks under each root block into a single list with a parent-child relationship. This is set to true by default, and setting it to false will preserve the original block hierarchy, but will require nested inner block queries to the desired depth. Default: true',

chriszarate · 2023-12-08T17:26:54Z

src/graphql/graphql-api.php

+							],
+						],
+						'resolve'     => function ( $blocks, $args ) {
+							if ( ! isset( $args['flatten'] ) || true === $args['unflatten'] ) {


bug, probably needs tests

Suggested change

if ( ! isset( $args['flatten'] ) || true === $args['unflatten'] ) {

if ( ! isset( $args['flatten'] ) || false === $args['flatten'] ) {

Good catch! I fixed this, tested it manually, and put it in d578fe4. I spent some time trying to add a test, but after my attempt I think @ingeniumed has already done a decent job trying to wrap the core logic from the resolver with a test:

vip-block-data-api/src/graphql/graphql-api.php

Lines 220 to 233 in d578fe4

'resolve' => function ( $blocks, $args ) {

if ( ! isset( $args['flatten'] ) || true === $args['flatten'] ) {

$blocks['blocks'] = array_map( function ( $block ) {

// Flatten the inner blocks, if any.

if ( isset( $block['innerBlocks'] ) ) {

$block['innerBlocks'] = self::flatten_inner_blocks( $block['innerBlocks'], $block['id'] );

}

return $block;

}, $blocks['blocks'] );

}

return $blocks['blocks'];

},

Our unit testing does not include the WPGraphQL plugin, so the logic that's in this resolver isn't directly tested in test_flatten_inner_blocks(). The bug here is in the thin shell of WPGraphQL code around the flatten_inner_blocks() function, which we test directly. Ideally we could have some end-to-end tests that include the request through WPGraphQL (like we do for our REST tests), but I think the testing here is a good enough start. As long as we catch the interface bugs, like this.

chriszarate · 2023-12-08T17:32:40Z

src/graphql/graphql-api.php

+								$blocks['blocks'] = array_map(function ( $block ) {
+									// Flatten the inner blocks, if any.
+									if ( isset( $block['innerBlocks'] ) ) {
+										$block['innerBlocks'] = self::flatten_inner_blocks( $block['innerBlocks'], $block['id'] );
+									}
+
+									return $block;
+								}, $blocks['blocks'] );


You can just do this, no?

Suggested change

$blocks['blocks'] = array_map(function ( $block ) {

// Flatten the inner blocks, if any.

if ( isset( $block['innerBlocks'] ) ) {

$block['innerBlocks'] = self::flatten_inner_blocks( $block['innerBlocks'], $block['id'] );

}

return $block;

}, $blocks['blocks'] );

// Flatten the inner blocks, if any.

$blocks['blocks'] = self::flatten_inner_blocks( $blocks['blocks'], null );

This replacement doesn't work as-is, because flatten_inner_blocks() needs the $parent_id parameter, which is passed to innerBlocks to determine their parent IDs:

vip-block-data-api/src/graphql/graphql-api.php

Lines 116 to 119 in d578fe4

public static function flatten_inner_blocks( $inner_blocks, $parent_id, $flattened_blocks = [] ) {

foreach ( $inner_blocks as $inner_block ) {

// Set the parentId to be the ID of the parent block whose inner blocks are being flattened.

$inner_block['parentId'] = $parent_id;

I think this is due to the design that top level blocks are treated differently as "root" blocks, but their children are flattened beneath them all together. I'm sure it's possible to rewrite flatten_inner_blocks() so this is done internally, but I think this works fine.

Wait a minute, I'm rereading this and I'm not sure why it's not working anymore. Let me take a second look.

alecgeatches · 2023-12-13T00:07:14Z

@chriszarate Thanks a ton for your review! I believe I've addressed all of your suggestions above. Please take another look when you have a chance. Thank you!

chriszarate · 2023-12-13T03:17:18Z

src/graphql/graphql-api.php

+	 */
+	public static function transform_block_format( $block ) {
+		// Generate a unique ID for the block.
+		$block['id'] = Relay::toGlobalId( 'ID', wp_unique_id() );


The first argument to this function should the type name of the object in question, so BlockData in this case. This is what ensures uniqueness across the graph. Also, since wp_unique_id just increments integers, you should probably also incorporate the post ID. Otherwise, you risk collisions in client caches which will be really hard to diagnose. (I've seen this happen and it's bewildering when a paragraph from post 2 shows up in post 1).

Suggested change

$block['id'] = Relay::toGlobalId( 'ID', wp_unique_id() );

$block['id'] = Relay::toGlobalId( 'BlockData', sprintf( "%d:%d", $post->ID, wp_unique_id() ) );

Thank you for clarifying this @chriszarate! Changed in 6e2c367.

chriszarate

All my feedback has been addressed, but I'll leave it to Bistro to approve :)

ingeniumed · 2023-12-14T02:14:39Z

I'll merge this into our release branch, and we can cut a release later this week via that. Thanks for the amazing feedback @chriszarate, and @smithjw1.

ingeniumed added 14 commits October 31, 2023 14:42

Add WP-GraphQL to Block Data API, import it on plugin startup and mak…

b1ee4fa

…e a blank graphQL class

Add the skeleton layout of the graphQL endpoint

5d9a6a6

Add the parent_id and id to each block

a386950

Transform the block attributes, and add an extra param to the sourced…

bf73c1c

…Block filter for this

Require the new graphQL api file

83bfd74

Set the minimum version of the plugin to be 8.0

7e42577

Remove the graphQL plugin from the plugin itself.

cf0286d

Remove the lib folder from the PHPCS

61bee9d

Attempting to flatten the nested blocks

4c00b0d

Flattened the innerblocks, but still have a duplicate showing up

d810a0f

Fixed the bug with duplicates in the child list

71dd8ae

Downgrade php version to 7.4

d98425a

Add some php docs and fix linting problems

d5ab09e

Tweak todo comments

778509d

ingeniumed self-assigned this Nov 14, 2023