Skip to content

feat(#2075): migrate marts__combined_course_enrollment_detail to dimensional models#2314

Open
quazi-h wants to merge 6 commits into
mainfrom
copilot/2075-migrate-combined-course-enrollment-detail
Open

feat(#2075): migrate marts__combined_course_enrollment_detail to dimensional models#2314
quazi-h wants to merge 6 commits into
mainfrom
copilot/2075-migrate-combined-course-enrollment-detail

Conversation

@quazi-h

@quazi-h quazi-h commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

What are the relevant tickets?

Closes #2075 (mart migration — depends on prerequisite PR #2317)

⚠️ This PR must be merged and deployed AFTER #2317 (dim_course_run and tfact_enrollment column additions, deployed with --full-refresh). Deploying this mart before #2317 will result in NULL values for courserun_upgrade_deadline and courserunenrollment_is_edx_enrolled.

⚠️ This PR must also be merged and deployed AFTER #2406 (adds Emeritus/Global Alumni platform support to tfact_enrollment, requires its own --full-refresh). Without it, tfact_enrollment has no rows for the emeritus/global_alumni platforms, so this migration would silently drop those enrollments from the mart.

Description (What does it do?)

Migrates marts__combined_course_enrollment_detail from intermediate combined models (int__combined__courserun_enrollments, int__combined__course_runs, int__combined__users) to the dimensional layer.

The 6-way per-platform UNION ALL is replaced by a single flat query joining dimensional models:

Notable join strategy decisions:

  • Certificate join: uses the platform-specific user_username (from dim_user, or the deduped bootcamps_users CTE for Bootcamps) as the join key, restoring the pre-migration int__combined__courserun_enrollments semantics. user_email on dim_user is a deduped/canonicalized value and can silently miss platform-specific certificate records, so it is only used as a secondary match for the edX.org MITxOnline certificate cross-lookup (alongside user_mitxonline_username).
  • edX.org MITxOnline certificate cross-lookup: previously used int__micromasters__users CTE; now uses dim_user.user_mitxonline_username directly, eliminating that CTE.
  • edX.org MicroMasters order join: uses dim_user.micromasters_user_id replacing the micromasters_users CTE user_id lookup.
  • Bootcamps order join: changed from user_username-based to user_id-based (dim_user.bootcamps_application_user_id = bootcamps_completed_orders.order_purchaser_user_id), since Bootcamps username is not in dim_user.
  • MITxPro orders: bridged via a supplemental mitxpro_enrollment_orders CTE sourcing ecommerce_order_id from int__mitxpro__courserunenrollments (not yet in tfact_enrollment).
  • Grades: sourced from tfact_grade joined on user_fk + courserun_fk — clean dimensional join, no per-platform grade CTEs needed.

All 38 output columns preserved and verified via systematic diff against the original final SELECT.

How can this be tested?

Prerequisite: #2317 must be deployed first (with --full-refresh on both dimensional models).

-- Q1: Row count parity by platform
SELECT
    platform,
    count(*) AS row_count
FROM ol_warehouse_marts_qa.marts__combined_course_enrollment_detail
GROUP BY 1
ORDER BY 1;
-- Q2: Verify courserun_upgrade_deadline is populated for MITxOnline
SELECT
    platform,
    count(*) AS total_rows,
    count(courserun_upgrade_deadline) AS non_null_upgrade_deadline
FROM ol_warehouse_marts_qa.marts__combined_course_enrollment_detail
WHERE platform = 'MITx Online'
GROUP BY 1;
-- Q3: Verify courserunenrollment_is_edx_enrolled distribution
SELECT
    platform,
    courserunenrollment_is_edx_enrolled,
    count(*) AS row_count
FROM ol_warehouse_marts_qa.marts__combined_course_enrollment_detail
GROUP BY 1, 2
ORDER BY 1, 2;
-- Expected: edxorg and residential = all true; mitxonline/mitxpro = mix of true/false/null;
-- bootcamps/emeritus/global_alumni = all null
-- Q4: Verify grade data populates from tfact_grade
SELECT
    platform,
    count(*) AS total_enrollments,
    count(courserungrade_grade) AS enrollments_with_grade,
    count(courserungrade_is_passing) AS enrollments_with_pass_flag
FROM ol_warehouse_marts_qa.marts__combined_course_enrollment_detail
GROUP BY 1
ORDER BY 1;

Additional Context

Deployment order (no breaking change if followed):

  1. Merge and deploy feat(#2075): add courserun_upgrade_deadline to dim_course_run and enrollment_is_edx_enrolled to tfact_enrollment #2317 — run dim_course_run --full-refresh and tfact_enrollment --full-refresh
  2. Verify Q1/Q2 queries in feat(#2075): add courserun_upgrade_deadline to dim_course_run and enrollment_is_edx_enrolled to tfact_enrollment #2317 show correct values
  3. Merge and deploy this PR — run marts__combined_course_enrollment_detail normally (no full-refresh needed)

Downstream consumers (dashboards, reports) using courserun_upgrade_deadline or courserunenrollment_is_edx_enrolled will see no regression as long as step 1 completes before step 3.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates the marts__combined_course_enrollment_detail mart from a set of per-platform intermediate “combined” models to a dimensional-layer join strategy centered on tfact_enrollment, dim_course_run, dim_course, dim_user, and tfact_grade, while retaining intermediate ecommerce/certificate CTEs where dimensional equivalents don’t yet exist.

Changes:

  • Replaces the prior 6-way per-platform UNION ALL pattern with a single flattened query joining dimensional facts/dims by user_fk + courserun_fk.
  • Reworks certificate and ecommerce order enrichment joins, including a supplemental MITxPro enrollment→order bridge CTE.
  • Switches grade sourcing to tfact_grade using the dimensional grain (user_fk, courserun_fk).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ol_dbt/models/marts/combined/marts__combined_course_enrollment_detail.sql Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment thread src/ol_dbt/models/marts/combined/marts__combined_course_enrollment_detail.sql Outdated
@quazi-h
quazi-h requested a review from Copilot July 7, 2026 21:38
@quazi-h
quazi-h marked this pull request as ready for review July 7, 2026 21:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

blarghmatey pushed a commit that referenced this pull request Jul 16, 2026
- Add emeritus_enrollments/global_alumni_enrollments CTEs sourced from
  stg__emeritus__api__bigquery__user_enrollments /
  stg__global_alumni__api__bigquery__user_enrollments (the same staging
  models dim_user already uses for its emeritus_user_id/global_alumni_user_id
  columns), following the existing per-platform CTE pattern.
- Add mitxpro_external_readable_id_lookup CTE (sourced from
  int__mitxpro__course_runs) to resolve these platforms' course run FK:
  Emeritus/Global Alumni don't own course run records, so their enrollments
  map onto existing MITxPro course runs via courserun_external_readable_id,
  mirroring the join logic previously used in
  int__combined__courserun_enrollments.
- Extend user_lookup to select emeritus_user_id/global_alumni_user_id from
  dim_user (already present there) and thread through the user_fk coalesce
  chain.
- Add dim_course_run_mitxpro CTE + join to resolve courserun_fk for these
  two platforms against MITxPro-platform dim_course_run rows.
- enrollment_is_edx_enrolled is null for both (same treatment as
  program/bootcamps enrollments).

Prerequisite for #2075/PR #2314 (migrate marts__combined_course_enrollment_detail
to the dimensional layer): without this, sourcing that mart exclusively from
tfact_enrollment would silently drop all Emeritus/Global Alumni enrollment rows.

Closes #2405

Requires a --full-refresh of tfact_enrollment after deploy to backfill
historical Emeritus/Global Alumni enrollment rows (incremental_strategy is
delete+insert, so new CTEs alone only capture new activity going forward).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
quazi-h and others added 6 commits July 16, 2026 10:16
…nsional models

- Add courserun_upgrade_deadline to dim_course_run (MITxOnline only; null for other platforms)
- Add enrollment_is_edx_enrolled to tfact_enrollment (per-platform: true for edxorg/residential,
  from source field for mitxonline/mitxpro, null for program/bootcamps)
- Rewrite marts__combined_course_enrollment_detail to source from dimensional layer:
  - tfact_enrollment + dim_course_run + dim_course + dim_user replace 6-way UNION ALL
  - Grade data from tfact_grade (joined on user_fk + courserun_fk)
  - Certificate join uses user_email (from dim_user) as cross-table key
  - edX.org MITxOnline certificate cross-lookup uses dim_user.user_mitxonline_username
  - Bootcamps order join changed from username-based to user_id-based
  - MITxPro order join bridged via supplemental ecommerce_order_id CTE
  - edX.org MicroMasters order join uses dim_user.micromasters_user_id
  - All 38 output columns preserved and verified via systematic diff

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ment_detail

- course_readable_id: fall back to extract_course_readable_id(courserun_readable_id)
  instead of the raw courserun_readable_id (a course-run id, not a course id) when
  dim_course doesn't join, matching int__combined__course_runs' existing logic.
- Add bootcamps_users CTE (joined on dim_user.bootcamps_application_user_id) since
  dim_user doesn't expose a bootcamps username attribute; restores user_username
  for Bootcamps enrollments (previously always NULL, a regression vs the prior mart).
- user_username: add a Residential branch (dim_user.user_residential_username),
  previously NULL despite dim_user exposing this value (also a regression).
- Certificate join: match on the platform-specific username (mitxonline/edxorg/
  mitxpro/bootcamps) instead of dim_user.email, restoring the pre-migration
  int__combined__courserun_enrollments join semantics. dim_user.email is a
  deduped/canonicalized value and can silently miss platform-specific cert records.
- micromasters_completed_orders: fix row_number() dedup partition to match the
  actual join key (courserun_edxorg_readable_id) instead of the MicroMasters-native
  courserun_readable_id, a different id namespace -- the mismatch could drop valid
  orders by assigning row_num=1 within the wrong partition.

Verified with dbt compile (clean) and sqlfluff-lint/-fix (no changes needed).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The prior fix added a Residential branch to the user_username case
expression referencing users.user_residential_username, but the users
CTE never selected that column from dim_user, causing a COLUMN_NOT_FOUND
compile failure.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ate fan-out

int__bootcamps__users joins profiles_legaladdress without dedup, so it can
return multiple rows per user_id. Since only user_username is selected
(invariant per user_id, sourced from the users table not legaladdress),
switching to select distinct prevents duplicate rows downstream without
needing a tie-breaker.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The combined_certificates join's platform-specific username CASE expression
omitted the Residential platform, making courseruncertificate_* fields
always NULL for Residential enrollments even when
dim_user.user_residential_username is available and certificates exist.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e_enrollment_detail

mitxonline_certificates selected from int__mitxonline__courserun_certificates
without filtering revoked certs. Since the edX.org cross-lookup coalesces
mitxonline_certificates.courseruncertificate_* fields, a revoked cert could
still populate URL/UUID/timestamps even when courseruncertificate_is_earned
is false, and could fan out rows if multiple certificate records exist.
Filter to non-revoked to match int__combined__courserun_certificates.
@blarghmatey
blarghmatey force-pushed the copilot/2075-migrate-combined-course-enrollment-detail branch from 851f46f to 0340b5c Compare July 16, 2026 14:33
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.

Migrate marts__combined_course_enrollment_detail to use dimensional layer

2 participants