Skip to content
This repository has been archived by the owner on Dec 19, 2019. It is now read-only.

Implement updates all properties in updateCustomer mutation #397

Merged
merged 2 commits into from Mar 8, 2019

Conversation

pmclain
Copy link
Contributor

@pmclain pmclain commented Feb 24, 2019

Description (*)

The current schema defines properties that were ignored when processing user
input.

Fixed Issues (if relevant)

  1. updateCustomer mutation doesn't update undefined fields #388: updateCustomer mutation doesn't update undefined fields

Manual testing scenarios (*)

  1. Create customer
mutation {
    createCustomer(
        input: {
            firstname: "Melanie"
            lastname: "Shaw"
            email: "mshaw@example.com"
            password: "Password1"
        }
    ) {
        customer {
            id
            firstname
            lastname
            email
        }
    }
}
  1. Generate customer token required for update mutation
  2. Update customer information
mutation {
    updateCustomer(
        input: {
            prefix: "Dr"
            middlename: "Jessica"
            suffix: "Esq"
            dob: "3/11/1972"
            taxvat: "sometaxvat"
        }
    ) {
        customer {
            prefix
            firstname
            middlename
            lastname
            suffix
            dob
            taxvat
        }
    }
}
  1. Validate all properties updated as requested

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@@ -19,6 +19,8 @@
*/
class UpdateCustomerData
{
private const RESTRICTED_DATA_KEYS = ['email', 'password'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this data to constructor parameter (values should be set via di)
It more extensible way

BTW maybe we can reuse some data provider for these fields? Do we have the corresponding model in Magento?
Thanks


if (isset($data['lastname'])) {
$customer->setLastname($data['lastname']);
foreach ($newCustomerData as $key => $value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Check can we reuse dataObjectHelper?

The current schema defines properties that were ignored when processing user
input.

Fixes magento#388
* update how customer object is updated
* add di to restrict updating defined properties
@ghost
Copy link

ghost commented Mar 8, 2019

Hi @pmclain, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants