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.

Sign up for GitHub

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

Jump to bottom

[Bug] <General Ledger/Balance Sheet accounts Don't balance because there are no entries for Credit Card Payments> #47

Closed
2 of 4 tasks
mikerenderco opened this issue Sep 20, 2022 · 38 comments
Closed
2 of 4 tasks

[Bug] <General Ledger/Balance Sheet accounts Don't balance because there are no entries for Credit Card Payments> #47

mikerenderco opened this issue Sep 20, 2022 · 38 comments
Assignees
@fivetran-joemarkiewicz
Labels
priority:p2 Affects most users; fix needed status:in_review Currently in review type:enhancement New functionality or enhancement update_type:feature Primary focus is to add new functionality

Comments

@mikerenderco
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Describe the issue

We created a Balance Sheet from the General Ledger and we have also pulled the Balance Sheet that is provided. Fivetrans has the Quickbooks Table pulling in a table called credit_card_payment_txn. When you try to balance a Credit Card Account or the Cash account from a bank account from the General Ledger and the Balance these two accounts will always be off by the amounts in the credit_card_payment_txn table.

It seems like these need to be entries in the General Ledger.

Relevant error log or model output

No response

Expected behavior

The expected behavior is to have all accounts balance from the general_ledger and the balance_sheet that this code produces.

dbt Project configurations

This is the standard configuration.

Package versions

packages:

  • package: fivetran/quickbooks
    version: [">=0.5.1", "<0.6.0"]

What database are you using dbt with?

bigquery

dbt Version

DBT Cloud Environment

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.
@mikerenderco mikerenderco added the bug Something isn't working label Sep 20, 2022
@fivetran-joemarkiewicz
Copy link
Contributor

Hi @mikerenderco thanks for opening this issue!

I agree that in this situation including the credit_card_payment_txn source table into our General Ledger (and other financial statement) downstream models would account for the differences you are currently seeing. The data we used to develop and maintain this package actually does not leverage the credit_card_payment_txn table and therefore was not included in the original build of the package.

However, we are happy to consider adding the table. In order to do so, I am curious if you would be open to sharing more details about this table. For example, do you have sample data (could be a few hashed records within a .csv) that you would be comfortable sharing for our development efforts? Additionally, can you confirm that this table contains all the required information to fold into the downstream financial statements, or are there additional joins (outside of the account joins) that you believe would need to be factored in?

Let me know your thoughts. We would be happy to incorporate this model, but want to make sure we are making correct updates before jumping into the new table and downstream models.

@mikerenderco
Copy link
Contributor Author

@fivetran-joemarkiewicz great points here. I don't know that the credit_card_payment_txn contains all the fields you would need to build data into the GL and Balance Sheet. What I ended up doing for our client is union-ing to additional tables in Big query to the GL. I have attached our table with example records.
quickbooks.credit_card_payment_txn.csv
Also here is where the SQL that I was using built off of this table to build a view that contains your GL (Plus Class info) and unioning two other tables - 1 for the credit card account debit and 1 for the bank account debit. Please note this is standard sql from big query.

Select t1.unique_id,t1.transaction_id, t1.transaction_index,t1.transaction_date,t1.customer_id,t1.vendor_id,t1.amount,t1.account_id, t1.account_number, t1.account_name,t1.is_sub_account,t1.parent_account_number,t1.account_type,t1.account_sub_type,t1.financial_statement_helper,t1.account_current_balance,t1.account_class,t1.transaction_type,t1.transaction_source,t1.account_transaction_type,t1.adjusted_amount,t1.running_balance,
case
when t1.transaction_source = 'journal_entry' then t2.fully_qualified_name
when t1.transaction_source = 'purchase' then t3.fully_qualified_name
when t1.transaction_source = 'bill' then t4.fully_qualified_name
when t1.transaction_source = 'deposit' then t5.fully_qualified_name
--when t1.transaction_source = 'bill_payment' then t6.fully_qualified_name
when t1.transaction_source = 'vendor_credit' then t7.fully_qualified_name
when t1.transaction_source = 'invoice' then t8.fully_qualified_name
when t1.transaction_source = 'sales_receipt' then t9.fully_qualified_name
else null
END as FullyQualifiedClassName,
case
when t1.transaction_source = 'journal_entry' then t2.name
when t1.transaction_source = 'purchase' then t3.name
when t1.transaction_source = 'bill' then t4.name
when t1.transaction_source = 'deposit' then t5.name
--when t1.transaction_source = 'bill_payment' then t6.name
when t1.transaction_source = 'vendor_credit' then t7.name
when t1.transaction_source = 'invoice' then t8.name
when t1.transaction_source = 'sales_receipt' then t9.name
else null
END as ClassName,
last_day(t1.transaction_date) AS period_end_date

From qb_quickbooks.quickbooks__general_ledgert1

--JOURNAL ENTRY JOIN
left join quickbooks_custom_views.je_summary t2 on t1.transaction_source = 'journal_entry' and t1.transaction_id = t2.journal_entry_id and t1.transaction_index = t2.index

--PURCHASE JOIN
left join quickbooks_custom_views.purchase_summary t3 on t1.transaction_source = 'purchase' and t1.transaction_id = t3.purchase_id and t1.transaction_index = t3.index

--BILL JOIN
left join quickbooks_custom_views.bill_summary t4 on t1.transaction_source = 'bill' and t1.transaction_id = t4.bill_id and t1.transaction_index = t4.index

--DEPOSIT JOIN
left join quickbooks_custom_views.deposit_summary t5 on t1.transaction_source = 'deposit' and t1.transaction_id = t5.deposit_id and t1.transaction_index = t5.index

--BILL PAYMENT JOIN
--left join quickbooks_custom_views.bp_summary t6 on t1.transaction_source = 'bill_payment' and t1.transaction_id = t6.bill_payment_id and t1.transaction_index = t6.index

--VENDOR CREDIT PAYMENT JOIN
left join quickbooks_custom_views.vc_summary t7 on t1.transaction_source = 'vendor_credit' and t1.transaction_id = t7.vendor_credit_id and t1.transaction_index = t7.index

--INVOICE PAYMENT JOIN
left join quickbooks_custom_views.invoice_summary t8 on t1.transaction_source = 'invoice' and t1.transaction_id = t8.invoice_id and t1.transaction_index = t8.index

--SALES RECEIPT PAYMENT JOIN
left join quickbooks_custom_views.sr_summary t9 on t1.transaction_source = 'sales_receipt' and t1.transaction_id = t9.sales_receipt_id and t1.transaction_index = t8.index

where t1.unique_id not in (select unique_id from qb_quickbooks.quickbooks__general_ledger where account_number ='1010' and transaction_source = 'bill payment' and transaction_index=1)

UNION ALL
--Credit Card Payment Cedit Card Account General Ledger Transactions

Select
GENERATE_UUID() as unique_id,
t1.id as transaction_id,
0 as transaction_index,
t1.transaction_date,
null as customer_id,
null as vendor_id,
t1.amount,
t1.credit_card_account_id as account_id,
'2104' as account_number,
'Credit Card' as account_name,
false as is_sub_account,
'2104' as parent_account_number,
'Cradit Card' as account_type,
'CreditCard' as account_sub_type,
'balance_sheet' as fincial_statement_helper,
(select max(account_current_balance) From quickbooks_custom_views.general_ledger where account_number ='2104') as account_current_balance,
'Liability' as account_class,
'credit' as transaction_type,
'credit_card_payment' as transaction_source,
'credit' as account_transaction_type,
-t1.amount as adjusted_amount,
0 as running_balance,
null as FullyQualifiedClassName,
null as ClassName,
last_day(t1.transaction_date) AS period_end_date
From quickbooks.credit_card_payment_txn t1

union all

--Credit Card Payment Bank Account General Ledger Transactions
Select
GENERATE_UUID() as unique_id,
t1.id as transaction_id,
0 as transaction_index,
t1.transaction_date,
null as customer_id,
null as vendor_id,
t1.amount,
t1.bank_account_id as account_id,
'1010' as account_number,
'BOK Checking (3276)' as account_name,
false as is_sub_account,
'1010' as parent_account_number,
'Bank' as account_type,
'Checking' as account_sub_type,
'balance_sheet' as fincial_statement_helper,
(select max(account_current_balance) From quickbooks_custom_views.general_ledger where account_number ='1010') as account_current_balance,
'Asset' as account_class,
'debit' as transaction_type,
'credit_card_payment' as transaction_source,
'debit' as account_transaction_type,
-t1.amount as adjusted_amount,
0 as running_balance,
null as FullyQualifiedClassName,
null as ClassName,
last_day(t1.transaction_date) AS period_end_date
From quickbooks.credit_card_payment_txn t1

@fivetran-joemarkiewicz
Copy link
Contributor

This is all great information, thanks so much @mikerenderco! Our team can scope out this enhancement to the package in our next sprint. Realistically I would imagine we could fold this into our package in October.

The code you shared though is fantastic and will really help us out. I would love for your to be credited for this contribution. If you would like to open a PR to even just add the csv file to our integration_tests/seeds/ folder we can credit your contribution 😄.

Regardless, we will plan to coordinate more here on any other questions we may have or any updates on our end. Thanks and I will keep you posted!

@lxuis97
Copy link

lxuis97 commented Sep 26, 2022

Hey @fivetran-joemarkiewicz,

What's the expected time to solve this issue?

Thanks!

@fivetran-joemarkiewicz
Copy link
Contributor

Hi @lxuis97 you must have a sixth sense! I am actually exploring this as we speak. While I don't have a fix at the moment, I am working on the implementation outlined above. I will plan to keep you updated as I progress in this thread 😄

@lxuis97
Copy link

lxuis97 commented Sep 26, 2022

Haha great! Good to know 😊

Merged
14 tasks
@fivetran-joemarkiewicz
Copy link
Contributor

HI @mikerenderco and @lxuis97 I have been able to get a working version of the package with the credit_card_payment_txn source baked into the General Ledger model (see the draft PR linked above if you want to see the details)! It would be great if you could give the branch a try and let me know if the implementation works on your data.

To test out the branch, you can use the following in your packages.yml:

packages:
  - git: https://github.com/fivetran/dbt_quickbooks_source.git
    revision: feature/credit-card-payments
    warn-unpinned: false

Once you add the contents to your packages.yml you can run dbt deps to install the new code and dbt run to generate your models. I look forward to hearing your feedback and if the added credit card payments fixes your issue!

@mikerenderco
Copy link
Contributor Author

mikerenderco commented Oct 12, 2022 via email

@fivetran-joemarkiewicz
Copy link
Contributor

Hey @mikerenderco were you able to add the above packages.yml config to your project? If so, you will need to run dbt deps before running in order to see the updates.

@mikerenderco
Copy link
Contributor Author

mikerenderco commented Oct 12, 2022 via email

@mikerenderco
Copy link
Contributor Author

mikerenderco commented Oct 12, 2022 via email

@fivetran-joemarkiewicz
Copy link
Contributor

@mikerenderco my apologies, I forgot to mention that you will need to add a variable to your dbt_project.yml

vars:
    using_credit_card_payment_txn: true

We set it to false by default since only a number of customers leverage this table.

@mikerenderco
Copy link
Contributor Author

mikerenderco commented Oct 13, 2022 via email

@fivetran-joemarkiewicz
Copy link
Contributor

Thanks for the quick reply and looks like we are getting somewhere! Did you scope the variable above globally? You will need to make sure the variable is global or either listed under both quickbooks_source and quickbooks.

Are you able to see that transaction ID you are sharing in the staging table? Additionally, I am unable to see the images you are including in the messages 😢

@mikerenderco
Copy link
Contributor Author

Hey Joe - Sorry about the images. I don't think i understand how to declare my variables globally. Let me try to post directly in Git.

  1. I checkout the branch in development
  2. I open the dbt_project.yml under the quickbooks_source folder
  3. on the last line i paste your addition of using_credit_card_payment_txn: true
  4. I then save the file
  5. I then merge to main
  6. I then dbt run
  7. I don't see a folder titled quickbooks or a way to declare the variables globally
  8. This is my unfamiliarity with dbt I think - if you can walk me through it i can test this.

@fivetran-joemarkiewicz
Copy link
Contributor

Ahhhh you want to edit the variable in your root dbt_project.yml instead of the dbt_project.yml in the package.

Here is how I have it setup on my project:
image

@mikerenderco
Copy link
Contributor Author

Ok Joe - I was able to get that in the global dbt_project. It runs successfully and moves the temp tables over for credit_card_payment_txn but I still don't see the records coming into the GL.

Here is my dbt_project.yml:
Screenshot 2022-10-13 142146

I tried matching my global dbt_project.yml exactly like yours but it won't compile. Says that there are errors.

@fivetran-joemarkiewicz
Copy link
Contributor

Ah ha!! I think I see what the issue is. It looks like you are only installing the source package (as the snippet I sent above only is for the source package 🤦 , my apologies). Would you be able to try this snippet and see if you see the GL model updated?

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

@mel-restori
Copy link
Contributor

mel-restori commented Nov 7, 2022 •
edited
Loading

Hi @fivetran-joemarkiewicz ! I just started working with a team using Fivetran's dbt_quickbook integration and I'm investigating a credit card payment discrepancy. I'm diving into this thread and the PR you linked to. I'm new to Fivetran's dbt projects so apologies in advance if some of these questions are rather basic.

  • First question - is that PR the latest place to look for updates on credit card payments?

  • If so, I notice that stg_quickbooks__credit_card_payment_txn is never defined in dbt_project.yml. I assume it needs that definition to tie it to the raw table credit_card_payment_txn or is that defined elsewhere?

  • Related, I'm confused by this line - I don't see the field is_most_recent_record in credit_card_payment_txn. Where can I see the logic that builds the field?

  • Also, I haven't used dbt code as an imported package before so this question is pretty basic - if we set a variable in our dbt_project.yml (using_credit_card_payment_txn), will it overwrite the variable that is set in Fivetran's package dbt_project.yml? It doesn't matter in this case because you haven't declared that variable as far as I can tell in that PR, but I guess one day you will and the default will likely be false and we'll want in flipped to true.

Thank you!

@fivetran-joemarkiewicz
Copy link
Contributor

Hi @mel-restori thanks for jumping into the thread! Hopefully I can help with some of your questions!

First question - is that PR the latest place to look for updates on credit card payments?

  • Currently I would reference the feature/backwards-credit-card branch instead for the time being. At the moment we have a number of changes in flight (including this one) due to an upcoming dbt-utils v1.0.0 release that we are working on migration support within PR Updates for dbt-utils to dbt-core cross-db macro migration #51. However, we have noticed from this thread that users of the QuickBooks dbt package could benefit from these updates being rolled out prior to the migration updates. Therefore we are working on the branch highlighted above to possible merge before the migration updates later this month. In the packages.yml I would just reference the new branch for the time being if you wanted to try it out.

If so, I notice that stg_quickbooks__credit_card_payment_txn is never defined in dbt_project.yml. I assume it needs that definition to tie it to the raw table credit_card_payment_txn or is that defined elsewhere?

  • It will be best practice for us to define the variable you mentioned in the dbt_project.yml. However, the current configuration should still work for the time being. But you are correct, and we will make sure to define the variable before official release.

Related, I'm confused by this line - I don't see the field is_most_recent_record in credit_card_payment_txn. Where can I see the logic that builds the field?

  • This field is actually generated within the dbt_quickbooks_source package (which is a dependency defined within the packages.yml). This line in particular is generated here

Also, I haven't used dbt code as an imported package before so this question is pretty basic - if we set a variable in our dbt_project.yml (using_credit_card_payment_txn), will it overwrite the variable that is set in Fivetran's package dbt_project.yml? It doesn't matter in this case because you haven't declared that variable as far as I can tell in that PR, but I guess one day you will and the default will likely be false and we'll want in flipped to true.

  • That is correct! The variables are created and defined so users are able to adjust the behavior of the package without needing to modify the code of the package directly. We will set the variable to false by default since the majority of users do not yet leverage this source table. However, since you do, you will be able to edit the variable in your own dbt_project.yml to change how the package functions.

Let me know if you have any other questions!

@mel-restori
Copy link
Contributor

Hi Joe, thanks so much for all this!
It's strange because stg_quickbooks__credit_card_payment_txn was never materialized, which makes me think maybe the variable wasn't set in the right spot. But I'll see about pointing to the new branch and rerunning to see if that helps resolve our cc payment issues. Thanks again!

@mel-restori
Copy link
Contributor

Hi @fivetran-joemarkiewicz ! We went ahead and pointed to that other branch and finally see the credit card payments come through! 🎉 However, our discrepancy doubled in size 😭

It seems that the payments are coming through with the opposite signs. I think in the credit card accounts, the credit card payments should be positive, while in the bank account they should be negative. I think that's determined by debit vs credit in lines 36 and 50 here)., but I'll be honest that accounting terminology is new to me.

Does that sound right to you? Thanks again!

@fivetran-joemarkiewicz
Copy link
Contributor

Hey @mel-restori that was actually something I was not entirely sure about when I added it to the branch and wanted to get confirmation on. I think you pointing this out is a great step forward in understanding the behavior of these transactions! I believe to fix this, we would just need to switch credit_card_account_id on line 35, with bank_account_id on line 59.

You have been really helpful in this process and if you wanted to contribute that quick switch in a PR, I would be happy to merge it into this working branch! Otherwise, I can make the update and we can see if this fix does the trick for you.

@mel-restori mel-restori mentioned this issue Nov 10, 2022
Merged
14 tasks
@fivetran-joemarkiewicz
Copy link
Contributor

Thanks so much for opening the PR! I just approved and merged it into the working branch. Let me know if that reduces the discrepancy on your end. If not, we can switch back and continue investigating 😄

@mel-restori
Copy link
Contributor

thank you!! hopefully that does it 🤞🏻. I'll update likely tomorrow.

@mikerenderco
Copy link
Contributor Author

mikerenderco commented Dec 5, 2022 via email

@fivetran-joemarkiewicz
Copy link
Contributor

Hey @mikerenderco this has not been pushed to prod yet. We have this included in the migration PR #51 and were planning to officially release with those changes. However, that PR is waiting for the release of dbt-utils v1.0.0 (which has now been delayed a number of times).

As the release of dbt-utils v1.0.0 is taking longer than expected, we can prioritize releasing these updates before dbt Labs releases dbt-utils v1.0.0.

@mikerenderco
Copy link
Contributor Author

mikerenderco commented Dec 5, 2022 via email

Merged
14 tasks
@fivetran-joemarkiewicz
Copy link
Contributor

Sure thing @mikerenderco! I just opened the PR above to merge these changes in before the migration changes.

Before doing so, would you be able to give the changes a test and confirm all looks good on your end?

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

@fivetran-joemarkiewicz fivetran-joemarkiewicz added type:enhancement New functionality or enhancement priority:p2 Affects most users; fix needed update_type:feature Primary focus is to add new functionality status:in_review Currently in review and removed bug Something isn't working labels Dec 5, 2022
@mikerenderco
Copy link
Contributor Author

mikerenderco commented Dec 5, 2022 via email

@mel-restori
Copy link
Contributor

@mikerenderco I was troubleshooting this myself a couple weeks ago and seems to work for me now. Note that the branch Joe pointed you to today is different than the branch you were working off of in October, so maybe that's what's still causing issues for you? 🤷🏻‍♀️ Good luck!

@fivetran-joemarkiewicz
Copy link
Contributor

Thanks for confirming @mel-restori and that is great to hear that it worked on your end!

@mikerenderco, @mel-restori is correct in pointing out that the branch has changed. Would you be able to use the following in your packages.yml and try again?

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

Merged
14 tasks
@mikerenderco
Copy link
Contributor Author

mikerenderco commented Dec 12, 2022 via email

@mikerenderco
Copy link
Contributor Author

mikerenderco commented Dec 12, 2022 via email

@fivetran-joemarkiewicz
Copy link
Contributor

Hey all, this should be live now in v0.5.4!!

@mel-restori
Copy link
Contributor

awesome, thank you!!

@fivetran-joemarkiewicz
Copy link
Contributor

Closing this ticket as the updates are now live. Thanks @mel-restori and @mikerenderco for all your help in getting these updates tested and raising the initial issue to our team 😃

@mikerenderco
Copy link
Contributor Author

mikerenderco commented Dec 13, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fivetran-joemarkiewicz fivetran-joemarkiewicz

Labels
priority:p2 Affects most users; fix needed status:in_review Currently in review type:enhancement New functionality or enhancement update_type:feature Primary focus is to add new functionality
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mel-restori @lxuis97 @fivetran-joemarkiewicz @mikerenderco