[Bug] Discount on Sales Receipt Debit should be negative but is coming in positive

[mikerenderco]

Is there an existing issue for this?

• I have searched the existing issues

Describe the issue

We have a few transactions that are causing the General Ledger not to balance. For Debit records for discounts on sales receipts the models/double_entry_transactions/int_quickbooks__sales_receipt_double_entry.sql should be marking the debit records as negative values in the adjusted amount but they are coming in as positive.

SELECT * FROM firehook-bakery.prod_quickbooks.quickbooks__general_ledger where transaction_id = '79547'

bquxjob_5be39b77_18fe1365034.csv

See transaction id 79547 the amount is getting pulled in as a positive 8.7 to account id 4 Undeposited funds rather than negative amount to the adjusted amount.

Relevant error log or model output

No response

Expected behavior

I looked through models/double_entry_transactions/int_quickbooks__sales_receipt_double_entry.sql but somehow these transactions going to Undeposited funds should be negative values.

dbt Project configurations

vars:
using_credit_card_payment_txn: true # enable if you want to include credit card payment transactions in your staging models
using_estimate: false # disable if you don't have estimates in QuickBooks
using_invoice_bundle: false # disable if you don't have invoice bundles in QuickBooks
using_purchase_order: true #enable if you want to include purchase orders in your staging models
quickbooks_union_schemas: ['qb_retail','qb_virginia'] # use this if the data is in different databases/projects but uses the same schema name

Package versions

packages:

package: fivetran/quickbooks
version: [">=0.13.0", "<0.14.0"] # we recommend using ranges to capture non-breaking changes automatically

What database are you using dbt with?

bigquery

dbt Version

1.7

Additional Context

No response

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.

[fivetran-avinash]

Hi @mikerenderco ! Thanks for reporting this.

Taking a look at our models and your CSV, it appears there's a few reasons the debits are positive.

• In int_quickbooks__account_classifications (link), the transaction_type is classified as 'debit` since the classification is Asset. It seems that this is expected.
• In quickbooks__general_ledger (link), the adjusted_amount is only multiplied by -1 if transaction_type and the account transaction_type aren't equivalent. but they're both debit, so the adjusted_amount is the same as the amount.

Let me know if you see anything else that jumps out.

So for followup on your side...

• It looks as if you expect the sales_receipt_line amount line in int_quickbooks__sales_receipt_double_entry (link) to be negative here from the sales_receipt_line table. Is that what you're seeing?
• If not, do you think additional logic needs to be applied somewhere to modify the adjusted_amount, based on whether the sales receipt is a discount type and whether the account_sub_type is UndepositedFunds?

Let me know your thoughts!

[mikerenderco]

Thank @fivetran-avinash - So technically the DBT job for our quickbooks account for the general ledger creation is incorrect. These transactions hit all your criteria but when the account_sub_type is UndepositedFunds it should flip the polarity of the adjusted amount even though it is a debit and the classification is an asset.

UndepositedFunds is a temporary holding account that transactions will get held in until they are moved. But until they are moved the General Ledger will not balance. We believe that UndepositedFunds is an account_sub_type (sub-account) that would be in every Quickbooks environment.

You would have to trigger the sign flipping off of the account_sub_type field.

[fivetran-avinash]

Hi @mikerenderco ! Thanks for all of the thorough follow-ups.

After examining your data, I was able to confirm that indeed the 8.7 amount has an associated discount_acount_id. A few follow-ups:

1. Should all rows with a not-null discount_account_id values in sales_receipt_line have a negative value?
2. If so, do you think the appropriate solve for this issue is to create a condition for all sales_receipt_line rows that have a non-null discount_account_id value to multiply the amounts by -1? At first glance it seems like all the other 'UndepositedFunds' values in the csv you provided that are not discounts should remain positive, as the line items would then sum up to the actual sales_receipt value.

Let me know your thoughts!

[mikerenderco]

@fivetran-avinash I don't think that will work. I looked at the Discount_account_id's in our system and two occur account_id for Sales_receipts. 356 and 149. The sign fliping should only occur when the the transaction_id is associated to a sales_receipt_line that has a discount_account where the account_sub_type = DiscountsRefundsGiven. I don't know why the 149 account is listed as a discount_account_id in the sales_receipt_line.

[fivetran-avinash]

Ah, thanks for the context @mikerenderco!

Two follow-ups before proceeding forward with adding this logic.

• I did see three other account ids with this DIscountsRefundsGiven account_sub_type (315, 5, 7). If they were hypothetically sales refunds, we would want these to also have negative adjusted_amount values?

• Are there any other conditions you could see where discounts could be applied, or will this sub_type suffice?

Let me know if this will cover all cases you need for seeing negative debit adjusted amounts for sales receipts!

[mikerenderco]

Thanks @fivetran-avinash. Let me review these questions with our FP&A team and ill get back to you. As always I appreciate the support.

[mikerenderco]

@fivetran-avinash I don't think there are any other conditions to this logic. When I look at the rest of our account the only issue is when a discount is applied to undeposited funds. All other discounts are getting captured correctly. Its only an issue when a discount initially gets applied to undeposited funds.

Does that makes sense?

[mikerenderco]

@fivetran-avinash - any ideas on this one? I'm trying to figure out how I could patch this myself so that i can get our GL to balance but i'm having trouble figure out how to apply the logic to the model for when a discount hits undeposited funds that the amount should be negative.

[fivetran-avinash]

Hi @mikerenderco , sorry about not communicating this earlier! We agree with your approach here, ahave gone ahead and accepted this ticket into our sprint that just kicked off. We hope to have a branch for you to test and/or a new release that will incorporate these new changes.

Let us know if you have any questions!

[mikerenderco]

Perfect Thanks @fivetran-avinash.

[mikerenderco]

@fivetran-avinash what is the timing of your sprints. No rush from our side but just want to understand when we could expect testing.

[fivetran-avinash]

Hi @mikerenderco , we're actively working on this now!

I have created a working branch with the proposed changes within the int_quickbooks__sales_receipt_double_entry model that should hopefully resolve your issue. Could you rename the code in your packages.yml to be

• git: https://github.com/fivetran/dbt_quickbooks.git
  revision: bugfix/sales-receipt-debit-should-be-negative
  warn-unpinned: false

And see what results you get and if they properly balance out the general ledger?

One note is that this new logic makes both the debited and credited discount amounts as negative. Let me know if it's only the debit amount that should be negative and I can tweak the logic again.

[mikerenderco]

@fivetran-avinash I believe both the credit and the debit should be negative. I'll deploy this now and let you know the results.

[mikerenderco]

@fivetran-avinash - we tested this quite a bit and your change helped me figure this out but it turns out that all sales receipts with a discount the discount signs need to be flipped. I created a fork here and modified your code: https://github.com/mikerenderco/dbt_quickbooks. I just modified your case statement in int_quickbooks__sales_receipt_double_entry removing the criterea related to the sub account. Now our GL Balances with this change.

[fivetran-avinash]

Hi @mikerenderco ! Thanks for your thorough investigation.

Just to confirm before deploying your proposed changes, we created the initial logic to only look at accounts which had discounted amounts with the account_sub_type being DiscountsRefundsGiven, per this discussion. Did the discount account id 149 case turn out to require a negative amount too? Or did that account need to be adjusted internally?

Finally, can you imagine any edge cases where an account with a discount account id shouldn't be applied as negative (i.e. any account subtypes that should be excluded)? We want to make sure this change is as future proofed
as possible before applying the proposed changes.

If there are no additional changes you think that are required, we can merge your changes into our PR, and try and have it ready for release next week!

[mikerenderco]

@fivetran-iavinash all sales receipts with discount ID's should have their polarity reversed. We thought that the account_sub_type was the driver but for sales receipts but really its any sales receipt with the discount id. This would be relevant to any quickbooks client of yours.

[fivetran-avinash]

Sounds good @mikerenderco !

Would you be able to go to this link and create the pull request where indicated? That way we can merge your changes into the above working branch I mentioned and you can then utilize our package by utilizing that packages.yml configuration. (I'd also be happy to create a PR on your behalf and merge it into our branch if that works better.)

Our plan is for you to use this working branch and check and see if everything works for your purposes. We will then merge this branch into a larger release we are doing for Quickbooks in the next cycle, at which point you can revert back to the standard packages.yml.

Let me know if you have any questions!

[mikerenderco]

@fivetran-avinash - I created a pull request: #131
but I don't do this often and may not have answered all of the questions correctly. Could you guide me if you need anything else?

[fivetran-avinash]

No worries about the additional pieces of the PR @mikerenderco ! Since we're introducing this in a wider release next sprint you don't have to document those pieces.

I've gone ahead and merged your PR into the above branch, so you can use the above packages.yml configuration for the time being. Once this is fully released I'll let you know and you can return to the normal packages configuration.

Be in touch, let me know if you have any questions or concerns! Thanks for working through this with us!

[mikerenderco]

Sorry what branch should i use now? I'm looking through the above and not sure what branch i should be using.

[fivetran-avinash]

Apologies @mikerenderco ! Sorry for the confusion.

I think using your branch for now is actually best, as the branch I suggested is getting folded into a release branch.

[fivetran-avinash]

Hi @mikerenderco, thanks for your patience! This feature is now 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.

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