Skip to content

Commit

Permalink
feat(surveys): add column with responses count (#17623)
Browse files Browse the repository at this point in the history
* add count_responses endpoint

* add frontend logic

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (1)

* fetch responses count in a separate loader

* pass team_id as a stored parameter

* use a separate kea loader, add ui spinner

* use shorthand for conditional rendering

* omit i in the loop

* add endpoint tests

* Update UI snapshots for `chromium` (2)

* adjust endpoint tests

* add /responses_counts mock to the snapshot

* enable sorting

* Update UI snapshots for `chromium` (2)

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
jurajmajerik and github-actions[bot] authored Sep 27, 2023
1 parent ced6337 commit 02007d1
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 35 deletions.
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
// {
// 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
},
},
{
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 () => {
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, {})

0 comments on commit 02007d1

Please sign in to comment.