diff --git a/django_goals/models.py b/django_goals/models.py index 6d8511a..1c85457 100644 --- a/django_goals/models.py +++ b/django_goals/models.py @@ -276,13 +276,42 @@ def _mark_as_unfailed(goal_ids): """ if not goal_ids: return - Goal.objects.filter(id__in=goal_ids).update(state=GoalState.WAITING_FOR_DATE) - # update waiting-for failed count in dependent goals + + now = timezone.now() + + # Process each goal individually to determine correct state + for goal in Goal.objects.filter(id__in=goal_ids).select_for_update(): + # Determine correct state based on preconditions and date + if goal.precondition_date > now: + # Future date - should wait for date (original behavior for future dates) + new_state = GoalState.WAITING_FOR_DATE + elif goal.waiting_for_count > 0: + # Current/past date but has unachieved preconditions - should wait for preconditions + new_state = GoalState.WAITING_FOR_PRECONDITIONS + else: + # Current/past date and no preconditions - go to waiting for date (let normal flow handle it) + # This preserves the original behavior for goals with no preconditions + new_state = GoalState.WAITING_FOR_DATE + + goal.state = new_state + goal.save(update_fields=['state']) + + # Update waiting-for failed count in dependent goals - but avoid going negative Goal.objects.filter( precondition_goals__id__in=goal_ids, + waiting_for_failed_count__gt=0, ).update( waiting_for_failed_count=models.F('waiting_for_failed_count') - 1, ) + + # For goals with PROCEED failure behavior, we need to increase waiting_for_count + # when preconditions are unfailed (since they were previously being skipped) + Goal.objects.filter( + precondition_goals__id__in=goal_ids, + precondition_failure_behavior=PreconditionFailureBehavior.PROCEED, + ).update( + waiting_for_count=models.F('waiting_for_count') + 1, + ) @transaction.atomic diff --git a/django_goals/models_tests.py b/django_goals/models_tests.py index fd99e27..710f5f5 100644 --- a/django_goals/models_tests.py +++ b/django_goals/models_tests.py @@ -6,7 +6,7 @@ from .models import ( GoalState, PreconditionFailureBehavior, PreconditionsMode, handle_unblocked_goals, handle_waiting_for_worker, schedule, - unblock_retry_goal, + unblock_retry_goal, _add_precondition_goals, ) from .pickups import GoalPickup @@ -65,6 +65,52 @@ def test_schedule_updates_deadline(): assert goal_a.deadline == now - datetime.timedelta(minutes=1) +@pytest.mark.django_db +def test_blocked_goal_with_dependencies_unblock_transitions(): + """ + Test the scenario from the problem statement: + 1. Schedule a blocked goal + 2. Add dependency + 3. Unblock the goal + 4. Verify correct state transitions + + This tests the bug where _mark_as_unfailed would always go to WAITING_FOR_DATE + instead of determining the correct state based on preconditions. + """ + # Test case 1: blocked goal with achieved precondition + precondition_goal = GoalFactory(state=GoalState.ACHIEVED) + blocked_goal = schedule(noop, blocked=True) + assert blocked_goal.state == GoalState.BLOCKED + + # Add dependency while blocked + _add_precondition_goals(blocked_goal, [precondition_goal]) + blocked_goal.refresh_from_db() + assert blocked_goal.waiting_for_count == 0 # precondition already achieved + + # Unblock the goal - since precondition is achieved and date is current, + # it should go to WAITING_FOR_DATE (then get processed by normal flow) + unblock_retry_goal(blocked_goal.id) + blocked_goal.refresh_from_db() + assert blocked_goal.state == GoalState.WAITING_FOR_DATE + + # Test case 2: blocked goal with unachieved precondition + precondition_goal2 = GoalFactory(state=GoalState.WAITING_FOR_WORKER) + blocked_goal2 = schedule(noop, blocked=True) + assert blocked_goal2.state == GoalState.BLOCKED + + # Add dependency while blocked + _add_precondition_goals(blocked_goal2, [precondition_goal2]) + blocked_goal2.refresh_from_db() + assert blocked_goal2.waiting_for_count == 1 # waiting for unachieved precondition + + # Unblock the goal - since precondition is not achieved, + # it should go to WAITING_FOR_PRECONDITIONS (not WAITING_FOR_DATE) + unblock_retry_goal(blocked_goal2.id) + blocked_goal2.refresh_from_db() + assert blocked_goal2.state == GoalState.WAITING_FOR_PRECONDITIONS + assert blocked_goal2.waiting_for_count == 1 + + @pytest.mark.django_db @pytest.mark.parametrize( ('goal', 'expected_waiting_for', 'expected_waiting_for_failed_count'),