fivetran / dbt_netsuite

Data models for Fivetran's Netsuite connector, built using dbt.
https://fivetran.github.io/dbt_netsuite/
Apache License 2.0
36 stars 34 forks source link

[Bug] Income Statement model not accounting for renamed account types #83

Closed JessVagnoni closed 11 months ago

JessVagnoni commented 12 months ago

Is there an existing issue for this?

Describe the issue

Sql script income_statement_sort_helper references account type name but it doesn't include account types renamed in Netsuite environments. In our instance, we have renamed our 'cost of goods sold' to 'cost of service' and the Income Statement model no longer picks up our accounts.

Relevant error log or model output

No response

Expected behavior

Instead of using account type name, we would expect the ID to be used instead so that any renamed account types are not excluded

dbt Project configurations

https://github.com/fivetran/dbt_netsuite/blob/main/dbt_project.yml

config-version: 2 name: 'netsuite' version: '0.9.0' require-dbt-version: [">=1.3.0", "<2.0.0"]

models: netsuite: +materialized: table +schema: netsuite netsuite: intermediate: +materialized: ephemeral netsuite2: intermediate: +materialized: ephemeral

vars: netsuite:

Netsuite staging models

netsuite_accounting_books: "{{ ref('stg_netsuite__accounting_books') }}"
netsuite_accounting_periods: "{{ ref('stg_netsuite__accounting_periods') }}"
netsuite_accounts: "{{ ref('stg_netsuite__accounts') }}"
netsuite_classes: "{{ ref('stg_netsuite__classes') }}"
netsuite_consolidated_exchange_rates: "{{ ref('stg_netsuite__consolidated_exchange_rates') }}"
netsuite_currencies: "{{ ref('stg_netsuite__currencies') }}"
netsuite_customers: "{{ ref('stg_netsuite__customers') }}"
netsuite_departments: "{{ ref('stg_netsuite__departments') }}"
netsuite_expense_accounts: "{{ ref('stg_netsuite__expense_accounts') }}"
netsuite_income_accounts: "{{ ref('stg_netsuite__income_accounts') }}"
netsuite_items: "{{ ref('stg_netsuite__items') }}"
netsuite_locations: "{{ ref('stg_netsuite__locations') }}"
netsuite_subsidiaries: "{{ ref('stg_netsuite__subsidiaries') }}"
netsuite_transaction_lines: "{{ ref('stg_netsuite__transaction_lines') }}"
netsuite_transactions: "{{ ref('stg_netsuite__transactions') }}"
netsuite_vendor_types: "{{ ref('stg_netsuite__vendor_types') }}"
netsuite_vendors: "{{ ref('stg_netsuite__vendors') }}"
netsuite2_account_types: "{{ ref('stg_netsuite2__account_types') }}"
netsuite2_accounting_book_subsidiaries: "{{ ref('stg_netsuite2__accounting_book_subsidiaries') }}"
netsuite2_accounting_books: "{{ ref('stg_netsuite2__accounting_books') }}"
netsuite2_accounting_period_fiscal_calendars: "{{ ref('stg_netsuite2__accounting_period_fiscal_cal') }}"
netsuite2_accounting_periods: "{{ ref('stg_netsuite2__accounting_periods') }}"
netsuite2_accounts: "{{ ref('stg_netsuite2__accounts') }}"
netsuite2_classes: "{{ ref('stg_netsuite2__classes') }}"
netsuite2_consolidated_exchange_rates: "{{ ref('stg_netsuite2__consolidated_exchange_rates') }}"
netsuite2_currencies: "{{ ref('stg_netsuite2__currencies') }}"
netsuite2_customers: "{{ ref('stg_netsuite2__customers') }}"
netsuite2_departments: "{{ ref('stg_netsuite2__departments') }}"
netsuite2_entities: "{{ ref('stg_netsuite2__entities') }}"
netsuite2_entity_address: "{{ ref('stg_netsuite2__entity_address') }}"
netsuite2_items: "{{ ref('stg_netsuite2__items') }}"
netsuite2_jobs: "{{ ref('stg_netsuite2__jobs') }}"
netsuite2_location_main_address: "{{ ref('stg_netsuite2__location_main_address') }}"
netsuite2_locations: "{{ ref('stg_netsuite2__locations') }}"
netsuite2_subsidiaries: "{{ ref('stg_netsuite2__subsidiaries') }}"
netsuite2_transaction_accounting_lines: "{{ ref('stg_netsuite2__transaction_accounting_lines') }}"
netsuite2_transaction_lines: "{{ ref('stg_netsuite2__transaction_lines') }}"
netsuite2_transactions: "{{ ref('stg_netsuite2__transactions') }}"
netsuite2_vendor_categories: "{{ ref('stg_netsuite2__vendor_categories') }}"
netsuite2_vendors: "{{ ref('stg_netsuite2__vendors') }}"
accounts_pass_through_columns: []
classes_pass_through_columns: []
departments_pass_through_columns: []
transactions_pass_through_columns: []
transaction_lines_pass_through_columns: []
balance_sheet_transaction_detail_columns: []
income_statement_transaction_detail_columns: []

Package versions

https://github.com/fivetran/dbt_netsuite/blob/main/packages.yml

packages:

What database are you using dbt with?

snowflake

dbt Version

% dbt --version Core:

Plugins:

Additional Context

     transactions_with_converted_amounts.account_category as account_category,
        case when lower(accounts.type_name) = 'income' then 1
            when lower(accounts.type_name) = 'cost of goods sold' then 2
            when lower(accounts.type_name) = 'expense' then 3
            when lower(accounts.type_name) = 'other income' then 4
            when lower(accounts.type_name) = 'other expense' then 5
            else null
            end as income_statement_sort_helper

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

fivetran-jamie commented 12 months ago

Hi there @JessVagnoni, thanks for taking the time to open this issue!

I agree that it would make way more sense to look at the account type IDs. I'd first like to confirm that the account type IDs are indeed standardized across different Netsuite environments

Are you using netsuite or netsuite2?

If Netsuite(1) can you confirm that your netsuite.accounts.type and netsuite.accounts.type_sequence fields map onto each other like this?

image

If Netsuite2, can you confirm that your netsuite2.accounttype.id IDs match the rightside of this

image

(source)

Thank ya!

JessVagnoni commented 12 months ago

Hi Jamie,

We are using Netsuite2 and I can confirm our account type IDs look like the second example.

Thank you!

On Tue, Sep 12, 2023 at 4:12 PM Jamie Rodriguez @.***> wrote:

Hi there @JessVagnoni https://github.com/JessVagnoni, thanks for taking the time to open this issue!

I agree that it would make way more sense to look at the account type IDs. I'd first like to confirm that the account type IDs are indeed standardized across different Netsuite environments

Are you using netsuite or netsuite2?

If Netsuite(1) can you confirm that your netsuite.accounts.type and netsuite.accounts.type_sequence fields map onto each other like this? [image: image] https://user-images.githubusercontent.com/65564846/267449044-6b6b8031-2af8-435f-af34-d91fb7d6f706.png

If Netsuite2, can you confirm that your netsuite2.accounttype.id IDs match the rightside of this [image: image] https://user-images.githubusercontent.com/65564846/267447386-cc5fa263-5cf0-4274-8713-9cf7ddeca73a.png (source https://blog.prolecto.com/2013/09/10/netsuite-searchfilter-internal-account-type-codes/ )

Thank ya!

— Reply to this email directly, view it on GitHub https://github.com/fivetran/dbt_netsuite/issues/83#issuecomment-1716357623, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQKCR272DUK2TJVOZAYZS5LX2C6ZLANCNFSM6AAAAAA4VBHVLE . You are receiving this because you were mentioned.Message ID: @.***>

fivetran-jamie commented 11 months ago

Great! We'll definitely fold this in! Ideally we'd also apply this to the Netsuite(1) models, so I'm going to first investigate if the numerical account_type IDs are consistent across different NS1 environments. We'll roll this out to NS2 regardless though

JessVagnoni commented 11 months ago

Awesome thanks Jamie!

Is there a timeline for when this is expected to be completed? And will this change be for just the Income Statement, or will it also take effect for any other models using names for account types?

Thank you

On Thu, Sep 14, 2023 at 5:30 PM Jamie Rodriguez @.***> wrote:

Great! We'll definitely fold this in! Ideally we'd also apply this to the Netsuite(1) models, so I'm going to first investigate if the numerical account_type IDs are consistent across different NS1 environments. We'll roll this out to NS2 regardless though

— Reply to this email directly, view it on GitHub https://github.com/fivetran/dbt_netsuite/issues/83#issuecomment-1720177701, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQKCR233UR5OK7COKHRC5BDX2NZNJANCNFSM6AAAAAA4VBHVLE . You are receiving this because you were mentioned.Message ID: @.***>

fivetran-joemarkiewicz commented 11 months ago

Hi @JessVagnoni we will be adding this to our upcoming sprint (starting tomorrow). You can expect our team to work on incorporating these updates and share details as we work through them including sharing a WIP branch which you will be able to test to validate the changes work on your side.

We will also plan to make the same changes to the Balance Sheet model, unless there are any unforeseen issues during development. However, we will likely restrict this update to only apply to NS2 data models.

alexandra-plassaras commented 11 months ago

We are facing a similar issue and the int_netsuite2__tran_with_converted_amounts model is marking many of our accounts as false for is_income_statement and not categorizing our account types correctly because our account type names don't match the logic in the account_category column

We are using Netsuite2, Redshift

  - package: fivetran/netsuite
    version: [">=0.9.0", "<0.10.0"]

Our netsuite2.accounttype.id also matched what you included in the posts above Screenshot 2023-09-22 at 3 02 52 PM

The type_name values from our int_netsuite2__accounts table are as follows: Screenshot 2023-09-22 at 3 07 51 PM

So for example our account type labeled Cost of Revenue is being marked as null in the int_netsuite__transactions_with_converted_amounts model even though that is what we named our cost of goods sold account type. And operating expense should be marked as expense but is not.

Will the fix you are currently working on also fix the issue we are seeing? Or should I open up a different issue?

fivetran-catfritz commented 11 months ago

Hi @JessVagnoni and @alexandra-plassaras thanks for the additional information! I am picking up this issue and aim to have a test branch ready for you to try in the next week or so.

fivetran-catfritz commented 11 months ago

Hi @JessVagnoni and @alexandra-plassaras I have created a test branch that you can try out. I have incorporated the changes in all the models I saw were affected, but I would also really appreciate your feedback. You can install the test branch using the below code in place of the normal dbt_netsuite installation code.

- git: https://github.com/fivetran/dbt_netsuite.git
  revision: bug/account-type-id
  warn-unpinned: false

Note, the models updated are:

fivetran-catfritz commented 11 months ago

Version 0.10.0, which incorporates these changes, is now live! Closing this issue, but please let us know if you have any additional feedback.

Update can be installed using the below:

packages:
  - package: fivetran/netsuite
    version: [">=0.10.0", "<0.11.0"]