[Bug] Invoices with 0 balance due because of deposit being applied result in incorrect revenue on P&L

[sbrudz]

Is there an existing issue for this?

• I have searched the existing issues

Describe the issue

UPDATE from @fivetran-avinash: After investigating with @sbrudz, we've determined that the invoice_lines.amount is the right value to bring in for most transaction amounts.

The reason that invoices.total_amount=0 was ever brought in was due to unique issues presented by zero invoice bundle amounts that had non-zero line items that summed up to a non-zero invoice when it should have been zero, leading to overcounting in the general ledger. These specific invoice line bundle cases will need to be addressed, but for the majority of issues the invoice_line.amount should suffice for transaction amounts to bring in all the needed transaction values.

Initial issue presented by @sbrudz:

We have clients that pay an upfront deposit and then when we complete work for them, we apply that deposit to their final invoice. The workflow is described in this Quickbooks article.

We've found that if the total balance of the invoice is 0, then the revenue from that invoice isn't being counted in the quickbooks__profit_and_loss model.

Relevant error log or model output

No response

Expected behavior

Given: A customer has a $10k deposit in a liability account and then we apply that $10k deposit to their last invoice which has $10k in charges resulting in a $0 invoice total, the resulting stg_quickbooks_invoice_line table looks like:

Invoice ID	Index	Amount	Detail Type	Sales Item Account ID
14510	0	10000	SalesItemLineDetail	218
14510	1	-10000	SalesItemLineDetail	210
14510	2	0	SubTotalLineDetail

If that's the only invoice for the month, I should see $10k in revenue from account 218 on the Profit and Loss statement.

dbt Project configurations

name: "bug_report"
version: "1.0.0"
require-dbt-version: ">=1.7.0"
config-version: 2

profile: "bug_report"

model-paths: ["models"]
analysis-paths: ["analyses"]
test-paths: ["tests"]
seed-paths: ["seeds"]
macro-paths: ["macros"]
snapshot-paths: ["snapshots"]

clean-targets:
  - "target"
  - "dbt_packages"

models:
  salesforce:
    +schema: sales
  salesforce_source:
    +schema: stg_salesforce
  quickbooks:
    +schema: finance
    double_entry_transactions:
      +schema: int_quickbooks
  quickbooks_source:
    +schema: stg_quickbooks

vars:
  using_department: false
  using_estimate: false
  using_invoice_bundle: false
  using_refund_receipt: false
  using_sales_receipt: false
  using_purchase_order: false

Package versions

packages:
  - package: fivetran/salesforce
    version: [">=1.0.0", "<1.1.0"]
  - package: fivetran/quickbooks
    version: [">=0.12.0", "<0.13.0"]

What database are you using dbt with?

snowflake

dbt Version

Core:
  - installed: 1.7.10
  - latest:    1.7.11 - Update available!

  Your version of dbt-core is out of date!
  You can find instructions for upgrading here:
  https://docs.getdbt.com/docs/installation

Plugins:
  - snowflake: 1.7.2 - Update available!
  - duckdb:    1.7.3 - Up to date!

  At least one plugin is out of date or incompatible with dbt-core.
  You can find instructions for upgrading here:
  https://docs.getdbt.com/docs/installation

Additional Context

If the invoice has a non-zero total, then the revenue shows up as expected. It seems like the problem is with these lines in int_quickbooks__invoice_double_entry.sql. I'd be happy to contribute a PR, but I don't understand enough about why the logic was added (in this commit). It seems like changing it incorrectly could break other things.

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

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

[sbrudz]

Also, the "office hours" Calendly link from the template is currently not working.

[fivetran-avinash]

Hi @sbrudz , thanks for bringing this issue to our attention!

It looks like this other customer originally introduced this logic this to ensure discount line items are being included, but didn't account for situations where invoices could be zero where the invoice lines are non-zero, since they were primarily focused in on discounts which are rarely cancelling out the total invoice.

I think it might be worth getting on a call to discuss this in more detail, but I do think that in most cases that just taking in the invoice_lines.amount makes the most sense. Can you think of any cases where that might be the case?

Great job noticing the Calendly link! We've actually moved on from using Calendly, so if you'd like to set up a time to schedule an office hours, you can book a time on my Calendar, or if those times don't work we recommend you contact us via our solutions@fivetran.com email and we figure out a time that works for both of us.

[sbrudz]

@fivetran-avinash I've booked a meeting with you to discuss further.

[fivetran-avinash]

Hi @sbrudz, thank you again for taking the time out to discuss this with us. Debugging this with you live helped us reach a likely path to resolving this issue.

After some further investigation with our team, we discovered that the initial reason we introduced the invoice.total_amount = 0 logic was due to some irregular behavior with invoice_line_bundles, where bundles would have line amounts that summed to zero, but the total_amount would instead be zero. So we accounted for those cases by deferring to the total_amount. This might be due to some irregularities we saw with the customer's data at the time though, as this logic was introduced sometime ago.

Therefore, our plan is to proceed forward with reconfiguring this logic so that in most cases, the invoice_line.amount is what is brought in for the transaction amount, while we figure out the best way to account for potential edge cases in the invoice bundle.

Our plan is to take this task in a coming sprint and hopefully have a branch for you to test the changes after completion. If you're willing to contribute a PR, I can work with you and provide more details on what's happening with the invoice bundle and our proposed solution.

Thanks for all your patience, and we hope to have an update for you in the coming weeks!

[sbrudz]

Great — Thank you so much!

I am willing to contribute a PR but wouldn’t have time to work on it until the week of May 6th. I’ll check back in with you then to see where things are at.

[fivetran-reneeli]

Hi @sbrudz! We're taking this on this sprint and wanted to see if you had a chance to get started on a PR? I've started making updates on our end, but feel it's always good to check with customers who may be more tuned-in based on their data.

[sbrudz]

Hi @fivetran-reneeli, I haven't had a chance to start on a PR yet. Since you've started making updates on your end, I'll hold off. When there's a branch ready for testing, please let me know. I'll be happy to help test it out against our data.

[fivetran-reneeli]

Thanks @sbrudz ! I've applied changes on this working branch below. Let me know how it goes!

  - git: https://github.com/fivetran/dbt_quickbooks.git 
    revision: bugfix/v0.12.5
    warn-unpinned: false

[sbrudz]

@fivetran-reneeli I tested using your branch and it fixed the issue for us! Thank you!

[fivetran-reneeli]

Great @sbrudz , thank you for confirming! This is slated to be released in the next few days :)