fivetran / dbt_quickbooks

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

[Bug] invoice.total_tax isn't going into general_ledger.amount #56

Open mel-restori opened 1 year ago

mel-restori commented 1 year ago

Is there an existing issue for this?

Describe the issue

I am investigating discrepancies in the balance sheet between quickbooks UI and quickbooks__balance_sheet.

One source of discrepancy I found is that for invoices with tax amounts, the totals aren't reflecting properly in the ledger. The tax amounts are included in the Quickbooks UI balance sheet, but not in Fivetran's ledger.

The general ledger is summing the invoice_line amounts (this code), which do not include tax and have no line for the tax. The tax can be found in invoice.total_tax and it's included in the invoice.total_amount.

When the invoice is paid, it is paid in full, including the tax amounts.

Note that I think (but not sure) that the revenue side of the invoice double entry is behaving correctly. When I look up that line item in the Quickbooks UI balance sheet, it does not include tax. So I think taxes need to be added to the A/R account total, but continue to be left out of the revenue accounts total. But this needs confirmation.

I'm attaching a csv with an example invoice and its eventual payment, including the columns for invoice, invoice_line, quickbooks__general_ledger. The invoice is for $1168.67 plus tax of $30.21 for total of $1198.88

invoice tax example.csv

Relevant error log or model output

No response

Expected behavior

the balance sheet amount should match quickbook's balance sheet

dbt Project configurations

quickbooks:

using_address: true using_bill: true using_credit_memo: true using_department: true using_deposit: true using_estimate: true using_invoice: true using_invoice_bundle: true using_journal_entry: true using_payment: true using_refund_receipt: true using_transfer: true using_vendor_credit: true using_sales_receipt: true using_purchase_order: true using_credit_card_payment_txn: true

Package versions

packages:

git: https://github.com/fivetran/dbt_quickbooks.git
revision: feature/backwards-credit-card
warn-unpinned: false

note: using this branch to make sure we're properly counting credit card payment transactions

What database are you using dbt with?

bigquery

dbt Version

unknown

Additional Context

No response

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

fivetran-joemarkiewicz commented 1 year ago

Hi @mel-restori thanks for opening this incredibly thorough issue!

I would agree with you that I believe tax in this situation should be excluded from the revenue piece. However, I could see that this may need to be included within the AR totals. Based on what you are proposing, it may be as simple as swapping the amount in the line you called out to be total_amount. If you make that adjustment on your end, do you see the totals tie out with the UI and the revenue totals still be consistent with their expected results?

mel-restori commented 1 year ago

Hi @fivetran-joemarkiewicz, I believe yes in these examples. But that means the whole case statement would collapse and it would just always be invoices.total_amount. I'm not sure if there was a reason that the case statement was put in in the first place so not sure the risk of swapping that for other types of transactions.

And I believe changing that line would then include it in the revenue piece, which, to your point, shouldn't be.

fivetran-joemarkiewicz commented 1 year ago

Hi @mel-restori I am looking back into this and still a bit concerned to make the adjustment when the tax amount is factoring into the revenue calculation. I was wondering though, is there an account in your QuickBooks implementation that is for Taxes? If that is the case, there may be a way we can break these out while not erroneously adding to revenue.

mel-restori commented 1 year ago

@fivetran-joemarkiewicz not that I know of. In Quickbooks UI they totals are included in the accounts tied to the invoices

fivetran-avinash commented 1 year ago

Hi @mel-restori ! After initial investigation, we're thinking that the best approach here would be to create an additional double entry model that pulls the invoices.total_tax into its own model, and then create an Invoice Tax line item type that you can then factor into the general ledger and reconcile these discrepancies.

Let us know your thoughts! We will attempt development of this model in a coming sprint.

caffeinebounce commented 1 year ago

I had this same issue. It didn't affect the income statement, but it did affect my balance sheet as we had tax payables accounts for tax owed.

I think this is actually a difficult problem to fix because I couldn't find a mapping in QBO online from the tax agency owed tax to a payables account in the chart of accounts. To solve this, I had to include a manual account mapping in my dbt transformation

I'm not sure if a new double-entry model just for the invoice tax is needed, I just made edits to the invoice double-entry model.

https://github.com/fivetran/dbt_quickbooks/commit/66e20011404ea666fcb9b55f9d0c540b3467a068

fivetran-joemarkiewicz commented 1 year ago

@caffeinebounce thanks for sharing that you are experiencing the same issue and the additional insight and code. If you are interested in opening a PR I would be happy to review your changes and collaborate with you to integrate a version of these updates into the package.

In the meantime I am checking with our product team to determine if there is a way we can infer the mapping from the data. If not, there is likely a way we will be able to leverage variables to allow users to customize the mapping. Out of curiosity, how were you able to determine the mapping you included manually?

fivetran-joemarkiewicz commented 1 year ago

@mel-restori @caffeinebounce I have been able to link up with our product team and it looks like you may be able to link accounts to the tax agency by using the following join path.

ACCOUNT → TAX_CODE → TAX_RATE_DETAIL → TAX_RATE → TAX_AGENCY

Unfortunately, I do not have data that links accounts to the tax agency so I am unable to verify this. Would either of you be able to validate that this path works on your end for what we are trying to achieve.

Thanks!

caffeinebounce commented 1 year ago

I think this is probably a good add to the dbt in general, but unfortunately, it does not work for me. It may be the case that my bookkeepers are not following best practices, but the TAX_CODE_ID column in my ACCOUNT table is completely blank.

In my QB, the TAX_CODE table is the lowest segmentation of sales tax, which is basically by state and local jurisdiction because sales tax can vary between jurisdictions within a state. However, when the sales tax is mapped to the balance sheet (for tax payable), they've aggregated by state (e.g., CA Sales Tax Payable) to avoid an excessive amount of line items. Quickbooks apparently isn't great for calculating and tracking sales tax, so we use another tool to manage sales tax, and only the aggregate number is useful for our financial reporting. It's not clear to me how QB is handling the mapping, because things are being mapped to specific accounts but I can't trace the logic in the table.

I will ask my bookkeepers if they could be doing something differently and best practice would to be to maintain via an account mapping.

fivetran-joemarkiewicz commented 10 months ago

Unfortunately at the moment I am still unable to find a way with the given data to resolve this discrepancy 🤔.

If others run into the same issue please respond to this thread so we may understand the urgency of this ticket and so we may work with more folks to try and find a resolution. However, for the time being I will mark this ticket as stale until we are able to pick it back up and get more insight into resolving the issue.

caffeinebounce commented 10 months ago

Yes, pretty sure this is only fixable with manual mapping of tax accounts to tax agencies because qb doesn't seem to make the mapping available

finalspace-alwyn commented 5 months ago

I noticed that Value added tax on the JOURNAL_ENTRY table isn't posted to the VAT Control account somehow. So I joined the total tax of JOURNAL_ENTRY_TAX_LINE into the correct period's VAT PAYABLE account which balanced it out.

What puzzles me is that the general__ledger doesn't balance at all, yet this error is known since November 2022 without any fixes? It's not even a bug, the transformation is just plain wrong.

fivetran-joemarkiewicz commented 5 months ago

Hi @finalspace-alwyn thanks for responding to this issue. As stated in the previous comments, this is a known issue which we were unable to solve at the time and we were waiting until another member of the community chimed in to get more information on how we may solve this issue.

Would you be able to elaborate on the code changes you made to support the below statement? I would be very interested in seeing if we may apply a similar code update to address the issue. Our previous understanding was that we would need a manual mapping to ensure the tax amounts would be accounted for properly.

I noticed that Value added tax on the JOURNAL_ENTRY table isn't posted to the VAT Control account somehow. So I joined the total tax of JOURNAL_ENTRY_TAX_LINE into the correct period's VAT PAYABLE account which balanced it out.

I would also be willing to meet over a call with you to better understand the discrepancies you are seeing and determine if we can apply updates to the transformation.

finalspace-alwyn commented 5 months ago

Hi @fivetran-joemarkiewicz, I first did a full aggregation of the general ledger and noticed it was out of balance, I then did the same aggregation by period/transaction_id and realised that the journals weren't balancing to zero, so I went to those transactions in Quickbooks itself and saw that whenever there's value added tax on a transaction, the tax portion is missing from the journal.

The reasoning is that if the expense / loan account legs of the transactions are fine, the difference needs to be in vat payable / suspense.

I then at random noticed that my differences are exactly the contents of the JOURNAL_TAX_ENTRY_LINE. Since I have 25+ quickbooks companies to manage, I simply ran a script to check the account_id of the VAT payable accounts in each, and then based on the period of the tax transaction_id, I joined the TAX_ENTRY_LINE table onto the tax payable account based on those.

I used BI software called Qlik Sense to do the data modelling, which you'll hopefully to make out more or less what I did:

JOURNAL_TAX_ENTRY: LOAD JOURNAL_ENTRY_ID, INDEX, AMOUNT, PERCENT_BASED, NET_AMOUNT_TAXABLE, TAX_INCLUSIVE_AMOUNT, OVERRIDE_DELTA_AMOUNT, TAX_PERCENT, TAX_RATE_ID, _FIVETRAN_SYNCED;

SELECT "JOURNAL_ENTRY_ID", "INDEX", "AMOUNT", "PERCENT_BASED", "NET_AMOUNT_TAXABLE", "TAX_INCLUSIVE_AMOUNT", "OVERRIDE_DELTA_AMOUNT", "TAX_PERCENT", "TAX_RATE_ID", "_FIVETRAN_SYNCED" FROM "FIVETRAN_DATABASE"."$(voriginal_schema)"."JOURNAL_ENTRY_TAX_LINE";

LEFT JOIN (JOURNAL_TAX_ENTRY) LOAD DISTINCT TRANSACTION_ID as JOURNAL_ENTRY_ID, TRANSACTION_DATE;

SELECT "TRANSACTION_ID", "TRANSACTION_DATE", "ACCOUNT_ID", "ACCOUNT_NAME", "PARENT_ACCOUNT_NAME", "TRANSACTION_TYPE", "ACCOUNT_TRANSACTION_TYPE" FROM "FIVETRAN_DATABASE"."$(vformatted_schema)"."QUICKBOOKS__GENERAL_LEDGER";

BALANCE_PREPARATION: LOAD DISTINCT DATE_MONTH RESIDENT BALANCE_SHEET;

LEFT JOIN (BALANCE_PREPARATION) LOAD MAXSTRING(ACCOUNT_ID) AS ACCOUNT_ID RESIDENT BALANCE_SHEET WHERE WILDMATCH(UPPER(ACCOUNT_NAME),'PAYABLE') AND WILDMATCH(UPPER(ACCOUNT_NAME),'VAT');

LEFT JOIN (BALANCE_PREPARATION) LOAD
MONTHSTART(TRANSACTION_DATE) AS DATE_MONTH, SUM(AMOUNT) as TAX_AMOUNT RESIDENT JOURNAL_TAX_ENTRY GROUP BY TRANSACTION_DATE; DROP TABLE JOURNAL_TAX_ENTRY;

FINAL_TAX_PREPARATION: // TEST: LOAD ACCOUNT_ID&'|'&NUM(DATE(DATE_MONTH)) as ACCOUNT_PERIOD_KEY, TAX_BALANCE; LOAD ACCOUNT_ID, DATE_MONTH, TAX_AMOUNT, RANGESUM(PEEK('TAX_BALANCE'),TAX_AMOUNT) AS TAX_BALANCE RESIDENT BALANCE_PREPARATION ORDER BY DATE_MONTH ASC;

LEFT JOIN (BALANCE_SHEET) LOAD ACCOUNT_PERIOD_KEY, SUM(TAX_BALANCE) AS TAX_BALANCE RESIDENT FINAL_TAX_PREPARATION GROUP BY ACCOUNT_PERIOD_KEY;

DROP TABLE FINAL_TAX_PREPARATION; DROP TABLE BALANCE_PREPARATION;

BALANCE_SHEET_UPDATED: NOCONCATENATE LOAD SOURCE, COMPANY, DATE_MONTH, ACCOUNT_NAME, ACCOUNT_CODE, ACCOUNT_ID, ACCOUNT_TYPE, ACCOUNT_CLASS, ACCOUNT_PERIOD_KEY, [ACCOUNT_ORDINAL], NET_AMOUNT+IF(ISNULL(TAX_BALANCE),0,TAX_BALANCE) AS NET_AMOUNT RESIDENT BALANCE_SHEET; DROP TABLE BALANCE_SHEET;

RENAME TABLE BALANCE_SHEET_UPDATED TO BALANCE_SHEET;

fivetran-joemarkiewicz commented 5 months ago

@finalspace-alwyn thanks so much for this insightful finding!

This would make sense why your general ledger isn't balancing out as we do not currently take the journal_entry_tax_line source into account when building the general ledger. The primary reason this source table was not included originally was solely due to lack of available data. When we designed and as we have updated the QuickBooks data model, we did not come across a dataset with these transactions. Now that you have raised this, we can clearly see these are records need to be included in order for balances to tie out when journal entry tax lines are present.

The only concern I have (which also resembles the same issue highlighted for the invoice tax line above) is which account_id the tax line should be associated with? I see you have included the following line which seems to match the tax line to the proper account:

WHERE WILDMATCH(UPPER(ACCOUNT_NAME),'PAYABLE') AND WILDMATCH(UPPER(ACCOUNT_NAME),'VAT')

This resembles a manual mapping that @caffeinebounce included in their solution here (but for invoice tax line). As the account_name can be different from customer to customer I don't feel this is the most scalable approach. Do you know if there is a way to dynamically associate the tax line entries with the proper account?

In the meantime, I will do some investigating to see if this suggestion above will work for the journal_entry_tax_line records

finalspace-alwyn commented 5 months ago

Hi @fivetran-joemarkiewicz,

Thanks for the prompt and detailed response. The current wildmatch solution I'm using is a very cowboy way of dealing with the issue since there's no procedural way to identify these accounts. Their ID's also differ from company to company, which is why I had to go for the account name.

I'll investigate further and let you know what I find - but I understand that this approach prohibits you from rolling out a fix.

fivetran-joemarkiewicz commented 5 months ago

Thanks for the partnership and your understanding here @finalspace-alwyn. It's important we find the right solution, and your input is extremely helpful in making sure we can have all the proper information to make the correct adjustments.

Since we are at the same crossroads with not directly being able to associate the proper tax account with these tax entries, I have reached out to our product manager of the QuickBooks connector to determine if there is a way to make this connection. It may result in us needing to talk with QuickBooks directly, but I will keep you all informed here if I am able to come up with a possible solution.

finalspace-alwyn commented 5 months ago

Always a pleasure @fivetran-joemarkiewicz, a solution would be mutually beneficial.

Logic dictates that since quickbooks has a whole function for dealing with value added tax and creating custom reports for it, the system must have an identifier to know which account it should use for it's built in VAT function.

I'm look forward to hearing back from the quicbkooks connector team.

finalspace-alwyn commented 4 months ago

@fivetran-joemarkiewicz We have a more serious issue at the moment - none of the balance sheets are reconciling to quickbooks due to the way Fivetran handles VAT Inclusive transactions.

The result is that all accounts balance except for the VAT inclusive affected accounts, being:

  1. Customer Control.
  2. Supplier Control.
  3. Current Account / Bank
  4. VAT Payable / (Receivable)

I noticed in the Supplier control account that VAT transactions are exclusive of VAT where they are actually Inclusive if you view it on the front end.

An example is a bill in Quickbooks:

Cr Supplier Control R16,562 Dr Municipal Expense R14,402 DR Vat Control R2,160

But Fivetran's "General Ledger" table shows:

Cr Supplier Control R14,402 Dr Municipal Expense R14,402

Completely ignoring the VAT portion of the transaction. This affects customer control as well.

What's strange though, is that despite the General ledger "ADJUSTED_AMOUNT" and "RUNNING_BALANCE" being wrong, the "ACCOUNT_CURRENT_BALANCE" field shows the correct balance.

This is obviously a huge problem as my entire client base is receiving incorrect information regarding their customers, suppliers and cash balances, adversely affecting their ability to make decisions.

I've also made a ticket on Fivetran, this issue is critical.

fivetran-joemarkiewicz commented 4 months ago

Hi @finalspace-alwyn thanks for raising this issue and I understand the criticality of this data needing to be accurate.

Based on the discovery in this thread, being that VAT is not properly accounted for, I believe I understand how you are seeing client environments running into the above issue. In the example you shared the Fivetran generated General Ledger entries are not able to link the proper tax amount to the VAT account. Therefore, resulting in an entry without any reference to VAT. In order to rectify this discrepancy I believe we would need to be able to understand how to attribute the tax amount to the appropriate VAT account. However, here seems to be a gap in the raw data which is stalling us from making this connection and therefore the proper GL entry in these cases. Please let me know if you disagree or if you believe this issue is entirely separate. I want to make sure I am understanding the issue properly and how it is occurring.

Regarding the "ACCOUNT_CURRENT_BALANCE" field being correct, this is expected as the value in this field is populated directly from QuickBooks. However, we are not able to rely on this field for our General Ledger or other financial report generation due to the field only reporting on the current balance as of the latest sync. This field will change following every sync and we will lose all historical information. Ideally, the latest value generated by the "RUNNING_BALANCE" should match what you see in "ACCOUNT_CURRENT_BALANCE".

Following my previous message from a few weeks ago, we have been trying to make contact with QuickBooks to get an understanding of why this is not possible giving the available endpoints. However, we have not had any luck establishing contact with the right individuals. While we have not had much luck on our end, there may be some steps we can take with you to get this pressing question addressed with help from QuickBooks.

Nevertheless, I believe a call to address this open issue would be most beneficial. Please feel free to schedule some time with me using the following link. Let me know if no times are available on your end. I am happy to adjust where possible to accommodate timezone differences.

finalspace-alwyn commented 4 months ago

Hi @fivetran-joemarkiewicz, this is a separate issue which I believe to have a much easier solution as the actual base tables you use to do the transformation has the right information in it - so I'm not sure how it's actually being rolled up incorrectly.

if I query "FIVETRAN_DATABASE.JETLINE_JDS_AFROPULSE.BILL" I get the following amount of R16,562.32 - which is correct and should be what's displayed in the GENERAL_LEDGER, it also clearly shows "GLOBAL_TAX_CALCULATION" as "Tax Inclusive" meaning there's no further action needed.

ID DOC_NUMBER GLOBAL_TAX_CALCULATION TOTAL_AMOUNT TRANSACTION_DATE 141 7678 TaxInclusive 16562.320000000000000000 2019-04-11

However, if I pull BILL_LINE for the same ID 141 I get 14,402.02:

BILL_ID | INDEX | DESCRIPTION | AMOUNT 141 | 0 | Ferum House 42712 | 14402.020000000

TRANSACTION_DATE AMOUNT | ACCOUNT_ID | TRANSACTION_SOURCE | ADJUSTED_AMOUNT 2019-04-11 | 14402.020000000 | 101 | bill | credit | 14402.020000000

So somehow when you transform the BILL table you transform the amount to remove tax where it's not necessary.

So finally, I did a SUM of AMOUNT in both BILL and BILL_ID and the difference is literally the difference on the supplier control account as per the below image:

image

fivetran-joemarkiewicz commented 4 months ago

@finalspace-alwyn thanks for your response and for clarifying the issue you are seeing.

Based on your example, it seems that the tax item is not recorded in the bill_line table. When looking at the transformation logic for double entry bills I can see that we are basing the granularity of bill records off the raw bill_line table. We can further see that we are only utilizing the bill_line.amount when recording the amount for the bill entries. It seems then that your source bill_line table does not include the tax entry within the bill_line for this bill. Is that correct? If so, is that expected?

My understanding in this scenario is we should see in the bill_line table an entry for the appropriate bill amount of 14402.02 (which you do already see) and then the appropriate tax entry in the bill_line table for this bill of the amount 2160.30. If we do not see this tax entry in the bill_line table, then I wonder if this is something missing from the connector as opposed to the transformations. I can also confirm we are not filtering out any tax entries within the bill_line table from our staging model. If this tax entry is not intended to be recorded in the bill_line raw table, then we likely will need to find some way to link the delta between bill and bill_line some other way.

I see you scheduled time to meet this week. We can discuss in more detail and investigate further when we meet. Looking forward to chatting then.

finalspace-alwyn commented 4 months ago

Hi Joe,

I reviewed the dbt models and understandably it seems that there is something missing from the connector - however I wonder if it's not possible to join BILL onto bill_line to check whether the global tax classification is "TaxInclusive" in which case we can calculate the tax using the appropriate TAX_ID.

But I agree, might be better to have that discussion in our session as I'm guessing more than anything at this point.

fivetran-joemarkiewicz commented 4 months ago

@finalspace-alwyn thanks for meeting with me today! Following our conversation, it seems there are a couple of updates we can explore on our end within the data model:

I believe there were still some outstanding discrepancies which we were unable to identify for another account type, but you were going to continue investigating to see if there were any trends that could lead to possible updates on the data model end.

Let me know if there were any items I missed in this recap, and I look forward to confirming these updates as we work to address them next week!