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

Added createCustomerGraphQl plugin #763

Merged

Conversation

sedonik
Copy link
Contributor

@sedonik sedonik commented Jul 5, 2019

Description

Added Magento\CustomerGraphQl\Model\Plugin\Customer\CreateCustomerAccount
Added createCustomerGraphQl plugin to di.xml

Fixed Issues

  1. Subscribe for newsletter even when not allowed #762: Subscribe for newsletter even when not allowed

Manual testing scenarios

  1. Navigate to Stores > Settings > Configuration > Customers > Newsletter
  2. In the General Options set Enabled to No
  3. Save config and Flush Cache
  4. Perform query:
mutation {
  createCustomer(
    input: {
      firstname: "John"
      lastname: "Lennon"
      email: "[email protected]"
      is_subscribed: true
    }
  ) {
    customer {
      email
      is_subscribed
    }
  }
}

Response

{
  "data": {
    "createCustomer": {
      "customer": {
        "email": "[email protected]",
        "is_subscribed": false
      }
    }
  }
}

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 are green)

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Jul 5, 2019

CLA assistant check
All committers have signed the CLA.

@sedonik
Copy link
Contributor Author

sedonik commented Jul 16, 2019

Hi @naydav,
I've updated PR's code.
Pls, check.

Copy link
Contributor

@lenaorobei lenaorobei left a comment

Choose a reason for hiding this comment

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

Please replace the core usage Magento\Newsletter\Observer\PredispatchNewsletterObserver with your implementation and remove unnecessary dependency.

*/
public function __construct(
\Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig,
\Magento\Config\Model\ResourceModel\Config $resourceConfig
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a redundant dependency.

@lenaorobei
Copy link
Contributor

@sedonik, how is your progress with this ticket, are you interested to finish it?

Copy link
Contributor

@lenaorobei lenaorobei left a comment

Choose a reason for hiding this comment

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

Please remove redundant fixtures and use @magentoConfigFixture instead.

@lenaorobei
Copy link
Contributor

lenaorobei commented Aug 30, 2019

Once it is fixed, please squash all commits into one. Thanks.

@sedonik
Copy link
Contributor Author

sedonik commented Sep 2, 2019

Hi @lenaorobei,
I've set @magentoConfigFixture but after that WebAPI Tests build failed. Could you advice, please?

@lenaorobei
Copy link
Contributor

@sedonik need to check ScopeInterface::SCOPE_STORE in the isActive method.

@magento-engcom-team
Copy link
Contributor

Hi @lenaorobei, thank you for the review.
ENGCOM-5817 has been created to process this Pull Request

@lenaorobei lenaorobei modified the milestone: Release: 2.3.4 Sep 9, 2019
@magento-engcom-team magento-engcom-team merged commit 0768d89 into magento:2.3-develop Sep 10, 2019
@ghost
Copy link

ghost commented Sep 10, 2019

Hi @sedonik, 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