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

#271 [My Account] Add support of Customer attributes #489

Merged

Conversation

furseyev
Copy link
Contributor

Description (*)

Frontend customer attributes validation added for graphQL update and create customer.
#271 [My Account] Add support of Customer attributes

- Customer attributes validation added
- Updating API functional test: adding
- Customer attributes validation added for create customer action
- Updating API functional test: adding empty first name scenario for customer account creation API
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Mar 16, 2019

CLA assistant check
All committers have signed the CLA.

@TomashKhamlai
Copy link
Contributor

Thank you for creating this PR. There could be some delays in processing this because we focus our effort on Cart operations. Please be patient.

@furseyev
Copy link
Contributor Author

@TomashKhamlai ,

Thanks for the update!

…ation

Conflicts:
	dev/tests/api-functional/testsuite/Magento/GraphQl/Customer/CreateCustomerTest.php
@naydav
Copy link
Contributor

naydav commented Apr 24, 2019

Hello @furseyev,
Sorry for the delay with the PR processing. My account functionality will be one of the main focuses for the next release.
This ticket already included in backlog of 2.3.3 release
Thanks

@naydav naydav added on hold and removed on hold labels Apr 24, 2019
@furseyev
Copy link
Contributor Author

@naydav ,

Thanks for the update!

*/
public function execute(): array
{
$attributes = $this->eavConfig->getEntityAttributes(
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we load just some attributes fields (via collection) instead of loading the whole attribute entity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@TomashKhamlai TomashKhamlai added Progress: ready for review Progress: ready for qa Add this in any case when you need some feedback, even if automated tests are failing and removed Progress: needs update labels May 13, 2019
@TomashKhamlai TomashKhamlai self-assigned this May 13, 2019
@furseyev
Copy link
Contributor Author

@naydav , @TomashKhamlai ,

Sorry for the delay. PR is updated.

Thanks!

@furseyev
Copy link
Contributor Author

Hello @furseyev,
Could you check failed static tests?

Thanks

@naydav

Fixed except this one:

PHP Code Mess has found error(s):
/var/www/html/app/code/Magento/CustomerGraphQl/Model/Customer/UpdateCustomerAccount.php:22 The class UpdateCustomerAccount has a coupling between objects value of 16. Consider to reduce the number of dependencies under 13.

I though we should ignore coupling limitation for now. Please let me know if this changed and I will check how this can be fixed.

Thanks!

@lenaorobei
Copy link
Contributor

@magento run all tests

…ation

Conflicts:
	app/code/Magento/CustomerGraphQl/Model/Customer/CreateCustomerAccount.php
	app/code/Magento/CustomerGraphQl/Model/Customer/UpdateCustomerAccount.php
@furseyev
Copy link
Contributor Author

@naydav ,
All checks are green and merge conflict is resolved.
Thanks!

@magento-engcom-team
Copy link
Contributor

Hi @lenaorobei, thank you for the review.
ENGCOM-5606 has been created to process this Pull Request
✳️ @lenaorobei, could you please add one of the following labels to the Pull Request?

Label Description
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests

@ghost
Copy link

ghost commented Aug 16, 2019

Hi @furseyev, 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.

6 participants