Backfill missing LegalAddress/UserProfile for migrated users; guard should_skip_onboarding

[shaidar]

What are the relevant tickets?

N/A

Description (What does it do?)

migrate_edx_data --type users has a historical bug: _bulk_create_users calls User.objects.bulk_create(new_users, batch_size=batch_size, ignore_conflicts=True) and returns that result directly. bulk_create(ignore_conflicts=True) never populates .pk on the objects passed into it, on any backend, since it can't reliably map generated ids back to input objects when some rows are silently skipped on conflict. Every user created through this path therefore has id=None in the returned list, so _migrate_users builds id_row_lookup keyed on None, and _bulk_create_legal_addresses/_bulk_create_user_profiles both do User.objects.filter(id__in=[None, ...]), which matches zero rows in Postgres (IN (NULL) never matches). The net effect: every user created by this migration type ends up with no LegalAddress or UserProfile row at all - confirmed in production (User.objects.filter(scim_external_id__isnull=False, user_profile__isnull=True).count() → 63,852). The root-cause fix for future runs is a separate PR; this one repairs the users already affected.

This PR:

1. Adds a new migrate_edx_data --type repair_migrated_profiles that re-scans the same edxorg_to_mitxonline_users Trino table, finds users already missing a LegalAddress and/or UserProfile row, and repairs them using the existing, already-correct _bulk_create_legal_addresses/_bulk_create_user_profiles helpers - those helpers only ever broke because they were fed PK-less objects; given real, already-saved User rows (real .id), they work exactly like they already do for the enrollments migration's opportunistic repair path (_migrate_enrollments). Supports --dry-run and --limit, matching the existing conventions in this command. Idempotent: both helpers already skip users who already have the row, so re-running is always safe.
2. Guards User.should_skip_onboarding (users/models.py) against a missing UserProfile. This is a separate, more urgent problem than the backfill itself: it currently does self.user_profile.completed_onboarding with no guard, which raises RelatedObjectDoesNotExist today, in production, on every login for any of the affected users, via OpenedxAndApiGatewayLoginView.get()'s not user.should_skip_onboarding check. This fix stops that crash regardless of how/when the backfill itself is run.

How can this be tested?

courses/management/commands/test_migrate_edx_data.py (new) covers the new repair_migrated_profiles type against a fake Trino cursor/connection: creating both missing rows, backfilling only the missing piece when one already exists, leaving already-correct users untouched, leaving users with no matching edX row still-broken-but-not-crashing, --dry-run writing nothing, and --limit capping how many get repaired.

users/models_test.py adds two cases for should_skip_onboarding: a user with no UserProfile row no longer crashes and falls back to the enrollment check, and the existing completed-onboarding behavior is unchanged.

To validate against real data before running unbounded in production:

python manage.py migrate_edx_data --type repair_migrated_profiles --dry-run

then a small --limit run, spot-checking a couple of repaired users' LegalAddress/UserProfile in the admin, before running unbounded.

Additional Context

Depends on nothing merged yet, and nothing depends on it - it's independent of the SCIM name-mapping PRs (#3836, #3837) and the root-cause _bulk_create_users PK fix (separate PR). Verified via code reading that #3836/#3837's LearnUserAdapter already tolerates users missing these rows (it uses getattr(self.obj, "user_profile", UserProfile()), which works because Django's RelatedObjectDoesNotExist for reverse one-to-ones is deliberately a subclass of AttributeError), so this backfill is not a blocker for those - it's being done because it's the right fix for the underlying data, and because of the separate live should_skip_onboarding crash.

[github-actions]

OpenAPI Changes

Show/hide changes

## Changes for v0.yaml:
No changes detected

## Changes for v1.yaml:
No changes detected

## Changes for v2.yaml:
No changes detected

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

[sentry]

Bug: The repair query for migrated users is too broad, potentially corrupting data for non-migrated users with missing profiles and matching emails.
_{Severity: LOW}

Suggested Fix

Filter the initial query in _repair_migrated_user_profiles to only include users who were part of the migration. This can be done by checking for a migration-specific marker, such as a scim_external_id, instead of querying for all users missing a LegalAddress or UserProfile.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: courses/management/commands/migrate_edx_data.py#L428-L432

Potential issue: The function `_repair_migrated_user_profiles` queries for all users
missing a `LegalAddress` or `UserProfile` record. This is intended to fix users created
via `User.objects.bulk_create`, which bypasses the signals that normally create these
records. However, the query is not restricted to only migrated users. In the unlikely
event that a non-migrated user is also missing one of these records (due to some other
bug) and shares an email address with a user in the migration data, this command would
silently backfill the non-migrated user's profile with incorrect data from the migrated
user, causing data corruption.

[rachellougee]

@shaidar if the plan is to use edxorg_to_mitxonline_users to fix the migrated users who are missing a LegalAddress/UserProfile, then this is the right place. But Q(legal_address__isnull=True) | Q(user_profile__isnull=True) matches every user in the database missing one of these rows, including users who were not part of the edX users we migrated, and we won't have the data to populate theirs.

The simple fix might be to create a blank LegalAddress/UserProfile @rhysyngsun any thoughts on whether we scope this to migrated users only, or backfill blanks for everyone missing a row?

[rhysyngsun]

I think because this command is scoped to repair migrated users, let's just leave the case as-is for now and we'll look into repairing everyone else separately. I mentioned yesterday that users somehow not having a legaladdress/profile seems to be happening through other means so we need some kind of self-healing mechanism that repairs these users on an ongoing basis but that should be it's own PR.

[shaidar]

Sounds good - leaving the query as-is here. Happy to help scope the self-healing follow-up once we've got a sense of how it's happening outside this migration path.

[rhysyngsun]

I think because this command is scoped to repair migrated users, let's just leave the case as-is for now and we'll look into repairing everyone else separately. I mentioned yesterday that users somehow not having a legaladdress/profile seems to be happening through other means so we need some kind of self-healing mechanism that repairs these users on an ongoing basis but that should be it's own PR.

[rhysyngsun]

I think based on the fact we're only using id and email, I'd make this change to reduce the amount of data we load into memory and make it less likely we'll get oom-killed:

Suggested change

	)
	).only("id", "email")

[shaidar]

Added in 5a8208c - good catch, these are the only two fields either helper actually needs off this queryset.

[rhysyngsun]

LGTM