Problem/Motivation

The Review class which triggers the review makes the assumption that order items have a related product variation. In case of order items with no purchased entity or related to another entity type this raises an error.

Proposed resolution

We could first fill the product data with the order item data and complete with additional information if it is related to a product variation.

Remaining tasks

To go a bit further and allow websites to customize the data sent to TrustedShops we could trigger an event to alter the product data and fill more fields like the brand or the product image.

Comments

Aerzas created an issue. See original summary.

aerzas’s picture

Here is a patch proposition.

aerzas’s picture

Issue summary: View changes
aerzas’s picture

Status: Active » Needs review
aerzas’s picture

Here is a further proposition to open product data to alterations via the event dispatcher.

wengerk’s picture

Status: Needs review » Needs work

Thank you for that huge improvements !!

I opened a PR on Github with that patch to run Tests & Code Styles on Github Actions (faster than Drupal CI).
https://github.com/antistatique/drupal-commerce-trustedshops/pull/11

We still have some tests issues to works on before merging.

aerzas’s picture

Here is the updated patch with the fixed kernel tests.

aerzas’s picture

Re-roll patch against latest changes.

aerzas’s picture

Status: Needs work » Needs review
wengerk’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

For me the code is pretty perfect ! Many thanks for that huge quality code !!

Still, before merging, I would have a tests that cover the following code

+++ b/src/API/Review.php
@@ -80,17 +94,26 @@ class Review {
+      // Open product data to alterations.
+      $event = new AlterProductDataEvent($trusted_product, $order_item);
+      $this->eventDispatcher->dispatch(TrustedShopsEvents::ALTER_PRODUCT_DATA, $event);

Indeed, the current tests show how works AlterProductDataEvent but nothing demonstrate the event is fired properly.

aerzas’s picture

Thank you very much for your feedback.
Here is the updated patch with an additional test on the fired event.

aerzas’s picture

StatusFileSize
new10.3 KB
new1.87 KB

Fixing wrong import.

aerzas’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
wengerk’s picture

StatusFileSize
new12.57 KB
new2.64 KB

Thanks Aerzas !

I added another tests that ensure Drupal\commerce_trustedshops\API\Review::triggerShopReview work with a order item without purchased entity.

  • wengerk committed a1dee54 on 8.x-2.x authored by Aerzas
    Issue #3201543 by Aerzas, wengerk: Order items may not be product...
  • wengerk authored dec632d on 8.x-2.x
    Merge pull request #13 from antistatique/8.x-2.x--3201543--orders-items-...
wengerk’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.