MultiSafepay / magento2-core

Provides core functionalities
Open Software License 3.0
2 stars 10 forks source link

Core module has dependency by Magento GiftCardAccount. #25

Closed Citizen4our closed 1 year ago

Citizen4our commented 1 year ago

https://github.com/MultiSafepay/magento2-core/blob/8da67b874c024bb65c9484c5b8fc50a9593562e9/Model/Api/Builder/OrderRequestBuilder/ShoppingCartBuilder/CustomTotalBuilder.php#L161

Firstly, composer.json hasn't dependency for Magento GiftCardAccount, and I think this is not necessary. Needed another way for this functionality (new module etc.) In additional, for total with code giftcardaccount may have other custom module.

vinodsowdagar commented 1 year ago

Hi @Citizen4our ,

Thank you for the report, we'll look into it a bit later and come back to you when we have investigated the reported issue.

vinodsowdagar commented 1 year ago

Hi @Citizen4our ,

We decided to remove the import of the Giftcardaccount class for now and created a backlog item to create separate modules for the different custom total implementations. When we have released the new version with the change in it, we will keep you updated.

vinodsowdagar commented 1 year ago

Hi @Citizen4our ,

The change has been added and released in the following version: https://github.com/MultiSafepay/magento2/releases/tag/3.1.3

I'm hereby closing this issue. If there are any more related questions, please feel free to leave a comment.

Citizen4our commented 1 year ago

Hello @Vinod-MultiSafepay, I still see a reference to the gift card module - \Magento\GiftCardAccount\Model\Giftcardaccount::CODE. It will be caused the same error when the gift card module is not installed, right? image

vinodsowdagar commented 1 year ago

Hi @Citizen4our ,

It will only execute that part of the code whenever there is a total found with the name 'giftcardaccount'. The file import has been removed, which means that the class doesn't have a dependency on the Gift Card module anymore.

I understand this is not an ideal fix, since technically it will still fail whenever you have a total with the name 'giftcardaccount' that has not been added by the Gift Card module. But we've weighed the options and the chance of that happening seems pretty low. To create a perfect solution for that, would mean that we want to create separate support modules for each total, which is a task that is currently on the backlog and does not have a release date yet. So for now, this will have to do.

Citizen4our commented 1 year ago

@Vinod-MultiSafepay Yeah, it's my case - I'm using giftcardaccount in my own module as name=) Let me know when you release separate support modules and thanks in advance!

vinodsowdagar commented 1 year ago

@Citizen4our ,

Could you try to update to the latest version of the plugin and then replace line 135 to 138 with the following and see if that works for you?

        if (class_exists(\Magento\GiftCardAccount\Model\Giftcardaccount::class)) {
            if ($total->getCode() === 'giftcardaccount' && ($giftCards = $total->getGiftCards())) {
                $title = $this->getGiftCardAccountTitle($total, $giftCards);
            }
        }