Skip to content

Conversation

@arendarenko
Copy link
Contributor

Original Pull Request

#18488

Description

I found that every time when customer data is saving, Newsletter module triggers "afterSave" (https://github.com/magento/magento2/blob/2.3-develop/app/code/Magento/Newsletter/Model/Plugin/CustomerPlugin.php#L67) plugin for CustomerRepository, which updates customer subscription status. According to this plugin code, it has two customers in arguments: customer that was actually passed to save (it called $customer) and customer which was modified after during save method (it called $result). Those two variables are comparing their "extensionAttributes" which are setting in the same plugin by method "afterGetById" (https://github.com/magento/magento2/blob/2.3-develop/app/code/Magento/Newsletter/Model/Plugin/CustomerPlugin.php#L151). And if they are exact - module didn't changes customer subscription, in other case - it did.

The problem is that "save" method triggers afterGetById only for $customer, but not for $result (which is actually users "get" method: https://github.com/magento/magento2/blob/2.3-develop/app/code/Magento/Customer/Model/ResourceModel/CustomerRepository.php#L253). In this case, extensionAttributes are always different for $customer and $result, and this is why every time when "afterSave" plugin triggers, module tries to make subscribe/unsubscribe user depends on it current subscription status.

Manual testing scenarios

  1. Create new customer
  2. Go to backend -> Customers -> Edit -> and open previously created customer for edit
  3. Create breakpoint via XDebug on https://github.com/magento/magento2/blob/2.3-develop/app/code/Magento/Newsletter/Model/Plugin/CustomerPlugin.php#L83
  4. Edit customer personal data (only) and click Save
  5. Compare property data of variables $newExtensionAttributes and $initialExtensionAttributes.

Expected result:
Both variables would have the same extension attributes.

Actual result:
$newExtensionAttributes have extension attributes, but $initialExtensionAttributes is not (empty array).

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)

hostep and others added 30 commits September 23, 2018 15:50
…ng fotorama library to not change to a different image when pressing left or right arrow key in combination with certain modifier keys on your keyboard.
- add to CPD blacklist
…rocessUrlRewriteMovingObserver by Unit Test #18211

 - Merge Pull Request #18211 from eduard13/magento2:2.3-develop-PR-port-18004
 - Merged commits:
   1. 47f791a
   2. 94ed408
   3. 36d1a95
   4. a733933
   5. fd2cb4c
 - Merge Pull Request #18122 from hryvinskyi/magento2:table-schema-comments
 - Merged commits:
   1. d4d23fd
   2. e20dfc3
…poser removal w/out regression #18205

 - Merge Pull Request #18205 from mage2pratik/magento2:2.3-develop-PR-port-18002
 - Merged commits:
   1. 8b9fff5
   2. fa4289f
…sells, crosss… #18207

 - Merge Pull Request #18207 from hostep/magento2:fix-for-issue-13720-for-magento-2.3
 - Merged commits:
   1. 74e1ea9
   2. d5b6231
…ing Integration Tests #18201

 - Merge Pull Request #18201 from mage2pratik/magento2:2.3-develop-PR-port-16361
 - Merged commits:
   1. 0d0b5c0
   2. 81076e1
   3. 421ba0b
   4. c0cb405
   5. 99b1ca7
Fix CE integration and static tests on Magento + MSI
…to-engcom/magento2ce into ENGCOM-2948-magento-graphql-ce-132
[honey] MAGETWO-91034: Renaming of new setup commands and setup:install/setup:upgrade attributes
Accepted Public Pull Requests:
 - #18142: Replace intval() function by using direct type casting to (int) where no default value is needed (by @mage2pratik)
 - #18136: [2.3] Update labels section in README.md (by @sidolov)
 - #18117: Change sort order for customer group options (by @dmytro-ch)
 - #18102: Replace sort callbacks to spaceship operator (by @mage2pratik)
 - magento-commerce/async-import#15: add support HTTP DELETE method for async bulk request by adding mergi� (by @anvasiliev)


Fixed GitHub Issues:
 - #18101: Wrong sort order for customer groups in customer grid filter (reported by @sreichel) has been fixed in #18117 by @dmytro-ch in 2.3-develop branch
   Related commits:
     1. 568fa46
     2. ed99490
Valeriy Nayda and others added 21 commits October 3, 2018 12:26
…ee-root-regexp

# Conflicts:
#	app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/CategoryTree.php
 - Merge Pull Request magento/graphql-ce#121 from mhhansen/graphql-ce:fix-categorytree-root-regexp
 - Merged commits:
   1. 7b1c4dd
   2. 10afe1d
   3. 0ba4b71
   4. 93c3ded
   5. 4195613
   6. 4a355a0
   7. 590b84d
   8. cb7e807
   9. 481c7bf
   10. 3b04c94
 - Merge Pull Request magento/graphql-ce#182 from magento/graphql-ce:graphql-128-extends-store-config-coverage
 - Merged commits:
   1. 8f3dd40
 - Merge Pull Request magento/graphql-ce#107 from magento/graphql-ce:product-url-rewrites
 - Merged commits:
   1. d6d0872
   2. b583ee7
   3. 57c2e4c
   4. 46a871e
   5. dfabfe5
   6. e0f4132
   7. e9a06a6
   8. b062bd9
   9. 1700740
   10. f141a0d
   11. e2cad89
   12. 16b72a8
   13. df3360b
   14. 987b138
   15. 0fe1ae5
   16. 7b8799d
   17. 6d8e43d
Accepted Public Pull Requests:
 - #18290: Fix wrong reference in google analytics module layout xml (by @sambolek)
 - #18149: type casted $qty to float in \Magento\Catalog\Model\Product::setQty() (by @jayankaghosh)
 - #18273: [Forwardport] Correctly convert config integration api resources (by @mage2pratik)
 - #18173: Use version_compare to compare version strings correctly (by @schmengler)
 - #18272: [Forwardport] 9830 - Null order in Magento\Sales\Block\Order\PrintShipment.php (by @mage2pratik)
 - #18244: Don't set a source model on the attribute when it's not needed. This � (by @hostep)
 - #18231: [Forwardport] Update shipment collection to unserialize `packages` attribute after load (by @mage2pratik)
 - #18228: [Forwardport] Fix sitemap grid render incorrect base urls for multiple stores (by @nntoan)
 - #18212: Allow keyboard navigation in browser on product detail pages, by fixi� (by @hostep)
 - #18201: [Forwardport] Allow usage of config-global.php when running Integration Tests (by @mage2pratik)
 - #18334: [Forwardport] Integration test for swatches types in attribute configuration added (by @jignesh-baldha)
 - #18288: Fix issue 17152 - prevent email being marked as not sent if email cop� (by @sambolek)
 - #18331: Fix throwing error by checkout error processor model (by @ihor-sviziev)
 - #18322: Fix for removing the dirs while creating a TAR archive (by @haroldclaus)
 - #18308: Fix the issue with customer inline edit when password is expired  (by @dmytro-ch)
 - #18312: [Forwardport] fix: reset search mini-form when we have no data / an empty response (by @dmytro-ch)
 - #18292: Do not use new Phrase in Link Current class (by @VincentMarmiesse)
 - #18209: Introducing a dedicated cron.log file for logging cron related info, � (by @hostep)
 - #18303: [Forwardport] Cast products getStoreId() to int (by @sreichel)
 - #18295: [Forwardport] fix #17582 ./bin/magento config:show fails with a fatal error (by @mage2pratik)
 - #18064: Fixing Snake Case To Camel Case (by @hryvinskyi)
 - #18344: Added unit test for CRON converter plugin (by @rogyar)


Fixed GitHub Issues:
 - #16497: Magento 2.2.5: Google Analytics not added to head correctly (reported by @WalterSmulders) has been fixed in #18290 by @sambolek in 2.3-develop branch
   Related commits:
     1. f6fcf41

 - #18094: Should getQty() return int/float or string? (reported by @sreichel) has been fixed in #18149 by @jayankaghosh in 2.3-develop branch
   Related commits:
     1. 76676ce
     2. 2ad1031

 - #12095: Update 2.2.1: One or more integrations have been reset because of a change to their xml configs. (reported by @thomvanderboon) has been fixed in #18273 by @mage2pratik in 2.3-develop branch
   Related commits:
     1. 6c33d20
     2. 0b3fe71
     3. 31b7f07
     4. 999d8e5
     5. 401b423

 - #9830: Null order in Magento\Sales\Block\Order\PrintShipment.php (reported by @dnadle) has been fixed in #18272 by @mage2pratik in 2.3-develop branch
   Related commits:
     1. 37ff9e9

 - #10530: Print order error on magento 2.1.8 (reported by @ionutdicu) has been fixed in #18272 by @mage2pratik in 2.3-develop branch
   Related commits:
     1. 37ff9e9

 - #13156: Updating attribute option data through API will set unwanted source_model on the attribute (reported by @koenner01) has been fixed in #18244 by @hostep in 2.3-develop branch
   Related commits:
     1. 477ca51

 - #17999: Sitemap grid display incorrect base URL in the grid if using multiple stores (reported by @nntoan) has been fixed in #18228 by @nntoan in 2.3-develop branch
   Related commits:
     1. 5144320

 - #15196: 2.2.4 : Magento 2 integration tests enables all modules (reported by @kanduvisla) has been fixed in #18201 by @mage2pratik in 2.3-develop branch
   Related commits:
     1. 0d0b5c0
     2. 81076e1
     3. 421ba0b
     4. c0cb405
     5. 99b1ca7

 - #17152: Failure of "Send Order Email Copy" spams customers, every minute, forever. (reported by @lingwooc) has been fixed in #18288 by @sambolek in 2.3-develop branch
   Related commits:
     1. 5548c9d
     2. 8c8b0a8
     3. 6fe9303

 - #18330: Checkout - Infinite loading indicator when server returned error (reported by @ihor-sviziev) has been fixed in #18331 by @ihor-sviziev in 2.3-develop branch
   Related commits:
     1. eea11c7

 - #18162: Cannot edit customer using inline edit if password is expired (reported by @JeroenVanLeusden) has been fixed in #18308 by @dmytro-ch in 2.3-develop branch
   Related commits:
     1. 2eeb65c
     2. 34830c6

 - #17190: system.log rapidly increasing after Magento CE 2.2.5 update (cron logs) (reported by @mad-develop) has been fixed in #18209 by @hostep in 2.3-develop branch
   Related commits:
     1. a4f58c8

 - #18079: Inconsistent return type for getStoreId() (reported by @sreichel) has been fixed in #18303 by @sreichel in 2.3-develop branch
   Related commits:
     1. 5411c0d

 - #17582: ./bin/magento config:show fails with a fatal error (reported by @simonworkhouse) has been fixed in #18295 by @mage2pratik in 2.3-develop branch
   Related commits:
     1. 20af5a9
…e-merge

# Conflicts:
#	app/code/Magento/Sales/view/frontend/layout/sales_order_print.xml
[EngCom] Public Pull Requests - GraphQL
Update module logic when customer have store_id = 0
Fix unit tests according to new logic
Add integration tests for case when customer have store_id = 0
@magento-cicd2
Copy link
Contributor

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
4 out of 15 committers have signed the CLA.

✅ hostep
✅ magento-engcom-team
✅ sidolov
✅ slavvka
❌ okolesnyk
❌ Mariana
❌ svitja
❌ StasKozar
❌ zakdma
❌ mastiuhin-olexandr
❌ cpartica
❌ magento-cicd2
❌ dvoskoboinikov
❌ naydav
❌ rganin


Mariana seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

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.