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] Net Income Adjustment is not applied for QBO instances with no revenue lines #145

Open
2 of 4 tasks
brandonrf94 opened this issue Nov 13, 2024 · 8 comments
Open
2 of 4 tasks
Labels
error:unforced status:scoping Currently being scoped type:bug Something is broken or incorrect

Comments

@brandonrf94
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the issue

I have a QBO instance that has expenses logged and the net income adjustment is not coming through.

Following the code path for net income adjustment it looks like this code here: https://github.com/fivetran/dbt_quickbooks/blob/ea05fe5647b67fc04b3eb355856d5ca2254495ec/models/intermediate/int_quickbooks__retained_earnings.sql#L35C1-L42C3

assumes that QBO will have revenue and expenses to compute net income adjustment. But what about in cases where there is no revenue (or expenses) recognized yet - only one of the two or neither. The account should still be returned.

Relevant error log or model output

No response

Expected behavior

Net income adjustment is available regardless of revenue or expenses existing.

Possible solution

No response

dbt Project configurations

Most recent quickstart

Package versions

Most recent quickstart

What database are you using dbt with?

snowflake

How are you running this dbt package?

Fivetran Quickstart Data Model

dbt Version

Most recent quickstart

Additional Context

No response

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

  • Yes.
  • Yes, but I will need assistance.
  • No.
@brandonrf94 brandonrf94 added the type:bug Something is broken or incorrect label Nov 13, 2024
@fivetran-avinash
Copy link
Contributor

Hey @brandonrf94 , good callout! I wonder if we can adjust the base of the CTE to account for this by pulling directly from the original intermediate model. Something like

net_income_loss as (

    select 
        general_ledger_balances.period_first_day,
        general_ledger_balances.source_relation,
        coalesce(revenue_net_change, 0) as revenue_net_change, 
        coalesce(revenue_net_converted_change, 0) as revenue_net_converted_change,
        coalesce(expense_net_change, 0) as expense_net_change,
        coalesce(expense_net_converted_change, 0) as expense_net_converted_change
    from general_ledger_balances
    left join revenue_starter
        on general_ledger_balances.period_first_day = revenue_starter.period_first_day
        and general_ledger_balances.source_relation = revenue_starter.source_relation
    left join expense_starter 
        on general_ledger_balances.period_first_day = expense_starter.period_first_day
        and general_ledger_balances.source_relation = expense_starter.source_relation
),

Might do the trick in terms of bringing in those periods which are lacking revenue/expense lines. If you grab the compiled code from your target folder and substitute this CTE instead of the existing one, does that make a difference?

@brandonrf94
Copy link
Author

How do I get the compiled code when using a quickstart?

@fivetran-avinash
Copy link
Contributor

Ahh good call @brandonrf94 🤦🏽 . Let me try and get a pre-release branch ready for you to test these changes by early next week.

We also are going to refactor the CTE to be more optimal computationally. Something like


net_income_loss as (

    select
        period_first_day,
        source_relation,
        sum(case when account_class = 'Revenue' then period_net_change else 0) as revenue_net_change,
        sum(case when account_class = 'Revenue' then period_net_converted_change else 0) as revenue_net_converted_change,
        sum(case when account_class = 'Expense' then period_net_change else 0) as expense_net_change,
        sum(case when account_class = 'Expense' then period_net_converted_change else 0) as expense_net_converted_change
    from general_ledger_balances
    {{ dbt_utils.group_by(2) }} 
),

will replace all the existing CTEs.

@fivetran-avinash
Copy link
Contributor

fivetran-avinash commented Nov 21, 2024

Hi @brandonrf94 , just to give you an update, we are creating a pre-release branch for you to test and validate that these changes will work for you in Quickstart. We are hoping to have this released soon, and then we can sync with you on how to test this branch internally.

@brandonrf94
Copy link
Author

Hi @fivetran-avinash - is this ready for testing?

@fivetran-avinash
Copy link
Contributor

Hi @brandonrf94, I've created this pre-release for testing, but we are dealing with issues in our Quickstart deployment process. Once this is live on Quickstart, we'll let you know the process for testing this out.

@brandonrf94
Copy link
Author

hi @fivetran-avinash - any luck here?

@fivetran-avinash
Copy link
Contributor

Hi @brandonrf94, we are unfortunately still waiting for a resolution from our engineering team, there are a few dependencies that need to be resolved before this is ready to go live. We will be in touch when this is ready to try out in Quickstart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error:unforced status:scoping Currently being scoped type:bug Something is broken or incorrect
Projects
None yet
Development

No branches or pull requests

2 participants