Skip to content

Commit

Permalink
Merge pull request #95 from fivetran/bug/multibook-subsidiary-updates
Browse files Browse the repository at this point in the history
Bug/multibook subsidiary updates
  • Loading branch information
fivetran-catfritz authored Nov 14, 2023
2 parents 6e8f101 + ab90970 commit 24bc6f6
Show file tree
Hide file tree
Showing 20 changed files with 427 additions and 81 deletions.
2 changes: 1 addition & 1 deletion .buildkite/scripts/run_models.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ dbt deps
dbt seed --target "$db" --full-refresh
dbt run --target "$db" --full-refresh
dbt test --target "$db"
dbt run --vars '{netsuite2__multibook_accounting_enabled: false, netsuite2__using_exchange_rate: false, netsuite2__using_vendor_categories: false, netsuite2__using_jobs: false}' --target "$db" --full-refresh
dbt run --vars '{netsuite2__using_to_subsidiary: true, netsuite2__multibook_accounting_enabled: true, netsuite2__using_exchange_rate: false, netsuite2__using_vendor_categories: false, netsuite2__using_jobs: false}' --target "$db" --full-refresh
dbt test --target "$db"
dbt run-operation fivetran_utils.drop_schemas_automation --target "$db"
44 changes: 9 additions & 35 deletions .github/PULL_REQUEST_TEMPLATE/maintainer_pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,48 +4,22 @@
**This PR will result in the following new package version:**
<!--- Please add details around your decision for breaking vs non-breaking version upgrade. If this is a breaking change, were backwards-compatible options explored? -->

**Please detail what change(s) this PR introduces and any additional information that should be known during the review of this PR:**
**Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:**
<!--- Copy/paste the CHANGELOG for this version below. -->

## PR Checklist
### Basic Validation
Please acknowledge that you have successfully performed the following commands locally:
- [ ] dbt compile
- [ ] dbt run –full-refresh
- [ ] dbt run
- [ ] dbt test
- [ ] dbt run –vars (if applicable)
- [ ] dbt run –full-refresh && dbt test
- [ ] dbt run (if incremental models are present)

Before marking this PR as "ready for review" the following have been applied:
- [ ] The appropriate issue has been linked and tagged
- [ ] You are assigned to the corresponding issue and this PR
- [ ] 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 acknowledge that the following validation checks have been performed prior to marking this PR as "ready for review":
- [ ] You have validated these changes and assure this PR will address the respective Issue/Feature.
- [ ] You are reasonably confident these changes will not impact any other components of this package or any dependent packages.
- [ ] You have provided details below around the validation steps performed to gain confidence in these changes.
Please share any and all of your validation steps:
<!--- Provide the steps you took to validate your changes below. -->

### Standard Updates
Please acknowledge that your PR contains the following standard updates:
- Package versioning has been appropriately indexed in the following locations:
- [ ] indexed within dbt_project.yml
- [ ] indexed within integration_tests/dbt_project.yml
- [ ] CHANGELOG has individual entries for each respective change in this PR
<!--- If there is a parallel upstream change, remember to reference the corresponding CHANGELOG as an individual entry. -->
- [ ] README updates have been applied (if applicable)
<!--- Remember to check the following README locations for common updates. →
<!--- Suggested install range (needed for breaking changes) →
<!--- Dependency matrix is appropriately updated (if applicable) →
<!--- New variable documentation (if applicable) -->
- [ ] DECISIONLOG updates have been updated (if applicable)
- [ ] Appropriate yml documentation has been added (if applicable)

### dbt Docs
Please acknowledge that after the above were all completed the below were applied to your branch:
- [ ] docs were regenerated (unless this PR does not include any code or yml updates)

### If you had to summarize this PR in an emoji, which would it be?
<!--- For a complete list of markdown compatible emojis check our this git repo (https://gist.github.com/rxaviers/7360908) -->
:dancer:
78 changes: 78 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,81 @@
# dbt_netsuite v0.12.0-b1
## 📈 New Visualization Support (BigQuery & Snowflake users) 📊
- Our team has created the [Netsuite Streamlit app](https://fivetran-netsuite.streamlit.app/) to help you visualize the end reports created in this package! [See instructions here](https://github.com/fivetran/streamlit_netsuite) on how to fork our Streamlit repo and configure your own reports.

## Beta Release Notes for Netsuite2

[PR #95](https://github.com/fivetran/dbt_netsuite/pull/95) (built upon [#90](https://github.com/fivetran/dbt_netsuite/issues/90)) introduces the following updates. These changes are released in beta format to encourage community feedback and insights before the final release.
## 🚨 Breaking Changes 🚨
- Multi-book functionality is now disabled by default. To enable it, set the variable `netsuite2__multibook_accounting_enabled` to `true` in your `dbt_project.yml`.
- ❗Note: The default behavior was updated due to addition of `accounting_book` fields. Depending on your Netsuite setup, **adding this field can significantly increase the row count of the end models**.
- See additional details in the multi-book section below.

## 🎉 Features 🎉
### Model updates
- For more accurate categorization of accounts, accounts having the following `special_account_type_id` are now categorized as:

special_account_type_id | account_type_name | account_type_id
--- | --- | ---
retearnings | Retained Earnings | retained_earnings
cta-e | Cumulative Translation Adjustment | cumulative_translation_adjustment
cumultransadj | Cumulative Translation Adjustment | cumulative_translation_adjustment

- The below fields have been added for all configurations.
- If you are leveraging a `*_pass_through_columns` variable to include `accounting_period_id` or `subsidiary_id`, you may need to remove them to avoid a duplicate column error.

model | new cols
----- | -----
netsuite2__transaction_details | accounting_period_id <br> subsidiary_id <br> transaction_details_id
netsuite2__income_statement | income_statement_id
netsuite2__balance_sheet | balance_sheet_id

- `balance_sheet_id`, `income_statement_id`, and `transaction_details_id` are surrogate keys created for each end model. These keys are now tested for uniqueness and replaces the previous combination-of-columns tests for these models.

- For detailed descriptions on the added columns, refer to our [dbt docs for this package](https://fivetran.github.io/dbt_netsuite/#!/overview/netsuite).

### multi-book
- Expanded `accounting_book` information that is included. As mentioned above, this feature is now disabled by default. To enable it, set the below variable to `true` in your `dbt_project.yml`.
- ❗Notes:
- If you choose to enable this feature, this will add rows for transactions for your non-primary accounting_book_ids, and any of your downstream use cases may need to be adjusted.
- The surrogate keys mentioned above are dynamically generated depending on your enabled/disabled features, so adding these rows should not cause test failures.

This comment has been minimized.

Copy link
@erwann-pleo

erwann-pleo Nov 21, 2023

Hello, according to me, it is causing test failure for stg_netsuite2__transaction_accounting_lines where the test has not been amended there. What would your recommendation be to proceed without duplicate in the netsuite_source package?

image

This comment has been minimized.

Copy link
@fivetran-catfritz

fivetran-catfritz Nov 22, 2023

Author Contributor

Hi @erwann-pleo thank you for bringing this to our attention! I have updated this test to include accounting_book_id, so it should be included the next time you run dbt deps. Does this solve your test failure?

This comment has been minimized.

Copy link
@erwann-pleo

erwann-pleo Nov 22, 2023

All good now! ✅ Thank you very much

- If you are leveraging a `*_pass_through_columns` variable to include the below columns, you may need to remove them to avoid a duplicate column error.
```yml
vars:
netsuite2__multibook_accounting_enabled: true # False by default.
```
- The resulting fields added by enabling this feature are:
model | new cols
----- | -----
netsuite2__transaction_details | accounting_book_id <br> accounting_book_name
netsuite2__income_statement | accounting_book_id <br> accounting_book_name
netsuite2__balance_sheet | accounting_book_id <br> accounting_book_name
### to_subsidiary
- Added the option to include `to_subsidiary` information in all end models. This feature is disabled by default, so to enable it, set the below variable to `true` in your `dbt_project.yml`. You will also need to be using exchange rates, which is enabled by default.
- ❗Notes:
- If you choose to enable this feature, this will add rows for transactions where `to_subsidiary` is not a top-level subsidiary. Your downstream use cases may need to be adjusted.
- The surrogate keys mentioned above are dynamically generated depending on your enabled/disabled features, so adding these rows should not cause test failures.
- If you are leveraging a `*_pass_through_columns` variable to include the below columns, you may need to remove them to avoid a duplicate column error.
```yml
vars:
netsuite2__using_to_subsidiary: true # False by default.
```
- The resulting fields added by enabling this feature are:

model | new cols
----- | -----
netsuite2__transaction_details | to_subsidiary_id <br> to_subsidiary_name <br> to_subsidiary_currency_symbol
netsuite2__income_statement | to_subsidiary_id <br> to_subsidiary_name <br> to_subsidiary_currency_symbol
netsuite2__balance_sheet | to_subsidiary_id <br> to_subsidiary_name <br> to_subsidiary_currency_symbol <br> subsidiary_name

## 🚘 Under the hood 🚘
- Removed previously deprecated, empty model `int_netsuite2__consolidated_exchange_rates`.

## Contributors:
- [@jmongerlyra](https://github.com/jmongerlyra) ([#90](https://github.com/fivetran/dbt_netsuite/issues/90))
- [@rwang-lyra](https://github.com/rwang-lyra ) ([#90](https://github.com/fivetran/dbt_netsuite/issues/90))

# dbt_netsuite v0.11.0

## 🚨 Breaking Changes 🚨:
Expand Down
42 changes: 36 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ The following table provides a detailed list of all models materialized within t
| [netsuite__transaction_details](https://fivetran.github.io/dbt_netsuite/#!/model/model.netsuite.netsuite__transaction_details) or [netsuite2__transaction_details](https://fivetran.github.io/dbt_netsuite/#!/model/model.netsuite.netsuite2__transaction_details) | All transactions with the associated accounting period, account and subsidiary information. Where applicable, you can also see data about the customer, location, item, vendor, and department. |
| [netsuite__income_statement](https://fivetran.github.io/dbt_netsuite/#!/model/model.netsuite.netsuite__income_statement) or [netsuite2__income_statement](https://fivetran.github.io/dbt_netsuite/#!/model/model.netsuite.netsuite2__income_statement) | All transaction lines necessary to generate an income statement (converted for the appropriate exchange rate of the parent subsidiary). Department, class, and location information are included for additional reporting functionality. |
| [netsuite__balance_sheet](https://fivetran.github.io/dbt_netsuite/#!/model/model.netsuite.netsuite__balance_sheet) or [netsuite2__balance_sheet](https://fivetran.github.io/dbt_netsuite/#!/model/model.netsuite.netsuite2__balance_sheet) | All transaction lines necessary to generate a balance sheet (converted for the appropriate exchange rate of the parent subsidiary). Non balance sheet transactions are categorized as either Retained Earnings or Net Income. |

Many of the above reports are now configurable for [visualization via Streamlit](https://github.com/fivetran/streamlit_netsuite)! Check out some [sample reports here](https://fivetran-netsuite.streamlit.app/).
<!--section-end-->


Expand Down Expand Up @@ -100,7 +102,7 @@ Include the following netsuite package version in your `packages.yml` file:
```yaml
packages:
- package: fivetran/netsuite
version: [">=0.11.0", "<0.12.0"]
version: [">=0.12.0", "<0.13.0"]
```
## Step 3: Define Netsuite.com or Netsuite2 Source
As of April 2022 Fivetran made available a new Netsuite connector which leverages the Netsuite2 endpoint opposed to the original Netsuite.com endpoint. This package is designed to run for either or, not both. By default the `netsuite_data_model` variable for this package is set to the original `netsuite` value which runs the netsuite.com version of the package. If you would like to run the package on Netsuite2 data, you may adjust the `netsuite_data_model` variable to run the `netsuite2` version of the package.
Expand All @@ -122,7 +124,7 @@ vars:
It's possible that your Netsuite connector does not sync every table that this package expects. If your syncs exclude certain tables, it is because you either don't use that feature in Netsuite or actively excluded some tables from your syncs. To disable the corresponding functionality in the package, you must add the relevant variables. By default, all variables are assumed to be true. Add variables for only the tables you would like to disable:
```yml
vars:
netsuite2__multibook_accounting_enabled: false # True by default. Disable `accountingbooksubsidiary` and `accountingbook` if you are not using the Multi-Book Accounting feature
netsuite2__multibook_accounting_enabled: true # False by default. Disable `accountingbooksubsidiary` and `accountingbook` if you are not using the Multi-Book Accounting feature
netsuite2__using_exchange_rate: false #True by default. Disable `exchange_rate` if you don't utilize exchange rates. If you set this variable to false, ensure it is scoped globally so that the `netsuite_source` package can access it as well.
netsuite2__using_vendor_categories: false # True by default. Disable `vendorcategory` if you don't categorize your vendors
netsuite2__using_jobs: false # True by default. Disable `job` if you don't use jobs
Expand All @@ -132,7 +134,32 @@ vars:
> To determine if a table or field is activated by a feature, access the [Records Catalog](https://docs.oracle.com/en/cloud/saas/netsuite/ns-online-help/article_159367781370.html).

## (Optional) Step 6: Additional configurations
<details><summary>Expand for configurations</summary>

### Enable additional features

### Multi-Book (Netsuite2 only)
To include `accounting_book_id` and `accounting_book_name` columns in the end models, set the below variable to `true` in your `dbt_project.yml`. This feature is disabled by default.
>❗Notes:
> - If you choose to enable this feature, this will add rows for transactions for any non-primary `accounting_book_id`, and your downstream use cases may need to be adjusted.
> - The surrogate keys for the end models are dynamically generated depending on the enabled/disabled features, so adding these rows will not cause test failures.
> - If you are leveraging a `*_pass_through_columns` variable to include the added columns, you may need to remove them to avoid a duplicate column error.
```yml
vars:
netsuite2__multibook_accounting_enabled: true # False by default.
```

### To Subsidiary (Netsuite2 only)
To include `to_subsidiary_id` and `to_subsidiary_name` columns in the end models, set the below variable to `true` in your `dbt_project.yml`. This feature is disabled by default. You will also need to be using exchange rates, which is enabled by default.

>❗Notes:
> - If you choose to enable this feature, this will add rows for transactions where `to_subsidiary` is not a top-level subsidiary. Your downstream use cases may need to be adjusted.
> - The surrogate keys for the end models are dynamically generated depending on the enabled/disabled features, so adding these rows will not cause test failures.
> - If you are leveraging a `*_pass_through_columns` variable to include the added columns, you may need to remove them to avoid a duplicate column error.

```yml
vars:
netsuite2__using_to_subsidiary: true # False by default.
```

### Passing Through Additional Fields
This package includes all source columns defined in the macros folder. To add additional columns to this package, do so by adding our pass-through column variables to your `dbt_project.yml` file:
Expand Down Expand Up @@ -206,9 +233,12 @@ vars:
### Override the data models variable
This package is designed to run **either** the Netsuite.com or Netsuite2 data models. However, for documentation purposes, an additional variable `netsuite_data_model_override` was created to allow for both data model types to be run at the same time by setting the variable value to `netsuite`. This is only to ensure the [dbt docs](https://fivetran.github.io/dbt_netsuite/) (which is hosted on this repository) is generated for both model types. While this variable is provided, we recommend you do not adjust the variable and instead change the `netsuite_data_model` variable to fit your configuration needs.

</details>
## (Optional) Step 7: Produce Analytics-Ready Reports with Streamlit App (Bigquery and Snowflake users only)
For those who want to take their reports a step further, our team has created the [Fivetran Netsuite Streamlit App](https://fivetran-netsuite.streamlit.app/) to generate end model visualizations based off of the reports we created in this package. This way you can replicate much of the reporting you see internally in Netsuite and automate a lot of the work needed to report on your core metrics.

[We recommend following the instructions here](https://github.com/fivetran/streamlit_netsuite) to fork the app for your own data and create end reports leveraging our Netsuite models. You can see a sample version of [these reports here]((https://fivetran-netsuite.streamlit.app/)).

## (Optional) Step 7: Orchestrate your models with Fivetran Transformations for dbt Core™
## (Optional) Step 8: Orchestrate your models with Fivetran Transformations for dbt Core™
<details><summary>Expand for details</summary>
<br>

Expand All @@ -223,7 +253,7 @@ This dbt package is dependent on the following dbt packages. Please be aware tha
```yml
packages:
- package: fivetran/netsuite_source
version: [">=0.7.0", "<0.8.0"]
version: [">=0.9.0", "<0.10.0"]
- package: fivetran/fivetran_utils
version: [">=0.4.0", "<0.5.0"]
Expand Down
2 changes: 1 addition & 1 deletion dbt_project.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
config-version: 2
name: 'netsuite'
version: '0.11.0'
version: '0.12.0'
require-dbt-version: [">=1.3.0", "<2.0.0"]

models:
Expand Down
2 changes: 1 addition & 1 deletion docs/catalog.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion docs/manifest.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion docs/run_results.json

Large diffs are not rendered by default.

7 changes: 6 additions & 1 deletion integration_tests/dbt_project.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
name: 'netsuite_integration_tests'
version: '0.11.0'
version: '0.12.0'
profile: 'integration_tests'
config-version: 2
models:
+materialized: table
vars:
netsuite_schema: netsuite_integration_tests_5
netsuite_data_model_override: netsuite

# Enable below when generating docs
# netsuite2__multibook_accounting_enabled: true
# netsuite2__using_to_subsidiary: true

netsuite_source:
# Netsuite Seed Data
netsuite_accounting_books_identifier: "netsuite_accounting_books_data"
Expand Down
4 changes: 1 addition & 3 deletions integration_tests/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,4 @@ dbt-redshift>=1.3.0,<2.0.0
dbt-postgres>=1.3.0,<2.0.0
dbt-spark>=1.3.0,<2.0.0
dbt-spark[PyHive]>=1.3.0,<2.0.0
dbt-databricks>=1.3.0,<2.0.0

oscrypto @ git+https://github.com/wbond/oscrypto.git@d5f3437
dbt-databricks>=1.3.0,<2.0.0
Loading

0 comments on commit 24bc6f6

Please sign in to comment.