From 862545494082d707da0fc1b2abd80a35ffc902f6 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Wed, 16 Sep 2026 12:04:08 -0400 Subject: [PATCH] feat(b2b_learner_records): serve activity from the learner-records MVs ol-data-platform#2693 adds last_active_on, days_active and the three counters to mv_b2b_learner_enrollment, and last_active_on and courses_in_progress to mv_b2b_learner. The tenant projected all of them as NULL. in_progress now also counts activity. mv_b2b_learner.courses_in_progress is defined that way, so /learners and /enrollments disagree unless the completion_status CASE matches it. The counter descriptions said "distinct blocks". The MV sums per-day distinct counts, so a block used on two days counts twice. The spec also says activity doesn't move updated_since: record_updated_on can't carry a day-granular activity date without falling below the partner's cursor. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XMuwnLQwHtHuH7aECS7sf3 --- docs/b2b-learner-records-design.md | 11 +++- docs/b2b-learner-records-onepager.md | 6 +- docs/openapi/b2b-learner-records-v1.yaml | 52 +++++++++++----- .../tenants/b2b_learner_records/models.py | 57 +++++++++++------- .../tenants/b2b_learner_records/queries.py | 29 ++++----- tests/test_learner_records.py | 60 +++++++++++++++++++ 6 files changed, 157 insertions(+), 58 deletions(-) diff --git a/docs/b2b-learner-records-design.md b/docs/b2b-learner-records-design.md index 9fed3d4..b28eee0 100644 --- a/docs/b2b-learner-records-design.md +++ b/docs/b2b-learner-records-design.md @@ -58,9 +58,14 @@ The second is more work but produces a correct `user_fk`-keyed `(learner × courserun × day)` fact that the aggregate MVs would also benefit from. Recommend the second. -Until one lands, `last_active_on`, `days_active` and the per-run activity -counters ship as `null`. The spec marks them `x-data-readiness: pending-model` -so a partner sizing the integration knows which columns to expect empty. +Resolved by the second: `afact_learner_courserun_daily_activity` +(ol-data-platform#2672) feeds `last_active_on`, `days_active` and the per-run +counters in both learner-records MVs (ol-data-platform#2693). The counters sum +the fact's per-day distinct counts, so a block used on two days counts twice, +as the `b2b_dashboard` totals do. Activity does not move `record_updated_on`: a +day's activity first appears at a refresh after that day began, so a cursor +taken from it would already sort below the `updated_since` a partner passes. +Partners pick activity up from a full reload. **3. `organization_key` is unreliable for activity attribution.** In `organization_administration_report` it is diff --git a/docs/b2b-learner-records-onepager.md b/docs/b2b-learner-records-onepager.md index 893bcc8..381dc9c 100644 --- a/docs/b2b-learner-records-onepager.md +++ b/docs/b2b-learner-records-onepager.md @@ -232,8 +232,10 @@ Options: an end-date claim on the client, or a warehouse check against Degrading, not blocking — the service ships without these and fills in as they land: the learner-consent field, without which every record reads `outcomes_shared: false`; a per-learner activity model, without which "last -active" and the engagement counters are null; and learner removal on -`mv_b2b_learner`, without which `is_current` is always true. +active" and the engagement counters are null (wired in tenant-side, gated on +ol-data-platform#2693 merging and the MVs rebuilding with the new columns); +and learner removal on `mv_b2b_learner`, without which `is_current` is always +true. Blocking the first partner: the per-contract Keycloak client template and a bearer-only gateway route. No partner can authenticate without both. diff --git a/docs/openapi/b2b-learner-records-v1.yaml b/docs/openapi/b2b-learner-records-v1.yaml index 3f49106..7bcd487 100644 --- a/docs/openapi/b2b-learner-records-v1.yaml +++ b/docs/openapi/b2b-learner-records-v1.yaml @@ -33,8 +33,9 @@ info: `outcomes_shared: false` and outcome fields nulled, a deactivated enrollment with `enrollment_is_active: false`, and a learner who left the organization with `is_current: false`. A client that upserts drops what - it held. Activity changes don't arrive on sync; reload in full - periodically. + it held. Activity (`last_active_on`, `days_active`, the counters, and + the `in_progress` status and count they drive) does not move + `updated_since`; a full reload picks it up. * **Paging is stable within one refresh.** Every collection has a unique order, but a refresh between two pages can shift rows across the offset. If `as_of` changes between pages, restart from offset 0. @@ -593,12 +594,20 @@ components: last_active_on: type: [string, 'null'] format: date - description: Most recent day with recorded course activity. A date, not a timestamp — activity is aggregated per day. Changes to it do not move `updated_since`; a full reload picks them up. - x-data-readiness: pending-model + description: >- + Most recent day with tracked course activity (video play, problem + check, navigation, discussion or chatbot submit) across the + enrollments the request covers. The course platform's local day, + not a UTC day. Null with no activity. + x-data-readiness: available courses_in_progress: type: [integer, 'null'] minimum: 0 - x-data-readiness: pending-model + description: >- + Distinct course runs whose `completion_status` is `in_progress`: + not passed or certified, with a nonzero grade or any tracked + activity. + x-data-readiness: derived courses_passed: type: [integer, 'null'] minimum: 0 @@ -717,7 +726,10 @@ components: oneOf: - $ref: '#/components/schemas/CompletionStatus' - type: 'null' - description: Single derived answer per row. Null when outcomes are withheld. + description: >- + Single derived answer per row. Null when outcomes are withheld. + `in_progress` means not passed or certified, with a nonzero grade + or any tracked activity in the run; `not_started` means neither. x-data-readiness: derived is_passing: type: [boolean, 'null'] @@ -747,28 +759,36 @@ components: last_active_on: type: [string, 'null'] format: date - description: Most recent day with recorded activity in this course run. Changes to it do not move `updated_since`; a full reload picks them up. - x-data-readiness: pending-model + description: >- + Most recent day with tracked activity in this course run. The + course platform's local day, not a UTC day. Null with no activity. + x-data-readiness: available days_active: type: [integer, 'null'] minimum: 0 - description: Distinct days with recorded activity in this course run. Changes to it do not move `updated_since`; a full reload picks them up. - x-data-readiness: pending-model + description: Distinct days with tracked activity in this course run. + x-data-readiness: available videos_watched: type: [integer, 'null'] minimum: 0 - description: Distinct video blocks played. Changes to it do not move `updated_since`; a full reload picks them up. - x-data-readiness: pending-model + description: >- + Video blocks played, counted once per day: a block played on two + days counts twice. + x-data-readiness: available problems_attempted: type: [integer, 'null'] minimum: 0 - description: Distinct problem blocks attempted. Changes to it do not move `updated_since`; a full reload picks them up. - x-data-readiness: pending-model + description: >- + Problem blocks checked, counted once per day: a block attempted on + two days counts twice. Viewing an answer is not an attempt. + x-data-readiness: available chatbot_interactions: type: [integer, 'null'] minimum: 0 - description: Changes to it do not move `updated_since`; a full reload picks them up. - x-data-readiness: pending-model + description: >- + Chatbot submits in this course run, counting each (session, block) + once per day. + x-data-readiness: available CourseRun: type: object diff --git a/src/ol_analytics_api/tenants/b2b_learner_records/models.py b/src/ol_analytics_api/tenants/b2b_learner_records/models.py index 3690aac..1632c87 100644 --- a/src/ol_analytics_api/tenants/b2b_learner_records/models.py +++ b/src/ol_analytics_api/tenants/b2b_learner_records/models.py @@ -9,9 +9,9 @@ NULL for them (see queries._outcomes_shared). This is a second check, so a query change that projects a raw outcome column still can't disclose it. -Fields the warehouse doesn't carry yet (consent date, activity) are projected -as NULL and ship null until upstream models land. They have no default, so -the generated schema lists them as required and nullable, as the contract does. +The consent date isn't in the warehouse yet, so it is projected as NULL and +ships null until the upstream model lands. It has no default, so the generated +schema lists it as required and nullable, as the contract does. """ from __future__ import annotations @@ -34,13 +34,10 @@ def _assume_utc(value: datetime.datetime) -> datetime.datetime: UtcDatetime = Annotated[datetime.datetime, AfterValidator(_assume_utc)] -# Activity is a separate gap from consent. These fields are hardcoded NULL in -# the queries until the activity fact is wired into the MVs, so they stay null -# even for a record whose outcomes are shared. -_ACTIVITY_PENDING = ( - "Not yet populated upstream: null for every record until activity data lands, " - "whatever outcomes_shared says." -) +# record_updated_on carries no activity: a day's activity first appears at a +# refresh after that day began, so a cursor derived from it would already sort +# below the updated_since a partner passes and the change would never be sent. +_ACTIVITY_NOT_SYNCED = "Changes to it do not move updated_since; a full reload picks them up." def _withhold_outcomes[RecordT: BaseModel](record: RecordT, fields: tuple[str, ...]) -> RecordT: @@ -157,14 +154,17 @@ class Learner(BaseModel): ) last_active_on: datetime.date | None = Field( description=( - "Most recent day with recorded course activity. A date, not a timestamp: " - f"activity is aggregated per day. {_ACTIVITY_PENDING}" + "Most recent day with tracked course activity (video play, problem check, " + "navigation, discussion or chatbot submit) across the enrollments the request " + "covers. A date in the course platform's local day, not a UTC day. Null with no " + f"activity. {_ACTIVITY_NOT_SYNCED}" ) ) courses_in_progress: int | None = Field( description=( - "Distinct course runs the learner has started but not yet passed or certified. " - f"{_ACTIVITY_PENDING}" + "Distinct course runs whose completion_status is in_progress: not passed or " + "certified, with a nonzero grade or any tracked activity. " + f"{_ACTIVITY_NOT_SYNCED}" ) ) courses_passed: int | None = Field( @@ -238,10 +238,9 @@ class Enrollment(BaseModel): ) completion_status: CompletionStatus | None = Field( description=( - "Single derived answer per row. Null when outcomes are withheld. Until activity " - "data lands, not_started and in_progress come from the grade alone: in_progress " - "means a nonzero grade, so a learner active in the run with no graded work yet " - "reads not_started." + "Single derived answer per row. Null when outcomes are withheld. in_progress " + "means not passed or certified, with a nonzero grade or any tracked activity in " + "the run; not_started means neither." ) ) is_passing: bool | None = Field(description="Null where no grade has been computed.") @@ -261,20 +260,32 @@ class Enrollment(BaseModel): ) last_active_on: datetime.date | None = Field( description=( - f"Most recent day with recorded activity in this course run. {_ACTIVITY_PENDING}" + "Most recent day with tracked activity in this course run. A date in the course " + f"platform's local day, not a UTC day. Null with no activity. {_ACTIVITY_NOT_SYNCED}" ) ) days_active: int | None = Field( - description=f"Distinct days with recorded activity in this course run. {_ACTIVITY_PENDING}" + description=( + f"Distinct days with tracked activity in this course run. {_ACTIVITY_NOT_SYNCED}" + ) ) videos_watched: int | None = Field( - description=f"Distinct video blocks played. {_ACTIVITY_PENDING}" + description=( + "Video blocks played, counted once per day: a block played on two days counts " + f"twice. {_ACTIVITY_NOT_SYNCED}" + ) ) problems_attempted: int | None = Field( - description=f"Distinct problem blocks attempted. {_ACTIVITY_PENDING}" + description=( + "Problem blocks checked, counted once per day: a block attempted on two days " + f"counts twice. Viewing an answer is not an attempt. {_ACTIVITY_NOT_SYNCED}" + ) ) chatbot_interactions: int | None = Field( - description=f"Chatbot interactions recorded for this course run. {_ACTIVITY_PENDING}" + description=( + "Chatbot submits in this course run, counting each (session, block) once per day. " + f"{_ACTIVITY_NOT_SYNCED}" + ) ) @model_validator(mode="after") diff --git a/src/ol_analytics_api/tenants/b2b_learner_records/queries.py b/src/ol_analytics_api/tenants/b2b_learner_records/queries.py index 7b3bcff..3d72631 100644 --- a/src/ol_analytics_api/tenants/b2b_learner_records/queries.py +++ b/src/ol_analytics_api/tenants/b2b_learner_records/queries.py @@ -20,8 +20,8 @@ it and this becomes ``COALESCE(, )``, so a recorded decision always wins and the setting only covers learners with none. -Columns the warehouse doesn't carry yet (consent date, activity) are projected -as NULL by the inner selects, so filling one in touches the inner select only. +The consent date isn't in the warehouse yet, so the inner selects project it +as NULL; filling it in touches the inner select only. """ from __future__ import annotations @@ -46,12 +46,13 @@ def _outcomes_shared() -> str: # An unrevoked certificate is certified without requiring is_passing: production # has enrollments with an unrevoked certificate and is_passing false # (ol-data-platform#2669). A revoked certificate falls through to the grade. -# Until activity data lands, "in progress" can only mean a nonzero grade. +# in_progress must match mv_b2b_learner.courses_in_progress: a nonzero grade or +# any tracked activity (ol-data-platform#2693). _COMPLETION_STATUS = ( "CASE" " WHEN certificate_is_revoked = FALSE THEN 'certified'" " WHEN is_passing = TRUE THEN 'passed'" - " WHEN grade_value > 0 THEN 'in_progress'" + " WHEN grade_value > 0 OR last_active_on IS NOT NULL THEN 'in_progress'" " ELSE 'not_started'" " END" ) @@ -76,9 +77,7 @@ def _outcomes_shared() -> str: "courses_certified", "certificates_earned", ) -_LEARNER_PENDING = ( - " NULL AS outcomes_consent_on, NULL AS last_active_on, NULL AS courses_in_progress," -) +_LEARNER_PENDING = " NULL AS outcomes_consent_on," _ENROLLMENT_COLUMNS = ( "learner_id", @@ -109,10 +108,6 @@ def _outcomes_shared() -> str: "problems_attempted", "chatbot_interactions", ) -_ENROLLMENT_PENDING = ( - " NULL AS last_active_on, NULL AS days_active, NULL AS videos_watched," - " NULL AS problems_attempted, NULL AS chatbot_interactions," -) @dataclass(frozen=True) @@ -218,7 +213,8 @@ def enrollments(schema: str, filters: RecordFilters) -> RecordQuery: " enrollment_created_on AS enrolled_on, enrollment_is_active, enrollment_mode," f" enrollment_status, {_COMPLETION_STATUS} AS completion_status, is_passing," " grade_value AS grade, letter_grade, certificate_issued_on, certificate_is_revoked," - f"{_ENROLLMENT_PENDING} record_updated_on" + " last_active_on, days_active, videos_played AS videos_watched, problems_attempted," + " chatbot_interactions, record_updated_on" f" FROM {table} WHERE {' AND '.join(scope)}" ) @@ -271,7 +267,7 @@ def learners(schema: str, filters: RecordFilters) -> RecordQuery: " is_organization_manager, first_enrolled_on, last_enrolled_on, courses_enrolled," " courses_passed, courses_certified," " courses_certified + program_certificates_earned AS certificates_earned," - f"{_LEARNER_PENDING} record_updated_on" + f"{_LEARNER_PENDING} last_active_on, courses_in_progress, record_updated_on" f" FROM {schema}.{LEARNER_MV} WHERE sso_organization_id = %s" ) record_params: list[Any] = [str(filters.organization_id)] @@ -340,6 +336,9 @@ def _recomputed_learners(schema: str, filters: RecordFilters) -> tuple[str, list " COUNT(DISTINCT CASE WHEN is_passing = TRUE THEN courserun_pk END) AS courses_passed," " COUNT(DISTINCT CASE WHEN certificate_is_revoked = FALSE THEN courserun_pk END)" " AS courses_certified," + " MAX(last_active_on) AS last_active_on," + f" COUNT(DISTINCT CASE WHEN {_COMPLETION_STATUS} = 'in_progress'" + " THEN courserun_pk END) AS courses_in_progress," " MAX(record_updated_on) AS record_updated_on" f" FROM {schema}.{ENROLLMENT_MV} WHERE {' AND '.join(scope)} GROUP BY user_pk" ) @@ -362,7 +361,9 @@ def _recomputed_learners(schema: str, filters: RecordFilters) -> tuple[str, list " COALESCE(e.courses_passed, 0) AS courses_passed," " COALESCE(e.courses_certified, 0) AS courses_certified," f" COALESCE(e.courses_certified, 0) + {program_certificates} AS certificates_earned," - f"{_LEARNER_PENDING} {record_updated_on} AS record_updated_on" + f"{_LEARNER_PENDING} e.last_active_on," + " COALESCE(e.courses_in_progress, 0) AS courses_in_progress," + f" {record_updated_on} AS record_updated_on" f" FROM (SELECT * FROM {schema}.{LEARNER_MV} WHERE sso_organization_id = %s) l" f" {join} ({enrollment_rollup}) e ON l.user_pk = e.user_pk" ) diff --git a/tests/test_learner_records.py b/tests/test_learner_records.py index 6690d33..4314860 100644 --- a/tests/test_learner_records.py +++ b/tests/test_learner_records.py @@ -297,6 +297,66 @@ def test_models_keep_outcomes_when_shared(): assert (enrollment.completion_status, enrollment.grade) == ("passed", 0.8) +def test_models_keep_activity_when_shared(): + enrollment = Enrollment( + **_enrollment_row( + outcomes_shared=1, + completion_status="in_progress", + last_active_on=datetime.date(2026, 8, 11), + days_active=34, + videos_watched=212, + problems_attempted=88, + chatbot_interactions=14, + ) + ) + assert ( + enrollment.last_active_on, + enrollment.days_active, + enrollment.videos_watched, + enrollment.problems_attempted, + enrollment.chatbot_interactions, + ) == (datetime.date(2026, 8, 11), 34, 212, 88, 14) + + +async def test_enrollments_project_the_activity_columns(app, monkeypatch): + pool = _FakePool() + await _get( + app, f"/organizations/{ORG_ID}/enrollments", _partner_header(ORG_ID), pool, monkeypatch + ) + query, _ = pool.page_call() + assert "videos_played AS videos_watched" in query + assert "NULL AS days_active" not in query + # Activity alone is enough to count as started. + assert "WHEN grade_value > 0 OR last_active_on IS NOT NULL THEN 'in_progress'" in query + + +async def test_default_learners_project_the_activity_columns(app, monkeypatch): + pool = _FakePool() + await _get(app, f"/organizations/{ORG_ID}/learners", _partner_header(ORG_ID), pool, monkeypatch) + query, _ = pool.page_call() + assert "NULL AS last_active_on" not in query + assert "NULL AS courses_in_progress" not in query + assert "last_active_on, courses_in_progress, record_updated_on FROM" in query + + +async def test_recomputed_learners_count_in_progress_with_the_enrollment_status(app, monkeypatch): + pool = _FakePool() + await _get( + app, + f"/organizations/{ORG_ID}/learners?contract_id=42", + _partner_header(ORG_ID), + pool, + monkeypatch, + ) + query, _ = pool.page_call() + assert "MAX(last_active_on) AS last_active_on" in query + assert ( + f"COUNT(DISTINCT CASE WHEN {queries._COMPLETION_STATUS} = 'in_progress'" # noqa: SLF001 + " THEN courserun_pk END) AS courses_in_progress" + ) in query + assert "COALESCE(e.courses_in_progress, 0) AS courses_in_progress" in query + + async def test_default_learners_read_the_precomputed_rollup(app, monkeypatch): pool = _FakePool() await _get(app, f"/organizations/{ORG_ID}/learners", _partner_header(ORG_ID), pool, monkeypatch)