Bug Fix: Date spine timestamp error

[fivetran-avinash]

PR Overview

This PR will address the following Issue/Feature: [#23]

This PR will result in the following new package version: 0.5.0

No fields were changed in this update, just the behavior of the spine for empty general ledger models.

Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:

Bug Fixes

• Updated the int_sage_intacct__general_ledger_date_spine model to accommodate for the cases when the compiled sage_intacct__general_ledger model has no transactions. In this case, the model now defaults to a range of one-month from the current date.

Under The Hood

• Updated the structure of the int_sage_intacct__general_ledger_date_spine model for improved performance and maintainability.
• Added flags.WHICH in ('run', 'build') as a condition in int_sage_intacct__general_ledger_date_spine to prevent call statements from querying the staging models during a dbt compile.
• Addition of integrity and consistency validation tests within integration tests for the sage_intacct__general_ledger and sage_intacct__general_ledger_by_period model.

PR Checklist

Basic Validation

Please acknowledge that you have successfully performed the following commands locally:

• dbt run –full-refresh && dbt test
• [NA] 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, tagged, and properly assigned.
• [NA] 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 share any and all of your validation steps:

To reproduce the error, I created empty versions of the seed files that flowed into the general ledger model, in this case gl_account, gl_batch and gl_detail, then ran it in snowflake.

Adding the new logic fixed the error on a dbt run.

Also updating the date spine model to match our recent salesforce updates led to a successful dbt compile.

To validate these changes on devprod, I created:

• An integrity test that compared row counts to the general_ledger_by_period model that flows downstream from the new general_ledger_date_spine.
• I created a consistency test for the general_ledger to compare values between dev and prod.

One note:

• I originally created a consistency test on the general_ledger_by_period as well, but this test was failing. It appears the old logic uses the current date as the last date. So prod takes a lot of extra records that probably shouldn't exist since the max(entry_date_at) that created the max date on prod and dev is almost a year old.

So this test should fail on this PR, but should work for all future PRs. I've left it in here with the expectation that it'll fail here but is a useful test for future models.

If you had to summarize this PR in an emoji, which would it be?

🦴

[fivetran-reneeli]

Looks good just reminder to set it as a breaking change!

[fivetran-reneeli]

lgtm

[fivetran-joemarkiewicz]

@fivetran-avinash thanks for your work on this PR. It's looking great, I just have a few small notes before approval. Let me know if you want to sync on any of my notes.

[fivetran-joemarkiewicz]

I would adjust the wording of this sub bullet as it reads a bit confusing. We are saying the records will change but then don't directly reference what exactly is changing. It sounds like we are being selective as to which transaction records will be included in the final model. We are not limiting the transactions but instead changing the behavior of the date spine. Ultimately we are providing a more accurate representation of their transactions and this may adjust the records in their end model.

[fivetran-avinash]

Updated

[fivetran-avinash]

@fivetran-avinash thanks for your work on this PR. It's looking great, I just have a few small notes before approval. Let me know if you want to sync on any of my notes.

@fivetran-joemarkiewicz This is ready for re-review.

[fivetran-joemarkiewicz]

LGTM