Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions django_goals/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 47 additions & 1 deletion django_goals/models_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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'),
Expand Down