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 cfd9592..754ea4d 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/learner_queries.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/learner_queries.py @@ -45,6 +45,10 @@ " END" ) +# Upstream stores "" rather than NULL for learners who never set a name. Null +# blank names so they sort with the missing ones instead of before every name. +_BLANK_AS_NULL_NAME = "NULLIF(TRIM(full_name), '')" + # The four branches of _COMPLETION_STATUS. Mutually exclusive, so these buckets # never overlap; a row with withheld outcomes falls into none of them. _STATUSES = ("not_started", "in_progress", "passed", "certified") @@ -118,7 +122,8 @@ def learner_progress(filters: ProgressFilters) -> ProgressQuery: if not filters.include_inactive: scope.append("enrollment_is_active = TRUE") records = ( - "SELECT user_pk, courserun_pk, user_global_id AS learner_id, email, full_name," # noqa: S608 + "SELECT user_pk, courserun_pk, user_global_id AS learner_id, email," # noqa: S608 + f" {_BLANK_AS_NULL_NAME} AS full_name," " courserun_readable_id, courserun_title, courserun_start_on, courserun_end_on," " enrollment_created_on AS enrolled_on, enrollment_is_active, enrollment_mode," f" {_COMPLETION_STATUS} AS completion_status, is_passing, grade_value AS grade," @@ -149,8 +154,8 @@ def learner_progress(filters: ProgressFilters) -> ProgressQuery: where = f" WHERE {' AND '.join(predicates)}" if predicates else "" direction = "DESC" if filters.descending else "ASC" - # Nulls last either way (full_name is often null), then a unique tie-break - # so LIMIT/OFFSET paging is deterministic. + # Nulls last either way (full_name is often null, and `records` nulls blank + # ones), then a unique tie-break so LIMIT/OFFSET paging is deterministic. order_by = ( f"{filters.sort.value} IS NULL, {filters.sort.value} {direction}, user_pk, courserun_pk" ) 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 45cb19f..58c9ea8 100644 --- a/src/ol_analytics_api/tenants/b2b_learner_records/queries.py +++ b/src/ol_analytics_api/tenants/b2b_learner_records/queries.py @@ -109,6 +109,10 @@ def _outcomes_shared() -> str: "chatbot_interactions", ) +# Upstream stores "" rather than NULL for learners who never set a name. Null +# blank names so they sort with the missing ones instead of before every name. +_BLANK_AS_NULL_NAME = "NULLIF(TRIM(full_name), '')" + @dataclass(frozen=True) class RecordFilters: @@ -248,7 +252,8 @@ def enrollments(schema: str, filters: RecordFilters) -> RecordQuery: scope.append("courserun_readable_id = %s") scope_params.append(filters.courserun_id) records = ( - "SELECT user_pk, user_global_id AS learner_id, email, full_name," # noqa: S608 + "SELECT user_pk, user_global_id AS learner_id, email," # noqa: S608 + f" {_BLANK_AS_NULL_NAME} AS full_name," " sso_organization_id AS organization_id, contract_id," " b2b_contract_name AS contract_name, courserun_readable_id AS courserun_id," " courserun_title, courserun_start_on, courserun_end_on," @@ -305,7 +310,8 @@ def learners(schema: str, filters: RecordFilters) -> RecordQuery: schema = validate_sql_identifier(schema) if filters.contract_id is None and not filters.include_inactive: records = ( - "SELECT user_pk, user_global_id AS learner_id, email, full_name," # noqa: S608 + "SELECT user_pk, user_global_id AS learner_id, email," # noqa: S608 + f" {_BLANK_AS_NULL_NAME} AS full_name," " sso_organization_id AS organization_id, organization_name, membership_source," " is_organization_manager, first_enrolled_on, last_enrolled_on, courses_enrolled," " courses_passed, courses_certified," @@ -376,7 +382,8 @@ def _recomputed_learners(schema: str, filters: RecordFilters) -> tuple[str, list enrollment_rollup = ( "SELECT user_pk, MAX(user_global_id) AS user_global_id, MAX(email) AS email," # noqa: S608 - " MAX(full_name) AS full_name, MAX(sso_organization_id) AS sso_organization_id," + f" MAX({_BLANK_AS_NULL_NAME}) AS full_name," + " MAX(sso_organization_id) AS sso_organization_id," " MAX(organization_name) AS organization_name," f" MIN({enrolled_on}) AS first_enrolled_on," f" MAX({enrolled_on}) AS last_enrolled_on," @@ -400,7 +407,8 @@ def _recomputed_learners(schema: str, filters: RecordFilters) -> tuple[str, list records = ( "SELECT COALESCE(l.user_pk, e.user_pk) AS user_pk," # noqa: S608 " COALESCE(l.user_global_id, e.user_global_id) AS learner_id," - " COALESCE(l.email, e.email) AS email, COALESCE(l.full_name, e.full_name) AS full_name," + " COALESCE(l.email, e.email) AS email," + " COALESCE(NULLIF(TRIM(l.full_name), ''), e.full_name) AS full_name," " COALESCE(l.sso_organization_id, e.sso_organization_id) AS organization_id," " COALESCE(l.organization_name, e.organization_name) AS organization_name," " CASE WHEN l.membership_source IN ('roster', 'both') AND e.courses_enrolled > 0" diff --git a/tests/test_dashboard_learner_progress.py b/tests/test_dashboard_learner_progress.py index c20a16b..75a93c3 100644 --- a/tests/test_dashboard_learner_progress.py +++ b/tests/test_dashboard_learner_progress.py @@ -252,6 +252,12 @@ async def test_sort_puts_nulls_last_with_a_unique_tie_break(app): ) +async def test_blank_names_read_as_null_so_they_sort_last(app): + pool = _FakePool() + await _get(app, pool) + assert "NULLIF(TRIM(full_name), '') AS full_name," in pool.page_call()[0] + + async def test_unknown_sort_key_is_rejected(app): response = await _get(app, _FakePool(), params={"sort": "grade"}) assert response.status_code == 422 diff --git a/tests/test_learner_records.py b/tests/test_learner_records.py index 16c2c06..ec79303 100644 --- a/tests/test_learner_records.py +++ b/tests/test_learner_records.py @@ -433,6 +433,17 @@ async def test_include_inactive_learners_keep_every_roster_member(app, monkeypat assert params == (ORG_ID, ORG_ID, 100, 0) +@pytest.mark.parametrize( + "path", + ["enrollments", "learners", "learners?contract_id=42", "learners?include_inactive=true"], +) +async def test_blank_names_read_as_null(app, monkeypatch, path): + pool = _FakePool() + await _get(app, f"/organizations/{ORG_ID}/{path}", _partner_header(ORG_ID), pool, monkeypatch) + query, _ = pool.page_call() + assert "NULLIF(TRIM(full_name), '')" in query + + async def test_recomputed_learners_report_the_staler_view(app, monkeypatch): older = datetime.datetime(2026, 8, 12, 6, 0) # noqa: DTZ001 pool = _FakePool(as_of_by_mv={queries.ENROLLMENT_MV: older})