Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dbt_quickbooks package: quickbooks__profit_and_loss - 'amount' column is mixing currencies #115

Closed
2 of 4 tasks
waldencastbaa opened this issue Jan 12, 2024 · 33 comments · Fixed by #134 or #136
Closed
2 of 4 tasks
Assignees
Labels
status:in_progress Currently being worked on type:enhancement New functionality or enhancement update_type:feature Primary focus is to add new functionality

Comments

@waldencastbaa
Copy link

Is there an existing feature request for this?

  • I have searched the existing issues

Describe the Feature

Hi, I am using the quickbooks__profit_and_loss model to re-create the Profit and Loss by Class report in Quickbooks platform. We noticed there are some inconsistencies in the totals for some of the accounts. The differences are not massive but they exist. After investigating further we arrived to the conclusion that the 'amount' column in the model is mixing currencies, while Quickbooks platform report shows the P&L totals in USD. I would need the quickbooks__profit_and_loss model to reflect the totals in a unified currency (USD).

Describe alternatives you've considered

I have noticed that under quickbooks table 'invoice' exist the columns 'currency', 'exchange rate', 'home_total_amount' and 'total_amount', which means the exchange rate was applied in Quickbooks already. After doing some tests 'total_amount' seems to be the total in the local currency and 'home_total_amount' is the total in USD. My understanding is the model is using 'total_amount' instead of 'home_total_amount' and that's why the quickbooks__profit_and_loss table (column 'amount') is mixing currencies.

Are you interested in contributing this feature?

  • Yes.
  • Yes, but I will need assistance and will schedule time during your office hours for guidance.
  • No.

Anything else?

No response

@waldencastbaa waldencastbaa added the type:enhancement New functionality or enhancement label Jan 12, 2024
@fivetran-jamie
Copy link
Contributor

fivetran-jamie commented Jan 12, 2024

Hey there, thanks for taking the time to open this up! your recommendation makes sense, i think we'd need to update this bit of logic specifically.

i imagine we'd also need to switch total_amount with home_total_amount in the other double_entry models whose source tables have this field, specifically:

  • deposit
  • estimate
  • credit memo
  • journal entry
  • sales receipt
  • refund receipt
  • invoice

we'll also need to adjust the staging models, as they currently exclude home_total_amount.

we will prioritize folding this into our upcoming sprint (starts in 2 weeks), but in the meantime could you test out this quick-test branch? in it, i've just switched the fields and aliased home_total_amount as total_amount in the staging models so that the downstream double_entry models automatically use the home_total_amount values.

# packages.yml - replace references to quickbooks with 
packages:
  - git: https://github.com/fivetran/dbt_quickbooks.git
    revision: explore/home-total-amount
    warn-unpinned: false 

@waldencastbaa
Copy link
Author

Hi Jamie,
Thanks for your response. I'm happy to test out but we don't have access to DBT. We only have the outcome of the pre-built Fivetran analytics models stored in our BigQuery (because we included the transformations in the connector set-up). Let me know if I can help in any other way.
Thanks,

@jeromemayaud
Copy link

Hi @fivetran-jamie , I think we're also the same issue as @waldencastbaa. We can't switch to that custom branch either. Is there a way you'd suggest we could test out whether we are indeed being fed the wrong numbers because of currency?
(and, separately, do you have an idea of when you'll be able to release a fix? We're currently facing a decision as to whether we wait for you to fix before building our dashboards, or relying on a different source for our Quickbooks data.

@fivetran-jamie
Copy link
Contributor

fivetran-jamie commented Jan 18, 2024

Hi @waldencastbaa @jeromemayaud - if i compiled the quickbooks__profit_and_loss model code to one long purely-SQL script that uses home_total_amount, would you be able to run that directly in your warehouse and compare against the Quickbooks UI? i'm also digging for ways to test on our end 🙂

As for the timeline, we can prioritize working on this in our next sprint, which starts next thursday

@fivetran-jamie fivetran-jamie added status:scoping Currently being scoped update_type:feature Primary focus is to add new functionality labels Jan 18, 2024
@waldencastbaa
Copy link
Author

Hi Jamie, that would work for us :)

@jeromemayaud
Copy link

Hi @fivetran-jamie – that would also work for us, thanks for proposing to do this!

@jeromemayaud
Copy link

@fivetran-jamie Update here: we managed to run your branch with the revision locally, and it made a small change, but didn't fix the wider issue.

The only difference I can see is that your branch creates a 100% accurate subscription revenue (which is good!), and I expect that’s because subscription revenue comes into Quickbooks in multiple currencies. It therefore makes sense that your fix that makes use of actual currencies made it more accurate.

However, for something like COGS, which ingests expenses that will just be in the same unified currency to begin with, the numbers don’t change in the new branch compared to the original; they're still way off.

Do you think your fix will address this, or should we be getting in touch with another part of Fivetran support? If the latter, could you put me in touch with the right person please? Thanks.

@CPAanalytics
Copy link

The larger issue is that multi-currency accounts can have multiple AP and AR accounts to support different currencies. This results in a wildly inaccurate balance sheet as transactions on the double entry models are assigned to all accounts of type Accounts Receivable or Accounts Payable.

The fundamental assumption that a quickbooks client will not have multiple AR or AP accounts is wrong.

@fivetran-jamie
Copy link
Contributor

Apologies for the delay! Yes so multi-currency is indeed a known limitation of the package currently, but it is clearly something that lots of folks will find useful.

After investigating this further, adding multi-currency support will actually be a hefty feature request and require more changes than currently included in the dev branch, in order to calculate all kinds of transactions' exchange rates and handle nuances/assumptions like the one pointed out by @CPAanalytics. We are aiming to include this work in the upcoming sprint but it will definitely take more effort than first anticipated

@CPAanalytics
Copy link

@fivetran-jamie Happy to assist in testing as work is complete. I have access to multi-currency quickbooks file and can run branches for testing.

The largest issue I see right now with this dbt package is the philosophy that invoice transactions will always debit the Accounts Recievable type and similarly for bill transactions that they will credit Accounts Payable type.

Quickbooks allows you to specify the account invoices are posted against in multi account situations. This is a carry forward from files migrated from QB desktop. I suspect there is missing information in the API that you guys need to access to accurately build balance sheets.

@fivetran-joemarkiewicz
Copy link
Contributor

Hi @CPAanalytics thanks so much for chiming in! As @fivetran-jamie mentioned previously the current QuickBooks package does not support multi-currency, but we feel this is an important feature to support and we will start scoping out the updates necessary to ensure accurate results when using multi-currency.

We believe we have a good understanding of the individual currency exchange and home_total_amount updates that will be required in order to ensure the transaction lines are using the proper currency amount when populating the end models. However, @CPAanalytics you have shed a light on some other nuances we were not aware of (such as the AR and AP account mappings for invoices using multi-currency). Based on this observation, I was wondering if you would be open to meeting with myself and my team to discuss multi-currency behaviors within QuickBooks to ensure when we apply the updates to the package, they are all encompassing and have taken all proper precautions to verify the end results are accurate?

If you are open to meeting, it would be great if you could schedule a call via this meeting link so we may ask some additional questions and review our approach with you before going further. Thanks!

@CPAanalytics
Copy link

@fivetran-jamie happy to schedule a call.

@fivetran-joemarkiewicz
Copy link
Contributor

@CPAanalytics great thanks! I saw you scheduled the meeting and look forward to chatting more then 😄

@waldencastbaa
Copy link
Author

Hi team, is there any update on this? an estimated time for completion to set expectations internally? Thank you!

@fivetran-joemarkiewicz
Copy link
Contributor

Hi @waldencastbaa thanks for reaching back out. Currently we do not have a definitive timeline for the feature enhancement of supporting multi-currency. We believe we have roughly 75% of this feature scoped out, but the final 25% is deeming more difficult than we expected. See below for our current scoping of this feature:
This will be a hefty feature request that will require updates low-key across the board of this package. See our current scoping notes and also expected timeline and other thoughts below.

Scoping

PART 1: CURRENCY + EXCHANGE RATES

  1. For all transaction types, we will need to include the exchange_rate field in their respective _line staging models, whose amount fields we use to generate the general ledger.

    • For the tables that do not have exchange_rate, we will need to calculate it at the transaction-header level (ie not the line-item level) by including the total_home_amount fields in their staging models and calculating the transaction's exchange rate by dividing the total_amount and home_total_amount (though if the exchange_rate field is available in the transaction header table, we could also just grab it from there)
  2. Then in the double entry models themselves, we will add a field called calculated_home_amount that multiplies the above exchange rate with the line item's amount.

    • Note: I noticed that for invoices we actually have logic that looks at the transaction-level amount if it != 0. Unsure of why this model is slightly different but something to consider (and potentially update?)
  3. Persist the new calculated_home_amount field in the general ledger and downstream end models.

PART 2: SUPPORT MULTIPLE AP + AR ACCOUNTS

As we came to understand based off @CPAanalytics previous comments, we now know that the assumption of each QuickBooks environment containing 1 AR account and 1 AP account is not accurate. QuickBooks in fact allows for multiple AP/AR accounts to be used. While this is not encouraged by QuickBooks, it is still allowed due to a legacy effect of users migrating from QuickBooks Desktop to QuickBooks Online. QuickBooks does in fact have a feature which allows customers to merge these multiple accounts into the preferred one account, but this only works for single currency environments. QuickBooks is not able to merge AR/AP accounts with different home currencies. The big challenge with this part will be understanding how to match line item transactions to the appropriate AR/AP account. As right now the package logic assumes there to only be one account.

I have not been able to find a way with the given data from the Fivetran connector to associate transaction line items with the proper AP/AR currency account. My next step was I have reached out to our PM of the QuickBooks connector who is investigating how this may be achieved with the available information from the QuickBooks API.

Possible Timeline

At the moment we are currently waiting to hear back from our QuickBooks connector PM to identify if we are able to associate the transaction line items properly. If we are able to identify these, we will fold this into the upcoming sprint and address these updates! If not, we will likely still move forward with this update in an upcoming sprint but make an explicit callout or workaround for the environments with multiple AP/AR accounts that the solution may require some more improvements.

Out of curiosity, @waldencastbaa do you have multiple AP/AR accounts in your environment for the different currencies?

@waldencastbaa
Copy link
Author

Hi team,
Yes we do have AR/AP on QBO in different currencies. Did you hear back from Quickbooks connector PM? Any updates on timeline? Sorry to chase, but we have recently built a dashboard for our investors which release depends on this fix.
Thanks,

@fivetran-joemarkiewicz
Copy link
Contributor

Hi @waldencastbaa I was actually going to post back here this morning. I was able to sync up with our engineering team and they confirmed that they do not seem to have any available information in the current version of the connector that ties line items to the appropriate AR/AP account when multiple AR/AP are present. They are doing another pass to see if there are fields that could be possibilities and if there are any promising leads I will share the findings here and we can determine if we are able to leverage any existing data to update the data model.

In the meantime, the product and engineering teams at Fivetran are also reaching out to QuickBooks to determine if there is any information available in the API that may not be leveraged yet by the Fivetran connector to determine this information.

In terms of a timeline we are still stuck unless we are able to map line items to the appropriate AR/AP account when multiple are present. That being said, our team will likely start working on this update in the coming sprint (starting 2/22) to begin building the foundation of supporting multi-currency. Ideally we are able to hear back from QuickBooks or have a plan to handle the multiple AR/AP accounts, but we cannot be sure at this moment. I will continue to keep this thread updated as our team picks this up in the coming sprint and as we hear from our product/engineering teams as well as QuickBooks.

Thank you for your patience as we work through this update.

@waldencastbaa
Copy link
Author

Hi Team, I understand your sprint starts today. Do we have any updates or plan for this? Thanks!

@fivetran-joemarkiewicz
Copy link
Contributor

Hi @waldencastbaa thanks for responding and yes today is the start of our new sprint. In terms of plan we still are unsure on how to map balance sheet items with the appropriate AP or AR account when dealing with multi currency. I will start exploring setting the foundation of this feature in the current sprint; however, I do not feel confident this will be completed by the end of the sprint. There is still some key information we are waiting to hear back from QuickBooks on regarding any API endpoints that the connector may be missing which will include this information.

In regards to the AR/AP mapping for multi currency transactions, we did receive a recommendation from our connector engineering team suggested to attempt to leverage the deposit_to_account_id field from the stg_quickbooks__invoice table to determine what account the individual invoice line items should be mapped to. This is only an exercise for the Invoice transactions, but possibly this could lead us to a solution for the others as well. In the sandbox data we have, all of these records are null. I am curious if you see this field populated for any of your invoices? If so, do they seem to map to an AR account? Ideally we would be able to map the invoice_line_item directly in this case, but I am curious if this is a viable option if the field is populated and points to an AR account. Let me know if you are able to find any insights from this!

In the meantime, I will share the draft PR here once we begin to develop the framework so the folks in this thread may chime in and possibly test the WIP branch once it is ready.

@waldencastbaa
Copy link
Author

Hi there,
Thanks for the update. I did a test, but in our 'invoice table' all the deposit_to_account_id values are null as well.

@fivetran-joemarkiewicz
Copy link
Contributor

Thanks for looking into this @waldencastbaa. That means this is not a viable solution for us to leverage in our efforts to support multi-currency. This is still blocking for us at the moment. We need to determine how to associate the balance sheet line item transactions to the appropriate currency AP/AR accounts.

I have shared this finding with our product and engineering team and they are still working to reach out to QuickBooks to find a way to make these associations.

@fivetran-joemarkiewicz
Copy link
Contributor

Hi All I just wanted to provide another update on the status of this update.

The connector team and I have been able to replicate a scenario in our testing environment where we now have multiple A/R accounts due to multiple invoices being associated with customers using different currencies. We have also been able to see in the QuickBooks UI the relevant invoices being associated with the correct A/R currency accounts. However, we still cannot associate these invoice line items with the appropriate account. We have been able to associate them when doing a join on the currency field of the customer table and the account table. However, I do not have full confidence in this approach and would prefer we are able to join more concretely with a proper key.

Now that we have been able to replicate this in our testing environment we are hopeful that we will be able to more directly coordinate with QuickBooks to determine how to replicate their reports using the API and then using the Fivetran connector.

I will continue to share our progress here. Thanks again for your patience on this update.

@CPAanalytics
Copy link

@fivetran-joemarkiewicz Good to hear you guys are approaching it the more robust way and trying to extract the mapping data from the api.

@waldencastbaa
Copy link
Author

Thank you guys, looking forward to your updates.

@waldencastbaa
Copy link
Author

waldencastbaa commented Mar 12, 2024

Hi team, any updates on this? do we have an ETA on the fix?

@fivetran-joemarkiewicz
Copy link
Contributor

Hi @waldencastbaa unfortunately we still do not have an ETA on the fix. Our connector team is currently trying to get ahold of the right people at QuickBooks to understand how we may be able to address this. I will continue to keep this thread updated as we hear anything and can determine next steps. However, for right now we are awaiting communication with QuickBooks to be able to proceed appropriately.

@fivetran-joemarkiewicz
Copy link
Contributor

Hi All,

I just wanted to follow up again that we have not yet heard from QuickBooks. We are trying to get in contact with the right individuals to ensure we can make the proper updates to the data model. I will continue to share updates and hopefully we will have an idea of a path forward soon.

@brandonrf94
Copy link

Hi @fivetran-joemarkiewicz - this may be the same issue as the one I just opened here. But it's been 2-3 months since the last update here. Is there any resolution in sight? Or do you have a workaround SQL query you've been sharing to fix any forex rates?

@fivetran-joemarkiewicz
Copy link
Contributor

HI @brandonrf94 thanks for chiming in and sharing you are also needing the transformation to support currency exchange when building out the GL from the transactions.

Unfortunately, we have not been able to make progress on this outstanding gap 😢. I proved more details around the specifics in the issue you just opened. I am happy to continue the conversation around your issue there. For the multiple AP/AR account issue we have not been able to make contact with someone at QuickBooks who is able to provide us with the necessary information. Therefore, we are struggling to understand how to properly map the accounts to the offsetting debit/credit transactions.

That being said, we do not use QuickBooks internally and that likely is resulting in us not being routed to the proper folks at QuickBooks to help with our inquiry. What I am going to attempt today is to create an open question on QuickBooks Developer forum. Once I create the question I will link it below. It would be a huge help if the folks involved in this thread could upvote the question so we may get closer to being able to properly solve the multi-currency support in the transformation.

@fivetran-joemarkiewicz
Copy link
Contributor

Following up on this thread - given the demand for multi-currency support and the fact that we have been unable to determine how to connect multi-currency transactions to the appropriate AP/AR account, we will be moving forward with the suggestions outlined in Issue #128 and what was initially discussed here. While this is not the most robust solution, it seems to be the only one for the time being. Once we have a working branch using the proposed currency mapping, I will share it here before an official release. This way we can gain some comfort around the approach and validate it is sufficient before rolling it out to all users of this package.

Thanks everyone for your patience in this endeavor to have the dbt package support multi-currency. Hopefully, this option proves successful. Our next sprint begins on (6/26). We aim to share more once we begin development after the upcoming sprint starts.

@fivetran-avinash fivetran-avinash self-assigned this Jul 9, 2024
@fivetran-avinash fivetran-avinash added status:in_progress Currently being worked on and removed status:scoping Currently being scoped labels Jul 9, 2024
@fivetran-avinash
Copy link
Contributor

Hi @waldencastbaa, thanks again for your patience here!

I just wanted to let you know we've been working on adding multicurrency, 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:

  • We have created new multicurrency fields that basically perform the same operations as the single currency fields, while maintaining the original fields. We document the new fields we've added in the CHANGELOG.
  • We introduced *_converted_* type fields in most of our intermediate models. We leverage the exchange_rate where it's available (ex: You can see converted_amount in many of the double-entry models has a multiplier on the exchange_rate, but defaults to a multiplier of 1 to bring in the original amount values if a customer is using only one currency).
  • There are a few edge cases that we'd love to get your thoughts on:
    • With the int_quickbooks__deposit_double_entry model, there was no exchange_rate available, so we brought in home_total_amount and divided it by total_amount to gather the exchange rate, per the documentation. Do you believe this is a proper way to calculate a converted amount via exchange rate in this model?
    • With the int_quickbooks__credit_card_pymt_double_entry and int_quickbooks__transfer_double_entry intermediate models, there are currently no exchange_rate fields available in the source models. So for now we've kept the converted_amount the same as the amount. We are researching if we can bring these fields in, but feel free to file an issue if not having the exchange rates here will prevent you from balancing your financial models.

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.

@fivetran-avinash
Copy link
Contributor

Also cc: @CPAanalytics and @jeromemayaud , if you want to take a look at this branch and provide any feedback, that'd be great! We are planning on releasing this in the next week or two.

@fivetran-avinash
Copy link
Contributor

Hi @waldencastbaa , @CPAanalytics , and @jeromemayaud , 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 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:in_progress Currently being worked on type:enhancement New functionality or enhancement update_type:feature Primary focus is to add new functionality
Projects
None yet
7 participants