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

PostObjectMutations #140

Merged
merged 26 commits into from May 26, 2017
Merged

Conversation

jasonbahl
Copy link
Collaborator

@jasonbahl jasonbahl commented May 23, 2017

This introduces postObjectMutations, and includes tests for said mutations.

Also includes lots of formatting changes, which I know I should do in separate PRs, but whatever, deal with it 😜 (I'll really try to stop doing that though, because I know it's annoying to try and code review when there's a bunch of misc formatting changes included)

====

Here's an example of trying to create a page but not being allowed to because I don't have permissions.

So, I add my Authorization headers to the request (using Basic auth in this example), then I repeat the mutation and you can see data created!

Then I do a follow-up updatePage mutation to change the title, and you see the same page is returned, just with a new title.

pagemutations

- Fixes to AvatarType
Update the WPInputObjectType to accept a $config array as the input
Update the CommentConnectionArgs to use the $config array for the parent input
Update the PostObjectConnectionArgs to use the $config array for the parent input
Update the PostObjectConnectionArgsDateQuery to use the $config array for the parent input
Update the TermObjectConnectionArgs to use the $config array for the parent input
Update the TermObjectConnectionResolver to use the $config array for the parent input
Update the UserConnectionArgs to use the $config array for the parent input
Introduce a shared “postObjectMutation” class for the CRUD classes to utilize
Introduce PostObjectDelete class for handling deletes for postObjects
Introduce PostObjectCreate class for handling inserts for postObjects
Introduce PostObjectUpdate class for handling updates for postObjects
Update /tests/test-nodes.php to include a query with variables
Update /tests/test-plugin-queries.php to set the role properly
Update /tests/test-post-connection-queries.php to set the role properly
Update /tests/test-post-object-queries.php to set the role properly
Add unit test for Relay Mutation Schema
Add unit test for Post Object Mutations
Introduce a shared “postObjectMutation” class for the CRUD classes to utilize
Introduce PostObjectDelete class for handling deletes for postObjects
Introduce PostObjectCreate class for handling inserts for postObjects
Introduce PostObjectUpdate class for handling updates for postObjects
Update /tests/test-nodes.php to include a query with variables
Update /tests/test-plugin-queries.php to set the role properly
Update /tests/test-post-connection-queries.php to set the role properly
Update /tests/test-post-object-queries.php to set the role properly
Add unit test for Relay Mutation Schema
Add unit test for Post Object Mutations
Introduce a shared “postObjectMutation” class for the CRUD classes to utilize
Introduce PostObjectDelete class for handling deletes for postObjects
Introduce PostObjectCreate class for handling inserts for postObjects
Introduce PostObjectUpdate class for handling updates for postObjects
Update /tests/test-nodes.php to include a query with variables
Update /tests/test-plugin-queries.php to set the role properly
Update /tests/test-post-connection-queries.php to set the role properly
Update /tests/test-post-object-queries.php to set the role properly
Add unit test for Relay Mutation Schema
Add unit test for Post Object Mutations
@coveralls
Copy link

coveralls commented May 23, 2017

Coverage Status

Coverage decreased (-0.01%) to 79.35% when pulling 973b769 on jasonbahl:feature/#4-post-mutations into 5ab9cd1 on wp-graphql:master.

@coveralls
Copy link

coveralls commented May 23, 2017

Coverage Status

Coverage decreased (-0.01%) to 79.35% when pulling 3abcb39 on jasonbahl:feature/#4-post-mutations into 5ab9cd1 on wp-graphql:master.

@coveralls
Copy link

coveralls commented May 24, 2017

Coverage Status

Coverage increased (+0.05%) to 81.966% when pulling ef01be1 on jasonbahl:feature/#4-post-mutations into 60c8a0e on wp-graphql:master.

@@ -33,7 +33,7 @@ matrix:
# Composer package installation
install:
# Install composer packages, will also trigger dump-autoload
- composer install --no-interaction
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Travis tests were bombing because of this

*/
public function __construct() {

$this->config = apply_filters( 'graphql_app_context_config', $this->config );
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This will allow the AppContext to have data added to it and accessed throughout the resolve tree. . .not used by anything (yet) but will be useful for WPGraphQL Insights / Apollo Optics support

@@ -82,9 +82,7 @@ public function graphql_wp_query_cursor_pagination_support( $where, \WP_Query $q
} else {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just formatting changes on this file

@@ -32,7 +32,7 @@ public static function resolve( $source, $args, AppContext $context, ResolveInfo
$query_args = static::get_query_args( $source, $args, $context, $info );
$query = static::get_query( $query_args );
$array_slice = self::get_array_slice( $query, $args );
$connection = static::get_connection( $array_slice, $args, $query );
$connection = static::get_connection( $query, $array_slice, $source, $args, $context, $info );
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding more params so context can be passed throughout easier

*
*
*/
public static function get_query_order( $args, $query_args ) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed because this never gets called now

@@ -1,4 +1,5 @@
<?php
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just formatting in this file

@@ -4,7 +4,6 @@
use GraphQL\Type\Definition\ResolveInfo;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just formatting changes to this file

self::$fields = $fields;

}
self::$fields = self::prepare_fields( $fields, 'commentArgs' );
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

make use of the prepare_fields method which handles sorting and filtering the fields.


self::$comments_orderby_enum = new EnumType([
if ( null === self::$comments_orderby_enum ) :
self::$comments_orderby_enum = new WPEnumType([
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

convert to use WPEnumType to take advantage of filters, etc

Copy link
Member

Choose a reason for hiding this comment

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

Are we using just EnumType other places? Should probably get an issue going to convert all EnumType's to WPEnumType

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@CodeProKid I tried to sweep and convert them all to WPEnumType. . .but I can double check to make sure I got em all

@@ -35,7 +35,7 @@ class CommentConnectionDefinition {
*/
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just formatting adjustments in this file

@@ -45,8 +45,7 @@ public static function debug_fields( $connection_name ) {
self::$debug_fields = [];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just formatting in this file

*
* @return array
*/
public static function get_connection( array $items, array $args, $query ) {
public static function get_connection( $query, array $items, $source, array $args, AppContext $context, ResolveInfo $info ) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

update to play nice with \Data\ConnectionResolver class

wp-graphql.php Outdated
@@ -250,7 +250,11 @@ public static function get_allowed_post_types() {
/**
* Get all post_types that have been registered to "show_in_graphql"
*/
$post_types = get_post_types( [ 'show_in_graphql' => true ] );
$post_types = get_post_types(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

codesniffer whining

@@ -371,27 +379,35 @@ public static function do_graphql_request( $request, $operation_name = '', $vari
* @param AppContext object The AppContext object containing all of the
* information about the context we know at this point
*/
$variables = ( array ) json_decode( $variables );
if ( ! is_array( $variables ) ) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

some clients (graphiql) send variables as a string, and some clients (Apollo) send variables as JSON, so this normalizes the $variables so either method will properly execute.

*/
$executable_schema = apply_filters( 'graphql_executable_schema', $executable_schema, $request, $variables, $app_context );
$schema = new \WPGraphQL\WPSchema( $executable_schema );
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Instead of filtering here, I created a new WPSchema class, and apply filters in there, where so the filtered methods can access methods within the WPSchma class. . .this is pretty essential for WPGraphQL Insights / Apollo Optics support

…o feature/#4-post-mutations

# Conflicts:
#	src/Type/PostObject/PostObjectType.php
#	src/Type/TermObject/TermObjectType.php
#	wp-graphql.php
…ahl/wp-graphql into feature/#4-post-mutations

# Conflicts:
#	wp-graphql.php
@coveralls
Copy link

coveralls commented May 24, 2017

Coverage Status

Coverage decreased (-0.3%) to 85.048% when pulling cd8c754 on jasonbahl:feature/#4-post-mutations into f565f3e on wp-graphql:master.

@coveralls
Copy link

coveralls commented May 24, 2017

Coverage Status

Coverage decreased (-0.3%) to 85.048% when pulling cd8c754 on jasonbahl:feature/#4-post-mutations into f565f3e on wp-graphql:master.

@jasonbahl jasonbahl requested a review from CodeProKid May 25, 2017 03:08
@@ -35,7 +35,7 @@ class CommentConnectionDefinition {
*/
public static function connection() {

if ( null === self::$connection ) {
if ( null === self::$connection ) :
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to get a developers handbook going for rules on stuff like this. When to use : vs {. Or at least an issue so we can start documenting these things.

Copy link
Collaborator Author

@jasonbahl jasonbahl May 25, 2017

Choose a reason for hiding this comment

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

@CodeProKid ya, I go back and forth on my usage sometimes. . .I usually prefer brackets, but use colon for longer blocks as it's more explicit where it's ending. . .but ya, we can definitely decide on a standard on when to go to colon vs. brackets. . .like a certain size of code block being wrapped or something

Copy link
Member

@CodeProKid CodeProKid left a comment

Choose a reason for hiding this comment

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

@jasonbahl some stuff in here. Let me know if you have any Q's.

$config = [
'name' => 'status',
'description' => __( 'The status of the object.', 'wp-graphql' ),
'values' => self::values(),
Copy link
Member

Choose a reason for hiding this comment

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

Formatting needed on this array for consistency.

$config = [
'name' => 'mimeType',
'description' => __( 'The MimeType of the object', 'wp-graphql' ),
'values' => self::values(),
Copy link
Member

Choose a reason for hiding this comment

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

Formatting needed on this array for consistency.

$config = [
'name' => 'relation',
'description' => __( 'The logical relation between each item in the array when there are more than one.', 'wp-graphql' ),
'values' => self::values(),
Copy link
Member

Choose a reason for hiding this comment

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

Formatting needed on this array for consistency.

$config = [
'name' => 'taxonomyEnum',
'description' => __( 'Allowed taxonomies', 'wp-graphql' ),
'values' => self::values(),
Copy link
Member

Choose a reason for hiding this comment

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

Formatting needed on this array for consistency.

public static function mutate( \WP_Post_Type $post_type_object ) {

if ( null === self::$mutation ) {
self::$mutation = [];
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't you just set $mutation to an empty array when you define it above? What's the upside of having it be null, and then define it here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no upside. changed this.

public static function mutate( \WP_Post_Type $post_type_object ) {

if ( null === self::$mutation ) {
self::$mutation = [];
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated

if ( ! current_user_can( $post_type_object->cap->edit_posts ) ) {
// translators: the $post_type_object->graphql_single_name placeholder is the name of the object being mutated
throw new \Exception( sprintf( __( 'Sorry, you are not allowed to update a %1$s', 'wp-graphql' ), $post_type_object->graphql_single_name ) );
}
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it would make sense to abstract these cap checks into a postObjectMutateCheck utility class or something. Just a thought. Not required before merge.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Created an issue to work on this: #143

/**
* Insert the post and retrieve the ID
*/
$post_id = wp_update_post( $post_args );
Copy link
Member

Choose a reason for hiding this comment

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

As with wp_insert_post() you actually have to pass true as the second param. See docs: https://developer.wordpress.org/reference/functions/wp_update_post/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated

*/
return array_merge(
[
'id' => [
Copy link
Member

Choose a reason for hiding this comment

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

What is your reformator trying to align this with? haha.

] );
$this->subscriber = $this->factory->user->create( [
'role' => 'subscriber',
] );
Copy link
Member

Choose a reason for hiding this comment

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

Indentation here is pretty ugly.

…o feature/wp-graphql#116-ancestor-support

# Conflicts:
#	src/Type/PostObject/PostObjectType.php
#	src/Type/TermObject/TermObjectType.php
#	wp-graphql.php
@jasonbahl
Copy link
Collaborator Author

@CodeProKid Updated this PR and replied to comments. Thanks!

@coveralls
Copy link

coveralls commented May 26, 2017

Coverage Status

Coverage decreased (-0.3%) to 85.033% when pulling d968f6e on jasonbahl:feature/#4-post-mutations into f565f3e on wp-graphql:master.

@jasonbahl jasonbahl merged commit 6cb33f7 into wp-graphql:master May 26, 2017
@jasonbahl jasonbahl mentioned this pull request Jun 24, 2017
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.

None yet

3 participants