From 822ec4a65007069b666b5c45ce9e58c5cf69c206 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Fri, 21 Aug 2026 14:12:53 -0400 Subject: [PATCH] fix(analytics): resync the hand-written types with what the API returns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `frontends/api/src/analytics/types.ts` mirrors ol-analytics-api's response models by hand, and has fallen behind them twice over. The `*_pk` fields are typed `number` and have never been numeric. They are dbt surrogate keys — `dbt_utils.generate_surrogate_key` MD5-hashes its inputs into a 32-character hex string. ol-analytics-api#29 fixed the same mistake server-side, where it was not cosmetic: `ContractUtilization`, `EnrollmentCompletionFunnel` and `ProgramFunnel` were 500ing in production on a Pydantic `int_parsing` error (Sentry OL-ANALYTICS-API-J, 120 events in two minutes). Nothing breaks on this side because every use is a React key, a Map key or Set membership, all of which behave the same for strings — but the `Map` in CoursePerformanceTable was only well-typed because the type it read was wrong. Separately, ol-analytics-api#33 moved the activity totals onto the cohort that produced them: a total is now suppressed when few enough learners are behind it, so `total_videos_watched` and its two siblings return null even when the total itself is large. Those three were still typed non-null here, and the five cohort counts the change added (`enrolling_learners`, `certified_learners`, `video_watchers`, `problem_attempters`, `chatbot_users`) were missing entirely, as were `video_watchers`/`problem_attempters` on `ContentEngagementDepth`. Nothing renders wrong today: EngagementTrendChart plots only `new_enrollments` and `certificates_earned`, both already nullable and already handled. The factories were the live hazard — they built fixtures with numeric pks, so every test asserting on one was agreeing with a shape production does not send. Fields were checked against `models.py` on ol-analytics-api@main rather than against the ticket, and deliberately stop there: `contract_id` and the contract-grained models arrive with ol-analytics-api#34, which has not merged. This is the third hand-chase (see also the sso_organization_id shim). ol-analytics-api#37 now publishes an OpenAPI spec per tenant and the generated TypeScript client typechecks clean against it, so this file is on its way out — once the clients repo and the Concourse PIPELINE_CONFIGS entry land, types.ts, clients.ts and the `./analytics-types` export all go with it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AVSXwars1LvgtqV1YWhrB1 --- .../api/src/analytics/test-utils/factories.ts | 27 +++++++++++++---- frontends/api/src/analytics/types.ts | 30 ++++++++++++++----- .../Analytics/CoursePerformanceTable.tsx | 2 +- .../DashboardPage/AnalyticsContent.test.tsx | 2 +- 4 files changed, 46 insertions(+), 15 deletions(-) diff --git a/frontends/api/src/analytics/test-utils/factories.ts b/frontends/api/src/analytics/test-utils/factories.ts index fa05457199..703b0af9bb 100644 --- a/frontends/api/src/analytics/test-utils/factories.ts +++ b/frontends/api/src/analytics/test-utils/factories.ts @@ -18,6 +18,16 @@ import type { const organizationId = () => faker.string.uuid() +/** + * `contract_pk`, `courserun_pk` and `program_pk` are dbt surrogate keys — + * `dbt_utils.generate_surrogate_key` MD5-hashes its inputs, so they are + * 32-character lowercase hex strings, never integers. Fixtures that generated + * numbers here were passing while production sent strings (ol-analytics-api#29, + * which 500ed on exactly that mismatch server-side). + */ +const surrogateKey = () => + faker.string.hexadecimal({ length: 32, casing: "lower", prefix: "" }) + const envelope = ( data: RowT[], overrides: Partial> = {}, @@ -36,7 +46,7 @@ const contractUtilization = ( ): ContractUtilization => ({ organization_key: faker.string.alphanumeric(6).toUpperCase(), organization_name: faker.company.name(), - contract_pk: faker.number.int({ min: 1, max: 10000 }), + contract_pk: surrogateKey(), b2b_contract_name: `${faker.company.name()} Contract`, b2b_contract_is_active: true, b2b_contract_start_date: "2026-01-01", @@ -56,9 +66,9 @@ const enrollmentCompletionFunnel = ( ): EnrollmentCompletionFunnel => ({ organization_key: faker.string.alphanumeric(6).toUpperCase(), organization_name: faker.company.name(), - contract_pk: faker.number.int({ min: 1, max: 10000 }), + contract_pk: surrogateKey(), b2b_contract_name: `${faker.company.name()} Contract`, - courserun_pk: faker.number.int({ min: 1, max: 10000 }), + courserun_pk: surrogateKey(), courserun_readable_id: `course-v1:MITx+${faker.string.alphanumeric(5)}+2026`, courserun_title: faker.commerce.productName(), enrolled_learners: 40, @@ -78,10 +88,15 @@ const monthlyEngagementTrend = ( activity_year_and_month: "2026-01", monthly_active_learners: 30, new_enrollments: 12, + enrolling_learners: 10, certificates_earned: 5, + certified_learners: 5, total_videos_watched: 900, + video_watchers: 22, total_problems_attempted: 1200, + problem_attempters: 25, total_chatbot_interactions: 80, + chatbot_users: 14, ...overrides, }) @@ -90,9 +105,9 @@ const programFunnel = ( ): ProgramFunnel => ({ organization_key: faker.string.alphanumeric(6).toUpperCase(), organization_name: faker.company.name(), - contract_pk: faker.number.int({ min: 1, max: 10000 }), + contract_pk: surrogateKey(), b2b_contract_name: `${faker.company.name()} Contract`, - program_pk: faker.number.int({ min: 1, max: 10000 }), + program_pk: surrogateKey(), program_title: `${faker.commerce.department()} Program`, total_courses: 6, enrolled_in_contract_courses: 50, @@ -112,8 +127,10 @@ const contentEngagementDepth = ( engaged_learners: 28, engagement_rate_pct: 70, total_videos_watched: 800, + video_watchers: 22, avg_videos_per_engaged_learner: 28.6, total_problems_attempted: 1000, + problem_attempters: 24, avg_problems_per_engaged_learner: 35.7, total_chatbot_interactions: 60, chatbot_users: 14, diff --git a/frontends/api/src/analytics/types.ts b/frontends/api/src/analytics/types.ts index dcb8d2e99d..e4a449e038 100644 --- a/frontends/api/src/analytics/types.ts +++ b/frontends/api/src/analytics/types.ts @@ -14,6 +14,13 @@ * therefore means "suppressed to protect learner privacy", NOT zero and NOT * missing — render it as such (see `SuppressibleValue` in the dashboard) and * never coerce it to 0 in a chart or an average. + * + * The activity totals (`total_videos_watched` and friends) are nullable for a + * less obvious reason: they count events, not learners, so the floor is applied + * through the cohort that produced them rather than to the total itself. A + * large total suppresses when few enough learners are behind it — 500 videos + * watched by 2 learners comes back `null`. Do not infer from a big number that + * it is safe to display, and do not treat a `null` total as "no activity". */ /** @@ -40,7 +47,7 @@ export type OrgAnalyticsResponse = { export type ContractUtilization = { organization_key: string organization_name: string - contract_pk: number + contract_pk: string b2b_contract_name: string b2b_contract_is_active: boolean b2b_contract_start_date: string | null @@ -58,9 +65,9 @@ export type ContractUtilization = { export type EnrollmentCompletionFunnel = { organization_key: string organization_name: string - contract_pk: number + contract_pk: string b2b_contract_name: string - courserun_pk: number + courserun_pk: string courserun_readable_id: string courserun_title: string enrolled_learners: number @@ -82,19 +89,24 @@ export type MonthlyEngagementTrend = { activity_year_and_month: string monthly_active_learners: number new_enrollments: number | null + enrolling_learners: number | null certificates_earned: number | null - total_videos_watched: number - total_problems_attempted: number - total_chatbot_interactions: number + certified_learners: number | null + total_videos_watched: number | null + video_watchers: number | null + total_problems_attempted: number | null + problem_attempters: number | null + total_chatbot_interactions: number | null + chatbot_users: number | null } /** `mv_b2b_program_funnel` — grain: org x contract x program. */ export type ProgramFunnel = { organization_key: string organization_name: string - contract_pk: number + contract_pk: string b2b_contract_name: string - program_pk: number + program_pk: string program_title: string total_courses: number enrolled_in_contract_courses: number @@ -112,8 +124,10 @@ export type ContentEngagementDepth = { engaged_learners: number | null engagement_rate_pct: number | null total_videos_watched: number | null + video_watchers: number | null avg_videos_per_engaged_learner: number | null total_problems_attempted: number | null + problem_attempters: number | null avg_problems_per_engaged_learner: number | null total_chatbot_interactions: number | null chatbot_users: number | null diff --git a/frontends/main/src/app-pages/DashboardPage/Analytics/CoursePerformanceTable.tsx b/frontends/main/src/app-pages/DashboardPage/Analytics/CoursePerformanceTable.tsx index e284a13fdd..08f8377d2d 100644 --- a/frontends/main/src/app-pages/DashboardPage/Analytics/CoursePerformanceTable.tsx +++ b/frontends/main/src/app-pages/DashboardPage/Analytics/CoursePerformanceTable.tsx @@ -98,7 +98,7 @@ const CoursePerformanceTable: React.FC<{ // The view's grain includes the contract, so a course run appears once per // contract it is offered under. Grouping by contract keeps those rows from // reading as duplicates; the label is dropped when there is only one. - const contracts = new Map() + const contracts = new Map() rows.forEach((row) => { const existing = contracts.get(row.contract_pk) if (existing) { diff --git a/frontends/main/src/app-pages/DashboardPage/AnalyticsContent.test.tsx b/frontends/main/src/app-pages/DashboardPage/AnalyticsContent.test.tsx index 489e40963f..e62b47aca7 100644 --- a/frontends/main/src/app-pages/DashboardPage/AnalyticsContent.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/AnalyticsContent.test.tsx @@ -431,7 +431,7 @@ describe("AnalyticsContent", () => { const courseRows = (count: number) => Array.from({ length: count }, (_, index) => analyticsFactories.enrollmentCompletionFunnel({ - courserun_pk: index + 1, + courserun_pk: String(index + 1), courserun_title: `Course ${index + 1}`, }), )