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.

Sign up for GitHub

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

Jump to bottom

fix(int_zendesk__schedule_spine): offset holiday start and end time too #118

Merged
merged 2 commits into from
Nov 27, 2023
Merged

fix(int_zendesk__schedule_spine): offset holiday start and end time too #118

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01a81bc
fix(int_zendesk__schedule_spine): offset holiday start/end time too
Oct 24, 2023
5c3c3da
delete .idea/.gitignore
cth84 Oct 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions models/intermediate/int_zendesk__schedule_spine.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ with timezone as (
schedule.schedule_name,
schedule.start_time - coalesce(split_timezones.offset_minutes, 0) as start_time_utc,
schedule.end_time - coalesce(split_timezones.offset_minutes, 0) as end_time_utc,

coalesce(split_timezones.offset_minutes, 0) as offset_minutes_to_add,
-- we'll use these to determine which schedule version to associate tickets with
cast(split_timezones.valid_from as {{ dbt.type_timestamp() }}) as valid_from,
cast(split_timezones.valid_until as {{ dbt.type_timestamp() }}) as valid_until
Expand All @@ -105,11 +105,11 @@ with timezone as (
-- Now we need take holiday's into consideration and perform the following transformations to account for Holidays in existing schedules
), holiday_start_end_times as (

select
select
calculate_schedules.*,
schedule_holiday.holiday_name,
schedule_holiday.holiday_start_date_at,
cast({{ dbt.dateadd("second", "86400", "schedule_holiday.holiday_end_date_at") }} as {{ dbt.type_timestamp() }}) as holiday_end_date_at, -- add 24*60*60 seconds
cast({{ dbt.dateadd("minute", "offset_minutes_to_add", "schedule_holiday.holiday_start_date_at") }} as {{ dbt.type_timestamp() }}) as holiday_start_date_at,
cast({{ dbt.dateadd("minute", "1440 - offset_minutes_to_add", "schedule_holiday.holiday_end_date_at") }} as {{ dbt.type_timestamp() }}) as holiday_end_date_at, -- add 24*60 = 1440 minutes
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain the change you're making here? Im curious about the second to minute switch as well as the 1440 - offset_minutes_to_add

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed from second to minute to account for the fact that the offset is stored in minutes (the alternative could be to keep it in seconds, 86400 and then multiple offset_minutes by 60). Figured since it was a hard coded time to add, hardcoding it to 1440 and the not having to multiply the offset minutes made more sense.

1440 - offset_minutes_to_add is mimicking the logic applied to the start/time, which subtracts the offset so we can compare the two set of times:

image

potentially adding the _to_add to the offset_minutes and then subtracting it in that statement is a bit confusing, especially since our value is -240, currently. Most definitely open to a rename here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome, thanks for explaining!

cast({{ dbt_date.week_start("schedule_holiday.holiday_start_date_at") }} as {{ dbt.type_timestamp() }}) as holiday_week_start,
cast({{ dbt_date.week_end("schedule_holiday.holiday_end_date_at") }} as {{ dbt.type_timestamp() }}) as holiday_week_end
from schedule_holiday
Expand Down