craftcms / commerce-sagepay

SagePay payment gateway for Craft Commerce.
https://plugins.craftcms.com/commerce-sagepay
MIT License
4 stars 6 forks source link

Legacy Basket Format SKUs #8

Open matt-adigital opened 5 years ago

matt-adigital commented 5 years ago

Hi all,

We've used the legacy basket format for a client on Sage 50 Cloud Accounts Professional but we aren't seeing the skus coming through. This is forcing them to put through a separate (fake) manual order in their software to update their stock figures accordingly.

I've tried the event code noted in the docs but this hasn't resolved the issue (or had any effect). Can you please advise if the example code in the docs needs any changes apart from the typo and missing use statement I found? For reference here is the updated code I ended up using which allows payments without triggering any errors:

use craft\commerce\omnipay\base\Gateway as BaseGateway;
use craft\commerce\omnipay\events\ItemBagEvent;

Event::on(
    BaseGateway::class,
    BaseGateway::EVENT_AFTER_CREATE_ITEM_BAG,
    function(ItemBagEvent $itemBagEvent) {
        $orderLineItems = $itemBagEvent->order->getLineItems();
        /**
         * @var $item Item
         */
        foreach ($itemBagEvent->items as $key => $item) {
            if (!isset($orderLineItems[$key])) {
                return;
            }
            $orderLineItem  = $orderLineItems[$key];
            // Make sure that the description and price are the same as we are relying upon the order
            // of the Order Items and The OmniPay Item Bag to be the same
            if ($orderLineItem->getDescription() != $item->getDescription()) {
                return;
            }
            if ($orderLineItem->price != $item->getPrice()) {
                return;
            }
            $sku = $orderLineItem->getSku();
            // Place the SKU within [] as the Product Record for the Sage 50 Accounts Integration
            $description = '['.$sku.']'.$item->getDescription();
            $item->setDescription($description);
        }
    }
);

Ideally we would like to populate the Product Code column in Sage with the SKU. I've added a screenshot below of what our client is seeing in Sage after having added the documented event code. As you can see, there are no SKUs present in any of the columns. Thanks.

image003

andris-sevcenko commented 5 years ago

I'm not sure how Sage payments solutions integrate with their accounting solutions and I'm not able to find any documentation specifying that.

Without any documentation about how to achieve this, there's little hope in figuring out how (and if) that's possible.

matt-adigital commented 5 years ago

Thanks for your response Andris.

I've had a play around and it seems that the conditionals in the example code were preventing us from progressing the $item model. The $key variable was coming through as 0 (zero indexed array) when what we actually needed our array targetting to use {orderId}-{purchasableId}-{optionsSignature} as the key. I've updated my code now to loop through each item and check a few values to get the correct SKU before modifying the $item variable.

use craft\commerce\omnipay\base\Gateway as BaseGateway;
use craft\commerce\omnipay\events\ItemBagEvent;

Event::on(
    BaseGateway::class,
    BaseGateway::EVENT_AFTER_CREATE_ITEM_BAG,
    function(ItemBagEvent $itemBagEvent) {
        $orderLineItems = $itemBagEvent->order->getLineItems();
        /**
         * @var $item Item
         */
        foreach ($itemBagEvent->items as $key => $item) {
            foreach($orderLineItems as $lineItem) {
                // Make sure that the description, price, and qty are the same as we are relying upon
                //  the Order Item and The OmniPay Item Bag to be the same
                if ($lineItem->price == $item->getPrice() && $lineItem->description == $item->getDescription() && $lineItem->qty == $item->getQuantity()) {
                    // Place the SKU within [] as the Product Record for the Sage 50 Accounts Integration
                    $description = '['.$lineItem->sku.']'.$item->getDescription();
                    $item->setDescription($description);
                }
            }
        }
    }
);

By doing this I am now seeing an updated value when dumped to the screen which I wasn't seeing before, this value still isn't making it into Sage Accounts though sadly. For now this code modifies the description but it would be better to use the setProductCode function that omnipay have documented here: https://github.com/thephpleague/omnipay-sagepay#sage-50-accounts-software-integration which states that:

using \Omnipay\SagePay\Extend\Item you can use setProductCode

This would allow us to leave the description unmodified and just directly set the productCode instead. However, this would require the items from the ItemBagEvent to implement Omnipay\SagePay\Extend\Item rather than the current Omnipay\Common\Item model which could affect other gateways depending on how commerce initially populates the ItemBagEvent, some conditionals to check that the selected gateway type is SagePay would probably be required.

Do you have any ideas why the newly set description still isn't modifying Sage Accounts correctly? Have I missed something when setting the $item variable which prevents it from being saved back into it's respective array correctly before being passed through to SagePay?

I'd like to get this working so that the documentation around integrating with Sage 50 Accounts can be updated under the Using the legacy basket format section. If we can't get it working and this feature is unsupported, the section should probably be removed. Thanks for your time, I really appreciate it.

andris-sevcenko commented 5 years ago

@matt-adigital at this point it might be worth contacting @harnasz, who was the author of the original PR, and see whether he got it to work successfully and if he has any pointers.

I really have no deeper knowledge on integration with Sage 50.