silvershop / silvershop-core

SilverShop is an e-commerce shopping cart module for the SilverStripe CMS
http://silvershop.github.io
BSD 2-Clause "Simplified" License
113 stars 119 forks source link

OrderItem / Order: problem with Fluent #748

Open wernerkrauss opened 4 years ago

wernerkrauss commented 4 years ago

When an Order is finished, the OrderItem tries to get the exact version of the product to show in the order summary (e.g. on /account/order/).

Unfortunately with fluent installed, this only works if the Product was saved in the current locale. Otherwise Fluent augments the SQl in a way that it doesn't find the Product and therefor the link to the product or a product image is not shown after checkout.

While this is definitely a fluent bug, does anything speak against adding an extension point in OrderItems's Product() method to work around this problem until the fluent bug is finally solved? Working with Injecctors is pretty painful, as OrderItem has many used Subclasses (e.g. GiftVoucherOrderItem...)

Related issues: https://github.com/tractorcow-farm/silverstripe-fluent/issues/583 https://github.com/silverstripe/silverstripe-versioned/issues/252

wernerkrauss commented 3 years ago

Unfortunately our fix for Fluent4 isn't compatible with Fluent 5 any more. For others that have the same problems, here is how we solved it for Fluent5 with an injected class over OrderItem:

<?php

namespace MyShop\Models;

use SilverShop\Model\Product\OrderItem;
use SilverShop\Page\Product;
use SilverStripe\ORM\Queries\SQLSelect;
use SilverStripe\Versioned\Versioned;
use TractorCow\Fluent\State\FluentState;

class CustomOrderItem extends OrderItem
{

    /**
     * @param bool $forcecurrent
     * @return Product|\SilverStripe\ORM\DataObject|null
     */
    public function Product($forcecurrent = false)
    {
        //TODO: this might need some unit testing to make sure it compliles with comment description
        //ie use live if in cart (however I see no logic for checking cart status)

        if ($this->ProductID && $this->ProductVersion && !$forcecurrent) {
            $productVersion = $this->ProductVersion;
            $product = null;

            //get latest version of current Locale
            $locale = FluentState::singleton()->getLocale();

            $latestVersionInLocale = SQLSelect::create()
                ->setSelect('Max("Version")')
                ->setFrom('SilverShop_Product_Localised_Versions')
                ->addWhere('"Version" <= ' . $productVersion)
                ->addWhere([
                    '"RecordID"' => $this->ProductID,
                    '"Locale"' => $locale
                ])
                ->firstRow()
                ->execute()
                ->first();

            $product = Versioned::get_version(
                Product::class, $this->ProductID,
                $latestVersionInLocale['Max("Version")']
            );

            return $product;
        } elseif ($this->ProductID
            && $product = Versioned::get_one_by_stage(
                Product::class,
                'Live',
                '"SilverShop_Product"."ID"  = ' . $this->ProductID
            )
        ) {
            return $product;
        }
        return null;
    }
}
forsdahl commented 3 years ago

We have solved this problem with the following change in a custom OrderItem subclass:

           $product = Versioned::get_version(Product::class, $this->ProductID, $this->ProductVersion);
           if ($product) {
                $product->setSourceQueryParams([]);
           }

That is by setting source query parameters to none, FluentExtension will just skip the augmentSQL functionality, and the correct version will be returned, regardless of locale.