-
-
Notifications
You must be signed in to change notification settings - Fork 123
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
implements metadata for customer register and update #402
implements metadata for customer register and update #402
Conversation
Looks like the linting issues that this PR is failing on aren't related to the changes in this PR, is that correct? I really need this feature, any chance of merging it? I'm happy to help in any way I can to speed this up? |
@PaperPlaneSoftware feel free to check whatever you can add. I'm still waiting owners to review my PR. Hope they' will find some time, this library is so much great ! |
Hi! I also need this feature! I'm using custom fields (saved as meta data in Worpdress) in the billing and shipping form and it's the only way to update this information! @believelody Can we help you to speed this up? Thanks for your work by the way 馃殌 |
@aresrioja10 sure, feel free to check what you can do. It seems the big problem is lint code. Still waiting for @kidunot89 to check if something is wrong. |
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.
@believelody @aresrioja10 I'm sorry for the delayed response been fixing a few fires with CI here and there. To answer your question the hold up is testing.
I can't add new code without ensuring its covered by the tests, too much risk involved with sites in production. This typically means for every new PR I have to usually pull down the PR branch, add a new test if the incoming changes warrant it and this change definitely warrants a test.
I have two other PRs to do before this one, but if one of you adds a working test for the changes to this test file. I can expedite this PR immediately.
7bcc5af
to
6adfc19
Compare
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.
@believelody @aresrioja10 With the test I added, this PR is ready for merging as soon as CI passes.
$query = ' | ||
mutation( $input: UpdateCustomerInput! ) { | ||
updateCustomer( input: $input ) { | ||
customer { | ||
databaseId | ||
metaData { | ||
key | ||
value | ||
} | ||
} | ||
} | ||
} | ||
'; | ||
$variables = array( | ||
'input' => array( | ||
'clientMutationId' => 'some_id', | ||
'id' => \GraphQLRelay\Relay::toGlobalId( 'customer', $user->ID ), | ||
'metaData' => array( | ||
array( | ||
'key' => 'test_meta_key', | ||
'value' => 'new_meta_value', | ||
), | ||
), | ||
) | ||
); | ||
|
||
$actual = graphql( compact( 'query', 'variables' ) ); | ||
codecept_debug( $actual ); | ||
|
||
$expected = array( | ||
'data' => array( | ||
'updateCustomer' => array( | ||
'customer' => array( | ||
'databaseId' => $user->ID, | ||
'email' => 'user@woographql.test', | ||
'metaData' => array( | ||
array( | ||
'key' => 'test_meta_key', | ||
'value' => 'new_meta_value', | ||
) | ||
) | ||
), | ||
), | ||
), | ||
); | ||
|
||
$this->assertEquals( $expected, $actual ); | ||
|
||
/** | ||
* Assertion Three | ||
* | ||
* Test "metaData" input field with "updateCustomer" mutation on the session user. | ||
*/ | ||
$query = ' | ||
mutation( $input: UpdateCustomerInput! ) { | ||
updateCustomer( input: $input ) { | ||
customer { | ||
id | ||
metaData { | ||
key | ||
value | ||
} | ||
} | ||
} | ||
} | ||
'; | ||
$variables = array( | ||
'input' => array( | ||
'clientMutationId' => 'some_id', | ||
'metaData' => array( | ||
array( | ||
'key' => 'test_meta_key', | ||
'value' => 'test_meta_value', | ||
), | ||
), | ||
) | ||
); | ||
|
||
$actual = graphql( compact( 'query', 'variables' ) ); | ||
codecept_debug( $actual ); | ||
|
||
$expected = array( | ||
'data' => array( | ||
'updateCustomer' => array( | ||
'customer' => array( | ||
'id' => 'guest', | ||
'metaData' => array( | ||
array( | ||
'key' => 'test_meta_key', | ||
'value' => 'test_meta_value', | ||
) | ||
) | ||
), | ||
), | ||
), | ||
); | ||
|
||
$this->assertEquals( $expected, $actual ); | ||
} |
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.
@believelody @aresrioja10 Test added. Please use this as a reference in future when making PRs introducing logic that needs to be tested.
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.
Thank you so much @kidunot89.Don't worry for your delayed reply, I will use your test code and learn better about that.
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.
@kidunot89 Thank you so much! I'm gonna test it right now :)
Your checklist for this pull request
Thanks for sending a pull request! Please make sure you click the link above to view the contribution guidelines, then fill out the blanks below.
馃毃Please review the guidelines for contributing to this repository.
What does this implement/fix? Explain your changes.
Allow Customer to include metadata in registerCustomer mutation and updateCustomer mutation
Does this close any currently open issues?
No
Any relevant logs, error output, GraphiQL screenshots, etc?
Metadata in registerCustomer mutation
![image](https://user-images.githubusercontent.com/16539960/101964585-2d2e0280-3c12-11eb-8926-0e6ace1ee45a.png)
Metadata in customers query
![image](https://user-images.githubusercontent.com/16539960/101964674-6ebead80-3c12-11eb-9647-113998c15574.png)
Metadata in updateCustomer mutation
![image](https://user-images.githubusercontent.com/16539960/101964849-02907980-3c13-11eb-8a8d-ebf5f5972c5d.png)
Any other comments?
Tell me if this enough and meet your requirements. Inspired by woocommerce-rest-api customer register feature
Where has this been tested?
Operating System: Ubuntu 18.04.5
WordPress Version: 5.5.3