From 096438d4775afff4d6b767ac41e64fdf2eafa76a Mon Sep 17 00:00:00 2001 From: Danielle Frappier Date: Fri, 25 Sep 2026 09:38:29 -0400 Subject: [PATCH] fix(b2b): return blank learner names as null Upstream stores "" rather than NULL for learners without a profile name. Those rows sorted ahead of every real name and clumped on page 1 of the contract learner directory. Normalize with NULLIF(TRIM(full_name), '') in the learner-progress and learner-records queries so they sort last with the other missing names. Co-Authored-By: Claude Opus 5.5 --- .../tenants/b2b_dashboard/learner_queries.py | 11 ++++++++--- .../tenants/b2b_learner_records/queries.py | 17 +++++++++++++---- tests/test_dashboard_learner_progress.py | 6 ++++++ tests/test_learner_records.py | 11 +++++++++++ 4 files changed, 38 insertions(+), 7 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 2d5f083..e4fcee0 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), '')" + _COLUMNS = ( "learner_id", "email", @@ -114,7 +118,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," @@ -145,8 +150,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 7b3bcff..2fafdc0 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,11 @@ def _outcomes_shared() -> str: "problems_attempted", "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), '')" + _ENROLLMENT_PENDING = ( " NULL AS last_active_on, NULL AS days_active, NULL AS videos_watched," " NULL AS problems_attempted, NULL AS chatbot_interactions," @@ -211,7 +216,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," @@ -266,7 +272,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," @@ -332,7 +339,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," @@ -350,7 +358,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 133eb78..16c895d 100644 --- a/tests/test_dashboard_learner_progress.py +++ b/tests/test_dashboard_learner_progress.py @@ -197,6 +197,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 0b6a67b..18f35df 100644 --- a/tests/test_learner_records.py +++ b/tests/test_learner_records.py @@ -352,6 +352,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})