Skip to content

Conversation

swnsma
Copy link
Contributor

@swnsma swnsma commented Jun 24, 2019

Provide fallback to copy Extension Attributes for classes which extends Data Object.

Fixed Issues

  1. Copy Service does not works properly for Entities which extends Data Object and implements ExtensibleDataInterface #23386: Copy Service does not works properly for Entities which extends Data Object and implements ExtensibleDataInterface

Manual testing scenarios

See in issue.

…ds Data Object and implements ExtensibleDataInterface.

Provide fallback to copy Extension Attributes for classes which extends Data Object.
@m2-assistant
Copy link

m2-assistant bot commented Jun 24, 2019

Hi @swnsma. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@magento-engcom-team magento-engcom-team added Component: DataObject Release Line: 2.3 Partner: ISM eCompany Pull Request is created by partner ISM eCompany partners-contribution Pull Request is created by Magento Partner labels Jun 24, 2019
…for-Entities-which-extends-Data-Object-and-implements-ExtensibleDataInterface
@swnsma swnsma force-pushed the Copy-Service-does-not-works-properly-for-Entities-which-extends-Data-Object-and-implements-ExtensibleDataInterface branch from afd1aed to d70edfc Compare June 25, 2019 06:21
@aleron75 aleron75 self-assigned this Jun 25, 2019
aleron75
aleron75 previously approved these changes Jun 25, 2019
@magento-engcom-team
Copy link
Contributor

Hi @aleron75, thank you for the review.
ENGCOM-5352 has been created to process this Pull Request
✳️ @aleron75, 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

@aleron75
Copy link
Contributor

@swnsma do you think that the unit test related to the updated class should be updated to reflect the change in fallback logic or is it already ok?

@aleron75 aleron75 dismissed their stale review June 25, 2019 16:57

Not sure if changes require coverage by auto-tests

@swnsma
Copy link
Contributor Author

swnsma commented Jun 25, 2019

Hi @aleron75,

I think the best is to cover this behavior with Integration Tests.
But due to this is no easy way to cover it due to Extension Attribute usage I prefer to skip it.

But I can try if you can share an example of how test can looks like.

E.G.
Source: Quote
Field: Extension Attribute -> Shipping Assignment
Target: ?
Field: Extension Attribute -> ?

@aleron75
Copy link
Contributor

Hello @swnsma, I asked because I see there is already a unit test covering the class, it' \Magento\Framework\DataObject\Test\Unit\CopyTest so it would enough to take a look at that unit test and consider if it already covers the scenarios you extended or if it needs to be extended. That could be enough I think. Thanks!

…ds Data Object and implements ExtensibleDataInterface.

Add unit tests coverage.
@swnsma
Copy link
Contributor Author

swnsma commented Jun 28, 2019

Hi @aleron75,
I have updated unit tests!

…ds Data Object and implements ExtensibleDataInterface.

Reformat code.
@aleron75 aleron75 added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Jul 3, 2019
@magento-engcom-team
Copy link
Contributor

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

@swnsma swnsma changed the title magento/magento2#23386: Copy Service does not works properly for Entities which extends Data Object and implements ExtensibleDataInterface. magento/magento2#23386: Copy Service does not work properly for Entities which extends Data Object and implements ExtensibleDataInterface. Jul 3, 2019
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this pull request Jul 14, 2019
…Entities which extends Data Object and implements ExtensibleDataInterface. magento#23387
@magento-engcom-team magento-engcom-team merged commit 731a4fc into magento:2.3-develop Jul 14, 2019
@m2-assistant
Copy link

m2-assistant bot commented Jul 14, 2019

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

@magento-engcom-team magento-engcom-team added this to the Release: 2.3.3 milestone Jul 14, 2019
@swnsma swnsma deleted the Copy-Service-does-not-works-properly-for-Entities-which-extends-Data-Object-and-implements-ExtensibleDataInterface branch July 16, 2019 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Award: special achievement Component: DataObject Partner: ISM eCompany Pull Request is created by partner ISM eCompany partners-contribution Pull Request is created by Magento Partner Progress: accept Release Line: 2.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants