Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add isEqual() for product-specific price rules #1844

Conversation

BlackbitDevs
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no

This PR implements the isEqual() method for product-specific price rules.

@dpfaffenbauer
Copy link
Member

@BlackbitNeueMedien tests are failing, can you check?

@dpfaffenbauer
Copy link
Member

@BlackbitNeueMedien feel free to reopen when the tests are fixed

@BlackbitDevs
Copy link
Contributor Author

Have you ever looked at the tests? Is this failing in https://github.com/coreshop/CoreShop/runs/4869837375?check_suite_focus=true really coming from my change?

Install Resources for Environment behat.
0/20 [░ ] 0% %message%
4/20 [░░░░░░ ] 20% Install Class CoreShopCart (/home/runner/work/CoreShop/CoreShop/src/CoreShop/Bundle/CoreBundle/Resources/install/pimcore/classes/CoreShopOrderBundle/CoreShopCart.json)
6/20 [░░░░░░░░░ ] 30% Install Class CoreShopOrder (/home/runner/work/CoreShop/CoreShop/src/CoreShop/Bundle/CoreBundle/Resources/install/pimcore/classes/CoreShopOrderBundle/CoreShopOrder.json)
8/20 [░░░░░░░░░░░░ ] 40% Install Class CoreShopOrderShipment (/home/runner/work/CoreShop/CoreShop/src/CoreShop/Bundle/CoreBundle/Resources/install/pimcore/classes/CoreShopOrderBundle/CoreShopOrderShipment.json)
10/20 [░░░░░░░░░░░░░░░ ] 50% Install Class CoreShopQuote (/home/runner/work/CoreShop/CoreShop/src/CoreShop/Bundle/CoreBundle/Resources/install/pimcore/classes/CoreShopOrderBundle/CoreShopQuote.json)
11/20 [░░░░░░░░░░░░░░░░ ] 55% Install Class CoreShopQuoteItem (/home/runner/work/CoreShop/CoreShop/src/CoreShop/Bundle/CoreBundle/Resources/install/pimcore/classes/CoreShopOrderBundle/CoreShopQuoteItem.json)
12/20 [░░░░░░░░░░░░░░░░░ ] 60% Install Class CoreShopOrderInvoice (/home/runner/work/CoreShop/CoreShop/src/CoreShop/Bundle/OrderBundle/Resources/install/pimcore/classes/CoreShopOrderInvoice.json)
14/20 [░░░░░░░░░░░░░░░░░░░░ ] 70% Install Class CoreShopCustomerGroup (/home/runner/work/CoreShop/CoreShop/src/CoreShop/Bundle/CustomerBundle/Resources/install/pimcore/classes/CoreShopCustomerGroup.json)
16/20 [░░░░░░░░░░░░░░░░░░░░░░░ ] 80% Install Class CoreShopCompany (/home/runner/work/CoreShop/CoreShop/src/CoreShop/Bundle/CoreBundle/Resources/install/pimcore/classes/CoreShopCustomerBundle/CoreShopCompany.json)
Error: Process completed with exit code 255.

@dpfaffenbauer dpfaffenbauer reopened this Feb 17, 2022
@dpfaffenbauer
Copy link
Member

true, can you rebase to latest changes?

@dpfaffenbauer
Copy link
Member

@BlackbitNeueMedien I think it does fail cause of your changes... it fails trying to install the classes...

@BlackbitDevs
Copy link
Contributor Author

Implemented methods from TypeDeclarationSupportInterface which gets extended by Pimcore\Model\DataObject\ClassDefinition\Data - however this may have worked before because this PR has not changed anything about this.

@BlackbitDevs
Copy link
Contributor Author

Hmm, still aborting with error code 255. Have no idea... Do you, @dpfaffenbauer ?

@dpfaffenbauer
Copy link
Member

@BlackbitNeueMedien EqualComparisonInterface was added in a later version of Pimcore 6. CoreShop is tested all the way from Pimcore 6.6 upwards. I just added a BC Layer

@dpfaffenbauer
Copy link
Member

@BlackbitNeueMedien not it works

@dpfaffenbauer dpfaffenbauer merged commit af9e371 into coreshop:2.2 Feb 21, 2022
@dpfaffenbauer
Copy link
Member

alright, thanks then :) Maybe you want to add that into the 3.x branch as well? :)

@BlackbitDevs
Copy link
Contributor Author

Oh, I thought all the PRs which get merged to 2.2 will eventually get merged to 3.x?

@dpfaffenbauer
Copy link
Member

Merging things upwards to 3.x/master is not possible easily. To much has changed between the two versions already.

@dpfaffenbauer
Copy link
Member

@BlackbitNeueMedien managed to merge it

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.

2 participants