Add mitxonline_course_engagements_daily_report by quazi-h · Pull Request #1906 · mitodl/ol-data-platform

quazi-h · 2026-02-13T18:45:31Z

What are the relevant tickets?

https://github.com/mitodl/hq/issues/7644

Description (What does it do?)

This PR converts the Superset virtual dataset marts__mitxonline_course_engagements_daily into a physical dbt reporting model to improve query performance in Superset.

The new mitxonline_course_engagements_daily_report model extends the existing marts__mitxonline_course_engagements_daily mart by adding three columns that were previously calculated in the virtual dataset:

platform - Set to 'MITx Online' for consistent cross-platform filtering
courserun_is_current - Boolean indicating if the course run is currently active (checks if current date falls between courserun start and end dates)
course_readable_id - Course identifier joined from int__mitx__courses for additional course metadata

How can this be tested?

Build and test the model:

dbt build --select mitxonline_course_engagements_daily_report --vars 'schema_suffix: <your_username>' --target dev_production

Once this PR is merged and deployed, the corresponding Superset virtual dataset can be replaced with this physical table.

Copilot

Pull request overview

This PR adds a new dbt reporting model to materialize a previously-virtual Superset dataset, improving query performance by persisting derived fields for MITx Online daily course engagement.

Changes:

Adds mitxonline_course_engagements_daily_report reporting model built on marts__mitxonline_course_engagements_daily.
Introduces derived/reporting-friendly columns: platform, courserun_is_current, and course_readable_id.
Documents the new model and adds a compound-uniqueness test in the reporting models YAML.

Reviewed changes

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

File	Description
`src/ol_dbt/models/reporting/mitxonline_course_engagements_daily_report.sql`	New physical reporting model that extends the daily engagement mart with platform/current-run flag and course metadata.
`src/ol_dbt/models/reporting/_reporting__models.yml`	Adds documentation + tests for the new reporting model.

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

Copilot · 2026-02-20T17:24:18Z

+    , mitx_courses.course_readable_id
+from mitxonline_engagements
+left join mitx_courses
+    on mitxonline_engagements.course_number = mitx_courses.course_number


int__mitx__courses can have multiple rows per course_number (see model docs noting the field may not be unique), so joining it directly on course_number can duplicate engagement rows and break the uniqueness expectation / inflate metrics. Consider deduplicating mitx_courses to 1 row per course_number (e.g., row_number() partitioned by course_number and keep the preferred MITx Online row), or join via a more specific key (e.g., join int__mitxonline__course_runs to get course_id then join on mitxonline_course_id).

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-02-20T18:41:26Z

@quazi-h I've opened a new pull request, #1918, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull request overview

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

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

Copilot · 2026-02-20T20:02:11Z

+    , case
+        when cast(substring(mitxonline_engagements.courserun_start_on, 1, 10) as date) <= current_date
+            and (
+                mitxonline_engagements.courserun_end_on is null
+                or cast(substring(mitxonline_engagements.courserun_end_on, 1, 10) as date) >= current_date
+            )
+            then true
+        else false
+    end as courserun_is_current


courserun_is_current is computed by casting the first 10 chars of the ISO8601 timestamps to date and using an inclusive end-date check (>= current_date). Elsewhere (e.g., int__combined__course_runs) the project derives this flag using from_iso8601_timestamp(...) and compares timestamps with an exclusive end boundary (courserun_end_on > current_date). Consider aligning with that approach to avoid off-by-one behavior for runs that end at midnight and to keep “current” semantics consistent across reports.

Copilot · 2026-02-20T20:02:12Z

+)
+
+, mitx_courses as (
+    select * from {{ ref('int__mitx__courses') }}


The mitx_courses CTE uses select * from int__mitx__courses, but the final query only needs mitxonline_course_id and course_readable_id. Selecting only the required columns will reduce scan/IO and avoid accidental coupling to upstream schema changes.

Suggested change

select * from {{ ref('int__mitx__courses') }}

select

mitxonline_course_id

, course_readable_id

from {{ ref('int__mitx__courses') }}

quazi-h · 2026-02-20T20:36:33Z

@copilot open a new pull request to apply changes based on the comments in this thread

Copilot · 2026-02-20T20:36:41Z

@quazi-h I've opened a new pull request, #1924, to work on those changes. Once the pull request is ready, I'll request review from you.

quazi-h · 2026-02-20T21:33:11Z

@copilot review this branch with most recent changes

Copilot · 2026-02-20T21:33:19Z

@quazi-h I've opened a new pull request, #1928, to work on those changes. Once the pull request is ready, I'll request review from you.

rachellougee

Looks good. Just a small comment but it's not a blocker

rachellougee · 2026-02-23T14:42:26Z

+    , case
+        when
+            mitxonline_engagements.courserun_end_on is null
+            and from_iso8601_timestamp(mitxonline_engagements.courserun_start_on) <= current_date
+            then true
+        when
+            from_iso8601_timestamp(mitxonline_engagements.courserun_start_on) <= current_date
+            and from_iso8601_timestamp(mitxonline_engagements.courserun_end_on) > current_date
+            then true
+        else false


May be helpful to turn this into a macro so that it can be reused, since the same logic appear in another repor https://github.com/mitodl/ol-data-platform/blob/main/src/ol_dbt/models/reporting/mitxonline_video_engagements_w_video_counts.sql#L28-L38

- Create reporting model that extends marts__mitxonline_course_engagements_daily - Add platform column (MITx Online) - Add courserun_is_current boolean (checks if course is currently active) - Add course_readable_id from int__mitx__courses - Add full documentation and tests to _reporting__models.yml - Replaces Superset virtual dataset with physical table

…itxonline_course_engagements_daily_report (#1918) * Initial plan * Fix course join to use mitxonline_course_id instead of course_number to avoid duplicates Co-authored-by: quazi-h <59845076+quazi-h@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: quazi-h <59845076+quazi-h@users.noreply.github.com>

…engagements_daily_report (#1924) * Initial plan * Fix courserun_is_current and select * in mitxonline_course_engagements_daily_report Co-authored-by: quazi-h <59845076+quazi-h@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: quazi-h <59845076+quazi-h@users.noreply.github.com>

…8601 timestamps, selective CTE columns (#1928) * Initial plan * Replace select * with explicit columns in mitxonline_course_engagements_daily_report Co-authored-by: quazi-h <59845076+quazi-h@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: quazi-h <59845076+quazi-h@users.noreply.github.com>

- Replace inline case statement with is_courserun_current macro - Improves code maintainability and reusability - Provides cross-database compatibility (Trino, DuckDB, StarRocks) - Addresses PR feedback

Resolved conflict in _reporting__models.yml by keeping both model definitions: - mitxonline_course_engagements_daily_report (from this branch) - combined_video_engagements_counts_report (from main)

blarghmatey requested a review from Copilot February 20, 2026 17:19

Copilot started reviewing on behalf of blarghmatey February 20, 2026 17:19 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 20, 2026

Fix duplicate engagement rows from non-unique course_number join in mitxonline_course_engagements_daily_report #1918

Merged

quazi-h requested a review from Copilot February 20, 2026 19:57

Copilot started reviewing on behalf of quazi-h February 20, 2026 19:57 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 20, 2026

Fix courserun_is_current logic and select * in mitxonline_course_engagements_daily_report #1924

Merged

Copilot AI mentioned this pull request Feb 20, 2026

Fix mitxonline_course_engagements_daily_report: safe course join, iso8601 timestamps, selective CTE columns #1928

Merged

1 task

rachellougee self-assigned this Feb 23, 2026

rachellougee approved these changes Feb 23, 2026

View reviewed changes

rachellougee mentioned this pull request Feb 23, 2026

Standardize courserun_is_current in dbt #1939

Merged

quazi-h and others added 5 commits February 27, 2026 09:56

Use is_courserun_current macro from cross_db_functions

c9e46a6

- Replace inline case statement with is_courserun_current macro - Improves code maintainability and reusability - Provides cross-database compatibility (Trino, DuckDB, StarRocks) - Addresses PR feedback

quazi-h force-pushed the mitxonline-course-engagements-daily-report branch from 6e83c79 to c9e46a6 Compare February 27, 2026 15:41

Merge main into mitxonline-course-engagements-daily-report

5c8bdf4

Resolved conflict in _reporting__models.yml by keeping both model definitions: - mitxonline_course_engagements_daily_report (from this branch) - combined_video_engagements_counts_report (from main)

quazi-h merged commit 43b4053 into main Mar 2, 2026
6 checks passed

quazi-h deleted the mitxonline-course-engagements-daily-report branch March 2, 2026 16:21

Conversation

quazi-h commented Feb 13, 2026

What are the relevant tickets?

Description (What does it do?)

How can this be tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

quazi-h Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Feb 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

quazi-h commented Feb 20, 2026

Uh oh!

Copilot AI commented Feb 20, 2026

Uh oh!

quazi-h commented Feb 20, 2026

Uh oh!

Copilot AI commented Feb 20, 2026

Uh oh!

rachellougee left a comment

Choose a reason for hiding this comment

Uh oh!

rachellougee Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants