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] converted amount in transaction details can be NULL in some cases #115

Open
2 of 4 tasks
FrankTub opened this issue Apr 5, 2024 · 9 comments
Open
2 of 4 tasks
Assignees
Labels
priority:p4 Affects few users; pick up when available status:in_progress Currently being worked on type:enhancement New functionality or enhancement update_type:documentation Primary focus requires documentation updates

Comments

@FrankTub
Copy link

FrankTub commented Apr 5, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Describe the issue

In netsuite2__transaction_details the column converted_amount can be NULL, while the transaction_amount column is not empty. I guess this has to do with the way an exchange rate is determined in your code.

Our default currency is EUR. I have taken a look at the code that was used to determine the correct exchange rate but did not fully comprehend the logic behind it. The expected output for converted_amount when the currency is our default currency would be that the exchange rate would be 1.

Relevant error log or model output

Ran query:


select transaction_id , transaction_line_id , transaction_number, transaction_status, account_id , account_number , currency_symbol , converted_amount , transaction_amount , is_main_line , is_tax_line 
from transformations.netsuite2__transaction_details
where converted_amount is null
;


And this results in 719 rows, where the other 2000 rows all have a converted_amount as expected.

Expected behavior

The column netsuite2__transaction_details.converted_amount should never be NULL.

dbt Project configurations

Don't think it is relevant here, but this is what we have configured:

....

vars:
  netsuite_data_model: netsuite2
  netsuite_schema: netsuite_suiteanalytics
  netsuite2__using_vendor_categories: false
  netsuite2__using_jobs: false

  balance_sheet_transaction_detail_columns: ['bq_contract_id', 'amount_excl_vat', 'transaction_type']
  income_statement_transaction_detail_columns: ['bq_contract_id', 'amount_excl_vat', 'transaction_type']

  transactions_pass_through_columns:
      - name: "transaction_due_on"
        transform_sql: "cast(duedate at time zone 'UTC' as date)"
      - name: "transaction_on"
        transform_sql: "cast(trandate at time zone 'UTC' as date)"
      - name: "transaction_closed_on"
        transform_sql: "cast(closedate at time zone 'UTC' as date)"
  transaction_lines_pass_through_columns:
      - name: "custcol_bq_contract"
        alias: "bq_contract_id"
      - name: "foreignamount"
        alias: "amount_excl_vat"
...
models:
  netsuite_source:
    +schema: base
  netsuite:
    +schema: transformations

Package versions

Basicly I'm using 0.12.0, but due to #113 I started using the following in my packages.yml:

packages:
  - git: "https://github.com/FrankTub/dbt_netsuite"
    revision: feature/transaction-line-fields

What database are you using dbt with?

postgres

dbt Version

1.7.11

Additional Context

Example output of my query:
netsuite2__transaction_details_202404051706.csv

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.
@fivetran-avinash
Copy link
Contributor

Hi @FrankTub, thank you for bringing this to our attention!

This is actually something we've also spotted recently and were planning to spend some time investigating a proper solve for in the coming weeks. I think your initial solution is similar to the approach we landed on, but we didn't complete the investigation. Let me take some time to dig into this and see if this is the best possible solve for you.

Have a great weekend and we'll be in touch soon!

@fivetran-avinash
Copy link
Contributor

fivetran-avinash commented Apr 11, 2024

Hi @FrankTub ! Been doing some investigating, and were curious about your thoughts here:

  1. Are the majority of your accounting transactions that are null future-facing and have a true value for is_transaction_non_posting in the netsuite2__transaction_details model? At this point, our model does not convert any transactions in the future, as we do not know the exchange rates at those time. If so, would transaction_amount not suffice in those cases?
  2. For any of these cases for null converted amounts, were there any trends around the accounting_book_id? There might be particular cases where we don't convert values where the accounting book is not expected to be reported on and was curious.
  3. Do you spot any additional trends around these null converted amount cases?

Let us know your thoughts! This will hopefully lead us closer to what's lying at the root of this problem and allow us to address this with an appropriate fix.

@FrankTub
Copy link
Author

@fivetran-avinash , currently we are in the setup phase of Netsuite and this week we have removed all data from our Netsuite sandbox environment. So it will probably take some time before we have enough data to spot some trends in when the amount is not converted. I will come back on it when I can.

What did draw my attention was that on transaction level there is a field exchange rate, when I look at the code this field was ignored. Any reason why this is?

@fivetran-avinash
Copy link
Contributor

HI @FrankTub , thanks for the update! Let us know when you are able to check those trends and we can probably get closer to identifying the root issues to fix up.

Could you elaborate what you mean by the field exchange rate being ignored? If you look at our documentation, our intermediate models bring in the exchange rate for the transaction and reporting period and then use that to calculate the converted amount in our end model.

Have a great weekend!

@FrankTub
Copy link
Author

@fivetran-avinash , I can confirm that in our small test data case, all the rows where converted_amount is null, the condition is_transaction_non_posting = false is always true.

Could you elaborate what you mean by the field exchange rate being ignored? If you look at our documentation, our intermediate models bring in the exchange rate for the transaction and reporting period and then use that to calculate the converted amount in our end model.

With respect to above comment. What I meant was that on transaction level there is also a field exchange rate available.

image

I guess this is field transaction.exchangerate. Even for those cases where is_transaction_non_posting = false, the field transaction.exchangerate is still available. Maybe that helps?

@fivetran-avinash
Copy link
Contributor

Hi @FrankTub , thanks for the context! A few follow-ups and then we can probably decide on the proper course of action for this ticket.

  1. Are all these non-posting transactions future facing (i.e. transactions that will be posted in the future)? If not, what are the characteristics of these past transactions?

  2. Could you explain more about what you'd be doing with the converted amount for the future transactions? The reason we set up the converted amount to filter out non-posting transactions was to avoid any improper calculations of transactions that have not yet occurred. Is there any additional utility to bringing in converted transactions that haven't yet posted?

  3. If so, one potential solution we could explore is bringing in converted amounts for the default currency that is being used by multiplying by 1 for those cases (we would still not bring in the converted amounts for non-posting values that have an exchange rate other than 1, as those conversions are likely too volatile). However, there isn't a clear field I can find in Netsuite2 that brings in the default currency for a customer. Do you happen to know if there's a particular field that has that information?

Circle back to us when you can!

@FrankTub
Copy link
Author

Just verified and they are all transactions that will be posted in the future. For some reason our finance department starts creating these sales invoices a month in advance. They use it to forecast some things. So it would be usefull to have a best estimate for foreign currencies as well, if not possible I think most of our data is in local currency, and fixing it only for local currency would also be really helpful

@fivetran-avinash
Copy link
Contributor

fivetran-avinash commented Apr 16, 2024

Hi @FrankTub , thanks for all your excellent analysis and thoughts.

Unfortunately, after much discussion and thoughts, we have decided to keep the converted amount as is in our model to maintain financial fidelity.

Even though introducing new logic that would create future facing converted amounts is possible, we are concerned that other teams not familiar with your use case would suffer misaligned expectations for these particular converted amounts. The current transaction details end model does provide a proper set of end transactions with the correct converted amounts--adding logic that converts amount in the future is subject to volatility.

Particularly when foreign currency gets involved, you'd see forecasted amounts changing pretty frequently and potentially very volatile converted amount values. This can also be disruptive to customers who see constantly changing transaction figures without any real explanation. Whereas when the converted amount is posting, those numbers should stay fixed in the plurality of all cases.

My suggestion would be to leverage the transaction_amount in the end model, materialize the intermediate tables in your dbt_project.yml that contain the exchange rate data, and then create a new field in the end models in your warehouse that provides these converted amounts for the future transactions.

Sorry I couldn't provide a proper solution here internally! This feature does reminds us of the perils of developing a package meant to fit as many customers as possible as opposed to customized solutions for just one. While I agree that this solution would work for your own internal use, it would be one that would be difficult to work for the majority of customers.

We hope this alternative solution provides you with what you need. Let me know if you have any questions!

@fivetran-avinash fivetran-avinash added type:enhancement New functionality or enhancement priority:p4 Affects few users; pick up when available status:accepted Scoped and accepted into queue update_type:documentation Primary focus requires documentation updates labels Apr 16, 2024
@fivetran-avinash fivetran-avinash self-assigned this Apr 19, 2024
@fivetran-avinash fivetran-avinash added status:in_progress Currently being worked on and removed status:accepted Scoped and accepted into queue labels Apr 19, 2024
@fivetran-avinash
Copy link
Contributor

While we did not choose to proceed forward with working on this issue, we did create the above PR to update the DECISIONLOG for our reasons for not doing so. It will be part of a future Netsuite release.

Thanks again for all your proactive communication @FrankTub. Apologies that we could not go ahead and add that logic in, but hopefully our proposed intermediate solution will work for you! Always reach out and let us know what we can do to help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:p4 Affects few users; pick up when available status:in_progress Currently being worked on type:enhancement New functionality or enhancement update_type:documentation Primary focus requires documentation updates
Projects
None yet
Development

No branches or pull requests

2 participants