Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(surveys): add column with responses count #17623

Merged
merged 18 commits into from
Sep 27, 2023
Merged
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
Binary file modified frontend/__snapshots__/scenes-app-surveys--surveys-list.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 7 additions & 0 deletions frontend/src/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,10 @@ class ApiRequest {
return this.projectsDetail(teamId).addPathComponent('surveys')
}

public surveysResponsesCount(teamId?: TeamType['id']): ApiRequest {
return this.projectsDetail(teamId).addPathComponent('surveys/responses_count')
}

public survey(id: Survey['id'], teamId?: TeamType['id']): ApiRequest {
return this.surveys(teamId).addPathComponent(id)
}
Expand Down Expand Up @@ -1440,6 +1444,9 @@ const api = {
async update(surveyId: Survey['id'], data: Partial<Survey>): Promise<Survey> {
return await new ApiRequest().survey(surveyId).update({ data })
},
async getResponsesCount(): Promise<{ [key: string]: number }> {
return await new ApiRequest().surveysResponsesCount().get()
},
},

dataWarehouseTables: {
Expand Down
6 changes: 6 additions & 0 deletions frontend/src/scenes/surveys/Surveys.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ const MOCK_SURVEY_RESULTS = {
],
}

const MOCK_RESPONSES_COUNT = {
'0187c279-bcae-0000-34f5-4f121921f005': 17,
'0187c279-bcae-0000-34f5-4f121921f006': 25,
}

const meta: Meta = {
title: 'Scenes-App/Surveys',
parameters: {
Expand All @@ -164,6 +169,7 @@ const meta: Meta = {
]),
'/api/projects/:team_id/surveys/0187c279-bcae-0000-34f5-4f121921f005/': MOCK_BASIC_SURVEY,
'/api/projects/:team_id/surveys/0187c279-bcae-0000-34f5-4f121921f006/': MOCK_SURVEY_WITH_RELEASE_CONS,
'/api/projects/:team_id/surveys/responses_count/': MOCK_RESPONSES_COUNT,
},
post: {
'/api/projects/:team_id/query/': (req) => {
Expand Down
42 changes: 31 additions & 11 deletions frontend/src/scenes/surveys/Surveys.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { LemonButton, LemonTable, LemonDivider, Link, LemonTag, LemonTagType } from '@posthog/lemon-ui'
import { LemonButton, LemonTable, LemonDivider, Link, LemonTag, LemonTagType, Spinner } from '@posthog/lemon-ui'
import { PageHeader } from 'lib/components/PageHeader'
import { More } from 'lib/lemon-ui/LemonButton/More'
import stringWithWBR from 'lib/utils/stringWithWBR'
Expand Down Expand Up @@ -36,8 +36,16 @@ export enum SurveysTabs {
}

export function Surveys(): JSX.Element {
const { nonArchivedSurveys, archivedSurveys, surveys, surveysLoading, usingSurveysSiteApp } =
useValues(surveysLogic)
const {
nonArchivedSurveys,
archivedSurveys,
surveys,
surveysLoading,
surveysResponsesCount,
surveysResponsesCountLoading,
usingSurveysSiteApp,
} = useValues(surveysLogic)

const { deleteSurvey, updateSurvey } = useActions(surveysLogic)
const { user } = useValues(userLogic)
const { featureFlags } = useValues(featureFlagLogic)
Expand Down Expand Up @@ -136,14 +144,26 @@ export function Surveys(): JSX.Element {
)
},
},
// TODO: add responses count later
jurajmajerik marked this conversation as resolved.
Show resolved Hide resolved
// {
// title: 'Responses',
// render: function RenderResponses() {
// // const responsesCount = getResponsesCount(survey)
// return <div>{0}</div>
// },
// },
{
title: 'Responses',
dataIndex: 'id',
render: function RenderResponses(_, survey) {
return (
<>
{surveysResponsesCountLoading ? (
<Spinner />
) : (
<div>{surveysResponsesCount[survey.id]}</div>
)}
</>
)
},
sorter: (surveyA, surveyB) => {
const countA = surveysResponsesCount[surveyA.id] ?? 0
const countB = surveysResponsesCount[surveyB.id] ?? 0
return countA - countB
},
},
jurajmajerik marked this conversation as resolved.
Show resolved Hide resolved
{
dataIndex: 'type',
title: 'Type',
Expand Down
12 changes: 10 additions & 2 deletions frontend/src/scenes/surveys/surveysLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ export const surveysLogic = kea<surveysLogicType>([
surveys: {
__default: [] as Survey[],
loadSurveys: async () => {
const response = await api.surveys.list()
return response.results
const responseSurveys = await api.surveys.list()
return responseSurveys.results
},
deleteSurvey: async (id) => {
await api.surveys.delete(id)
Expand All @@ -46,6 +46,13 @@ export const surveysLogic = kea<surveysLogicType>([
return values.surveys.map((survey) => (survey.id === id ? updatedSurvey : survey))
},
},
surveysResponsesCount: {
__default: {} as { [key: string]: number },
loadResponsesCount: async () => {
jurajmajerik marked this conversation as resolved.
Show resolved Hide resolved
const surveysResponsesCount = await api.surveys.getResponsesCount()
return surveysResponsesCount
},
},
})),
listeners(() => ({
deleteSurveySuccess: () => {
Expand Down Expand Up @@ -87,5 +94,6 @@ export const surveysLogic = kea<surveysLogicType>([
}),
afterMount(async ({ actions }) => {
await actions.loadSurveys()
await actions.loadResponsesCount()
}),
])
23 changes: 21 additions & 2 deletions posthog/api/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@

from posthog.api.shared import UserBasicSerializer
from posthog.api.utils import get_token
from posthog.client import sync_execute
from posthog.exceptions import generate_exception_response
from posthog.models.feedback.survey import Survey
from rest_framework.response import Response
from rest_framework.decorators import action
from posthog.api.feature_flag import FeatureFlagSerializer, MinimalFeatureFlagSerializer
from posthog.api.routing import StructuredViewSetMixin
from rest_framework import serializers, viewsets
from rest_framework import serializers, viewsets, request
from rest_framework.permissions import IsAuthenticated
from rest_framework.request import Request
from rest_framework import status
Expand Down Expand Up @@ -138,7 +140,6 @@ def create(self, validated_data):
return super().create(validated_data)

def update(self, instance: Survey, validated_data):

if validated_data.get("remove_targeting_flag"):
if instance.targeting_flag:
instance.targeting_flag.delete()
Expand Down Expand Up @@ -207,6 +208,24 @@ def destroy(self, request: Request, *args: Any, **kwargs: Any) -> Response:

return super().destroy(request, *args, **kwargs)

@action(methods=["GET"], detail=False)
def responses_count(self, request: request.Request, **kwargs):
data = sync_execute(
f"""
SELECT JSONExtractString(properties, '$survey_id') as survey_id, count()
FROM events
WHERE event = 'survey sent' AND team_id = %(team_id)s
GROUP BY survey_id
""",
{"team_id": self.team_id},
)

counts = {}
for survey_id, count in data:
counts[survey_id] = count

return Response(counts)


class SurveyAPISerializer(serializers.ModelSerializer):
"""
Expand Down
11 changes: 11 additions & 0 deletions posthog/api/test/__snapshots__/test_survey.ambr
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
# name: TestResponsesCount.test_responses_count
'
/* user_id:0 request:_snapshot_ */
SELECT JSONExtractString(properties, '$survey_id') as survey_id,
count()
FROM events
WHERE event = 'survey sent'
AND team_id = 2
GROUP BY survey_id
'
---
# name: TestSurveysAPIList.test_list_surveys
'
SELECT "posthog_featureflag"."id",
Expand Down
62 changes: 42 additions & 20 deletions posthog/api/test/test_survey.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
from datetime import datetime, timedelta

from unittest.mock import ANY

from rest_framework import status
from django.core.cache import cache
from django.test.client import Client

from posthog.models.feature_flag.feature_flag import FeatureFlag
from posthog.models.feedback.survey import Survey
from posthog.test.base import (
APIBaseTest,
ClickhouseTestMixin,
BaseTest,
QueryMatchingTest,
snapshot_postgres_queries,
snapshot_clickhouse_queries,
_create_event,
)

from posthog.test.base import APIBaseTest, BaseTest, QueryMatchingTest, snapshot_postgres_queries
from posthog.models import FeatureFlag


class TestSurvey(APIBaseTest):
Expand Down Expand Up @@ -707,24 +717,36 @@ def test_list_surveys(self):
],
)

def test_get_surveys_errors_on_invalid_token(self):
self.client.logout()

with self.assertNumQueries(1):
response = self._get_surveys(token="invalid_token")
assert response.status_code == status.HTTP_401_UNAUTHORIZED
assert (
response.json()["detail"]
== "Project API key invalid. You can find your project API key in your PostHog project settings."
)
class TestResponsesCount(ClickhouseTestMixin, APIBaseTest):
@snapshot_clickhouse_queries
def test_responses_count(self):

def test_get_surveys_errors_on_empty_token(self):
self.client.logout()
survey_counts = {
"d63bb580-01af-4819-aae5-edcf7ef2044f": 3,
"fe7c4b62-8fc9-401e-b483-e4ff98fd13d5": 6,
"daed7689-d498-49fe-936f-e85554351b6c": 100,
}

with self.assertNumQueries(0):
response = self.client.get(f"/api/surveys/")
assert response.status_code == status.HTTP_401_UNAUTHORIZED
assert (
response.json()["detail"]
== "API key not provided. You can find your project API key in your PostHog project settings."
)
for survey_id, count in survey_counts.items():
for _ in range(count):
_create_event(
event="survey sent",
team=self.team,
distinct_id=self.user.id,
properties={"$survey_id": survey_id},
timestamp=datetime.now() - timedelta(days=count),
)

response = self.client.get(f"/api/projects/{self.team.id}/surveys/responses_count")
self.assertEqual(response.status_code, status.HTTP_200_OK)

data = response.json()
self.assertEqual(data, survey_counts)

def test_responses_count_zero_responses(self):
response = self.client.get(f"/api/projects/{self.team.id}/surveys/responses_count")
self.assertEqual(response.status_code, status.HTTP_200_OK)

data = response.json()
self.assertEqual(data, {})