From e7295d5e5e4cf91b97246893f128cc9cdfd8b721 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Tue, 11 Aug 2026 16:00:37 -0400 Subject: [PATCH 1/3] refactor: pass the Learn Fastly service ID at the call sites (#3804) Co-authored-by: Claude Opus 5 (1M context) --- cms/signals.py | 7 ++++++- cms/signals_test.py | 21 +++++++++++++++++---- cms/tasks.py | 9 +++++++-- cms/tasks_test.py | 16 +++++++++++++--- courses/signals.py | 19 ++++++++++++++++--- courses/signals_test.py | 32 +++++++++++++++++++++++++------- 6 files changed, 84 insertions(+), 20 deletions(-) diff --git a/cms/signals.py b/cms/signals.py index 9bf4a5be8b..04312afa3b 100644 --- a/cms/signals.py +++ b/cms/signals.py @@ -1,5 +1,6 @@ import logging +from django.conf import settings from django.db import transaction from wagtail.signals import page_published @@ -56,7 +57,11 @@ def purge_fastly_cache_on_publish(sender, **kwargs): # noqa: ARG001 logger.info( "Scheduling Fastly surrogate key purge on page publish: %s", surrogate_key ) - transaction.on_commit(lambda: queue_fastly_surrogate_key_purge.delay(surrogate_key)) + transaction.on_commit( + lambda: queue_fastly_surrogate_key_purge.delay( + surrogate_key, settings.MIT_LEARN_FASTLY_SERVICE_ID + ) + ) page_published.connect(flex_pricing_field_check) diff --git a/cms/signals_test.py b/cms/signals_test.py index 37684889a0..823e813d6d 100644 --- a/cms/signals_test.py +++ b/cms/signals_test.py @@ -10,10 +10,21 @@ pytestmark = pytest.mark.django_db +LEARN_SERVICE_ID = "test-learn-service-id" + + +@pytest.fixture +def learn_service_id(settings): + """Point the purge at a known MIT Learn Fastly service.""" + settings.MIT_LEARN_FASTLY_SERVICE_ID = LEARN_SERVICE_ID + return settings.MIT_LEARN_FASTLY_SERVICE_ID + @patch("cms.signals.transaction.on_commit", side_effect=lambda callback: callback()) @patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") -def test_purge_fastly_cache_on_publish_course_page(mock_purge_delay, mock_on_commit): +def test_purge_fastly_cache_on_publish_course_page( + mock_purge_delay, mock_on_commit, learn_service_id +): """Publishing a CoursePage purges the key for its course.""" course_page = CoursePageFactory.create() mock_purge_delay.reset_mock() @@ -26,13 +37,15 @@ def test_purge_fastly_cache_on_publish_course_page(mock_purge_delay, mock_on_com course_page.save_revision().publish() mock_purge_delay.assert_called_once_with( - f"mitxonline:course:{course_page.course.readable_id}" + f"mitxonline:course:{course_page.course.readable_id}", learn_service_id ) @patch("cms.signals.transaction.on_commit", side_effect=lambda callback: callback()) @patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") -def test_purge_fastly_cache_on_publish_program_page(mock_purge_delay, mock_on_commit): +def test_purge_fastly_cache_on_publish_program_page( + mock_purge_delay, mock_on_commit, learn_service_id +): """Publishing a ProgramPage purges the key for its program.""" program_page = ProgramPageFactory.create() mock_purge_delay.reset_mock() @@ -41,7 +54,7 @@ def test_purge_fastly_cache_on_publish_program_page(mock_purge_delay, mock_on_co program_page.save_revision().publish() mock_purge_delay.assert_called_once_with( - f"mitxonline:program:{program_page.program.readable_id}" + f"mitxonline:program:{program_page.program.readable_id}", learn_service_id ) diff --git a/cms/tasks.py b/cms/tasks.py index 6fbbf14c8b..c3af5b9512 100644 --- a/cms/tasks.py +++ b/cms/tasks.py @@ -121,8 +121,13 @@ def queue_fastly_surrogate_key_purge(surrogate_key, service_id=None): service_id = service_id or settings.MIT_LEARN_FASTLY_SERVICE_ID + # Logged at error level, not warning: a missing setting disables cache + # invalidation entirely, and Sentry only raises an issue from ERROR and above + # (main.sentry passes no event_level, so it keeps the sentry_sdk default). + # A warning here is a breadcrumb, which is how the unset service ID went + # unnoticed until it was found by reading the code. if not service_id: - logger.warning( + logger.error( "No Fastly service ID given; skipping surrogate key purge for %s. " "Is MIT_LEARN_FASTLY_SERVICE_ID set?", surrogate_key, @@ -130,7 +135,7 @@ def queue_fastly_surrogate_key_purge(surrogate_key, service_id=None): return False if not settings.FASTLY_AUTH_TOKEN: - logger.warning( + logger.error( "FASTLY_AUTH_TOKEN is not set; skipping surrogate key purge for %s", surrogate_key, ) diff --git a/cms/tasks_test.py b/cms/tasks_test.py index 9e3592ba8a..82ebef40ff 100644 --- a/cms/tasks_test.py +++ b/cms/tasks_test.py @@ -1,5 +1,7 @@ """Tests for cms.tasks""" +import logging + import pytest import responses @@ -71,25 +73,33 @@ def test_queue_fastly_surrogate_key_purge_falls_back_to_settings(fastly_settings @responses.activate -def test_queue_fastly_surrogate_key_purge_skips_without_service_id(fastly_settings): +def test_queue_fastly_surrogate_key_purge_skips_without_service_id( + fastly_settings, caplog +): """ With no service ID passed and none configured, the purge is skipped. - It must skip rather than raise or request `/service/None/purge/...`. + It must skip rather than raise or request `/service/None/purge/...`, and it + must say so at error level -- an unconfigured service ID disables cache + invalidation, and only ERROR and above reaches Sentry as an issue. """ fastly_settings.MIT_LEARN_FASTLY_SERVICE_ID = None assert queue_fastly_surrogate_key_purge(SURROGATE_KEY) is False assert not responses.calls + assert [record.levelno for record in caplog.records] == [logging.ERROR] @responses.activate -def test_queue_fastly_surrogate_key_purge_skips_without_auth_token(fastly_settings): +def test_queue_fastly_surrogate_key_purge_skips_without_auth_token( + fastly_settings, caplog +): """A missing auth token skips the purge rather than sending it unauthenticated.""" fastly_settings.FASTLY_AUTH_TOKEN = None assert queue_fastly_surrogate_key_purge(SURROGATE_KEY, LEARN_SERVICE_ID) is False assert not responses.calls + assert [record.levelno for record in caplog.records] == [logging.ERROR] @responses.activate diff --git a/courses/signals.py b/courses/signals.py index 232a4ae8cf..eeaf9d084d 100644 --- a/courses/signals.py +++ b/courses/signals.py @@ -2,6 +2,7 @@ Signals for mitxonline course certificates """ +from django.conf import settings from django.db import transaction from django.db.models.signals import post_save from django.dispatch import receiver @@ -90,7 +91,11 @@ def purge_fastly_cache_on_course_save( from cms.tasks import queue_fastly_surrogate_key_purge # noqa: PLC0415 surrogate_key = f"mitxonline:course:{instance.readable_id}" - transaction.on_commit(lambda: queue_fastly_surrogate_key_purge.delay(surrogate_key)) + transaction.on_commit( + lambda: queue_fastly_surrogate_key_purge.delay( + surrogate_key, settings.MIT_LEARN_FASTLY_SERVICE_ID + ) + ) @receiver(post_save, sender=CourseRun, dispatch_uid="courserun_post_save_fastly_purge") @@ -107,7 +112,11 @@ def purge_fastly_cache_on_course_run_save( from cms.tasks import queue_fastly_surrogate_key_purge # noqa: PLC0415 surrogate_key = f"mitxonline:course:{instance.course.readable_id}" - transaction.on_commit(lambda: queue_fastly_surrogate_key_purge.delay(surrogate_key)) + transaction.on_commit( + lambda: queue_fastly_surrogate_key_purge.delay( + surrogate_key, settings.MIT_LEARN_FASTLY_SERVICE_ID + ) + ) @receiver(post_save, sender=Program, dispatch_uid="program_post_save_fastly_purge") @@ -124,4 +133,8 @@ def purge_fastly_cache_on_program_save( from cms.tasks import queue_fastly_surrogate_key_purge # noqa: PLC0415 surrogate_key = f"mitxonline:program:{instance.readable_id}" - transaction.on_commit(lambda: queue_fastly_surrogate_key_purge.delay(surrogate_key)) + transaction.on_commit( + lambda: queue_fastly_surrogate_key_purge.delay( + surrogate_key, settings.MIT_LEARN_FASTLY_SERVICE_ID + ) + ) diff --git a/courses/signals_test.py b/courses/signals_test.py index 253ffc425b..232fae3366 100644 --- a/courses/signals_test.py +++ b/courses/signals_test.py @@ -123,41 +123,59 @@ def test_sync_program_certificate_with_hubspot_on_save( # Fastly surrogate-key purge signal tests # --------------------------------------------------------------------------- +LEARN_SERVICE_ID = "test-learn-service-id" + @patch("courses.signals.transaction.on_commit", side_effect=lambda callback: callback()) @patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") -def test_purge_fastly_cache_on_course_save_update(mock_purge_delay, mock_on_commit): +def test_purge_fastly_cache_on_course_save_update( + mock_purge_delay, mock_on_commit, settings +): """ Updating (re-saving) a Course enqueues a fresh purge each time. """ + settings.MIT_LEARN_FASTLY_SERVICE_ID = LEARN_SERVICE_ID + course = CourseFactory.create() - mock_purge_delay.assert_called_with(f"mitxonline:course:{course.readable_id}") + mock_purge_delay.assert_called_with( + f"mitxonline:course:{course.readable_id}", LEARN_SERVICE_ID + ) course.title = "Updated Title" course.save() - mock_purge_delay.assert_called_with(f"mitxonline:course:{course.readable_id}") + mock_purge_delay.assert_called_with( + f"mitxonline:course:{course.readable_id}", LEARN_SERVICE_ID + ) @patch("courses.signals.transaction.on_commit", side_effect=lambda callback: callback()) @patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") -def test_purge_fastly_cache_on_course_run_save(mock_purge_delay, mock_on_commit): +def test_purge_fastly_cache_on_course_run_save( + mock_purge_delay, mock_on_commit, settings +): """ Saving a CourseRun enqueues a Fastly surrogate-key purge for the parent course: mitxonline:course:. """ + settings.MIT_LEARN_FASTLY_SERVICE_ID = LEARN_SERVICE_ID + course_run = CourseRunFactory.create() mock_purge_delay.assert_called_with( - f"mitxonline:course:{course_run.course.readable_id}" + f"mitxonline:course:{course_run.course.readable_id}", LEARN_SERVICE_ID ) @patch("courses.signals.transaction.on_commit", side_effect=lambda callback: callback()) @patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") -def test_purge_fastly_cache_on_program_save(mock_purge_delay, mock_on_commit): +def test_purge_fastly_cache_on_program_save(mock_purge_delay, mock_on_commit, settings): """ Saving a Program enqueues a Fastly surrogate-key purge for mitxonline:program:. """ + settings.MIT_LEARN_FASTLY_SERVICE_ID = LEARN_SERVICE_ID + program = ProgramFactory.create() - mock_purge_delay.assert_called_with(f"mitxonline:program:{program.readable_id}") + mock_purge_delay.assert_called_with( + f"mitxonline:program:{program.readable_id}", LEARN_SERVICE_ID + ) From 3a8bcbb7965cac57168d729e1375c878dac45577 Mon Sep 17 00:00:00 2001 From: Sar <1447295+shaidar@users.noreply.github.com> Date: Wed, 12 Aug 2026 11:09:14 -0500 Subject: [PATCH 2/3] Backfill missing LegalAddress/UserProfile for migrated users; guard should_skip_onboarding (#3845) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- authentication/api_gateway/views.py | 3 +- authentication/api_gateway/views_test.py | 21 ++ .../management/commands/migrate_edx_data.py | 104 +++++++++- .../management/tests/migrate_edx_data_test.py | 185 ++++++++++++++++++ users/models.py | 6 +- users/models_test.py | 24 +++ 6 files changed, 338 insertions(+), 5 deletions(-) create mode 100644 courses/management/tests/migrate_edx_data_test.py diff --git a/authentication/api_gateway/views.py b/authentication/api_gateway/views.py index a58bafd55c..7952ea0fb4 100644 --- a/authentication/api_gateway/views.py +++ b/authentication/api_gateway/views.py @@ -29,6 +29,7 @@ USER_MSG_TYPE_PROFILE_CREATED, ) from main.utils import encode_json_cookie_value, is_success_response +from users.models import UserProfile User = get_user_model() @@ -135,7 +136,7 @@ def get( params = urlencode({"next": redirect_url}) redirect_url = f"{settings.MITXONLINE_NEW_USER_LOGIN_URL}?{params}" - profile = user.user_profile + profile, _ = UserProfile.objects.get_or_create(user=user) profile.completed_onboarding = True profile.save() return redirect(redirect_url) diff --git a/authentication/api_gateway/views_test.py b/authentication/api_gateway/views_test.py index d53a3d6559..b73d56b3d9 100644 --- a/authentication/api_gateway/views_test.py +++ b/authentication/api_gateway/views_test.py @@ -101,6 +101,27 @@ def test_custom_login_view_authenticated_user_with_onboarding(settings, client): assert user.user_profile.completed_onboarding is True +def test_custom_login_view_with_no_user_profile(settings, client): + """A user with no UserProfile row at all (e.g. one created by the + historical migrate_edx_data bulk_create bug) must not crash logging in - + the view must create the missing UserProfile rather than accessing + user.user_profile directly + """ + settings.MITXONLINE_NEW_USER_LOGIN_URL = "/create-profile" + + user = UserFactory.create() + UserProfile.objects.filter(user=user).delete() + + client.force_login(user) + + qs = {"next": "/dashboard"} + response = client.get(f"{reverse('gateway-login')}?{urlencode(qs)}") + + assert response.status_code == 302 + assert response.url == "/create-profile?next=%2Fdashboard" + assert UserProfile.objects.get(user=user).completed_onboarding is True + + @pytest.mark.parametrize( "next_url", ["/dashboard", "http://openedx.odl.local/courses/abc"] ) diff --git a/courses/management/commands/migrate_edx_data.py b/courses/management/commands/migrate_edx_data.py index 63b941e9d8..5bd1f40bf3 100644 --- a/courses/management/commands/migrate_edx_data.py +++ b/courses/management/commands/migrate_edx_data.py @@ -404,6 +404,100 @@ def _migrate_users(self, conn, options): self.stdout.write(self.style.SUCCESS(f"{user_creation_count} users created")) + def _repair_migrated_user_profiles(self, conn, options): + """ + Backfill LegalAddress/UserProfile rows for users left without them by + a historical bug in _bulk_create_users: bulk_create(ignore_conflicts=True) + never populates .pk on the objects passed into it, so every call here + used to build id_row_lookup keyed on user.id=None and then filter + User.objects.filter(id__in=[None, ...]), which always matches zero + rows - LegalAddress/UserProfile silently never got created for any + user who went through this command, regardless of the row data + Trino actually had for them. + + This re-scans the same edX table used by _migrate_users, but only + acts on users already missing one of these rows - real, already-saved + User objects with real ids, so the existing bulk-create helpers work + correctly here the same way they already do for the enrollments + migration's opportunistic repair path. + """ + limit = options.get("limit") + batch_size = options.get("batch_size", 1000) + dry_run = options.get("dry_run") + + affected_users = list( + User.objects.filter( + Q(legal_address__isnull=True) | Q(user_profile__isnull=True) + ).only("id", "email") + ) + if limit is not None: + affected_users = affected_users[:limit] + affected_by_email = {user.email: user for user in affected_users if user.email} + + self.stdout.write( + f"{len(affected_by_email)} users found missing " + f"LegalAddress and/or UserProfile" + ) + + cur = conn.cursor() + cur.execute("SELECT * FROM edxorg_to_mitxonline_users") + columns = [desc[0] for desc in cur.description] + + # e.g. {"Male": "m", "Female": "f"} + GENDER_MAP = {label: code for code, label in GENDER_CHOICES} + + repaired_emails = set() + while True: + results = cur.fetchmany(batch_size) + if not results: + break + + rows = [dict(zip(columns, r)) for r in results] + + matched_users = [] + row_lookup_by_id = {} + for row in rows: + email = row.get("user_email") + user = affected_by_email.get(email) + if user is None or email in repaired_emails: + continue + matched_users.append(user) + row_lookup_by_id[user.id] = row + repaired_emails.add(email) + + if not matched_users: + continue + + if dry_run: + continue + + self._bulk_create_legal_addresses( + matched_users, row_lookup_by_id, batch_size + ) + self._bulk_create_user_profiles( + matched_users, row_lookup_by_id, batch_size, GENDER_MAP + ) + + unmatched = len(affected_by_email) - len(repaired_emails) + if dry_run: + self.stdout.write( + self.style.WARNING( + f"[DRY RUN] Would repair {len(repaired_emails)} users" + ) + ) + else: + self.stdout.write( + self.style.SUCCESS(f"Repaired {len(repaired_emails)} users") + ) + if unmatched: + self.stdout.write( + self.style.WARNING( + f"{unmatched} affected users had no matching row in " + f"edxorg_to_mitxonline_users - still missing " + f"LegalAddress/UserProfile, needs manual review" + ) + ) + @staticmethod def _bulk_create_enrollments( rows, @@ -1048,9 +1142,10 @@ def add_arguments(self, parser) -> None: "program_certificates", "entitlements", "future_enrollments", + "repair_migrated_profiles", ], default="course_runs", - help="Choose which migration to run: course_runs, users, course_certificates, program_certificates, entitlements, future_enrollments (default: course_runs)", + help="Choose which migration to run: course_runs, users, course_certificates, program_certificates, entitlements, future_enrollments, repair_migrated_profiles (default: course_runs)", ) parser.add_argument("--dry-run", action="store_true") parser.add_argument( @@ -1072,6 +1167,13 @@ def handle(self, *args, **options): # pylint: disable=unused-argument # noqa: A self.stdout.write("Migrating the edX users ...") self._migrate_users(conn, options) + if migrate_type == "repair_migrated_profiles": + self.stdout.write( + "Repairing LegalAddress/UserProfile rows missed by a " + "historical bug in the users migration ..." + ) + self._repair_migrated_user_profiles(conn, options) + if migrate_type == "course_certificates": self.stdout.write( "Migrating the edX course enrollments, grades and certificates ..." diff --git a/courses/management/tests/migrate_edx_data_test.py b/courses/management/tests/migrate_edx_data_test.py new file mode 100644 index 0000000000..d89327661a --- /dev/null +++ b/courses/management/tests/migrate_edx_data_test.py @@ -0,0 +1,185 @@ +"""Tests for migrate_edx_data management command's repair_migrated_profiles type""" + +import pytest + +from courses.management.commands.migrate_edx_data import Command +from users.factories import UserFactory +from users.models import LegalAddress, User, UserProfile + +pytestmark = pytest.mark.django_db + + +class FakeCursor: + """Minimal stand-in for a Trino DB-API cursor.""" + + def __init__(self, columns, rows): + self.description = [(col,) for col in columns] + self._rows = rows + self._offset = 0 + + def execute(self, query): + """No-op - the fake cursor already has its rows in memory.""" + + def fetchmany(self, size): + """Return the next slice of rows, matching the DB-API contract.""" + batch = self._rows[self._offset : self._offset + size] + self._offset += size + return batch + + +class FakeConnection: + """Minimal stand-in for a Trino DB-API connection.""" + + def __init__(self, columns, rows): + self._columns = columns + self._rows = rows + + def cursor(self): + """Return a fresh fake cursor over the same fixed rows.""" + return FakeCursor(self._columns, self._rows) + + +USER_COLUMNS = [ + "user_email", + "user_full_name", + "user_address_country", + "user_address_state", + "user_address_postal_code", + "user_address_street_1", + "user_address_street_2", + "user_address_city", + "user_gender", + "user_birth_year", +] + + +def _user_row(email, name, country="US"): + return ( + email, + name, + country, + "MA", + "02139", + "1 Main St", + "", + "Cambridge", + None, + None, + ) + + +def _break_user(user): + """Simulate the historical bug's effect: a User row with no + LegalAddress or UserProfile at all. + """ + LegalAddress.objects.filter(user=user).delete() + UserProfile.objects.filter(user=user).delete() + + +def test_repair_creates_missing_legal_address_and_profile(): + """A user missing both rows gets both created from the matching edX row""" + user = UserFactory.create(email="broken@example.com") + _break_user(user) + + conn = FakeConnection( + columns=USER_COLUMNS, + rows=[_user_row(user.email, "Broken User", country="US")], + ) + + Command()._repair_migrated_user_profiles(conn, {}) # noqa: SLF001 + + user.refresh_from_db() + assert user.legal_address.country == "US" + assert UserProfile.objects.filter(user=user).exists() + + +def test_repair_only_backfills_missing_piece(): + """A user missing only UserProfile (LegalAddress already exists) only + gets UserProfile created - the existing helpers must not touch or + duplicate the LegalAddress that's already there + """ + user = UserFactory.create(email="half-broken@example.com") + user.legal_address.country = "CA" + user.legal_address.save() + UserProfile.objects.filter(user=user).delete() + + conn = FakeConnection( + columns=USER_COLUMNS, + rows=[_user_row(user.email, "Half Broken", country="US")], + ) + + Command()._repair_migrated_user_profiles(conn, {}) # noqa: SLF001 + + user.refresh_from_db() + assert user.legal_address.country == "CA" + assert UserProfile.objects.filter(user=user).exists() + + +def test_repair_skips_users_with_no_missing_rows(): + """A user who already has both rows is left untouched, even though a + matching edX row exists for their email + """ + user = UserFactory.create(email="fine@example.com") + user.legal_address.country = "MX" + user.legal_address.save() + + conn = FakeConnection( + columns=USER_COLUMNS, + rows=[_user_row(user.email, "Fine User", country="US")], + ) + + Command()._repair_migrated_user_profiles(conn, {}) # noqa: SLF001 + + user.refresh_from_db() + assert user.legal_address.country == "MX" + + +def test_repair_leaves_unmatched_user_broken_without_crashing(): + """An affected user with no corresponding row in edxorg_to_mitxonline_users + (e.g. their edX data was never in that table) is reported as still + missing, not silently invented from nothing + """ + user = UserFactory.create(email="no-edx-row@example.com") + _break_user(user) + + conn = FakeConnection(columns=USER_COLUMNS, rows=[]) + + Command()._repair_migrated_user_profiles(conn, {}) # noqa: SLF001 + + assert not LegalAddress.objects.filter(user=user).exists() + assert not UserProfile.objects.filter(user=user).exists() + + +def test_dry_run_does_not_write(): + """--dry-run must not create any rows, even for a clearly matched user""" + user = UserFactory.create(email="dry-run@example.com") + _break_user(user) + + conn = FakeConnection( + columns=USER_COLUMNS, + rows=[_user_row(user.email, "Dry Run User", country="US")], + ) + + Command()._repair_migrated_user_profiles(conn, {"dry_run": True}) # noqa: SLF001 + + assert not LegalAddress.objects.filter(user=user).exists() + assert not UserProfile.objects.filter(user=user).exists() + + +def test_limit_caps_number_of_users_repaired(): + """--limit caps how many affected users are considered at all, leaving + the rest broken for a later run + """ + users = [UserFactory.create(email=f"limited-{i}@example.com") for i in range(3)] + for user in users: + _break_user(user) + + conn = FakeConnection( + columns=USER_COLUMNS, + rows=[_user_row(user.email, "Limited User", country="US") for user in users], + ) + + Command()._repair_migrated_user_profiles(conn, {"limit": 1}) # noqa: SLF001 + + repaired = User.objects.filter(legal_address__isnull=False).count() + assert repaired == 1 diff --git a/users/models.py b/users/models.py index b727788994..debc64ba2e 100644 --- a/users/models.py +++ b/users/models.py @@ -368,10 +368,10 @@ def b2b_organization_sso_ids(self): @property def should_skip_onboarding(self): + user_profile = getattr(self, "user_profile", None) return ( - self.user_profile.completed_onboarding - or self.courserunenrollment_set(manager="all_objects").exists() - ) + user_profile is not None and user_profile.completed_onboarding + ) or self.courserunenrollment_set(manager="all_objects").exists() @property def openedx_user_exists(self): diff --git a/users/models_test.py b/users/models_test.py index d6dcba0b29..d35e649d86 100644 --- a/users/models_test.py +++ b/users/models_test.py @@ -18,6 +18,7 @@ OPENEDX_HIGHEST_EDUCATION_MAPPINGS, LegalAddress, User, + UserProfile, ) pytestmark = pytest.mark.django_db @@ -139,6 +140,29 @@ def test_user_is_editor(is_staff, is_superuser, has_editor_group, exp_is_editor) assert user.is_editor is exp_is_editor +def test_should_skip_onboarding_with_no_user_profile(): + """A user with no UserProfile row at all (e.g. one created by the + historical migrate_edx_data bulk_create bug) must not crash accessing + should_skip_onboarding - it should fall back to the enrollment check + instead of raising RelatedObjectDoesNotExist + """ + user = UserFactory.create() + UserProfile.objects.filter(user=user).delete() + + assert user.should_skip_onboarding is False + + +def test_should_skip_onboarding_with_completed_onboarding(): + """A user with a UserProfile behaves as before: completed_onboarding + alone is enough to skip onboarding + """ + user = UserFactory.create() + user.user_profile.completed_onboarding = True + user.user_profile.save() + + assert user.should_skip_onboarding is True + + def test_legal_address_us_state(): """ Tests to make sure the us_state property is working properly. From 4803314d4f9b4fad562fc5c869f84da40b1b5ded Mon Sep 17 00:00:00 2001 From: Doof Date: Wed, 12 Aug 2026 18:57:03 +0000 Subject: [PATCH 3/3] Release 1.162.5 --- RELEASE.rst | 6 ++++++ main/settings.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/RELEASE.rst b/RELEASE.rst index 6f47990711..a31882c4a8 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,12 @@ Release Notes ============= +Version 1.162.5 +--------------- + +- Backfill missing LegalAddress/UserProfile for migrated users; guard should_skip_onboarding (#3845) +- refactor: pass the Learn Fastly service ID at the call sites (#3804) + Version 1.162.4 (Released August 12, 2026) --------------- diff --git a/main/settings.py b/main/settings.py index 7100542a78..f7930d4d28 100644 --- a/main/settings.py +++ b/main/settings.py @@ -39,7 +39,7 @@ from main.sentry import init_sentry from openapi.settings_spectacular import open_spectacular_settings -VERSION = "1.162.4" +VERSION = "1.162.5" log = logging.getLogger()