aimeos / aimeos-core

Aimeos PHP e-commerce framework for ultra fast online shops, scalable marketplaces, complex B2B applications and #gigacommerce
https://aimeos.org
Other
3.4k stars 118 forks source link

Add variant support to PropertyAdd plugin #252

Open iammart opened 3 years ago

iammart commented 3 years ago

Following on from the discussion on the below thread, I have updated the plugin to query on product code(s) not product ID.

This enables product attributes to be stored on order products that have types of selection or bundle.

https://aimeos.org/help/viewtopic.php?p=15713#p15713

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-2.3%) to 85.886% when pulling a6f08484937240eddd01c54f97dd1f7f14853830 on iammart:master into f04beb8a5cfe91b2e1d6edc3cd5e608126798aab on aimeos:master.

aimeos commented 3 years ago

Using codes only leads to the situation that it now works for variant articles but not for selection products any more. You need to fetch all products by their ID or their code.

iammart commented 3 years ago

If I update the existing expression to following:

$expr = [
    $search->combine('||', [
        $search->compare('==', 'product.id', array_unique($productIds)),
        $search->compare('==', 'product.code', array_unique($productCodes)),
    ]),
    $search->getConditions()
];
$search->setConditions($search->combine('&&', $expr));

And the getProductItems signature to accept an additional property $productCodes, I am now returned two products in the list.

The addAttributes filters the product from the array list using the productId stored on the order. Do we need to check here on the product type and switch between code and ID in order to get the correct product reference to grab the properties we are attaching to the order product?

Or, would you just filter the list of products by code first, followed by id if null is returned?

aimeos commented 3 years ago

It's expected that it returns two products for ordered variant articles (the article and the selection product) and we want to get properties from both if they exists.

You can shorten the code a bit:

$search->add( $search->or( [
    $search->is( 'product.id', '==', array_unique( $productIds ) ),
    $search->is( 'product.code', '==', array_unique( $productCodes ) ),
] );
$manager->search( $search );
iammart commented 3 years ago

That makes sense now, I was ignoring the possibility the article could still have properties that might need to be stored against the order product. This now introduces more questions :) Which properties should take priority?

For eg. an article and selection will have the same property options. If one of these is a radio group, either yes/no then there is always a value. This would be stored as two json objects, and result in two comma separated values when printed out in the back office under the order.

I'll make the changes for now and we can review.

aimeos commented 3 years ago

If both, selection products and variant articles has the same property type assigned, then article properties should have priority - but I wouldn't care much about that scenario yet

aimeos commented 3 years ago

$manager->search( $search, ['product/property'] ) ->col( null, 'product.code' ) ->map( function( $product ) use( $types ) { foreach ( $types as $type ) { return $product->getProperties( $type ); } });

That should work.

iammart commented 3 years ago

$manager->search( $search, ['product/property'] ) ->col( null, 'product.code' ) ->map( function( $product ) use( $types ) { foreach ( $types as $type ) { return $product->getProperties( $type ); } });

That should work.

I thought so too initially, but after I thought about it - it couldn't be :)

We're mapping over the products, and returning properties by type, but this will return early if there is more than a single type as we are not returning a collection of properties by the types given.

For eg. the below would only return a map of properties belonging to the type 'one'

$types = ['one', 'two'];

$properties = $manager->search( $search, ['product/property'] )
    ->col( null, 'product.code' )
    ->map( function( $product ) use( $types ) {
        foreach ( $types as $type ) {
            return $product->getProperties( $type );
        }
    });

I will come back to this when I have some more time, for me what I originally wrote before the refactoring was working and has enabled me to continue.

aimeos commented 2 years ago

@iammart Any news on this topic?