[Feature] Include location passthrough columns in transaction_details

[tom-rb]

Is there an existing feature request for this?

• I have searched the existing issues

Describe the Feature

It's already possible to extend netsuite2__transaction_details with the passthrough columns of accounts and departments, but not locations.

So, the same way accounts_pass_through_columns and departments_pass_through_columns are used in transaction details, I'd expect locations_pass_through_columns to do the same.

Describe alternatives you've considered

As a workaround (which I don't think it's ideal), I've used transaction_lines_pass_through_columns to include location_id in netsuite2__transaction_details, and then I perform the join again with stg_netsuite2__locations to fetch the additional columns.

Are you interested in contributing this feature?

• Yes.
• Yes, but I will need assistance and will schedule time during your office hours for guidance.
• No.

Anything else?

I tested a trivial change to include {{ fivetran_utils.persist_pass_through_columns('locations_pass_through_columns', identifier='locations') }} in netsuite2__transaction_details and I got my desired behavior, but I haven't run all tests of this package.

[fivetran-joemarkiewicz]

Hey @tom-rb thanks for opening this feature request!

I definitely can understand the desire to include the location information in the transaction detail end model. I also don't imagine this addition would cause any disruption to the granularity tests and should work by taking the approach you mentioned of including the fivetran_utils.persist_pass_through_columns() macro in the end model. The only other change I would recommend making is to include the location_id by default in the end model. This way, if users are not familiar or comfortable using the variables to passthrough columns, they can also more easily leverage the location_id to perform the join without needing to take the non ideal workaround you mentioned.

I see you are open to creating a PR! If you are open to contributing this change we would happily review it and plan to integrate into the next release once all is approved. Otherwise, we will plan to add this to our backlog and pick it up once we have capacity or if we see a high demand for this FR. The changes I recommend applying are as follows:

• Include transaction_lines.location_id here.
• Add the below macro/variable to below the default location fields.

    --The below script allows for locations table pass through columns.
    {{ fivetran_utils.persist_pass_through_columns('locationss_pass_through_columns', identifier='locations') }},

With those updates applied, and if you can confirm all looks good on your end, then this would be ready for a PR and my team and I can review this and let you know if there are any changes necessary before merging into main and including in the next release. Let me know if you would be interested in contributing this PR or if you have any questions. Thanks!

[tom-rb]

Done! Let me know if you need something else 👍

[fivetran-joemarkiewicz]

Great, thanks so much @tom-rb! One last request before I take a closer look. I'll take a closer look at your PR this week and let you know if I have any questions.

Most likely, we will fold your PR into a release branch, make a few under the hood updates (CHANGELOG, version bump, testing, etc.) and then will plan to integrate it into the upcoming release! I will let you know once I am starting the review in your PR. 😄

[fivetran-avinash]

Hi @tom-rb! Thanks for creating the PR! I've folded it into a working branch on our end and have validated that the passthrough functionality can bring in additional location fields.

Because these small changes will be breaking and require a versioning bump, we are planning to fold your changes into a larger future release. Are you okay with for now forking the repo and using the changes you implemented until we send the release live? Let us know your thoughts!

[tom-rb]

Are you okay with for now forking the repo and using the changes you implemented until we send the release live?

Of course! Thanks for moving it forward.