[TYPO3-commerce] RFC: #6634: Refactor tx_commerce_products get_attribute_matrix() and get_product_attribute_matrix()

[Christian Kuhn]

Hey Morten.

Morten Olesen wrote:
> I finally had some time to test the changes, how ever since issue 1345 
> remains unsolved it's kinda limited what I can test.

Happy thanks for your feedback!


> This test is done against svn 19/03-2010 ( patch 12.2 iirc );
> 
> your patch introduces
> "PHP Warning:  Call-time pass-by-reference has been deprecated" x 3  : 
> line 216, 299, 344 in pibase

Yeah, that is in the _test patch, just a minor hickup. Will fix before 
committing.


> Other than that it seems to crash randomly when selecting attribute 
> combinations - cause is;
> 
> (Note; this also happens in unpatched 12.2 - how ever since the patch 
> modifies function that gives out the attributes used for creating the 
> dropdowns I thougth I migth be good to bring it up aswell.)
> 
> Some further investigation reveals that the crashed is not random at 
> all, but due to the drop downs getting too many options.
> 
> Ie. a product with theese two attributes ( values in parantesis )
> 
> Colour ( Blue, Yellow, Beige )
> Size ( Large, Small, XLarge )
> 
> 
> All those choices are shown in the selects, how ever only the combinations;
> 
>     * Yellow - Large
>     * Yellow- Small
>     * Beige - Large
>     * Beige - Small
> 
> exists.
> 
> 
> (This also happens in 12.2 )
> 
> Altering the article order so that they are now
> 
> Beige    Small
> Yellow    Large
> Yellow    Small
> Beige    Large
> 
> 
> "Yellow, Large" is still chosen by default, where as "Beige, Small" 
> would be the logical choice ... (yes we do infact have customers who 
> thinks this is important ;) )
> 
> 
> Test product;
> http://testcommerce.idefahost.dk/index.php?id=12&tx_commerce_pi1[showUid]=7&tx_commerce_pi1[catUid]=8 

I didn't fully understand this combination ... will try to dig a bit 
before the patch set is committed. In fact we re-implemented some bugs 
of the old methods to the new logic, but they are commmented now with 
TODO tags and can be sorted out at a later stage.


> So in conclusion it seems your changes does work exactly like the 
> original (apart from added warnings) - how ever it's less that desired 
> that it does so.

This is the important news, the new methods should be smarter, but fully 
backwards compatible. Thanks!


Regards
Christian