Skip to content

Commit

Permalink
refactor(insights): remove include_query_insights flag (#23091)
Browse files Browse the repository at this point in the history
  • Loading branch information
thmsobrmlr authored Jun 24, 2024
1 parent 7b25e73 commit fb307b2
Show file tree
Hide file tree
Showing 7 changed files with 4 additions and 82 deletions.
1 change: 0 additions & 1 deletion frontend/src/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,6 @@ const api = {
.withQueryString(
toParams({
short_id: encodeURIComponent(shortId),
include_query_insights: true,
basic: basic,
})
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ export const projectHomepageLogic = kea<projectHomepageLogicType>([
[] as InsightModel[],
{
loadRecentInsights: async () => {
return await api.get(
`api/projects/${values.currentTeamId}/insights/my_last_viewed?include_query_insights=true`
)
return await api.get(`api/projects/${values.currentTeamId}/insights/my_last_viewed`)
},
},
],
Expand Down
3 changes: 1 addition & 2 deletions frontend/src/scenes/saved-insights/savedInsightsLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ export const savedInsightsLogic = kea<savedInsightsLogicType>([
const params = {
...values.paramsFromFilters,
basic: true,
include_query_insights: true,
}

const response = await api.get(
Expand All @@ -105,7 +104,7 @@ export const savedInsightsLogic = kea<savedInsightsLogicType>([
if (filters.search && String(filters.search).match(/^[0-9]+$/)) {
try {
const insight: InsightModel = await api.get(
`api/projects/${teamLogic.values.currentTeamId}/insights/${filters.search}/?include_query_insights=true`
`api/projects/${teamLogic.values.currentTeamId}/insights/${filters.search}/`
)
return {
...response,
Expand Down
5 changes: 0 additions & 5 deletions posthog/api/insight.py
Original file line number Diff line number Diff line change
Expand Up @@ -675,9 +675,6 @@ def safely_get_queryset(self, queryset) -> QuerySet:
queryset = queryset.prefetch_related("tagged_items__tag")
queryset = self._filter_request(self.request, queryset)

if self.request.query_params.get("include_query_insights", "false").lower() != "true":
queryset = queryset.exclude(Q(filters={}) & Q(query__isnull=False))

order = self.request.GET.get("order", None)
if order:
queryset = queryset.order_by(order)
Expand All @@ -697,8 +694,6 @@ def my_last_viewed(self, request: request.Request, *args, **kwargs) -> Response:
.exclude(insight__deleted=True)
.only("insight")
)
if self.request.query_params.get("include_query_insights", "false").lower() != "true":
insight_queryset = insight_queryset.exclude(Q(insight__filters={}) & Q(insight__query__isnull=False))

recently_viewed = [rv.insight for rv in (insight_queryset.order_by("-last_viewed_at")[:5])]

Expand Down
4 changes: 0 additions & 4 deletions posthog/api/test/__snapshots__/test_insight.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -1212,8 +1212,6 @@
SELECT COUNT(*) AS "__count"
FROM "posthog_dashboarditem"
WHERE (NOT ("posthog_dashboarditem"."deleted")
AND NOT ("posthog_dashboarditem"."filters" = '{}'::jsonb
AND "posthog_dashboarditem"."query" IS NOT NULL)
AND "posthog_dashboarditem"."team_id" = 2)
'''
# ---
Expand Down Expand Up @@ -1359,8 +1357,6 @@
LEFT OUTER JOIN "posthog_user" ON ("posthog_dashboarditem"."created_by_id" = "posthog_user"."id")
LEFT OUTER JOIN "posthog_user" T4 ON ("posthog_dashboarditem"."last_modified_by_id" = T4."id")
WHERE (NOT ("posthog_dashboarditem"."deleted")
AND NOT ("posthog_dashboarditem"."filters" = '{}'::jsonb
AND "posthog_dashboarditem"."query" IS NOT NULL)
AND "posthog_dashboarditem"."team_id" = 2)
ORDER BY "posthog_dashboarditem"."order" ASC
LIMIT 100
Expand Down
44 changes: 1 addition & 43 deletions posthog/api/test/test_insight.py
Original file line number Diff line number Diff line change
Expand Up @@ -2438,7 +2438,7 @@ def test_get_recently_viewed_insights(self) -> None:
self.assertEqual(response.status_code, status.HTTP_200_OK)
assert [r["id"] for r in response_data] == [insight_1_id]

def test_get_recently_viewed_insights_excludes_query_based_insights_by_default(self) -> None:
def test_get_recently_viewed_insights_include_query_based_insights(self) -> None:
insight_1_id, _ = self.dashboard_api.create_insight({"short_id": "12345678"})
insight_2_id, _ = self.dashboard_api.create_insight(
{
Expand Down Expand Up @@ -2475,48 +2475,6 @@ def test_get_recently_viewed_insights_excludes_query_based_insights_by_default(s
response = self.client.get(f"/api/projects/{self.team.id}/insights/my_last_viewed")
response_data = response.json()

# No results if no insights have been viewed
self.assertEqual(response.status_code, status.HTTP_200_OK)
assert [r["id"] for r in response_data] == [insight_1_id]

def test_get_recently_viewed_insights_can_include_query_based_insights(self) -> None:
insight_1_id, _ = self.dashboard_api.create_insight({"short_id": "12345678"})
insight_2_id, _ = self.dashboard_api.create_insight(
{
"short_id": "3456",
"query": {
"kind": "DataTableNode",
"source": {
"kind": "EventsQuery",
"select": [
"*",
"event",
"person",
"coalesce(properties.$current_url, properties.$screen_name)",
"properties.$lib",
"timestamp",
],
"properties": [
{
"type": "event",
"key": "$browser",
"operator": "exact",
"value": "Chrome",
}
],
"limit": 100,
},
},
}
)

self.client.post(f"/api/projects/{self.team.id}/insights/{insight_1_id}/viewed")
self.client.post(f"/api/projects/{self.team.id}/insights/{insight_2_id}/viewed")

response = self.client.get(f"/api/projects/{self.team.id}/insights/my_last_viewed?include_query_insights=true")
response_data = response.json()

# No results if no insights have been viewed
self.assertEqual(response.status_code, status.HTTP_200_OK)
assert [r["id"] for r in response_data] == [insight_2_id, insight_1_id]

Expand Down
25 changes: 1 addition & 24 deletions posthog/api/test/test_insight_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,29 +175,6 @@ def test_cannot_save_invalid_persons_table_query_to_an_insight(self) -> None:
expected_status=status.HTTP_201_CREATED,
)

def test_listing_insights_by_default_does_not_include_those_with_only_queries(self) -> None:
self.dashboard_api.create_insight({"name": "Insight with filters"})
self.dashboard_api.create_insight(
{
"name": "Insight with persons table query",
"query": {
"kind": "DataTableNode",
"columns": ["person", "id", "created_at", "person.$delete"],
"source": {
"kind": "EventsQuery",
"select": ["*"],
},
},
},
)

created_insights: list[Insight] = list(Insight.objects.all())
assert len(created_insights) == 2

listed_insights = self.dashboard_api.list_insights(query_params={"include_query_insights": False})
assert listed_insights["count"] == 1
assert listed_insights["results"][0]["name"] == "Insight with filters"

def test_can_list_insights_including_those_with_only_queries(self) -> None:
self.dashboard_api.create_insight({"name": "Insight with filters"})
self.dashboard_api.create_insight(
Expand All @@ -217,5 +194,5 @@ def test_can_list_insights_including_those_with_only_queries(self) -> None:
created_insights: list[Insight] = list(Insight.objects.all())
assert len(created_insights) == 2

listed_insights = self.dashboard_api.list_insights(query_params={"include_query_insights": True})
listed_insights = self.dashboard_api.list_insights()
assert listed_insights["count"] == 2

0 comments on commit fb307b2

Please sign in to comment.