kilbot / WooCommerce-POS

:bangbang: All development now at https://github.com/wcpos.
http://wcpos.com
GNU General Public License v3.0
353 stars 125 forks source link

Tax labels and values mixed on receipt #195

Open jgangso opened 6 years ago

jgangso commented 6 years ago

Hi @kilbot

Could you please check this scenario:

Tax classes: Standard tax: 24 % Reduced tax: 14 % Itemized tax: Enabled

Steps to reproduce:

  1. Open POS in Chrome
  2. Add item with standard rate to cart
  3. Add item with reduced rate to cart
  4. Make a note of the tax on cart totals, they should be correct.
  5. Pay the purchase and print receipt Expected: The taxes are listed on the receipt as they were in step 4. Current: The tax labels and values are mixed, showing Standard rate on the first line, but the tax amount belongs to Reduced rate. Second line is titled Reduced rate, but the amount belongs to Standard rate.

My observations so far:

  1. The sort order of tax class titles is dependent on which order they are returned from the WC REST API as per this:
this.tax_rates = Radio.request('entities', 'get', {
  type: 'option',
  name: 'tax_rates'
}) || {};
  1. The sort order of tax amounts is dependent on the order items. The tax class which happens to appear first on the order item list will be the first of the tax totals too as per this:
itemizedTax: function(){
    var items = _.clone(this.toJSON(), true);
    var taxes = _.map(items, function(item){
      if(!item.tax) { return; }
      _.each(item.tax, function(tax){
        tax.shipping = item.type === 'shipping' ? tax.total : 0 ;
      });
      return item.tax;
    });
    var obj = this.sumTaxes(taxes);

    // convert obj to array to be consistent with WC REST API output
    var arr = [];

    _.each(obj, function(value, key){
      //value.rate_id = parseInt(key, 10);
      value.rate_id = key.toString(); // make sure it's a string
      arr.push(value);
    });

    return arr;
  },
  1. The failure happens after the remote sync of the order, when merging the local(A) and remote(B) order objects, more specifically in the setNested -function which compares the indices of the tax_lines array, and merges A[0] with B[0] and A[1] with B[1], and doesn't notice the two arrays are sorted differently, thus mixing the labels and amounts.

Conclusions: I've tried to fix this by adjusting itemizedTax -function to mimic the sorting order of the POS.options.tax_rates, but that doesn't work due to the way Javascript traverses Objects (order not guaranteed). I also tried to unset the tax_lines from the local order before merging in order to use whatever the remote order includes, but that results in empty tax_lines after the merge, ie. the tax_lines of the remote order aren't being merged if the tax_lines of the local order don't exist.

Any suggestion on how to proceed? Thanks!

jgangso commented 6 years ago

While writing, I came up with an workaround idea and it seemed to work, which I'm sharing here for future readers:

/** Workaround for https://github.com/kilbot/WooCommerce-POS/issues/195
 * The POS may mix tax labels and amounts, thus we construct the label again based on the 'rate' property which is not being mixed 
 */
function my_wcpos_handlebars_tax_label_helper( $js ) {
    $inline_js =
    'Handlebars.registerHelper("tax_label", function(rate) {
        rate = parseInt(rate);
        return ' . __('Tax ', 'your-textdomain') . ' + rate.toString() + " %";
    });';
    $js['my_wcpos_handlebars_tax_label_helper'] = $inline_js;

    return $js;
}

add_filter( 'woocommerce_pos_enqueue_footer_js', 'my_wcpos_handlebars_tax_label_helper' );

This outputs simple "Tax XX %" label for each line, which is sufficient at least in my country (Finland).

Meanwhile, would be great to get this correct in the core plugin too. :+1: