feat(shopify): consolidate tax accounts in order #229
Conversation
|
Closes #12 |
| row = {"account_head": account, **tax_row} | ||
| row["item_wise_tax_detail"] = json.dumps(row.get("item_wise_tax_detail", {})) | ||
| taxes.append(row) | ||
|
|
||
| return taxes |
There was a problem hiding this comment.
Existing code is much more readable and we are adding a layer of complexity. It's better to break this and make it more readable.
Given a chance, I wouldn't touch existing functionality, but instead process the list of taxes just before returning it in a separate function (if applicable). Appending to items in update_taxes_with_shipping_lines would still be required though.
There was a problem hiding this comment.
Makes sense, have made consolidation an optional feature and also split it into a different function.
|
|
||
| shipping_charge_amount = flt(shipping_charge["price"]) - flt(total_discount) | ||
| if bool(taxes_inclusive): | ||
| shipping_charge_amount -= total_tax | ||
|
|
||
| taxes.append( |
There was a problem hiding this comment.
Shipping item at the top? 🤔
There was a problem hiding this comment.
Shipping item on top of Taxes table only. Not item table.
eg:
https://user-images.githubusercontent.com/1576808/120939785-f8f75280-c719-11eb-892c-7fefa6ac883d.png
| f"{get_tax_account_description(tax) or tax.get('title')}" | ||
| ), | ||
| "tax_amount": tax["price"], | ||
| "cost_center": setting.cost_center, |
There was a problem hiding this comment.
Why are you not updating item_wise_tax_detail here?
Also its just equal to using set_default instead of if / else
There was a problem hiding this comment.
Have added item_wise_tax_detail
rtdany10
force-pushed
the
consolidated-tax
branch
from
60dd863
to
4d60fb0
Compare
ecommerce_integrations/shopify/doctype/shopify_setting/shopify_setting.json
Outdated
Show resolved
Hide resolved
|
@ankush LGTM from a Logical standpoint. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #229 +/- ##
===========================================
- Coverage 40.96% 40.83% -0.14%
===========================================
Files 62 62
Lines 4245 4261 +16
===========================================
+ Hits 1739 1740 +1
- Misses 2506 2521 +15
|
ankush
force-pushed
the
consolidated-tax
branch
from
dbd97ca
to
90ddff1
Compare
|
🎉 This PR is included in version 1.15.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
caused by frappe#229 ``` Traceback (most recent call last): File "apps/ecommerce_integrations/ecommerce_integrations/shopify/order.py", line 49, in sync_sales_order create_order(order, setting) File "apps/ecommerce_integrations/ecommerce_integrations/shopify/order.py", line 61, in create_order so = create_sales_order(order, setting, company) File "apps/ecommerce_integrations/ecommerce_integrations/shopify/order.py", line 96, in create_sales_order taxes = get_order_taxes(shopify_order, setting, items) File "apps/ecommerce_integrations/ecommerce_integrations/shopify/order.py", line 223, in get_order_taxes row["item_wise_tax_detail"] = json.dumps(row["item_wise_tax_detail"]) KeyError: 'item_wise_tax_detail' ```
caused by frappe#229 ``` Traceback (most recent call last): File "apps/ecommerce_integrations/ecommerce_integrations/shopify/order.py", line 49, in sync_sales_order create_order(order, setting) File "apps/ecommerce_integrations/ecommerce_integrations/shopify/order.py", line 61, in create_order so = create_sales_order(order, setting, company) File "apps/ecommerce_integrations/ecommerce_integrations/shopify/order.py", line 96, in create_sales_order taxes = get_order_taxes(shopify_order, setting, items) File "apps/ecommerce_integrations/ecommerce_integrations/shopify/order.py", line 223, in get_order_taxes row["item_wise_tax_detail"] = json.dumps(row["item_wise_tax_detail"]) KeyError: 'item_wise_tax_detail' ```
caused by #229 ``` Traceback (most recent call last): File "apps/ecommerce_integrations/ecommerce_integrations/shopify/order.py", line 49, in sync_sales_order create_order(order, setting) File "apps/ecommerce_integrations/ecommerce_integrations/shopify/order.py", line 61, in create_order so = create_sales_order(order, setting, company) File "apps/ecommerce_integrations/ecommerce_integrations/shopify/order.py", line 96, in create_sales_order taxes = get_order_taxes(shopify_order, setting, items) File "apps/ecommerce_integrations/ecommerce_integrations/shopify/order.py", line 223, in get_order_taxes row["item_wise_tax_detail"] = json.dumps(row["item_wise_tax_detail"]) KeyError: 'item_wise_tax_detail' ```
Currently, the tax table have individual lines of tax for each item in the order.
This PR consolidates the tax table and also gives the user ability to move shipping charges to the item table, if required.
Settings page:
When Shipping Charge is added as an item:
Otherwise:
Related: resilient-tech/india-compliance#469
@ankush @vorasmit