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

[MTKA-1483] Register ACM fields for GraphQL mutations #499

Merged
merged 34 commits into from
Apr 29, 2022

Conversation

nickcernis
Copy link
Member

@nickcernis nickcernis commented Apr 14, 2022

Description

Registers ACM fields for create[ModelName] and update[ModelName] GraphQL mutations so that devs can send ACM fields as mutation inputs.

Addresses #240.

Note that:

https://wpengine.atlassian.net/browse/MTKA-1359
https://wpengine.atlassian.net/browse/MTKA-1483

Testing

Read the docs introduced by this PR and try making a mutation request against one of your models in GraphiQL:

https://github.com/wpengine/atlas-content-modeler/blob/mtka-1359/mutations/docs/mutations/index.md

Documentation Changes

I added docs to this repo. We could move them to https://developers.wpengine.com/ once we know what release this will be part of.

This mostly seems to impact test data where graphql_single_name
is not set.
When submitted via GraphQL, multiple choice fields are passed as
an array of choice keys: [ "choice1", "choice2" ].

When submitted via the publisher form, multiple choice fields are
passed as an array of keys and values in the case of a "multi":
[ "choice1" => "Choice 1", "choice2" => "Choice 2"].

This commit adjusts the sanize_field logic so that all inputs above
result in an output of [ "choice1", "choice2" ] and adds tests to
verify that behavior.
Form data for multis is passed as:

(
    [0] => Array
        (
            [test] => test
        )

    [1] => Array
        (
            [test2] => test2
        )
)
Derive model GraphQL name from raw field data instead of registered
field data.

To work around a timing issue where tests were failing due to GraphQL
types being registered in our testing environment before ACM post types
were registered.
- Updates correct boolean type.
- Removes resolver in mutation registration.
The existing resolver logic in register_content_fields_with_graphql
handles value resolution.
Comment on lines -508 to -512
// When you have multiple items in an array returned we need to define them as a list of strings.
if ( 'list' === $field['type'] ) {
$field['type'] = array( 'list_of' => 'String' );
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated map_html_field_type_to_graphql_field_type to return the correct type in place of this logic.

The GraphQL mutations logic also needs to grab the type so we may as well return it up front instead of changing it here.

Comment on lines +96 to +97
update_post_meta( $post_id, 'boolean', 'off' );
update_post_meta( $post_id, 'booleanRequired', 'on' );
Copy link
Member Author

Choose a reason for hiding this comment

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

Boolean values are ultimately stored in meta as 'off' or 'on' and not 'true' or 'false' so I changed the tests to reflect that.

* delete operations on ACM models.
* - Document how a delete mutation should work.
*/
public function test_graphql_delete_mutations_remove_acm_entries(): void {
Copy link
Member Author

Choose a reason for hiding this comment

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

I can omit this test if needed because it's WPGraphQL and not ACM that registers CPTs for delete mutations. I felt it worthwhile just to confirm and document delete mutation behavior, though.

For consistency with GraphQL responses.
Comment on lines +134 to +135
case 'boolean':
return $meta_value === 'on' ? true : false;
Copy link
Member Author

Choose a reason for hiding this comment

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

Updating the REST response for consistency with the GraphQL response for boolean fields, and so that tests pass with the updated test data (which changed to 'on' and 'off' instead of 'true' and 'false').

Update mutations don't seem like they should enforce required fields,
as the check for required fields was made during post creation.

Users should not have to pass required fields if they are updating
another field.
@nickcernis nickcernis changed the title [MTKA-1359] Register ACM fields for GraphQL mutations [MTKA-1483] Register ACM fields for GraphQL mutations Apr 26, 2022
@nickcernis nickcernis marked this pull request as ready for review April 26, 2022 12:43
@nickcernis nickcernis requested a review from a team as a code owner April 26, 2022 12:43
Copy link
Member

@mindctrl mindctrl left a comment

Choose a reason for hiding this comment

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

I left one minor nit/question. Excellent work!

mindctrl
mindctrl previously approved these changes Apr 27, 2022
@mindctrl mindctrl linked an issue Apr 27, 2022 that may be closed by this pull request
@mindctrl
Copy link
Member

@nickcernis there's a merge conflict here now. Sorry about that.

@nickcernis
Copy link
Member Author

@mindctrl Fixed, thanks for the note!

mindctrl
mindctrl previously approved these changes Apr 28, 2022
CHANGELOG.md Outdated Show resolved Hide resolved
@nickcernis nickcernis merged commit 81a250f into main Apr 29, 2022
@nickcernis nickcernis deleted the mtka-1359/mutations branch April 29, 2022 16:23
@nickcernis nickcernis restored the mtka-1359/mutations branch April 29, 2022 16:23
@mindctrl mindctrl deleted the mtka-1359/mutations branch May 26, 2022 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Requires 2 Approvals This PR should include 2 Approvals before merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WPGraphQL mutation support
3 participants