Feature/add all tax lines by fivetran-avinash · Pull Request #171 · fivetran/dbt_quickbooks

fivetran-avinash · 2025-07-17T16:29:20Z

PR Overview

Package version introduced in this PR:

0.22.0-a1

This PR addresses the following Issue/Feature(s):

[Feature] Incorporate Tax Lines into QBO Transform #153, [Bug] Sales Receipt Tax Amounts not flowing back up to the GL/IS/BS #132, [Bug] invoice.total_tax isn't going into general_ledger.amount #56
Will also address a piece of [Bug] Small fixes #101

Summary of changes:

Adding tax lines to our transaction models and thus adding those lines into the financial end models.

Submission Checklist

Alignment meeting with the reviewer (if needed)
- Timeline and validation requirements discussed
Provide validation details:
- Validation Steps: Check for unintentional effects (e.g., add/run consistency & integrity tests)
- Testing Instructions: Confirm the change addresses the issue(s)
- Focus Areas: Complex logic or queries that need extra attention
Merge any relevant open PRs into this PR

Changelog

Draft changelog for PR
Final changelog for release review

…e conflicts

Co-authored-by: Avinash Kunnath <avinash.kunnath@fivetran.com>

fivetran-joemarkiewicz

@fivetran-avinash thanks for working through these updates and combining these updates! A few comments/questions/suggestions before approving.

fivetran-avinash

@fivetran-joemarkiewicz Did some updates to the variable logic and I think everything is working now. Ready for re-review.

fivetran-joemarkiewicz

@fivetran-avinash a few more questions before pre-release.

fivetran-joemarkiewicz · 2025-08-06T14:48:24Z

+
+## Bringing in The Right Tax Accounts For Tax Lines
+- When bringing in tax lines ([see the README](https://github.com/fivetran/dbt_quickbooks?tab=readme-ov-file#step-4-enablingdisabling-models) for more details on how to enable/disable tax lines), we want to make sure we are associating each line with the right account, since they usually differ from the accounts for the regular lines. That way these lines are correctly tracked, generally as liabilities. 
+- Our initial logic maps the tax agency display name by appending 'Payable' to the end of the name. That should match at least one account name, which is the appropriate account for that tax line. 


I recall there was a QuickBooks doc that led us to this understanding. Can we link that here.

Finally found it I think in this support question. https://quickbooks.intuit.com/learn-support/en-ca/taxes/separate-tax-remittance-accounts/00/740623

fivetran-joemarkiewicz · 2025-08-06T14:55:54Z

+  # - package: fivetran/quickbooks_source
+  #   version: 0.14.0-a1
+  - git: https://github.com/fivetran/dbt_quickbooks_source.git
+    revision: feature/add-all-tax-lines
+    warn-unpinned: false


Reminder to update before pre-release

Bump to change before pre-release

fivetran-joemarkiewicz · 2025-08-06T15:44:04Z

 ),

+
+{% if var('using_sales_receipt_tax_line', False) %}


when the following variables are set I run into a general ledger unique id test failure. Can you look into why that would be the case.

using_sales_receipt_tax_line: true using_tax_agency: true using_tax_rate: false

The same happens for invoice and I imagine the others.

Tax agency shouldn't be true when tax rate is false, so reconfigured the variables so that tax agency is only true when tax rate is enabled.

fivetran-avinash

@fivetran-joemarkiewicz Ready for re-review. Variables reconfigured and tested that several configurations worked as expected.

fivetran-joemarkiewicz

@fivetran-avinash a few final questions before ready for pre-release.

fivetran-joemarkiewicz · 2025-08-07T16:08:54Z

+{% endif %}

-{% if var('using_invoice_bundle', True) %}
+{% if var('using_invoice_bundle', False) %}


Was this an intentional change? It looks like default is still meant to be true per our README docs.

No, it wasn't, reverted.

fivetran-joemarkiewicz · 2025-08-07T16:22:39Z

+{% if using_tax_agency %}
+tax_agencies as (
+
+    select *
+    from {{ ref('stg_quickbooks__tax_agency') }}
+),
+
+{% endif %}
+
+{% if using_tax_rate %}
+tax_rates as (
+
+    select *
+    from {{ ref('stg_quickbooks__tax_rate') }}
+),
+
+{% endif %} 


Would a customer ever use these tax variables, when using_journal_entry_tax_line is disabled? If not, why have them exist outside that conditional? They should all be in the same conditional block.

You can also likely consolidate conditionals this way

Consolidated.

fivetran-avinash

@fivetran-joemarkiewicz ready for re-review pending Buildkite

fivetran-joemarkiewicz

Looks good for pre-release following the latest updates!

fivetran-joemarkiewicz · 2025-08-11T19:19:41Z

+  # - package: fivetran/quickbooks_source
+  #   version: 0.14.0-a1
+  - git: https://github.com/fivetran/dbt_quickbooks_source.git
+    revision: feature/add-all-tax-lines
+    warn-unpinned: false


Bump to change before pre-release

* bugfix/invoice-bundles * additional bundle updates and minor global tax update * minor updates * Update README.md * Update CHANGELOG.md * review updates

…an/dbt_quickbooks into feature/add-all-tax-lines

fivetran-avinash · 2025-10-20T16:06:41Z

Closing this, as we have released newer versions of this package and [PR #179] now handles the latest update.

fivetran-avinash and others added 16 commits July 8, 2025 17:28

Update invoice tax line data

3e354c7

Update seed files to test new invoice tax line logic

95c441f

Documentation

771f86b

Q2 FY26: Apply automated update.

2512e79

Merge conflicts

60cb98e

docs

be5ae28

Add all tax lines

1685748

Variable flip to false enable

7219aea

New seed sources

49822ad

Update integration_tests data/config

b125eb1

update quickstart.yml and add variables

ecb8196

Model fixes

c68ebdf

comment out disabled models

153e8cb

Q2 FY26: Update auto-release workflow only.

82822a1

PR changes plus new tax line PR updates

0a6d16a

Apply new variable config logic to double entry Models + resolve merg…

e52d2d6

…e conflicts

fivetran-avinash self-assigned this Jul 30, 2025

fivetran-avinash and others added 4 commits July 29, 2025 17:38

Resolve merge conflicts from docs generator PR

a5c6f5c

small change to merge

3c32e2f

small change to merge

685cf75

Trigger merge to close PR (#170)

d2b70af

Co-authored-by: Avinash Kunnath <avinash.kunnath@fivetran.com>

fivetran-avinash marked this pull request as ready for review July 31, 2025 03:41

packages.yml

5cdeecb

fivetran-joemarkiewicz self-requested a review August 4, 2025 20:28

fivetran-joemarkiewicz requested changes Aug 4, 2025

View reviewed changes

PR review

b780f1b

fivetran-avinash commented Aug 5, 2025

View reviewed changes

fivetran-joemarkiewicz requested changes Aug 6, 2025

View reviewed changes

PR re-review

921024c

fivetran-avinash commented Aug 6, 2025

View reviewed changes

fivetran-joemarkiewicz requested changes Aug 7, 2025

View reviewed changes

PR notes

74129d2

fivetran-avinash commented Aug 7, 2025

View reviewed changes

fivetran-joemarkiewicz reviewed Aug 11, 2025

View reviewed changes

pin to pre-release package

edd4831

fivetran-avinash added docs:ready Triggers the docs generator workflow. pre-release Triggers the auto-releaser workflow. labels Aug 12, 2025

fivetran-joemarkiewicz and others added 5 commits August 26, 2025 13:45

address merge conflicts

1cc667b

cleanup merge conflicts

8a47956

bugfix/invoice-bundles (#175)

dd6bd1d

* bugfix/invoice-bundles * additional bundle updates and minor global tax update * minor updates * Update README.md * Update CHANGELOG.md * review updates

update for pre-release

1ff58e3

packages for docs gen

3d22662

fivetran-avinash added docs:ready Triggers the docs generator workflow. and removed docs:ready Triggers the docs generator workflow. pre-release Triggers the auto-releaser workflow. labels Aug 28, 2025

Generate dbt docs via GitHub Actions

166ed21

fivetran-avinash added the pre-release Triggers the auto-releaser workflow. label Aug 28, 2025

fivetran-avinash added 2 commits August 28, 2025 12:00

changelog + packages

b317823

Merge branch 'feature/add-all-tax-lines' of https://github.com/fivetr…

4488fa1

…an/dbt_quickbooks into feature/add-all-tax-lines

fivetran-avinash added docs:ready Triggers the docs generator workflow. and removed docs:ready Triggers the docs generator workflow. labels Aug 28, 2025

Generate dbt docs via GitHub Actions

8d2f73d

fivetran-avinash closed this Oct 20, 2025

Conversation

fivetran-avinash commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Overview

Submission Checklist

Changelog

Uh oh!

fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fivetran-avinash left a comment

Choose a reason for hiding this comment

Uh oh!

fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fivetran-avinash left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fivetran-avinash left a comment

Choose a reason for hiding this comment

Uh oh!

fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fivetran-avinash commented Oct 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fivetran-avinash commented Jul 17, 2025 •

edited

Loading

fivetran-avinash left a comment •

edited

Loading