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/date not recognized #54

Merged
merged 11 commits into from
Jan 15, 2025
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
# dbt_xero v0.7.0
[PR #54](https://github.com/fivetran/dbt_xero/pull/54) includes the following updates:

## Breaking Changes
- Corrected the calculation of `current_year_end_date` in the `xero__balance_sheet_report` model. Previously, `current_year_end_date` was miscalculated in certain scenarios, impacting the classification of records with the `account_name` value "Retained Earnings."
- This is labeled as a breaking change since it may affect prior labels assigned. We recommend reviewing your records to ensure they align with this corrected logic.

## Bug Fixes
- Updated the `xero__balance_sheet` model to resolve a run error when an organization's financial year end date is February 29.

## Under the Hood (maintainers only)
- Added consistency tests for the end models.

# dbt_xero v0.6.2
[PR #46](https://github.com/fivetran/dbt_xero/pull/46) includes the following updates:

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Include the following xero package version in your `packages.yml` file:
```yaml
packages:
- package: fivetran/xero
version: [">=0.6.0", "<0.7.0"] # we recommend using ranges to capture non-breaking changes automatically
version: [">=0.7.0", "<0.8.0"] # we recommend using ranges to capture non-breaking changes automatically
```
Do NOT include the `xero_source` package in this file. The transformation package itself has a dependency on it and will install the source package as well.
### Step 3: Define database and schema variables
Expand Down
2 changes: 1 addition & 1 deletion dbt_project.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: 'xero'
version: '0.6.2'
version: '0.7.0'
config-version: 2
require-dbt-version: [">=1.3.0", "<2.0.0"]
vars:
Expand Down
2 changes: 1 addition & 1 deletion docs/catalog.json

Large diffs are not rendered by default.

47 changes: 10 additions & 37 deletions docs/index.html

Large diffs are not rendered by default.

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

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion docs/run_results.json

This file was deleted.

6 changes: 5 additions & 1 deletion integration_tests/dbt_project.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
name: 'xero_integration_tests'
version: '0.6.2'
version: '0.7.0'
profile: 'integration_tests'
config-version: 2

vars:
xero_source:
xero_account_identifier: "xero_account_data"
Expand All @@ -15,6 +16,9 @@ vars:
xero_credit_note_identifier: "xero_credit_note_data"
xero_schema: xero_integration_tests_2

models:
+schema: "xero_{{ var('directed_schema','dev') }}"

dispatch:
- macro_namespace: dbt_utils
search_order: ['spark_utils', 'dbt_utils']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a necessary file? If not, can we remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope--removed!

Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
{{ config(
tags="fivetran_validations",
enabled=var('fivetran_validation_tests_enabled', false)
) }}

with prod as (
select *
from {{ target.schema }}_xero_prod.xero__balance_sheet_report
),

dev as (
select *
from {{ target.schema }}_xero_dev.xero__balance_sheet_report
),

prod_not_in_dev as (
-- rows from prod not found in dev
select * from prod
except distinct
select * from dev
),

dev_not_in_prod as (
-- rows from dev not found in prod
select * from dev
except distinct
select * from prod
),

final as (
select
*,
'from prod' as source
from prod_not_in_dev

union all -- union since we only care if rows are produced

select
*,
'from dev' as source
from dev_not_in_prod
)

select *
from final
45 changes: 45 additions & 0 deletions integration_tests/tests/consistency/consistency_general_ledger.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
{{ config(
tags="fivetran_validations",
enabled=var('fivetran_validation_tests_enabled', false)
) }}

with prod as (
select *
from {{ target.schema }}_xero_prod.xero__general_ledger
),

dev as (
select *
from {{ target.schema }}_xero_dev.xero__general_ledger
),

prod_not_in_dev as (
-- rows from prod not found in dev
select * from prod
except distinct
select * from dev
),

dev_not_in_prod as (
-- rows from dev not found in prod
select * from dev
except distinct
select * from prod
),

final as (
select
*,
'from prod' as source
from prod_not_in_dev

union all -- union since we only care if rows are produced

select
*,
'from dev' as source
from dev_not_in_prod
)

select *
from final
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
{{ config(
tags="fivetran_validations",
enabled=var('fivetran_validation_tests_enabled', false)
) }}

with prod as (
select *
from {{ target.schema }}_xero_prod.xero__invoice_line_items
),

dev as (
select *
from {{ target.schema }}_xero_dev.xero__invoice_line_items
),

prod_not_in_dev as (
-- rows from prod not found in dev
select * from prod
except distinct
select * from dev
),

dev_not_in_prod as (
-- rows from dev not found in prod
select * from dev
except distinct
select * from prod
),

final as (
select
*,
'from prod' as source
from prod_not_in_dev

union all -- union since we only care if rows are produced

select
*,
'from dev' as source
from dev_not_in_prod
)

select *
from final
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
{{ config(
tags="fivetran_validations",
enabled=var('fivetran_validation_tests_enabled', false)
) }}

with prod as (
select *
from {{ target.schema }}_xero_prod.xero__profit_and_loss_report
),

dev as (
select *
from {{ target.schema }}_xero_dev.xero__profit_and_loss_report
),

prod_not_in_dev as (
-- rows from prod not found in dev
select * from prod
except distinct
select * from dev
),

dev_not_in_prod as (
-- rows from dev not found in prod
select * from dev
except distinct
select * from prod
),

final as (
select
*,
'from prod' as source
from prod_not_in_dev

union all -- union since we only care if rows are produced

select
*,
'from dev' as source
from dev_not_in_prod
)

select *
from final
34 changes: 24 additions & 10 deletions models/xero__balance_sheet_report.sql
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, I noticed that fields like account_class or source_type have all-caps values... should we do the same with account_name ? (Not really an issue, more of a standardization thing) . I can make an feature flag if it's significant enough cc @fivetran-joemarkiewicz

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line in question is this one.

It is consistent with the account names coming as shown in the seed data, but I believe the question is should everything be all caps?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per conversation with @fivetran-joemarkiewicz, this is the source behavior, so we'll keep as is.

Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,35 @@ with calendar as (

), organization as (

select *
select
*,
cast(extract(year from current_date) as {{ dbt.type_string() }}) as current_year,
cast(extract(year from {{ dbt.dateadd('year', 1, 'current_date') }}) as {{ dbt.type_string() }}) as next_year
from {{ var('organization') }}


), year_end as (

-- Calculate the current financial year-end date for each organization:
-- For February, determine last day by subtracting 1 day from March 1, avoiding leap year logic.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brilliant!

-- Compare the year end date to the current date:
-- Use this year's date if it's on or after the current date.
-- Otherwise, use the next year's corresponding date.
select
case
when cast(extract(year from current_date) || '-' || financial_year_end_month || '-' || financial_year_end_day as date) >= current_date
then cast(extract(year from current_date) || '-' || financial_year_end_month || '-' || financial_year_end_day as date)
else case when financial_year_end_month = 2 and financial_year_end_day = 29
then cast(extract(year from {{ dbt.dateadd('year', -1, 'current_date') }}) || '-' || financial_year_end_month || '-28' as date) -- Necessary for organizations with a reported fiscal year end of 02-29 as the previous year will not be a leap year and must be the 28th.
else cast(extract(year from {{ dbt.dateadd('year', -1, 'current_date') }}) || '-' || financial_year_end_month || '-' || financial_year_end_day as date)
end
end as current_year_end_date,
source_relation
source_relation,
case when financial_year_end_month = 2 and financial_year_end_day = 29
Copy link
Contributor Author

@fivetran-catfritz fivetran-catfritz Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving this condition first appears to prevent the warehouse from trying to execute the other nested conditions that cause the error.

then
case when cast({{ dbt.dateadd('day', -1, "cast(current_year || '-03-01' as date)") }} as date) >= current_date
then cast({{ dbt.dateadd('day', -1, "cast(current_year || '-03-01' as date)") }} as date)
else cast({{ dbt.dateadd('day', -1, "cast(next_year || '-03-01' as date)") }} as date)
end
else
case when cast(current_year || '-' || financial_year_end_month || '-' || financial_year_end_day as date) >= current_date
then cast(current_year || '-' || financial_year_end_month || '-' || financial_year_end_day as date)
else cast(next_year || '-' || financial_year_end_month || '-' || financial_year_end_day as date)
end
end as current_year_end_date

from organization

), joined as (
Expand Down