Skip to content

Commit

Permalink
fix(surveys): do not reset survey iteration count if value is nil (#2…
Browse files Browse the repository at this point in the history
…5632)

1. Change the default value of iteration_count to be None if its not part of the request.
2. Guard for None value before setting the iteration_count on the Survey instance.
  • Loading branch information
Phanatic authored Oct 17, 2024
1 parent 4df898d commit 1c4f46a
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 4 deletions.
7 changes: 4 additions & 3 deletions posthog/api/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ def update(self, instance: Survey, validated_data):
instance.targeting_flag.active = False
instance.targeting_flag.save()

iteration_count = validated_data.get("iteration_count")
iteration_count = validated_data.get("iteration_count", None)
if (
instance.current_iteration is not None
and iteration_count is not None
Expand All @@ -396,8 +396,9 @@ def update(self, instance: Survey, validated_data):
f"Cannot change survey recurrence to {iteration_count}, should be at least {instance.current_iteration}"
)

instance.iteration_count = iteration_count
instance.iteration_frequency_days = validated_data.get("iteration_frequency_days")
if iteration_count is not None:
instance.iteration_count = iteration_count
instance.iteration_frequency_days = validated_data.get("iteration_frequency_days")

instance = super().update(instance, validated_data)

Expand Down
15 changes: 14 additions & 1 deletion posthog/api/test/test_survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -2371,6 +2371,19 @@ def test_can_create_recurring_survey(self):
assert len(response_data["iteration_start_dates"]) == 2
assert response_data["current_iteration"] == 1

def test_can_create_and_launch_recurring_survey(self):
survey = self._create_recurring_survey()
response = self.client.patch(
f"/api/projects/{self.team.id}/surveys/{survey.id}/",
data={
"start_date": datetime.now() - timedelta(days=1),
},
)
response_data = response.json()
assert response_data["iteration_start_dates"] is not None
assert len(response_data["iteration_start_dates"]) == 2
assert response_data["current_iteration"] == 1

def test_can_set_internal_targeting_flag(self):
survey = self._create_recurring_survey()
response = self.client.patch(
Expand Down Expand Up @@ -2493,7 +2506,7 @@ def test_guards_for_nil_iteration_count(self):
)
assert response.status_code == status.HTTP_200_OK
survey.refresh_from_db()
self.assertIsNone(survey.current_iteration)
self.assertIsNotNone(survey.current_iteration)
response = self.client.patch(
f"/api/projects/{self.team.id}/surveys/{survey.id}/",
data={
Expand Down

0 comments on commit 1c4f46a

Please sign in to comment.