From a3d16d661039f3671d86021154eeaff652d61a90 Mon Sep 17 00:00:00 2001 From: Danielle Frappier Date: Tue, 29 Sep 2026 11:54:24 -0400 Subject: [PATCH 1/3] feat(b2b_dashboard): filter learner-progress by course run Adds an optional courserun_readable_id filter to the learner-progress endpoint, and a new course-runs endpoint listing a contract's course runs (reads mv_b2b_contract_courserun, already exposed by the b2b_learner_records tenant). Together these let the frontend's already-built module filter on the contract learner directory actually filter, and populate its dropdown. Co-Authored-By: Claude Sonnet 5 --- openapi/specs/b2b_dashboard.yaml | 126 ++++++++++++++ .../tenants/b2b_dashboard/learner_models.py | 27 +++ .../tenants/b2b_dashboard/learner_queries.py | 37 ++++ .../tenants/b2b_dashboard/routers/learners.py | 34 ++++ tests/test_dashboard_course_runs.py | 161 ++++++++++++++++++ tests/test_dashboard_learner_progress.py | 15 ++ 6 files changed, 400 insertions(+) create mode 100644 tests/test_dashboard_course_runs.py diff --git a/openapi/specs/b2b_dashboard.yaml b/openapi/specs/b2b_dashboard.yaml index 2091fd7..28c42b0 100644 --- a/openapi/specs/b2b_dashboard.yaml +++ b/openapi/specs/b2b_dashboard.yaml @@ -509,6 +509,16 @@ paths: description: Repeat for several. `unknown` selects rows with withheld outcomes. title: Completion Status description: Repeat for several. `unknown` selects rows with withheld outcomes. + - name: courserun_readable_id + in: query + required: false + schema: + anyOf: + - type: string + - type: 'null' + description: Exact match. Narrows to one course run, e.g. the module filter. + title: Courserun Readable Id + description: Exact match. Narrows to one course run, e.g. the module filter. - name: include_inactive in: query required: false @@ -561,6 +571,55 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPValidationError' + /api/v1/analytics/organizations/{organization_id}/contracts/{contract_id}/course-runs: + get: + tags: + - learners + summary: Course runs under the contract, for the learner-progress module filter + operationId: learners_course_runs_retrieve + parameters: + - name: organization_id + in: path + required: true + schema: + type: string + title: Organization Id + - name: contract_id + in: path + required: true + schema: + type: integer + title: Contract Id + - name: limit + in: query + required: false + schema: + type: integer + maximum: 1000 + minimum: 1 + default: 100 + title: Limit + - name: offset + in: query + required: false + schema: + type: integer + minimum: 0 + default: 0 + title: Offset + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/CourseRunsResponse' + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' /api/v1/analytics/admin/contract-health: get: tags: @@ -1336,6 +1395,73 @@ components: ``seats_consumed`` over ``seat_limit``, null when the limit is zero or null (the view divides by ``nullif(seat_limit, 0)``).' + CourseRun: + properties: + courserun_id: + type: string + title: Courserun Id + description: The course run's ID, e.g. course-v1:MITxT+14.310x+2T2026. + courserun_title: + type: string + title: Courserun Title + description: The course's title. + courserun_start_on: + anyOf: + - type: string + format: date-time + - type: 'null' + title: Courserun Start On + description: When the course run starts. Empty if no start date is set. + courserun_end_on: + anyOf: + - type: string + format: date-time + - type: 'null' + title: Courserun End On + description: When the course run ends. Empty for self-paced courses. + type: object + required: + - courserun_id + - courserun_title + - courserun_start_on + - courserun_end_on + title: CourseRun + description: One course run under the contract, for the learner-progress module + filter. + CourseRunsResponse: + properties: + organization_id: + type: string + title: Organization Id + description: The organization's ID. + as_of: + anyOf: + - type: string + format: date-time + - type: 'null' + title: As Of + description: When the data was last updated. Empty before the first update. + total_count: + type: integer + title: Total Count + description: Course runs under the contract, across all pages. + data: + items: + $ref: '#/components/schemas/CourseRun' + type: array + title: Data + description: This page of course runs. + type: object + required: + - organization_id + - as_of + - total_count + - data + title: CourseRunsResponse + description: 'The org envelope (``organization_id``, ``as_of``, ``total_count``, + ``data``), + + matching ``LearnerProgressResponse``''s shape.' EnrollmentCompletionFunnel: properties: organization_key: diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py b/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py index fb00fc5..3c2827c 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py @@ -189,6 +189,33 @@ class CompletionStatusCounts(BaseModel): ) +class CourseRun(BaseModel): + """One course run under the contract, for the learner-progress module filter.""" + + courserun_id: str = Field( + description="The course run's ID, e.g. course-v1:MITxT+14.310x+2T2026." + ) + courserun_title: str = Field(description="The course's title.") + courserun_start_on: UtcDatetime | None = Field( + description="When the course run starts. Empty if no start date is set." + ) + courserun_end_on: UtcDatetime | None = Field( + description="When the course run ends. Empty for self-paced courses." + ) + + +class CourseRunsResponse(BaseModel): + """The org envelope (``organization_id``, ``as_of``, ``total_count``, ``data``), + matching ``LearnerProgressResponse``'s shape.""" + + organization_id: str = Field(description="The organization's ID.") + as_of: UtcDatetime | None = Field( + description="When the data was last updated. Empty before the first update." + ) + total_count: int = Field(description="Course runs under the contract, across all pages.") + data: list[CourseRun] = Field(description="This page of course runs.") + + class LearnerProgressResponse(BaseModel): """The org envelope (``organization_id``, ``as_of``, ``total_count``, ``data``) plus ``outcomes_withheld_count``, so a client can show how many diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/learner_queries.py b/src/ol_analytics_api/tenants/b2b_dashboard/learner_queries.py index 754ea4d..f748aab 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/learner_queries.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/learner_queries.py @@ -31,6 +31,7 @@ from ol_analytics_api.tenants.b2b_dashboard.config import settings ENROLLMENT_MV = "mv_b2b_learner_enrollment" +CONTRACT_COURSERUN_MV = "mv_b2b_contract_courserun" # Matches the b2b_learner_records tenant. An unrevoked certificate is certified # without requiring is_passing, since production has unrevoked certificates with @@ -88,6 +89,7 @@ class ProgressFilters: contract_id: int search: str | None = None completion_statuses: tuple[str, ...] = () + courserun_readable_id: str | None = None include_inactive: bool = False sort: SortKey = SortKey.FULL_NAME descending: bool = False @@ -137,6 +139,9 @@ def learner_progress(filters: ProgressFilters) -> ProgressQuery: pattern = _contains_pattern(filters.search) predicates.append("(LOWER(email) LIKE %s OR LOWER(full_name) LIKE %s)") params.extend([pattern, pattern]) + if filters.courserun_readable_id: + predicates.append("courserun_readable_id = %s") + params.append(filters.courserun_readable_id) if filters.completion_statuses: # Status values match only rows whose outcomes are shared; `unknown` # selects the withheld ones. Otherwise a status filter would reveal the @@ -182,3 +187,35 @@ def learner_progress(filters: ProgressFilters) -> ProgressQuery: f" FROM ({records}) records{where}" ) return ProgressQuery(page, count, tuple(params)) + + +@dataclass(frozen=True) +class CourseRunsQuery: + """``params`` binds ``count``; ``page`` takes ``params`` plus LIMIT and OFFSET.""" + + page: str + count: str + params: tuple[Any, ...] + + +def course_runs(organization_id: str, contract_id: int) -> CourseRunsQuery: + """The contract's course runs, for ``learner_progress``'s module filter. + + Catalog metadata, not learner rows: unlike ``learner_progress``, there is + no consent gating and no anonymization floor, matching + ``b2b_learner_records.queries.courses()`` for the same reason. + """ + table = f"{validate_sql_identifier(settings.learner_records_schema)}.{CONTRACT_COURSERUN_MV}" + where = "sso_organization_id = %s AND contract_id = %s" + params: tuple[Any, ...] = (organization_id, contract_id) + # Nulls last (self-paced runs have no start date), then title as a stable + # tie-break so LIMIT/OFFSET paging is deterministic. + page = ( + "SELECT courserun_readable_id AS courserun_id, courserun_title," # noqa: S608 + " courserun_start_on, courserun_end_on" + f" FROM {table} WHERE {where}" + " ORDER BY courserun_start_on IS NULL, courserun_start_on, courserun_title" + " LIMIT %s OFFSET %s" + ) + count = f"SELECT COUNT(*) AS total_count FROM {table} WHERE {where}" # noqa: S608 + return CourseRunsQuery(page, count, params) diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/routers/learners.py b/src/ol_analytics_api/tenants/b2b_dashboard/routers/learners.py index 9d0b197..a061bd7 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/routers/learners.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/routers/learners.py @@ -29,6 +29,8 @@ from ol_analytics_api.tenants.b2b_dashboard.config import settings from ol_analytics_api.tenants.b2b_dashboard.learner_models import ( CompletionStatusCounts, + CourseRun, + CourseRunsResponse, LearnerProgress, LearnerProgressResponse, ) @@ -74,6 +76,10 @@ async def learner_progress( # noqa: PLR0913 default_factory=list, ), ], + courserun_readable_id: Annotated[ + str | None, + Query(description="Exact match. Narrows to one course run, e.g. the module filter."), + ] = None, include_inactive: Annotated[ bool, Query(description="Include deactivated enrollments (unenrolled, refunded).") ] = False, @@ -86,6 +92,7 @@ async def learner_progress( # noqa: PLR0913 contract_id=contract_id, search=search, completion_statuses=tuple(status.value for status in completion_status or ()), + courserun_readable_id=courserun_readable_id, include_inactive=include_inactive, sort=sort, descending=descending, @@ -112,3 +119,30 @@ async def learner_progress( # noqa: PLR0913 ), data=[LearnerProgress(**row) for row in rows], ) + + +@router.get( + "/course-runs", + response_model=CourseRunsResponse, + name="course_runs", + operation_id="learners_course_runs_retrieve", + summary="Course runs under the contract, for the learner-progress module filter", +) +async def course_runs( + *, + organization_id: str, + contract_id: int, + page: Annotated[Pagination, Depends(pagination)], +) -> CourseRunsResponse: + query = learner_queries.course_runs(organization_id, contract_id) + as_of = await latest_refresh_timestamp( + settings.learner_records_schema, learner_queries.CONTRACT_COURSERUN_MV + ) + rows = await starrocks_pool.fetch_all(query.page, (*query.params, page.limit, page.offset)) + counts = (await starrocks_pool.fetch_all(query.count, query.params))[0] + return CourseRunsResponse( + organization_id=organization_id, + as_of=as_of, + total_count=int(counts["total_count"]), + data=[CourseRun(**row) for row in rows], + ) diff --git a/tests/test_dashboard_course_runs.py b/tests/test_dashboard_course_runs.py new file mode 100644 index 0000000..1b58976 --- /dev/null +++ b/tests/test_dashboard_course_runs.py @@ -0,0 +1,161 @@ +"""End-to-end tests for the b2b_dashboard course-runs endpoint. + +Same harness as test_dashboard_learner_progress.py: drives the mounted app +over ASGITransport with an org manager's X-Userinfo header, stubbing only the +StarRocks pool and the MITx Online manager check. +""" + +import base64 +import datetime +import json +import re +from unittest.mock import AsyncMock, patch + +import pytest +from httpx import ASGITransport, AsyncClient + +from ol_analytics_api.core.db.refresh_metadata import _clear_cache +from ol_analytics_api.main import create_app +from ol_analytics_api.tenants.b2b_dashboard.learner_models import CourseRun, CourseRunsResponse + +ORG_ID = "11111111-1111-1111-1111-111111111111" +CONTRACT_ID = 101 +PATH = f"/api/v1/analytics/organizations/{ORG_ID}/contracts/{CONTRACT_ID}/course-runs" +_AS_OF = datetime.datetime(2026, 9, 15, 6, 0) # noqa: DTZ001 - StarRocks returns naive UTC + + +def _manager_header(organization_id=ORG_ID): + claims = {"sub": "kc-uuid-1", "organization": {"an-alias": {"id": organization_id}}} + return base64.b64encode(json.dumps(claims).encode()).decode() + + +def _row(**overrides): + return { + "courserun_id": "course-v1:MITxT+14.310x+2T2026", + "courserun_title": "Data Analysis for Social Scientists", + "courserun_start_on": "2026-02-01T00:00:00", + "courserun_end_on": None, + **overrides, + } + + +class _FakePool: + """Answers the as_of probe, the contract gate, the count query and the page + query, recording every call. Course runs carry no PII, so there's no + consent or status-count bucketing to fake, unlike learner-progress's pool.""" + + def __init__(self, rows=(), total_count=0, *, contract_exists=True): + self.rows = list(rows) + self.total_count = total_count + self.contract_exists = contract_exists + self.calls = [] + + async def fetch_all(self, query, params=()): + self.calls.append((query, params)) + if "information_schema" in query: + return [{"as_of": _AS_OF}] + if query.startswith("SELECT 1 "): + return [{"1": 1}] if self.contract_exists else [] + if "COUNT(*)" in query: + return [{"total_count": self.total_count}] + return self.rows + + def page_call(self): + return next(call for call in self.calls if call[0].endswith("LIMIT %s OFFSET %s")) + + def count_call(self): + return next(call for call in self.calls if "COUNT(*)" in call[0]) + + +@pytest.fixture +def app(): + return create_app() + + +@pytest.fixture(autouse=True) +def _clear_as_of_cache(): + _clear_cache() + yield + _clear_cache() + + +async def _get(app, pool, path=PATH, *, is_manager=True, params=None): + with ( + patch("ol_analytics_api.core.db.client.starrocks_pool.fetch_all", new=pool.fetch_all), + patch( + "ol_analytics_api.tenants.b2b_dashboard.auth.mitxonline_client.is_org_manager", + new=AsyncMock(return_value=is_manager), + ), + ): + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + return await client.get(path, params=params, headers={"X-Userinfo": _manager_header()}) + + +async def test_envelope_lists_course_runs(app): + pool = _FakePool(rows=[_row()], total_count=1) + response = await _get(app, pool) + + assert response.status_code == 200 + body = response.json() + assert body["organization_id"] == ORG_ID + assert body["as_of"] == "2026-09-15T06:00:00Z" + assert body["total_count"] == 1 + [row] = body["data"] + assert row["courserun_id"] == "course-v1:MITxT+14.310x+2T2026" + assert row["courserun_end_on"] is None + assert set(row) == set(CourseRun.model_fields) + + +async def test_scopes_to_org_and_contract_with_bound_values(app): + pool = _FakePool() + await _get(app, pool) + page_query, page_params = pool.page_call() + assert "sso_organization_id = %s AND contract_id = %s" in page_query + assert "b2b_learner_records.mv_b2b_contract_courserun" in page_query + assert page_params[:2] == (ORG_ID, CONTRACT_ID) + count_query, count_params = pool.count_call() + assert "sso_organization_id = %s AND contract_id = %s" in count_query + assert count_params == page_params[:-2] + + +async def test_empty_contract_returns_no_rows(app): + pool = _FakePool(rows=[], total_count=0) + response = await _get(app, pool) + body = response.json() + assert body["total_count"] == 0 + assert body["data"] == [] + + +async def test_nulls_start_date_sorts_last(app): + pool = _FakePool() + await _get(app, pool) + assert pool.page_call()[0].endswith( + "ORDER BY courserun_start_on IS NULL, courserun_start_on, courserun_title" + " LIMIT %s OFFSET %s" + ) + + +async def test_contract_not_in_org_is_403(app): + pool = _FakePool(contract_exists=False) + response = await _get(app, pool) + assert response.status_code == 403 + assert not any("mv_b2b_contract_courserun" in query for query, _ in pool.calls) + + +async def test_non_manager_is_refused_before_any_query(app): + pool = _FakePool() + response = await _get(app, pool, is_manager=False) + assert response.status_code == 403 + assert pool.calls == [] + + +@pytest.mark.parametrize("model", [CourseRun, CourseRunsResponse]) +def test_every_field_has_a_manager_facing_description(model): + # Same rationale as learner-progress's own version of this test: the + # dashboard can show these as help text to a manager, who never sees field + # names, so every field needs one and none may lean on another field's name. + field_names = set(CourseRun.model_fields) | set(CourseRunsResponse.model_fields) + for name, field in model.model_fields.items(): + assert field.description, f"{name} has no description" + named = set(re.findall(r"\b[a-z]+(?:_[a-z]+)+\b", field.description)) & field_names + assert not named, f"{name}'s description names {named}" diff --git a/tests/test_dashboard_learner_progress.py b/tests/test_dashboard_learner_progress.py index 75a93c3..5eda55b 100644 --- a/tests/test_dashboard_learner_progress.py +++ b/tests/test_dashboard_learner_progress.py @@ -236,6 +236,21 @@ async def test_search_is_bound_with_wildcards_escaped(app): assert page_params[-4:-2] == ("%garcia\\_50\\%%", "%garcia\\_50\\%%") +async def test_courserun_filter_is_bound_when_given_and_omitted_otherwise(app): + pool = _FakePool() + await _get(app, pool, params={"courserun_readable_id": "course-v1:MITxT+14.310x+2T2026"}) + page_query, page_params = pool.page_call() + count_query, count_params = pool.count_call() + assert "courserun_readable_id = %s" in page_query + assert "courserun_readable_id = %s" in count_query + assert "course-v1:MITxT+14.310x+2T2026" in page_params + assert "course-v1:MITxT+14.310x+2T2026" in count_params + + pool = _FakePool() + await _get(app, pool) + assert "courserun_readable_id = %s" not in pool.page_call()[0] + + async def test_status_filter_cannot_reveal_withheld_statuses(app): pool = _FakePool() await _get(app, pool, params={"completion_status": ["passed", "unknown"]}) From bbbfd5dfcca97ac83e6e9a417afee5123c940562 Mon Sep 17 00:00:00 2001 From: Danielle Frappier Date: Tue, 29 Sep 2026 12:09:15 -0400 Subject: [PATCH 2/3] fix(b2b_dashboard): address Copilot review feedback on course-run filter Reject an explicit empty courserun_readable_id instead of silently treating it as "no filter" (a truthy check let "" fall through). Make course_runs() pagination deterministic by tie-breaking on the unique courserun_readable_id, since courserun_title can repeat across runs. Co-Authored-By: Claude Sonnet 5 --- .../tenants/b2b_dashboard/learner_queries.py | 8 +++++--- .../tenants/b2b_dashboard/routers/learners.py | 5 ++++- tests/test_dashboard_course_runs.py | 3 ++- tests/test_dashboard_learner_progress.py | 7 +++++++ 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/learner_queries.py b/src/ol_analytics_api/tenants/b2b_dashboard/learner_queries.py index f748aab..a79e7b2 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/learner_queries.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/learner_queries.py @@ -208,13 +208,15 @@ def course_runs(organization_id: str, contract_id: int) -> CourseRunsQuery: table = f"{validate_sql_identifier(settings.learner_records_schema)}.{CONTRACT_COURSERUN_MV}" where = "sso_organization_id = %s AND contract_id = %s" params: tuple[Any, ...] = (organization_id, contract_id) - # Nulls last (self-paced runs have no start date), then title as a stable - # tie-break so LIMIT/OFFSET paging is deterministic. + # Nulls last (self-paced runs have no start date), then title for a + # human-friendly order, then the readable id as a unique tie-break so + # LIMIT/OFFSET paging is deterministic even when runs share a title. page = ( "SELECT courserun_readable_id AS courserun_id, courserun_title," # noqa: S608 " courserun_start_on, courserun_end_on" f" FROM {table} WHERE {where}" - " ORDER BY courserun_start_on IS NULL, courserun_start_on, courserun_title" + " ORDER BY courserun_start_on IS NULL, courserun_start_on, courserun_title," + " courserun_readable_id" " LIMIT %s OFFSET %s" ) count = f"SELECT COUNT(*) AS total_count FROM {table} WHERE {where}" # noqa: S608 diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/routers/learners.py b/src/ol_analytics_api/tenants/b2b_dashboard/routers/learners.py index a061bd7..52e86f1 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/routers/learners.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/routers/learners.py @@ -78,7 +78,10 @@ async def learner_progress( # noqa: PLR0913 ], courserun_readable_id: Annotated[ str | None, - Query(description="Exact match. Narrows to one course run, e.g. the module filter."), + Query( + min_length=1, + description="Exact match. Narrows to one course run, e.g. the module filter.", + ), ] = None, include_inactive: Annotated[ bool, Query(description="Include deactivated enrollments (unenrolled, refunded).") diff --git a/tests/test_dashboard_course_runs.py b/tests/test_dashboard_course_runs.py index 1b58976..9ae1741 100644 --- a/tests/test_dashboard_course_runs.py +++ b/tests/test_dashboard_course_runs.py @@ -130,7 +130,8 @@ async def test_nulls_start_date_sorts_last(app): pool = _FakePool() await _get(app, pool) assert pool.page_call()[0].endswith( - "ORDER BY courserun_start_on IS NULL, courserun_start_on, courserun_title" + "ORDER BY courserun_start_on IS NULL, courserun_start_on, courserun_title," + " courserun_readable_id" " LIMIT %s OFFSET %s" ) diff --git a/tests/test_dashboard_learner_progress.py b/tests/test_dashboard_learner_progress.py index 5eda55b..5dc59e6 100644 --- a/tests/test_dashboard_learner_progress.py +++ b/tests/test_dashboard_learner_progress.py @@ -251,6 +251,13 @@ async def test_courserun_filter_is_bound_when_given_and_omitted_otherwise(app): assert "courserun_readable_id = %s" not in pool.page_call()[0] +async def test_empty_courserun_filter_is_rejected(app): + # An explicit empty value is a malformed request, not "no filter" -- silently + # falling back to the unfiltered list would hide the mistake. + response = await _get(app, _FakePool(), params={"courserun_readable_id": ""}) + assert response.status_code == 422 + + async def test_status_filter_cannot_reveal_withheld_statuses(app): pool = _FakePool() await _get(app, pool, params={"completion_status": ["passed", "unknown"]}) From 5c9ea94421d435689a4aeb76d154f216cbd47ef4 Mon Sep 17 00:00:00 2001 From: Danielle Frappier Date: Tue, 29 Sep 2026 12:42:27 -0400 Subject: [PATCH 3/3] chore(b2b_dashboard): regenerate OpenAPI spec Picks up the minLength: 1 constraint added to courserun_readable_id. Co-Authored-By: Claude Sonnet 5 --- openapi/specs/b2b_dashboard.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/openapi/specs/b2b_dashboard.yaml b/openapi/specs/b2b_dashboard.yaml index 28c42b0..c9bd0b1 100644 --- a/openapi/specs/b2b_dashboard.yaml +++ b/openapi/specs/b2b_dashboard.yaml @@ -515,6 +515,7 @@ paths: schema: anyOf: - type: string + minLength: 1 - type: 'null' description: Exact match. Narrows to one course run, e.g. the module filter. title: Courserun Readable Id