Skip to content

Conversation

@arendarenko
Copy link
Contributor

@arendarenko arendarenko commented Oct 9, 2018

Description

When working with newsletter module I found when customers update their subscription, Magento did it two and even three times (I mean by counting calling the same method in Subscribe model). My PR fixes that, so it may improve performance and stability.

Manual testing scenarios

1. When customers manages his subscription status from private office

1.1. Login as customer
1.2. Go to private office and click Edit on Newsletters block
1.3. Update customer subscription (if customer was subscribed then unsubscribe, otherwise subscribe)

Expected result:
Subscription status will be updated 1 time

Actual result:
Subscriber method "_updateCustomerSubscription" was called 3 times:

  • 2 times from customer plugin (because in newsletter manage controller there is a customer saving method) - without any changes
  • 1 times actually from newsletter manage controller - with changes, the way it should be

2. When admin manages subscriber status from backend

2.1. Go to Customers and edit customer
2.2. Update customer subscription (if customer was subscribed then unsubscribe, otherwise subscribe)

Expected result:
Subscription status will be updated 1 time

Actual result:
Subscriber method "_updateCustomerSubscription" was called 2 times:

  • 2 times from customer plugin - during first call it's just updating current status, without any changes, and the second call is with changes, the way it should be

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)

…quests

- Remove updateSubscription calling from CustomerPlugin, because if there is some changes (user was subscribed or unsubscribed, there are will be calls subscribeCustomerById or unsubscribeCustomerById which are located in the same plugin method)
- Add additional condition in newsletter manage controller to prevent re-saving customer if any customer data was changed
@arendarenko arendarenko changed the title Prevent customer subscription multiple requests Prevent newsletter subscription multiple callings Oct 9, 2018
@arendarenko arendarenko changed the title Prevent newsletter subscription multiple callings Fix newsletter subscription multiple callings Oct 9, 2018
@ihor-sviziev ihor-sviziev self-assigned this Oct 9, 2018
@arendarenko
Copy link
Contributor Author

As we discussed with @ihor-sviziev this solution is not proper. Better solution is #18488.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants