Problem/Motivation

If store has multiple applicable TaxTypes, there is no way how to specify order for them. This is an issue for applying tax for shipping.

Proposed resolution

Add weight for TaxType plugin.

Comments

mirom created an issue. See original summary.

mirom’s picture

Status: Active » Needs review
StatusFileSize
new4.48 KB

Status: Needs review » Needs work

The last submitted patch, 2: weight-tax-types-2999704-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mirom’s picture

Status: Needs work » Needs review
StatusFileSize
new4.61 KB

Fixing fails from previous patch.

bojanz’s picture

Status: Needs review » Needs work

This needs tests. Let's not define default weights on the existing plugins, there is no need. The shipping plugin can define its own weight that's higher than 0.

+  /**
+   * Helper function for sorting Tax Types by weight.
+   *
+   * @param \Drupal\commerce_tax\Entity\TaxTypeInterface $a
+   *   Tax Type A for comparison.
+   * @param \Drupal\commerce_tax\Entity\TaxTypeInterface $b
+   *   Tax Type B for comparison.
+   *
+   * @return int
+   *   Returns -1 if A<B; 1 if A>B; 0 id A=B
+   */
+  protected function sortTaxTypes(TaxTypeInterface $a, TaxTypeInterface $b) {
+    $aConfig = $a->getPlugin()->getPluginDefinition();
+    $bConfig = $b->getPlugin()->getPluginDefinition();
+    if ($aConfig['weight'] < $bConfig['weight']) {
+      return -1;
+    }
+    elseif ($aConfig['weight'] > $bConfig['weight']) {
+      return 1;
+    }
+    else {
+      return 0;
+    }
+  }
+

Every config entity already has a sort() method, we should override it on the TaxType class, and call it from there.
We also don't use camel case in code.

bojanz’s picture

StatusFileSize
new3.45 KB

Here's an initial cleanup. Still needs tests.

bojanz’s picture

       $this->taxTypes = $tax_type_storage->loadByProperties(['status' => TRUE]);
+      uasort($entities, [TaxType::class, 'sort']);

$entities should be $this->taxTypes.

johnjw59’s picture

StatusFileSize
new1.2 KB
new3.3 KB

Included an updated patch with the switch to $this->taxTypes as pointed out in #7. Also, I think we do need to set defaults in TaxTypeManager. I was getting errors that "weight" could not be found without that default set.

bojanz’s picture

Status: Needs work » Needs review
StatusFileSize
new5.49 KB

We don't need $defaults, the warnings disappear when you clear cache.

Here's the final patch. Added a getter for the weight, and a test for it. We don't have plugins with different weights, so I can't expand OrderIntegration test to cover that right now.

  • bojanz committed 2902414 on 8.x-2.x authored by mirom
    Issue #2999704 by mirom, bojanz, johnjw59: Add weight for TaxTypes
    
bojanz’s picture

Status: Needs review » Fixed

Commited. Just in time for 2.16 :)

Status: Fixed » Closed (fixed)

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