fix: various dashboard template fixes (#19799)

* add a loading spinner while getting templates

* limit what filters the api accepts

* add text search to API

* refactor a little

* fix scoped display and add UI filtering

* prettier

* Update query snapshots

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* fix

* Update UI snapshots for `webkit` (2)

* fix

* Update UI snapshots for `webkit` (2)

* Update UI snapshots for `webkit` (2)

* obey mypy

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

frontend/src/mocks/handlers.ts

@@ -32,6 +32,7 @@ export const defaultMocks: Mocks = {
         '/api/projects/:team_id/event_definitions/': EMPTY_PAGINATED_RESPONSE,
         '/api/projects/:team_id/cohorts/': toPaginatedResponse([MOCK_DEFAULT_COHORT]),
         '/api/projects/:team_id/dashboards/': EMPTY_PAGINATED_RESPONSE,
+        '/api/projects/:team_id/dashboard_templates': EMPTY_PAGINATED_RESPONSE,
         '/api/projects/:team_id/dashboard_templates/repository/': [],
         '/api/projects/:team_id/notebooks': () => {
             // this was matching on `?contains=query` but that made MSW unhappy and seems unnecessary

frontend/src/scenes/dashboard/DashboardTemplateChooser.tsx

@@ -3,20 +3,20 @@ import './DashboardTemplateChooser.scss'
 import clsx from 'clsx'
 import { useActions, useValues } from 'kea'
 import { FallbackCoverImage } from 'lib/components/FallbackCoverImage/FallbackCoverImage'
+import { Spinner } from 'lib/lemon-ui/Spinner'
 import BlankDashboardHog from 'public/blank-dashboard-hog.png'
 import { useState } from 'react'
-import { dashboardTemplatesLogic } from 'scenes/dashboard/dashboards/templates/dashboardTemplatesLogic'
+import {
+    DashboardTemplateProps,
+    dashboardTemplatesLogic,
+} from 'scenes/dashboard/dashboards/templates/dashboardTemplatesLogic'
 import { newDashboardLogic } from 'scenes/dashboard/newDashboardLogic'
 
-import { DashboardTemplateScope, DashboardTemplateType } from '~/types'
+import { DashboardTemplateType } from '~/types'
 
-interface DashboardTemplateChooserProps {
-    scope?: DashboardTemplateScope
-}
-
-export function DashboardTemplateChooser({ scope = 'global' }: DashboardTemplateChooserProps): JSX.Element {
+export function DashboardTemplateChooser({ scope = 'default' }: DashboardTemplateProps): JSX.Element {
     const templatesLogic = dashboardTemplatesLogic({ scope })
-    const { allTemplates } = useValues(templatesLogic)
+    const { allTemplates, allTemplatesLoading } = useValues(templatesLogic)
 
     const { isLoading, newDashboardModalVisible } = useValues(newDashboardLogic)
     const {
@@ -49,35 +49,38 @@ export function DashboardTemplateChooser({ scope = 'global' }: DashboardTemplate
                     index={0}
                     data-attr="create-dashboard-blank"
                 />
-                {allTemplates.map((template, index) => (
-                    <TemplateItem
-                        key={index}
-                        template={template}
-                        onClick={() => {
-                            if (isLoading) {
-                                return
-                            }
-                            setIsLoading(true)
-                            // while we might receive templates from the external repository
-                            // we need to handle templates that don't have variables
-                            if ((template.variables || []).length === 0) {
-                                if (template.variables === null) {
-                                    template.variables = []
+                {allTemplatesLoading ? (
+                    <Spinner className="text-6xl" />
+                ) : (
+                    allTemplates.map((template, index) => (
+                        <TemplateItem
+                            key={index}
+                            template={template}
+                            onClick={() => {
+                                if (isLoading) {
+                                    return
                                 }
-                                createDashboardFromTemplate(template, template.variables || [])
-                            } else {
-                                if (!newDashboardModalVisible) {
-                                    showVariableSelectModal(template)
+                                setIsLoading(true)
+                                // while we might receive templates from the external repository
+                                // we need to handle templates that don't have variables
+                                if ((template.variables || []).length === 0) {
+                                    if (template.variables === null) {
+                                        template.variables = []
+                                    }
+                                    createDashboardFromTemplate(template, template.variables || [])
                                 } else {
-                                    setActiveDashboardTemplate(template)
+                                    if (!newDashboardModalVisible) {
+                                        showVariableSelectModal(template)
+                                    } else {
+                                        setActiveDashboardTemplate(template)
+                                    }
                                 }
-                            }
-                        }}
-                        index={index + 1}
-                        data-attr="create-dashboard-from-template"
-                    />
-                ))}
-                {/*TODO @lukeharries should we have an empty state here? When no templates let people know what to do?*/}
+                            }}
+                            index={index + 1}
+                            data-attr="create-dashboard-from-template"
+                        />
+                    ))
+                )}
             </div>
         </div>
     )

frontend/src/scenes/dashboard/NewDashboardModal.tsx

@@ -1,7 +1,8 @@
-import { LemonButton } from '@posthog/lemon-ui'
+import { LemonButton, LemonInput } from '@posthog/lemon-ui'
 import { useActions, useMountedLogic, useValues } from 'kea'
 import { LemonModal } from 'lib/lemon-ui/LemonModal'
 import { pluralize } from 'lib/utils'
+import { dashboardTemplatesLogic } from 'scenes/dashboard/dashboards/templates/dashboardTemplatesLogic'
 import { newDashboardLogic } from 'scenes/dashboard/newDashboardLogic'
 
 import { DashboardTemplateChooser } from './DashboardTemplateChooser'
@@ -43,6 +44,12 @@ export function NewDashboardModal(): JSX.Element {
     const { hideNewDashboardModal } = useActions(newDashboardLogic)
     const { newDashboardModalVisible, activeDashboardTemplate } = useValues(newDashboardLogic)
 
+    const templatesLogic = dashboardTemplatesLogic({
+        scope: builtLogic.props.featureFlagId ? 'feature_flag' : 'default',
+    })
+    const { templateFilter } = useValues(templatesLogic)
+    const { setTemplateFilter } = useActions(templatesLogic)
+
     const _dashboardTemplateChooser = builtLogic.props.featureFlagId ? (
         <DashboardTemplateChooser scope="feature_flag" />
     ) : (
@@ -61,7 +68,18 @@ export function NewDashboardModal(): JSX.Element {
                         {pluralize((activeDashboardTemplate.variables || []).length, 'event', 'events', true)}.
                     </p>
                 ) : (
-                    'Choose a template or start with a blank slate'
+                    <div className="flex flex-col gap-2">
+                        <div>Choose a template or start with a blank slate</div>
+                        <div>
+                            <LemonInput
+                                type="search"
+                                placeholder="Filter templates"
+                                onChange={setTemplateFilter}
+                                value={templateFilter}
+                                fullWidth={true}
+                            />
+                        </div>
+                    </div>
                 )
             }
         >

frontend/src/scenes/dashboard/dashboards/templates/dashboardTemplatesLogic.tsx

@@ -1,4 +1,4 @@
-import { actions, afterMount, connect, kea, key, path, props } from 'kea'
+import { actions, afterMount, connect, kea, key, listeners, path, props, reducers } from 'kea'
 import { loaders } from 'kea-loaders'
 import api from 'lib/api'
 import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
@@ -8,7 +8,8 @@ import { DashboardTemplateScope, DashboardTemplateType } from '~/types'
 import type { dashboardTemplatesLogicType } from './dashboardTemplatesLogicType'
 
 export interface DashboardTemplateProps {
-    scope?: DashboardTemplateScope
+    // default is to present global templates _and_ those visible only in the current team
+    scope?: 'default' | DashboardTemplateScope
 }
 
 export const dashboardTemplatesLogic = kea<dashboardTemplatesLogicType>([
@@ -20,18 +21,40 @@ export const dashboardTemplatesLogic = kea<dashboardTemplatesLogicType>([
     }),
     actions({
         setTemplates: (allTemplates: DashboardTemplateType[]) => ({ allTemplates }),
+        setTemplateFilter: (search: string) => ({ search }),
     }),
-    loaders(({ props }) => ({
+    reducers({
+        templateFilter: [
+            '' as string,
+            {
+                setTemplateFilter: (_, { search }) => {
+                    return search
+                },
+            },
+        ],
+    }),
+    loaders(({ props, values }) => ({
         allTemplates: [
             [] as DashboardTemplateType[],
             {
                 getAllTemplates: async () => {
-                    const page = await api.dashboardTemplates.list(props)
+                    const params = {
+                        // the backend doesn't know about a default scope
+                        scope: props.scope !== 'default' ? props.scope : undefined,
+                        search: values.templateFilter.length > 2 ? values.templateFilter : undefined,
+                    }
+                    const page = await api.dashboardTemplates.list(params)
                     return page.results
                 },
             },
         ],
     })),
+    listeners(({ actions }) => ({
+        setTemplateFilter: async (_, breakpoint) => {
+            await breakpoint(100)
+            actions.getAllTemplates()
+        },
+    })),
     afterMount(({ actions }) => {
         actions.getAllTemplates()
     }),

frontend/src/types.ts

@@ -1445,6 +1445,8 @@ export interface DashboardBasicType {
 
 export interface DashboardTemplateListParams {
     scope?: DashboardTemplateScope
+    // matches on template name, description, and tags
+    search?: string
 }
 
 export type DashboardTemplateScope = 'team' | 'global' | 'feature_flag'

posthog/api/dashboards/dashboard_templates.py

@@ -90,5 +90,22 @@ def json_schema(self, request: request.Request, **kwargs) -> response.Response:
 
     def get_queryset(self, *args, **kwargs):
         filters = self.request.GET.dict()
-        default_condition = Q(team_id=self.team_id) | Q(scope=DashboardTemplate.Scope.GLOBAL)
-        return DashboardTemplate.objects.filter(Q(**filters) if filters else default_condition)
+        scope = filters.pop("scope", None)
+        search = filters.pop("search", None)
+
+        # if scope is feature flag, then only return feature flag templates
+        # they're implicitly global, so they are not associated with any teams
+        if scope == DashboardTemplate.Scope.FEATURE_FLAG:
+            query_condition = Q(scope=DashboardTemplate.Scope.FEATURE_FLAG)
+        elif scope == DashboardTemplate.Scope.GLOBAL:
+            query_condition = Q(scope=DashboardTemplate.Scope.GLOBAL)
+        # otherwise we are in the new dashboard context so show global templates and ones from this team
+        else:
+            query_condition = Q(team_id=self.team_id) | Q(scope=DashboardTemplate.Scope.GLOBAL)
+
+        if search:
+            query_condition &= (
+                Q(template_name__search=search) | Q(dashboard_description__search=search) | Q(tags__contains=[search])
+            )
+
+        return DashboardTemplate.objects.filter(query_condition)

posthog/api/dashboards/test/test_dashboard_templates.py

@@ -1,5 +1,8 @@
+from typing import Dict, Optional, List
+
 from rest_framework import status
 
+from posthog.models import User
 from posthog.models.dashboard_templates import DashboardTemplate
 from posthog.models.organization import Organization
 from posthog.models.team.team import Team
@@ -444,19 +447,120 @@ def test_cant_make_templates_without_teamid_private(self) -> None:
         assert response.status_code == status.HTTP_200_OK
         assert response.json()["results"][0]["scope"] == "global"
 
+    def test_search_when_listing_templates(self):
+        # ensure there are no templates
+        DashboardTemplate.objects.all().delete()
+
+        self.create_template({"scope": DashboardTemplate.Scope.GLOBAL, "template_name": "pony template"})
+        self.create_template(
+            {
+                "scope": DashboardTemplate.Scope.GLOBAL,
+                "template_name": "wat template",
+                "dashboard_description": "description with pony",
+            }
+        )
+        self.create_template(
+            {"scope": DashboardTemplate.Scope.GLOBAL, "template_name": "tagged wat template", "tags": ["pony", "horse"]}
+        )
+        self.create_template(
+            {
+                "scope": DashboardTemplate.Scope.GLOBAL,
+                "template_name": "tagged ponies template",
+                "tags": ["goat", "horse"],
+            }
+        )
+        not_pony_template_id = self.create_template(
+            {"scope": DashboardTemplate.Scope.GLOBAL, "template_name": "goat template"}
+        )
+
+        default_response = self.client.get(f"/api/projects/{self.team.pk}/dashboard_templates/")
+        assert default_response.status_code == status.HTTP_200_OK
+        assert len(default_response.json()["results"]) == 5
+
+        # will match pony and ponies
+        pony_response = self.client.get(f"/api/projects/{self.team.pk}/dashboard_templates/?search=pony")
+        assert pony_response.status_code == status.HTTP_200_OK
+        assert len(pony_response.json()["results"]) == 4
+        assert not_pony_template_id not in [r["id"] for r in pony_response.json()["results"]]
+
     def test_filter_template_list_by_scope(self):
+        # ensure there are no templates
+        DashboardTemplate.objects.all().delete()
+
+        # create a flag and a global scoped template
+        flag_template_id = self.create_template(
+            {"scope": DashboardTemplate.Scope.FEATURE_FLAG, "template_name": "flag scoped template"}
+        )
+        global_template_id = self.create_template(
+            {"scope": DashboardTemplate.Scope.GLOBAL, "template_name": "globally scoped template"}
+        )
+
+        default_response = self.client.get(f"/api/projects/{self.team.pk}/dashboard_templates/")
+        assert default_response.status_code == status.HTTP_200_OK
+        assert [(r["id"], r["scope"]) for r in default_response.json()["results"]] == [
+            (flag_template_id, "feature_flag"),
+            (global_template_id, "global"),
+        ]
+
+        global_response = self.client.get(f"/api/projects/{self.team.pk}/dashboard_templates/?scope=global")
+        assert global_response.status_code == status.HTTP_200_OK
+        assert [(r["id"], r["scope"]) for r in global_response.json()["results"]] == [(global_template_id, "global")]
+
+        flag_response = self.client.get(f"/api/projects/{self.team.pk}/dashboard_templates/?scope=feature_flag")
+        assert flag_response.status_code == status.HTTP_200_OK
+        assert [(r["id"], r["scope"]) for r in flag_response.json()["results"]] == [(flag_template_id, "feature_flag")]
+
+    def create_template(self, overrides: Dict[str, str | List[str]], team_id: Optional[int] = None) -> str:
+        template = {**variable_template, **overrides}
         response = self.client.post(
-            f"/api/projects/{self.team.pk}/dashboard_templates",
-            variable_template,
+            f"/api/projects/{team_id or self.team.pk}/dashboard_templates",
+            template,
         )
+        assert response.status_code == status.HTTP_201_CREATED
         id = response.json()["id"]
 
-        self.client.patch(
-            f"/api/projects/{self.team.pk}/dashboard_templates/{id}",
-            {"scope": "feature_flag"},
+        return id
+
+    def test_cannot_escape_team_when_filtering_template_list(self):
+        # create another team, and log in as a user from that team
+        another_team = Team.objects.create(name="Another Team", organization=self.organization)
+        another_team_user = User.objects.create_and_join(
+            organization=self.organization, first_name="Another", email="[email protected]", password="wat"
         )
+        another_team_user.current_team = another_team
+        another_team_user.is_staff = True
+        another_team_user.save()
 
-        response = self.client.get(f"/api/projects/{self.team.pk}/dashboard_templates/?scope=feature_flag")
+        self.client.force_login(another_team_user)
 
-        assert response.status_code == status.HTTP_200_OK
-        assert response.json()["results"][0]["scope"] == "feature_flag"
+        # create a dashboard template in that other team
+        id = self.create_template(
+            {"scope": DashboardTemplate.Scope.ONLY_TEAM, "template_name": "other teams template"},
+            team_id=another_team.pk,
+        )
+
+        # the user from another_team can access the new dashboard via the API on their own team
+        list_response = self.client.get(f"/api/projects/{another_team.pk}/dashboard_templates/")
+        assert list_response.status_code == status.HTTP_200_OK
+        assert id in [r["id"] for r in list_response.json()["results"]]
+
+        # the user from the home team cannot see the dashboard by default
+        self.client.force_login(self.user)
+        home_list_response = self.client.get(f"/api/projects/{self.team.pk}/dashboard_templates")
+
+        assert home_list_response.status_code == status.HTTP_200_OK
+        assert id not in [r["id"] for r in home_list_response.json()["results"]]
+
+        # the user form the home team cannot escape their permissions by passing filters
+        attempted_escape_response = self.client.get(
+            f"/api/projects/{self.team.pk}/dashboard_templates/?team_id={another_team.pk}"
+        )
+        assert attempted_escape_response.status_code == status.HTTP_200_OK
+        assert id not in [r["id"] for r in attempted_escape_response.json()["results"]]
+
+        # searching by text doesn't get around the team filtering
+        another_attempted_escape_response = self.client.get(
+            f"/api/projects/{self.team.pk}/dashboard_templates/?search=other"
+        )
+        assert another_attempted_escape_response.status_code == status.HTTP_200_OK
+        assert id not in [r["id"] for r in another_attempted_escape_response.json()["results"]]