Skip to content

Created fact_bookings.#38

Merged
EmilyYoung26 merged 6 commits into
mainfrom
create-fact-bookings
Apr 16, 2026
Merged

Created fact_bookings.#38
EmilyYoung26 merged 6 commits into
mainfrom
create-fact-bookings

Conversation

@EmilyYoung26
Copy link
Copy Markdown
Collaborator

Made the sql and yml files for fact_bookings. Added a singular test too to make sure the attendance flags work.

Ticket: Here

All ran, built and tests passed on individual machine.

Copy link
Copy Markdown
Collaborator

@harjeetkalsi harjeetkalsi left a comment

Choose a reason for hiding this comment

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

Great this looks good, almost there!
I think this would be a good candidate for a unit test with a few fake rows and expected outputs, just to lock in the behaviour. For example:

attended -> is_attended = true
cancelled -> is_cancelled = true
no_show -> is_no_show = true
possibly an unexpected or null status as well, depending on how we want to handle that case

One smaller point: I would also consider avoiding select * in the bookings CTE unless there is a reason to keep it broad, as being explicit can make schema changes easier to manage.

- {booking_id: 1, member_id: 101, class_id: 1001, booking_timestamp: '2025-01-10 09:00:00', attendance_status: 'attended', check_in_timestamp: '2025-01-10 09:55:00', cancellation_timestamp: null}
- {booking_id: 2, member_id: 102, class_id: 1002, booking_timestamp: '2025-01-11 10:00:00', attendance_status: 'cancelled', check_in_timestamp: null, cancellation_timestamp: '2025-01-10 15:00:00'}
- {booking_id: 3, member_id: 103, class_id: 1003, booking_timestamp: '2025-01-12 11:00:00', attendance_status: 'no_show', check_in_timestamp: null, cancellation_timestamp: null}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

could you add one more test case where attendance_status is just 'null' to begin with, just to check it is handled correctly, otherwise this all looks good :)

Comment thread models/marts/fact_bookings.sql
@rosie-t rosie-t self-requested a review April 15, 2026 16:03
Copy link
Copy Markdown
Collaborator

@rosie-t rosie-t left a comment

Choose a reason for hiding this comment

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

nice!

@EmilyYoung26 EmilyYoung26 merged commit 075050b into main Apr 16, 2026
1 check passed
@EmilyYoung26 EmilyYoung26 deleted the create-fact-bookings branch April 16, 2026 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants