feat(scim): add migrate_and_sync_users orchestrator command - #3837
Draft
shaidar wants to merge 1 commit into
Draft
feat(scim): add migrate_and_sync_users orchestrator command#3837shaidar wants to merge 1 commit into
shaidar wants to merge 1 commit into
Conversation
OpenAPI ChangesShow/hide changesUnexpected changes? Ensure your branch is up-to-date with |
shaidar
force-pushed
the
sar/migrate-and-sync-users-orchestrator
branch
2 times, most recently
from
August 10, 2026 19:57
574ee71 to
005b207
Compare
Replaces the old two-step manual migration process (running migrate_edx_data, then an ad hoc SCIM sync script with no field-level visibility or validation) with one auditable command: - Stage 1: backfill edX user data (call_command migrate_edx_data). - Stage 2: classify every sync candidate via LearnUserAdapter's _resolve_name() tiers - report only, never mutates legal_address. Only candidates with no name data anywhere are blocked by default (override with --force); a split-from-User.name candidate syncs normally, flagged as lower-confidence in the report. - Stage 3: sync via mitol.scim.api.sync_users_to_scim_remote, with per-user structured logging instead of only aggregate error counts. - Stage 4: verify what Keycloak actually stored by diffing the response body sync_users_to_scim_remote now returns, against what was sent - no extra API calls needed. - Stage 5: write a JSON report (synced-and-verified / blocked-or-failed / verified-but-mismatched). Depends on the response-capture change in mitodl/ol-django#544 - sync_users_to_scim_remote needs to return UserState.response_body for Stage 4 to work. Opening as a draft until that PR releases and the mitol-django-scim pin here is bumped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
shaidar
force-pushed
the
sar/migrate-and-sync-users-orchestrator
branch
from
August 11, 2026 13:49
3e806b6 to
b60c56c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What are the relevant tickets?
N/A
Description (What does it do?)
Replaces the old two-step manual migration process — running
migrate_edx_data, then an ad hoc SCIM sync script with no field-level visibility or validation — with one auditable command,migrate_and_sync_users:call_command("migrate_edx_data", type="users")unless--skip-edx-migration.is_active=True, not yet SCIM-synced) is run throughLearnUserAdapter._resolve_name()'s three tiers, purely for classification and reporting — this stage never writes tolegal_address.legal_addresscomplete → sync normally.User.nameavailable (split derived) → sync normally by default, flagged as lower-confidence in the report — not--force-gated, since for this population the split is the best data that exists.--forcesyncs anyway with a blank name, explicitly flagged rather than silently dropped.mitol.scim.api.sync_users_to_scim_remote(...)in batches, with per-user structured logging instead of only aggregate 409-conflict counts (the old ad hoc script's only error visibility).name.givenName/name.familyNamein each returnedUserState.response_bodyagainst what was sent — no extra Keycloak API calls, since the Bulk response already echoes the created resource.--report-pathor stdout.How can this be tested?
users/management/tests/migrate_and_sync_users_test.pymocksscim_api.sync_users_to_scim_remote's return value (a list of duck-typed fakeUserStateobjects, decoupled from whatevermitol-django-scimversion happens to be installed) and covers: dry-run classification without syncing, tier-3 (no name data) users being blocked by default and synced with--force, matching/mismatched response-body verification, and failed syncs being reported distinctly rather than silently swallowed.Ran locally with a fresh test DB (
uv run pytest users/management/tests/migrate_and_sync_users_test.py --create-db): all passing.ruff checkclean.Additional Context
Draft, blocked on a dependency: this command calls
scim_api.sync_users_to_scim_remote(...)and reads.success/.response_body/.erroroff the returned list — but the currently publishedmitol-django-scimstill returnsNonefrom that function, not a list. The companion PR mitodl/ol-django#544 adds that return value; until it's released and this repo'smitol-django-scimpin (pyproject.toml) is bumped, running this command for real would raise aTypeErroriteratingNone. Do not merge before that dependency is updated.Based on top of #3836 (the
LearnUserAdapterfix this command's Stage 2 relies on via_resolve_name()) — that should merge first.