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.

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

[Bug Fix] Net income adjustment for no revenue/expense lines #149

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

fivetran-avinash
Copy link
Contributor

@fivetran-avinash fivetran-avinash commented Dec 10, 2024

PR Overview

This PR will address the following Issue/Feature: [#145]

This PR will result in the following new package version:

Non breaking change as we are only changing the logic to ensure revenue and expense lines are brought down and kept not null

Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:

Bug Fix

  • Updated the logic in int_quickbooks__retained_earnings to ensure accounting periods with no revenue and expense class lines were accounted for.
    • This will ensure the net income adjustment is available regardless of existing revenue or expenses.

PR Checklist

Basic Validation

Please acknowledge that you have successfully performed the following commands locally:

  • dbt run –full-refresh && dbt test
  • [NA] dbt run (if incremental models are present) && dbt test

Before marking this PR as "ready for review" the following have been applied:

  • The appropriate issue has been linked, tagged, and properly assigned
  • All necessary documentation and version upgrades have been applied
  • docs were regenerated (unless this PR does not include any code or yml updates)
  • BuildKite integration tests are passing
  • Detailed validation steps have been provided below

Detailed Validation

Please share any and all of your validation steps:

Validation on development production was passing.

Screenshot 2024-12-16 at 9 55 09 PM

Customer also confirmed pre-release worked as expected on Quickstart.

If you had to summarize this PR in an emoji, which would it be?

🩺

Copy link
Contributor

@fivetran-joemarkiewicz fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not approving yet since this is not intended to be merged as of this moment. However, this is appropriate to cut a pre-release off for v0.17.1-a1

@fivetran-avinash fivetran-avinash changed the title [Pre-release v2] Bug fix for net income adjustment for no revenue/expense lines [Bug Fix] Net income adjustment for no revenue/expense lines Dec 17, 2024
@fivetran-avinash
Copy link
Contributor Author

This is now ready for official PR review.

Copy link
Contributor

@fivetran-joemarkiewicz fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look good to me. However, I noticed not related to these changes, but in this update we didn't bring the created_at and updated_at fields into the final invoice double entry query (see here). Can you please add those into this update as well.

Copy link
Contributor Author

@fivetran-avinash fivetran-avinash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fivetran-joemarkiewicz Changes applied. Ready for re-review

Copy link
Contributor

@fivetran-joemarkiewicz fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Net Income Adjustment is not applied for QBO instances with no revenue lines
2 participants