Skip to content

Commit 3a7c4b7

Browse files
committed
feat: add effective state evaluation for waffle flags in migration logic
1 parent 96b2e57 commit 3a7c4b7

1 file changed

Lines changed: 43 additions & 17 deletions

File tree

openedx_authz/handlers.py

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from django.conf import settings
1414
from django.db.models.signals import post_delete, post_save
1515
from django.dispatch import receiver
16+
from waffle.models import Flag
1617

1718
from openedx_authz.api.users import unassign_all_roles_from_user
1819
from openedx_authz.engine.utils import run_course_authoring_migration
@@ -149,39 +150,61 @@ def handle_org_waffle_flag_change(sender, instance, **kwargs) -> None:
149150
WaffleOverrideRecord = Union[WaffleFlagCourseOverrideModel, WaffleFlagOrgOverrideModel]
150151

151152

153+
def get_effective_state(record: WaffleOverrideRecord | None, global_flag_enabled: bool) -> bool:
154+
"""
155+
Determine the actual active state of the feature by evaluating the local override
156+
against the global flag state.
157+
158+
Args:
159+
record (WaffleOverrideRecord | None): The waffle flag record to evaluate.
160+
global_flag_enabled (bool): The state of the global flag.
161+
162+
Returns:
163+
bool: True if the feature is active, False otherwise.
164+
"""
165+
# If there is no override, or the override is disabled, the state falls back to the global flag.
166+
if not record or not record.enabled:
167+
return global_flag_enabled
168+
169+
# If there is an active override, it dictates the state, ignoring the global flag.
170+
if record.override_choice == WAFFLE_OVERRIDE_FORCE_ON:
171+
return True
172+
if record.override_choice == WAFFLE_OVERRIDE_FORCE_OFF:
173+
return False
174+
175+
# Safety fallback (in case override_choice is corrupted or empty)
176+
return global_flag_enabled
177+
178+
152179
def get_migration_type(
153-
current_record: WaffleOverrideRecord, previous_record: WaffleOverrideRecord | None
180+
current_record: WaffleOverrideRecord,
181+
previous_record: WaffleOverrideRecord | None,
182+
global_flag_enabled: bool,
154183
) -> MigrationType | None:
155184
"""
156-
Determine the migration type by comparing the current and previous record states.
185+
Determine the migration type by comparing the effective state before and after the transaction.
157186
158-
A FORWARD migration occurs when a flag transitions from disabled/not-forced
159-
to enabled and forced-on. A ROLLBACK occurs when it transitions back.
187+
This accounts for the global flag state, meaning a transition could be triggered by
188+
removing a FORCE_OFF override when the global flag is ON.
160189
161190
Args:
162191
current_record (WaffleOverrideRecord): The state of the record in the current transaction.
163192
previous_record (WaffleOverrideRecord | None): The state of the record prior to the current transaction.
193+
global_flag_enabled (bool): The state of the global flag.
164194
165195
Returns:
166196
MigrationType.FORWARD: If the flag is newly forced on.
167197
MigrationType.ROLLBACK: If the forced-on state is removed.
168198
None: If there is no effective change in the flag's behavior.
169199
"""
170-
# 1. Handle New Records (Creation)
171-
if not previous_record:
172-
is_active = current_record.enabled and current_record.override_choice == WAFFLE_OVERRIDE_FORCE_ON
173-
return MigrationType.FORWARD if is_active else None
174-
175-
# 2. Extract Effective States
176-
# A flag is "effectively active" only if enabled AND set to FORCE_ON
177-
was_effectively_active = previous_record.enabled and previous_record.override_choice == WAFFLE_OVERRIDE_FORCE_ON
178-
is_effectively_active = current_record.enabled and current_record.override_choice == WAFFLE_OVERRIDE_FORCE_ON
200+
was_effectively_active = get_effective_state(previous_record, global_flag_enabled)
201+
is_effectively_active = get_effective_state(current_record, global_flag_enabled)
179202

180-
# 3. Return None if state hasn't changed
203+
# If the effective behavior hasn't changed, we don't need to do anything
181204
if is_effectively_active == was_effectively_active:
182205
return None
183206

184-
# 4. Determine Transition Type
207+
# If it is now effectively active (and wasn't before), migrate forward. Otherwise, rollback.
185208
return MigrationType.FORWARD if is_effectively_active else MigrationType.ROLLBACK
186209

187210

@@ -254,9 +277,12 @@ def trigger_course_authoring_migration(
254277

255278
prev_record = sender.objects.filter(**filter_kwargs).exclude(id=instance.id).order_by("-change_date").first()
256279

257-
migration_type = get_migration_type(instance, prev_record)
280+
global_flag = Flag.objects.filter(name=AUTHZ_COURSE_AUTHORING_FLAG.name).first()
281+
global_flag_enabled = bool(global_flag and global_flag.everyone)
282+
283+
migration_type = get_migration_type(instance, prev_record, global_flag_enabled)
258284
if migration_type is None:
259-
logger.info("No change in waffle flag, skipping migration")
285+
logger.info("No effective change in waffle flag behavior, skipping migration")
260286
return
261287

262288
excluded_course_ids = frozenset()

0 commit comments

Comments
 (0)