Skip to content

Commit 03a33e8

Browse files
committed
feat: implement course authoring migration functionality
1 parent bc639d2 commit 03a33e8

10 files changed

Lines changed: 490 additions & 24 deletions

File tree

openedx_authz/admin.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from django import forms
55
from django.contrib import admin
66

7-
from openedx_authz.models import ExtendedCasbinRule
7+
from openedx_authz.models import AuthzCourseAuthoringMigrationRun, ExtendedCasbinRule
88

99

1010
class CasbinRuleForm(forms.ModelForm):
@@ -48,3 +48,13 @@ class CasbinRuleAdmin(admin.ModelAdmin):
4848
# TODO: In a future, possibly we should only show an inline for the rules that
4949
# have an extended rule, and show the subject and scope information in detail.
5050
inlines = [ExtendedCasbinRuleInline]
51+
52+
53+
@admin.register(AuthzCourseAuthoringMigrationRun)
54+
class AuthzCourseAuthoringMigrationRunAdmin(admin.ModelAdmin):
55+
"""Admin for AuthzCourseAuthoringMigrationRun to display additional metadata."""
56+
57+
list_display = ("id", "scope_type", "scope_key", "migration_type", "status", "created_at", "updated_at")
58+
search_fields = ("scope_type", "scope_key", "migration_type", "status")
59+
list_filter = ("scope_type", "migration_type", "status")
60+
readonly_fields = ("completed_at", "created_at", "updated_at")

openedx_authz/api/roles.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,5 @@ def get_all_role_assignments_per_scope_type(scope_types: tuple[type[ScopeData],
571571
list[RoleAssignmentData]: All assignments whose scope is an instance of any of the given scope types.
572572
"""
573573
return [
574-
role_assignment for role_assignment in get_role_assignments()
575-
if isinstance(role_assignment.scope, scope_types)
574+
role_assignment for role_assignment in get_role_assignments() if isinstance(role_assignment.scope, scope_types)
576575
]

openedx_authz/engine/utils.py

Lines changed: 105 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@
88
from collections import defaultdict
99

1010
from casbin import Enforcer
11+
from django.db import IntegrityError, transaction
1112
from django.db.models import Q
1213
from opaque_keys.edx.django.models import CourseKeyField
1314

14-
from openedx_authz.api.data import CourseOverviewData, OrgCourseOverviewGlobData
15+
from openedx_authz.api.data import CourseOverviewData, OrgCourseOverviewGlobData, RoleAssignmentData
1516
from openedx_authz.api.roles import get_all_role_assignments_per_scope_type
1617
from openedx_authz.api.users import (
1718
assign_role_to_user_in_scope,
@@ -24,6 +25,7 @@
2425
LIBRARY_AUTHOR,
2526
LIBRARY_USER,
2627
)
28+
from openedx_authz.models.migrations import AuthzCourseAuthoringMigrationRun, MigrationType, ScopeType
2729

2830
logger = logging.getLogger(__name__)
2931

@@ -264,8 +266,7 @@ def migrate_legacy_course_roles_to_authz(course_access_role_model, course_id_lis
264266

265267
# Permission applied to individual user
266268
logger.info(
267-
f"Migrating permission for User: {permission.user.username} "
268-
f"to Role: {role} in Scope: {scope_external_key}"
269+
f"Migrating permission for User: {permission.user.username} to Role: {role} in Scope: {scope_external_key}"
269270
)
270271

271272
is_user_added = assign_role_to_user_in_scope(
@@ -296,7 +297,7 @@ def migrate_legacy_course_roles_to_authz(course_access_role_model, course_id_lis
296297

297298
def migrate_authz_to_legacy_course_roles(
298299
course_access_role_model, user_subject_model, course_id_list, org_id, delete_after_migration
299-
):
300+
) -> tuple[list[RoleAssignmentData], list[RoleAssignmentData]]:
300301
"""
301302
Migrate permissions from the new Casbin-based authorization model back to the legacy CourseAccessRole model.
302303
This function reads permissions from the Casbin enforcer and creates equivalent entries in the
@@ -322,25 +323,23 @@ def migrate_authz_to_legacy_course_roles(
322323
_validate_migration_input(course_id_list, org_id)
323324

324325
role_assignments = get_all_role_assignments_per_scope_type(
325-
scope_types=(CourseOverviewData, OrgCourseOverviewGlobData,)
326+
scope_types=(CourseOverviewData, OrgCourseOverviewGlobData)
326327
)
327328

328329
# Two cases here:
329330
# 1. org_id provided: filter by org — includes org-level glob and course-level scopes for that org.
330331
# 2. only course_id_list provided: filter by course_id — org-level glob scopes are excluded (no course_id).
331332
if org_id:
332333
role_assignments = [
333-
role_assignment
334-
for role_assignment in role_assignments
335-
if role_assignment.scope.org == org_id
334+
role_assignment for role_assignment in role_assignments if role_assignment.scope.org == org_id
336335
]
337336

338337
if course_id_list and not org_id:
339338
role_assignments = [
340339
role_assignment
341340
for role_assignment in role_assignments
342-
if isinstance(role_assignment.scope, CourseOverviewData) and
343-
role_assignment.scope.course_id in course_id_list
341+
if isinstance(role_assignment.scope, CourseOverviewData)
342+
and role_assignment.scope.course_id in course_id_list
344343
]
345344

346345
roles_with_errors = []
@@ -350,13 +349,10 @@ def migrate_authz_to_legacy_course_roles(
350349
user_external_keys = {assignment.subject.external_key for assignment in role_assignments}
351350
users_by_username = {
352351
subject.user.username: subject.user
353-
for subject in user_subject_model.objects.filter(
354-
user__username__in=user_external_keys
355-
).select_related("user")
352+
for subject in user_subject_model.objects.filter(user__username__in=user_external_keys).select_related("user")
356353
}
357354

358355
for role_assignment in role_assignments:
359-
360356
# Per valid role assignment, create corresponding CourseAccessRole entry
361357
# depending on whether the scope is course-level or org-level glob
362358
try:
@@ -410,7 +406,7 @@ def migrate_authz_to_legacy_course_roles(
410406
logger.info(f"Total of {total_unassignments} role assignments unassigned after successful rollback migration.")
411407
for (role_external_key, scope), users in unassignments.items():
412408
logger.info(
413-
f"Unassigned Role: {role_external_key} from {len(users)} users \n"
409+
f"Unassigned Role: {role_external_key} from {len(users)} users "
414410
f"in Scope: {scope} after successful rollback migration."
415411
)
416412
batch_unassign_role_from_users(
@@ -420,3 +416,97 @@ def migrate_authz_to_legacy_course_roles(
420416
)
421417

422418
return roles_with_errors, roles_with_no_errors
419+
420+
421+
# pylint: disable=too-many-positional-arguments
422+
def run_course_authoring_migration(
423+
migration_type: MigrationType,
424+
scope_type: ScopeType,
425+
scope_key: str,
426+
course_access_role_model,
427+
user_subject_model=None,
428+
course_id_list=None,
429+
org_id=None,
430+
delete_after_migration=True,
431+
) -> None:
432+
"""Run a course authoring migration with full tracking and concurrency guard.
433+
434+
TODO: Add better documentation.
435+
436+
Args:
437+
migration_type (MigrationType): Direction of the migration
438+
(``FORWARD`` = legacy → authz, ``ROLLBACK`` = authz → legacy).
439+
scope_type (ScopeType): Whether the scope is a single course or an org.
440+
scope_key (str): Course ID string or org name used as the tracking key.
441+
course_access_role_model: The ``CourseAccessRole`` model class (passed
442+
explicitly to avoid import issues inside Django migrations).
443+
user_subject_model: The ``UserSubject`` model class. Required for
444+
``ROLLBACK`` migrations; ignored for ``FORWARD`` migrations.
445+
course_id_list (list[str] | None): List of course-v1 keys to filter.
446+
org_id (str | None): Organisation name to filter.
447+
delete_after_migration (bool): Whether to delete/unassign successfully
448+
migrated entries from the source system after migration.
449+
"""
450+
try:
451+
with transaction.atomic():
452+
run = AuthzCourseAuthoringMigrationRun.create_running(migration_type, scope_type, scope_key)
453+
except IntegrityError:
454+
logger.warning(
455+
"Skipping %s migration for %s:%s — an active run already exists.", migration_type, scope_type, scope_key
456+
)
457+
AuthzCourseAuthoringMigrationRun.create_skipped(migration_type, scope_type, scope_key)
458+
return
459+
460+
logger.info("Started %s migration run [%s] for %s:%s", migration_type, run.id, scope_type, scope_key)
461+
462+
try:
463+
with transaction.atomic():
464+
if migration_type == MigrationType.FORWARD:
465+
errors, successes = migrate_legacy_course_roles_to_authz(
466+
course_access_role_model, course_id_list, org_id, delete_after_migration
467+
)
468+
else:
469+
errors, successes = migrate_authz_to_legacy_course_roles(
470+
course_access_role_model, user_subject_model, course_id_list, org_id, delete_after_migration
471+
)
472+
except Exception as exc: # pylint: disable=broad-exception-caught
473+
# The inner atomic block is rolled back on exception; mark_failed() runs
474+
# outside it so the tracking record is always persisted.
475+
logger.exception(
476+
"Unexpected error in migration run [%s] for %s:%s", run.id, scope_type, scope_key, exc_info=exc
477+
)
478+
run.mark_failed(exception=exc)
479+
return
480+
481+
metadata_updates = {"successes": len(successes), "errors": len(errors)}
482+
483+
# TODO: Add details of each error in the metadata.
484+
if errors:
485+
error_metadata = {}
486+
for idx, error in enumerate(errors):
487+
error_metadata[f"error_{idx}"] = {
488+
"subject": error.subject.external_key,
489+
"role": error.roles[0].external_key,
490+
"scope": error.scope.external_key,
491+
}
492+
metadata_updates["errors"] = error_metadata
493+
run.mark_partial_success(metadata_updates=metadata_updates)
494+
logger.warning(
495+
"Partial success in %s migration run [%s] for %s:%s — successes=%d, errors=%d",
496+
migration_type,
497+
run.id,
498+
scope_type,
499+
scope_key,
500+
len(successes),
501+
len(errors),
502+
)
503+
else:
504+
run.mark_completed(metadata_updates=metadata_updates)
505+
logger.info(
506+
"Completed %s migration run [%s] for %s:%s — successes=%d",
507+
migration_type,
508+
run.id,
509+
scope_type,
510+
scope_key,
511+
len(successes),
512+
)

openedx_authz/handlers.py

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,33 @@
44
These handlers ensure proper cleanup and consistency when models are deleted.
55
"""
66

7+
from __future__ import annotations
8+
79
import logging
810

911
from casbin_adapter.models import CasbinRule
10-
from django.db.models.signals import post_delete
12+
from django.conf import settings
13+
from django.db.models.signals import post_delete, post_save
1114
from django.dispatch import receiver
1215

1316
from openedx_authz.api.users import unassign_all_roles_from_user
17+
from openedx_authz.engine.utils import run_course_authoring_migration
1418
from openedx_authz.models.core import ExtendedCasbinRule
19+
from openedx_authz.models.migrations import MigrationType, ScopeType
20+
from openedx_authz.models.subjects import UserSubject
1521

1622
try:
23+
from common.djangoapps.student.models import CourseAccessRole
1724
from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_CRITICAL
25+
from openedx.core.djangoapps.waffle_utils.models import WaffleFlagCourseOverrideModel, WaffleFlagOrgOverrideModel
26+
from openedx.core.toggles import AUTHZ_COURSE_AUTHORING_FLAG
1827
except ImportError:
1928
USER_RETIRE_LMS_CRITICAL = None
29+
WaffleFlagCourseOverrideModel = None
30+
WaffleFlagOrgOverrideModel = None
31+
AUTHZ_COURSE_AUTHORING_FLAG = None
32+
CourseAccessRole = None
33+
2034

2135
logger = logging.getLogger(__name__)
2236

@@ -82,3 +96,99 @@ def unassign_roles_on_user_retirement(sender, user, **kwargs): # pylint: disabl
8296
# Only register the handler if the signal is available (i.e., running in Open edX)
8397
if USER_RETIRE_LMS_CRITICAL is not None:
8498
USER_RETIRE_LMS_CRITICAL.connect(unassign_roles_on_user_retirement)
99+
100+
101+
def handle_course_waffle_flag_change(sender, instance, **kwargs) -> None:
102+
"""
103+
Handle changes to course-level waffle flags.
104+
105+
When the authz.enable_course_authoring flag is changed for a course,
106+
trigger the appropriate migration run. Only trigger if automatic migration
107+
is enabled in the settings.
108+
109+
Args:
110+
sender: The model class (WaffleFlagCourseOverrideModel)
111+
instance: The flag override instance being saved
112+
**kwargs: Additional keyword arguments from the signal
113+
"""
114+
trigger_course_authoring_migration(sender=sender, instance=instance, scope_key=str(instance.course_id))
115+
116+
117+
def handle_org_waffle_flag_change(sender, instance, **kwargs) -> None:
118+
"""
119+
Handle changes to organization-level waffle flags.
120+
121+
When the authz.enable_course_authoring flag is changed for an organization,
122+
trigger the appropriate migration run. Only trigger if automatic migration
123+
is enabled in the settings.
124+
125+
Args:
126+
sender: The model class (WaffleFlagOrgOverrideModel)
127+
instance: The flag override instance being saved
128+
**kwargs: Additional keyword arguments from the signal
129+
"""
130+
trigger_course_authoring_migration(sender=sender, instance=instance, scope_key=str(instance.org))
131+
132+
133+
# Only register the handlers if the models are available (i.e., running in Open edX)
134+
if WaffleFlagCourseOverrideModel is not None:
135+
post_save.connect(handle_course_waffle_flag_change, sender=WaffleFlagCourseOverrideModel)
136+
137+
if WaffleFlagOrgOverrideModel is not None:
138+
post_save.connect(handle_org_waffle_flag_change, sender=WaffleFlagOrgOverrideModel)
139+
140+
141+
def trigger_course_authoring_migration(
142+
sender: type[WaffleFlagCourseOverrideModel | WaffleFlagOrgOverrideModel],
143+
instance: WaffleFlagCourseOverrideModel | WaffleFlagOrgOverrideModel,
144+
scope_key: str,
145+
) -> None:
146+
"""
147+
Trigger a migration run in response to a waffle flag change.
148+
149+
Determines the migration direction from the flag state, guards against
150+
no-op saves, and delegates execution to ``run_course_authoring_migration``
151+
which handles tracking and concurrent-run protection.
152+
153+
Args:
154+
sender: The model class (WaffleFlagCourseOverrideModel or WaffleFlagOrgOverrideModel).
155+
instance: The waffle flag instance that triggered the migration.
156+
scope_key (str): Course ID or organization name.
157+
"""
158+
if not settings.ENABLE_AUTOMATIC_AUTHZ_COURSE_AUTHORING_MIGRATION:
159+
logger.info("ENABLE_AUTOMATIC_AUTHZ_COURSE_AUTHORING_MIGRATION is set to False, skipping migration")
160+
return
161+
162+
if instance.waffle_flag != AUTHZ_COURSE_AUTHORING_FLAG.name:
163+
return
164+
165+
course_id_list, org_id, scope_type = None, None, None
166+
filter_kwargs = {"waffle_flag": AUTHZ_COURSE_AUTHORING_FLAG.name}
167+
if isinstance(instance, WaffleFlagCourseOverrideModel):
168+
filter_kwargs["course_id"] = instance.course_id
169+
course_id_list = [scope_key]
170+
scope_type = ScopeType.COURSE
171+
elif isinstance(instance, WaffleFlagOrgOverrideModel):
172+
filter_kwargs["org"] = instance.org
173+
org_id = scope_key
174+
scope_type = ScopeType.ORG
175+
176+
prev_record = sender.objects.filter(**filter_kwargs).exclude(id=instance.id).order_by("-change_date").first()
177+
178+
if prev_record and prev_record.enabled == instance.enabled:
179+
logger.info("No change in waffle flag, skipping course migration")
180+
return
181+
182+
migration_type = MigrationType.FORWARD if instance.enabled else MigrationType.ROLLBACK
183+
184+
logger.info("Triggering %s migration for %s:%s due to waffle flag change", migration_type, scope_type, scope_key)
185+
186+
run_course_authoring_migration(
187+
migration_type=migration_type,
188+
scope_type=scope_type,
189+
scope_key=scope_key,
190+
course_access_role_model=CourseAccessRole,
191+
user_subject_model=UserSubject,
192+
course_id_list=course_id_list,
193+
org_id=org_id,
194+
)

0 commit comments

Comments
 (0)