fivetran / dbt_quickbooks

Fivetran data models for QuickBooks using dbt.
https://fivetran.github.io/dbt_quickbooks/
Apache License 2.0
25 stars 38 forks source link

[Bug] Currency Conversion not occurring in GL transformation #128

Closed brandonrf94 closed 4 weeks ago

brandonrf94 commented 2 months ago

Is there an existing issue for this?

Describe the issue

The GL is not applying a currency conversion when going from the bill/invoice to the GL.

We have 2 bills that were charged as Korean Won & Japanese Yen - however when the line hits the GL it looks like currency conversion rates are not applied.

Can you update the transformations to include the adjusted rate? Ideally you include currency recorded as well. my example was a bill, but I imagine invoices will have the same issue.

The: quickbooks_sparkloop_detail.invoice and quickbooks_sparkloop_detail.bill have exchange rates captured already.

https://github.com/fivetran/dbt_quickbooks/blob/e604697a51880e2f57bc89606517bb499eb6a017/models/transaction_lines/int_quickbooks__bill_transactions.sql#L39C9-L39C27

Relevant error log or model output

No response

Expected behavior

The GL should be in a unified currency.

dbt Project configurations

n.a

Package versions

n.a

What database are you using dbt with?

snowflake

dbt Version

n.a

Additional Context

n.a

Are you willing to open a PR to help address this issue?

fivetran-joemarkiewicz commented 2 months ago

Hi @brandonrf94 thanks for opening this issue. At the moment the transformation does not support multi-currency. That being said, as you have noticed there are a number of other users who are requesting and needing multi-currency support. Offering this support is definitely one we want to prioritize, but it has been quite the challenge to provide full support.

The main issue we continue to encounter is that some multi-currency QuickBooks environments allow for multiple AP/AR accounts which a few of our models map offsetting debit/credit entries to these accounts (see examples in bill_payment and invoice). We then take these accounts and perform a cross join to generate the offsetting debit/credit amounts. Typically QuickBooks only accepts one AP/AR account, but some multi-currency environments have an AR/AP for each currency which causes our cross joins to result in fannouts. We have been trying to get a hold of QuickBooks to try and understand how we can map these properly, but have not been able to make progress there.

The above has been the primary reason we have not been able to support multi-currency. Would you be able to confirm on your end, do you see multiple AP/AR accounts for the various currencies you have operated processing? If not, I don't see why we wouldn't be able to provide the currency and exchange rate features into the double entry and downstream models. As this would prove that not all multi-currency environments have the multiple AP/AR account scenario. If you do see multiple AP/AR accounts which are still active then we will run into the same issue. However, there may be a route we can take to get QuickBooks involved to help us close the gap stopping us from fully supporting multi-currency.

Let me know if you have any questions!

brandonrf94 commented 2 months ago

@fivetran-joemarkiewicz We have a single AR, but multiple Accounts Payable :(

fivetran-joemarkiewicz commented 2 months ago

Ahh darn 😢, that solidifies the fact that multiple AP or AR accounts is common across QuickBooks environments operating with multi-currency.

As I mentioned in the other thread, I am currently attempting to draft a developer open forum question for QuickBooks to help highlight how we can accurately map these accounts to transactions via the API. Unfortunately, I found this forum question from a number of years ago which may be the cause of these issues.

image

I know the above is highlighting AR accounts, but I imagine the same is true for AP.

Do you know if your version of QuickBooks at one point was QuickBooks for Windows (and then was upgraded to Online?). Additionally, when looking at your multiple Accounts Payable can you confirm if only one account matches this criteria?

https://github.com/fivetran/dbt_quickbooks/blob/e604697a51880e2f57bc89606517bb499eb6a017/models/double_entry_transactions/int_quickbooks__bill_payment_double_entry.sql#L33-L35

If not, do you see anything to identify with these multiple AP accounts that they are related to different currencies?

brandonrf94 commented 2 months ago

image

They are all type accounts payable.

The customer is almost positive they were always on QBO

fivetran-joemarkiewicz commented 2 months ago

Thanks for sharing @brandonrf94, a thought we had considered in the past is to map the transactions based on the currency_id field to the appropriate AP/AR account, but previously felt that was not the most sustainable solution.

Out of curiosity do you see for the two bills you mentioned in the original issue description that there is a reference to the appropriate currency_id which could map to the currency used in the respective AP/AR accounts?

brandonrf94 commented 2 months ago

Yes, the currency is there we could map to 👀

fivetran-joemarkiewicz commented 2 months ago

Thanks for confirming! Let me bring this into the team and see if we can get something working based off this assumption. I still don't know how I feel with this approach as I would like there to be a more direct connection provided by QuickBooks, but we have been stuck on this for quite a while and I would like to make some progress in supporting multi-currency as this has been a large need for users.

I will be on PTO Monday, but will plan to reserve some time Tuesday morning to see if I can get this to work for the int_quickbooks__bill_payment_double_entry and int_quickbooks__bill_double_entry models.

Also would you be able to share how you are running the QuickBooks data models? Are you running them via Quickstart or your own dbt project?

brandonrf94 commented 2 months ago

I'm running them via the quickstart - although because of this bug we're looking into kicking off our own dbt project to see if we can start fixing the more pressing issues asap

fivetran-joemarkiewicz commented 2 months ago

@brandonrf94 I just wanted to let you know that we decided to pursue this in greater detail as not supporting multi-currency is a large gap at the moment and we want to make progress and solve this gap for our users.

When exploring the currency_id mapping for the bill double entry models it seemed promising! I am going to explore offering support using the currency_id mapping approach for all double entry models in the coming sprint. I'll follow up with you here once we get a working branch for you to fully test out. If you have moved over to your own dbt project, you will be able to install the branch and confirm if the approach solves your original issue.

Let me know if you have any questions.

brandonrf94 commented 2 months ago

Hey @fivetran-joemarkiewicz - we ended up forking the repo and making the changes locally. Things do work.

We had to make a few changes to get everything to reconcile

Currency related: 1) In the "int_quickbooks_bil_double_entry.sql" we had to update (bill_lines.amount*bills.exchange_rate) as amount 2) In the same file we updated the "ap_accounts" cte to include currency_id and updated the ap_accounts join to also join on the currency_id (with the bill_payments.currency_id)

Other issues: 3) We had to update "int_quickbooks__invoice_double_entry.sql" to grab the correct account ID priority: line 110 - when coalesce(invoice_lines.sales_item_account_id, invoice_lines.account_id, items.parent_income_account_id, items.income_account_id, bundle_income_accounts.account_id) is not null then 'SalesItemLineDetail' line 112- when coalesce(invoice_lines.sales_item_account_id, invoice_lines.account_id, items.parent_income_account_id, items.income_account_id, bundle_income_accounts.account_id, invoice_lines.discount_account_id) is null then 'NoAccountMapping' line 114- coalesce(invoice_lines.sales_item_account_id, invoice_lines.account_id, items.parent_income_account_id, items.income_account_id, bundle_income_accounts.account_id, invoice_lines.discount_account_id) as account_id,

4) We had to enable using_credit_card_payment_txn

fivetran-joemarkiewicz commented 2 months ago

@brandonrf94 thanks so much for this information and I am glad you were able to get it to work on your end!

I made the same currency related changes, so that is great to see we are on the right track. I will explore making similar updates to our other double entry models.

I was unaware of those other issues you highlighted. This will be extremely helpful when applying this update. This should maybe be included outside of the currency changes so all customers may benefit sooner.

We have the credit_card_payment_txn disabled by default as at the time of adding the support to the data model it was not a widely used table. However, this likely has changed since it was added. We can evaluate and consider if this would make sense to enable by default going foward.

brandonrf94 commented 2 months ago

hey @fivetran-joemarkiewicz - did you happen to test this out?

fivetran-joemarkiewicz commented 2 months ago

Hey @brandonrf94, locally I was able to test the changes you mentioned and we plan to incorporate them in an upcoming release which will offer this support. Thank you again for sharing the changes you made on your end, this is incredibly helpful and will ensure we are able to propose a viable solution for the multi-currency gap.

We are currently in the middle of a sprint, but I am accepting this for dedicated development and completion next sprint (starting 6/26). I know you mentioned that you forked the repo to make you changes work on your end, if you would like to open a PR to contribute your changes we would happily accept these! Otherwise, we will plan to continue full development and plan for a release of these multi-currency updates for all double entry models next sprint.

Thank you!

fivetran-avinash commented 1 month ago

Hi @brandonrf94 , thanks again for your patience here!

I just wanted to let you know we've been working on this feature actively and are hopefully close to having it ready to deploy in our next release.

For now though, we have an open branch with many of these changes above implemented. Would you be able to update your packages.yml and test our live development branch where we are implementing multicurrency support:

packages:
  - git: https://github.com/fivetran/dbt_quickbooks.git
    revision: feature/multicurrency-support
    warn-unpinned: false

A few things to call out:

If you're able to test the above branch, and see if there are any immediate discrepancies with these new model additions, or if everything looks good, please let us know! We will then accelerate our release process for multicurrency in our latest version of dbt_quickbooks.

Thanks again for all your help! Feel free to reach out with any questions or thoughts you might have.

brandonrf94 commented 1 month ago

Thank you for the tag, unfortunately we won't be able to test this branch with our customers as we've had to layer in additional changes over time (as I've specified with other bug reports in this repo)

fivetran-avinash commented 4 weeks ago

Hi @brandonrf94 , we wanted to let you know the latest release of dbt_quickbooks incorporates multicurrency, and live on the dbt hub.

Take a look, let us know your thoughts, and of course if any issues arise in using this new feature. More details are in the release notes.

There are some limitations, (as we've discussed in the comment above), but we hope that this will resolve the plurality of the multicurrency issues, including the one mentioned here.

Reach out at any time, and thanks for this long and active thread in getting this feature to completion!