|
13 | 13 | from django.conf import settings |
14 | 14 | from django.db.models.signals import post_delete, post_save |
15 | 15 | from django.dispatch import receiver |
| 16 | +from waffle.models import Flag |
16 | 17 |
|
17 | 18 | from openedx_authz.api.users import unassign_all_roles_from_user |
18 | 19 | from openedx_authz.engine.utils import run_course_authoring_migration |
@@ -149,39 +150,61 @@ def handle_org_waffle_flag_change(sender, instance, **kwargs) -> None: |
149 | 150 | WaffleOverrideRecord = Union[WaffleFlagCourseOverrideModel, WaffleFlagOrgOverrideModel] |
150 | 151 |
|
151 | 152 |
|
| 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 | + |
152 | 179 | 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, |
154 | 183 | ) -> MigrationType | None: |
155 | 184 | """ |
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. |
157 | 186 |
|
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. |
160 | 189 |
|
161 | 190 | Args: |
162 | 191 | current_record (WaffleOverrideRecord): The state of the record in the current transaction. |
163 | 192 | 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. |
164 | 194 |
|
165 | 195 | Returns: |
166 | 196 | MigrationType.FORWARD: If the flag is newly forced on. |
167 | 197 | MigrationType.ROLLBACK: If the forced-on state is removed. |
168 | 198 | None: If there is no effective change in the flag's behavior. |
169 | 199 | """ |
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) |
179 | 202 |
|
180 | | - # 3. Return None if state hasn't changed |
| 203 | + # If the effective behavior hasn't changed, we don't need to do anything |
181 | 204 | if is_effectively_active == was_effectively_active: |
182 | 205 | return None |
183 | 206 |
|
184 | | - # 4. Determine Transition Type |
| 207 | + # If it is now effectively active (and wasn't before), migrate forward. Otherwise, rollback. |
185 | 208 | return MigrationType.FORWARD if is_effectively_active else MigrationType.ROLLBACK |
186 | 209 |
|
187 | 210 |
|
@@ -254,9 +277,12 @@ def trigger_course_authoring_migration( |
254 | 277 |
|
255 | 278 | prev_record = sender.objects.filter(**filter_kwargs).exclude(id=instance.id).order_by("-change_date").first() |
256 | 279 |
|
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) |
258 | 284 | 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") |
260 | 286 | return |
261 | 287 |
|
262 | 288 | excluded_course_ids = frozenset() |
|
0 commit comments