diff --git a/posthog/api/survey.py b/posthog/api/survey.py index 8c18fe6170032f..cf4e965b592768 100644 --- a/posthog/api/survey.py +++ b/posthog/api/survey.py @@ -379,7 +379,11 @@ def update(self, instance: Survey, validated_data): instance.targeting_flag.save() iteration_count = validated_data.get("iteration_count") - if instance.current_iteration is not None and instance.current_iteration > iteration_count > 0: + if ( + instance.current_iteration is not None + and iteration_count is not None + and instance.current_iteration > iteration_count > 0 + ): raise serializers.ValidationError( f"Cannot change survey recurrence to {iteration_count}, should be at least {instance.current_iteration}" ) diff --git a/posthog/api/test/test_survey.py b/posthog/api/test/test_survey.py index a5dcc5404a8143..e164cc89f256cb 100644 --- a/posthog/api/test/test_survey.py +++ b/posthog/api/test/test_survey.py @@ -986,7 +986,6 @@ def test_can_list_surveys(self): response_data = list.json() assert list.status_code == status.HTTP_200_OK, response_data survey = Survey.objects.get(team_id=self.team.id) - assert response_data == { "count": 1, "next": None, @@ -1047,7 +1046,7 @@ def test_can_list_surveys(self): "responses_limit": None, "iteration_count": None, "iteration_frequency_days": None, - "iteration_start_dates": None, + "iteration_start_dates": [], "current_iteration": None, "current_iteration_start_date": None, } @@ -2261,6 +2260,27 @@ def _create_recurring_survey(self) -> Survey: survey = Survey.objects.get(id=response_data["id"]) return survey + def _create_non_recurring_survey(self) -> Survey: + random_id = generate("1234567890abcdef", 10) + response = self.client.post( + f"/api/projects/{self.team.id}/surveys/", + data={ + "name": f"Recurring NPS Survey {random_id}", + "description": "Get feedback on the new notebooks feature", + "type": "popover", + "questions": [ + { + "type": "open", + "question": "What's a survey?", + } + ], + }, + ) + + response_data = response.json() + survey = Survey.objects.get(id=response_data["id"]) + return survey + def test_can_create_recurring_survey(self): survey = self._create_recurring_survey() response = self.client.patch( @@ -2374,6 +2394,41 @@ def test_cannot_reduce_iterations_lt_current_iteration(self): assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.json()["detail"] == "Cannot change survey recurrence to 1, should be at least 2" + def test_can_handle_non_nil_current_iteration(self): + survey = self._create_non_recurring_survey() + survey.current_iteration = 2 + survey.save() + response = self.client.patch( + f"/api/projects/{self.team.id}/surveys/{survey.id}/", + data={ + "start_date": datetime.now() - timedelta(days=1), + }, + ) + assert response.status_code == status.HTTP_200_OK + + def test_guards_for_nil_iteration_count(self): + survey = self._create_recurring_survey() + survey.current_iteration = 2 + survey.save() + response = self.client.patch( + f"/api/projects/{self.team.id}/surveys/{survey.id}/", + data={ + "start_date": datetime.now() - timedelta(days=1), + }, + ) + assert response.status_code == status.HTTP_200_OK + survey.refresh_from_db() + self.assertIsNone(survey.current_iteration) + response = self.client.patch( + f"/api/projects/{self.team.id}/surveys/{survey.id}/", + data={ + "start_date": datetime.now() - timedelta(days=1), + "iteration_count": 3, + "iteration_frequency_days": 30, + }, + ) + assert response.status_code == status.HTTP_200_OK + def test_can_turn_off_recurring_schedule(self): survey = self._create_recurring_survey() response = self.client.patch( diff --git a/posthog/models/feedback/survey.py b/posthog/models/feedback/survey.py index 40aabf0aa96dcf..d7d2008868c902 100644 --- a/posthog/models/feedback/survey.py +++ b/posthog/models/feedback/survey.py @@ -176,7 +176,12 @@ def update_survey_iterations(sender, instance, *args, **kwargs): iteration_count = 0 if instance.iteration_count is None else instance.iteration_count iteration_frequency_dates = 0 if instance.iteration_frequency_days is None else instance.iteration_frequency_days - if instance.iteration_count == 0 or instance.iteration_frequency_days == 0: + if ( + instance.iteration_count is None + or instance.iteration_frequency_days is None + or instance.iteration_count == 0 + or instance.iteration_frequency_days == 0 + ): instance.iteration_start_dates = [] instance.current_iteration = None instance.current_iteration_start_date = None