Discount line item to order item adjustment

1. +++ b/modules/commerce/src/Plugin/migrate/OrderItemDeriver.php
   @@ -103,34 +103,37 @@ class OrderItemDeriver extends DeriverBase implements ContainerDeriverInterface
   +        if ($line_item_type !== 'shipping') {
   

   The title is discounts. But there are fees that are not shipping. Comments or clarifications on what we are doing would help make this more clear.

2. +++ b/modules/commerce/src/Plugin/migrate/process/commerce1/CommercePrice.php
   @@ -19,11 +19,32 @@ class CommercePrice extends ProcessPluginBase {
   +      $price['number'] = bcdiv($price['number'], bcpow(10, $value['fraction_digits']), $value['fraction_digits']);
   

   It might make more sense to use's Drupal\commerce_price\Calculator.

3. +++ b/modules/commerce/src/Plugin/migrate/process/commerce1/OrderItemAdjustment.php
   @@ -0,0 +1,57 @@
   +            // todo: percentage.
   

   should we link to an issue here?

4. +++ b/modules/commerce/src/Plugin/migrate/process/commerce1/OrderItemAdjustment.php
   @@ -0,0 +1,57 @@
   +              // todo: use fraction digits?.
   

   Yes, fraction digits is important.

5. +++ b/modules/commerce/tests/src/Kernel/Migrate/commerce1/OrderTest.php
   @@ -148,6 +152,56 @@ class OrderTest extends Commerce1TestBase {
   +    $order = [
   +      'id' => 4,
   +      'type' => 'default',
   +      'number' => '4',
   +      'store_id' => '1',
   +      'created_time' => '1543271966',
   +      'changed_time' => '1543271966',
   +      'completed_time' => '1543271966',
   +      'email' => 'CommerceKickstart@example.com',
   +      'ip_address' => '127.0.0.1',
   +      'customer_id' => '1',
   +      'placed_time' => '1543271966',
   +      'total_price' => '24.000000',
   +      'total_price_currency' => 'USD',
   +      'adjustments' => [],
   

   There's no adjustments in here. Why?

6. +++ b/modules/commerce/tests/src/Kernel/Plugin/migrate/source/commerce1/OrderTest.php
   @@ -125,6 +125,48 @@ class OrderTest extends SourceTestBase {
   +    $tests[0]['source_data']['commerce_line_item'] = [
   +      [
   +        'line_item_id' => '1',
   +        'order_id' => '2',
   +        'type' => 'product',
   +        'line_item_label' => 'sku 1',
   +        'quantity' => '2',
   +        'data' => 'a:0{}',
   +        'created' => '1492868907',
   +        'changed' => '1498620003',
   +      ],
   +      [
   +        'line_item_id' => '11',
   +        'order_id' => '2',
   +        'type' => 'shipping',
   +        'line_item_label' => 'Express',
   

   I don't see any non-shipping adjustments. Am I reading this wrong?

I've been testing this today and was a little surprised to find that the D7 commerce_line_item table contains multiple rows for the same discount on the same order. My guess is that a row was being added each time the "Apply" button was pressed or upon transition from pane to pane...

|          752 |   100382 | commerce_discount |                    |     1.00 | 1482288725 | 1482288725 | a:1:{s:13:"discount_name";s:22:"discount_2016_closeout";}                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              |
|          753 |   100382 | commerce_discount |                    |     1.00 | 1482288725 | 1482288725 | a:1:{s:13:"discount_name";s:22:"discount_2016_closeout";}                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              |
|          754 |   100382 | commerce_discount |                    |     1.00 | 1482288934 | 1482288934 | a:1:{s:13:"discount_name";s:22:"discount_2016_closeout";}                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              |
|          756 |   100382 | commerce_discount |                    |     1.00 | 1482288935 | 1482288935 | a:1:{s:13:"discount_name";s:22:"discount_2016_closeout";}                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              |
|          758 |   100382 | commerce_discount |                    |     1.00 | 1482288957 | 1482288957 | a:1:{s:13:"discount_name";s:22:"discount_2016_closeout";}

Not sure if others will have this problem or if this is the best way to fix it, but I wrote a process plugin which does this to resolve it.

    $order_id = $row->getSourceProperty('order_id');

      $count = \Drupal::database()->select('commerce_order_item', 'oi')
        ->condition('oi.order_id', $order_id)
        ->condition('oi.type', 'default')
        ->condition('oi.unit_price__number', 0, '<')
        ->countQuery()
        ->execute()
        ->fetchField();

      if ($count > 0) {
        throw new MigrateSkipRowException('Order already has promotion order item - skipping.', TRUE);
      }
    }

In thinking about this further, why do we need to create commerce_order_item rows for discounts? When I place a new order with a discount, no row is created like this in commerce_order_item. Could we just create the adjustment in a process plugin and skip all commerce_discount rows?

Here's what I came up with... It works, but I'm hesitant to create a patch without further discussion. This is only going to work for order discounts (not line item discounts) and I'm not sure if it's necessary for everyone to have the check for duplicate discount rows I'm seeing. Notice I'm popping off the last discount row, not matter how many are found.

The percentage look-up is here, but it's not covering all use cases.

This is added to the OrderItemAdjustment.php process plugin and commerce_discount type line item rows are excluded from the normal line item source, i.e. commerce_discount is not in the static map on the type field.

This prevents the unnecessary discount rows in the order_item table.

      if ($value['name'] === 'base_price') {
        // Check for discount rows.
        $order_id = $row->getSourceProperty('order_id');

        Database::setActiveConnection('migrate');
        $connection = Database::getConnection();
        $query = $connection->select('commerce_line_item', 'li')
          ->fields('li', ['data'])
          ->fields('ct', ['commerce_total_data'])
          ->condition('li.order_id', $order_id)
          ->condition('li.type', 'commerce_discount');

        $query->innerJoin('field_data_commerce_total', 'ct', 'li.line_item_id = ct.entity_id');

	$discount_line_items = $query->execute()->fetchAll();
        Database::setActiveConnection();

        if ($discount_line_items) {
          $discount_line_item = array_pop((array_slice($discount_line_items, -1)));
          if ($discount_line_item) {
            $data = unserialize($discount_line_item->data);
            if ($data && !empty($data['discount_name'])) {

              // Look up the percentage.
              Database::setActiveConnection('migrate');
              $query = $connection->select('commerce_discount', 'cd')
                ->fields('cp', ['commerce_percentage_value'])
                ->condition('cd.name', $data['discount_name']);

              $query->innerJoin('field_data_commerce_discount_offer', 'do', 'do.entity_id = cd.discount_id');
              $query->innerJoin('field_data_commerce_percentage', 'cp', 'cp.entity_id = do.commerce_discount_offer_target_id');

              $commerce_percentage_value = $query->execute()->fetchField();

              Database::setActiveConnection();

              if (!empty($commerce_percentage_value)) {
                $commerce_total = $row->getSourceProperty('commerce_total');

                // Scale the incoming price by the fraction digits.
                $fraction_digits = isset($commerce_total[0]['fraction_digits']) ? $commerce_total[0]['fraction_digits'] : '2';
                $amount = (string) $commerce_total[0]['amount'];
                $currency_code = $commerce_total[0]['currency_code'];
                $input = [
                  'amount' => $amount,
                  'fraction_digits' => $fraction_digits,
                  'currency_code' => $currency_code,
                ];
                $price = new CommercePrice([], 'price', '');
                $price_scaled = $price->transform($input, $migrate_executable, $row, NULL);

                $commerce_total_data = unserialize($discount_line_item->commerce_total_data);
                $label = $commerce_total_data['components'][1]['price']['data']['discount_component_title'];

                $adjustment = [
                  'type' => 'promotion',
                  'label' => !empty($label) ? $label : 'Discount',
                  'amount' => $price_scaled['number'],
                  'currency_code' => $currency_code,
                  'percentage' => $commerce_percentage_value,
                  'source_id' => 'custom',
                  'included' => FALSE,
                  'locked' => TRUE,
                ];
              }
            }
          }
        }

      }
      else {
        $parts = explode('|', $value['name'], -1);

Decided to do a patch after all... This is combined with the tax adjustments patch as they are really dependent upon each other.

I am seeing two different types of discounts in my data. One where discounts are rows in the line_item table and another where the discount is in the price components. This patch handles both for me.

This fixes an issue with the percentage value being stored and also updates the adjustment about to be a proper negative number for discounts.

The previous patch was using the discounted subtotal and needed pre-discount.

Not sure what's going on with those tests. It could be because of the tax handing this patch includes.

+++ b/modules/commerce/src/Plugin/migrate/process/commerce1/OrderItemAdjustment.php
@@ -0,0 +1,64 @@
+    if (is_array($value)) {

Should we log an error or throw an exception if this happens?

No BC complaints.

Yeah, let's log something.

1. +++ b/modules/commerce/migrations/commerce1_order_item_type.yml
   @@ -7,7 +7,16 @@ migration_tags:
   +      bypass: TRUE
   
   +++ b/modules/commerce/src/Plugin/migrate/field/commerce1/CommerceLineItemReference.php
   @@ -43,9 +43,16 @@ class CommerceLineItemReference extends FieldPluginBase {
   +        'no_stub' => TRUE,
   

   I think true (lowercase) in yaml is more accurate?

2. +++ b/modules/commerce/src/Plugin/migrate/OrderItemDeriver.php
   @@ -103,34 +103,38 @@ class OrderItemDeriver extends DeriverBase implements ContainerDeriverInterface
   +                $plugin_id = $this->fieldPluginManager->getPluginIdFromFieldType($field_type, ['core' => 7], $migration);
   +                if (!isset($this->fieldPluginCache[$field_type])) {
   +                  $this->fieldPluginCache[$field_type] = $this->fieldPluginManager->createInstance($plugin_id, ['core' => 7], $migration);
   +                }
   +                $this->fieldPluginCache[$field_type]
   +                  ->defineValueProcessPipeline($migration, $field_name, $info);
   +              }
   +              catch (PluginNotFoundException $ex) {
   

   Do we need to handle CCK or not at this point?

3. +++ b/modules/commerce/src/Plugin/migrate/process/commerce1/OrderItemAdjustment.php
   @@ -0,0 +1,65 @@
   +class OrderItemAdjustment extends ProcessPluginBase {
   ...
   +            $price = new CommercePrice([], 'price', '');
   

   Could we extend CommercePrice and use inheritance instead of constructing a new object? If extended, this could just call parent::transform.