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

fix(insights): Improve load more for breakdowns #24231

Merged
merged 22 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
88b5441
Improve load more for breakdowns
webjunkie Aug 7, 2024
cd6d89e
Update UI snapshots for `webkit` (2)
github-actions[bot] Aug 7, 2024
06314b6
Update UI snapshots for `chromium` (1)
github-actions[bot] Aug 7, 2024
eae97fe
Update UI snapshots for `chromium` (2)
github-actions[bot] Aug 7, 2024
c6dd306
Update UI snapshots for `chromium` (1)
github-actions[bot] Aug 7, 2024
a2fd645
Update UI snapshots for `chromium` (1)
github-actions[bot] Aug 7, 2024
7ac91a6
Update UI snapshots for `chromium` (1)
github-actions[bot] Aug 7, 2024
d193189
Merge branch 'master' into fix/breakdown-limit-load-more
webjunkie Aug 12, 2024
d6f065d
Add hasMore to indicate if breakdown values have been truncated
webjunkie Aug 12, 2024
2f0f9be
Update UI snapshots for `webkit` (2)
github-actions[bot] Aug 12, 2024
ed51cfd
Update UI snapshots for `chromium` (1)
github-actions[bot] Aug 12, 2024
cf10f67
Update UI snapshots for `chromium` (2)
github-actions[bot] Aug 12, 2024
f62cb71
Update UI snapshots for `chromium` (1)
github-actions[bot] Aug 12, 2024
9fd7666
Merge branch 'master' into fix/breakdown-limit-load-more
webjunkie Aug 12, 2024
9f6f462
add comment
thmsobrmlr Aug 13, 2024
43c0781
gracefully handle missing breakdownFilter
thmsobrmlr Aug 13, 2024
b9dafd8
Merge branch 'master' into fix/breakdown-limit-load-more
webjunkie Aug 13, 2024
209cce7
Fix has more
webjunkie Aug 13, 2024
2f48b5c
Fix lemon table all series check
webjunkie Aug 13, 2024
48aa33c
Update query snapshots
github-actions[bot] Aug 13, 2024
c70a7f5
Merge branch 'master' into fix/breakdown-limit-load-more
webjunkie Aug 13, 2024
b7af002
Fix test
webjunkie Aug 14, 2024
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
thmsobrmlr marked this conversation as resolved.
Show resolved Hide resolved
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
thmsobrmlr marked this conversation as resolved.
Show resolved Hide resolved
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 12 additions & 0 deletions frontend/src/queries/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1702,6 +1702,10 @@
"description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
"type": "string"
},
"hasMore": {
"description": "Wether more breakdown values are available.",
"type": "boolean"
},
"hogql": {
"description": "Generated HogQL query.",
"type": "string"
Expand Down Expand Up @@ -7503,6 +7507,10 @@
"description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
"type": "string"
},
"hasMore": {
"description": "Wether more breakdown values are available.",
"type": "boolean"
},
"hogql": {
"description": "Generated HogQL query.",
"type": "string"
Expand Down Expand Up @@ -9144,6 +9152,10 @@
"description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
"type": "string"
},
"hasMore": {
"description": "Wether more breakdown values are available.",
"type": "boolean"
},
"hogql": {
"description": "Generated HogQL query.",
"type": "string"
Expand Down
5 changes: 4 additions & 1 deletion frontend/src/queries/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,10 @@ export const TRENDS_FILTER_PROPERTIES = new Set<keyof TrendsFilter>([
'hiddenLegendIndexes',
])

export interface TrendsQueryResponse extends AnalyticsQueryResponseBase<Record<string, any>[]> {}
export interface TrendsQueryResponse extends AnalyticsQueryResponseBase<Record<string, any>[]> {
/** Wether more breakdown values are available. */
hasMore?: boolean
}

export type CachedTrendsQueryResponse = CachedQueryResponse<TrendsQueryResponse>

Expand Down
45 changes: 22 additions & 23 deletions frontend/src/scenes/trends/Trends.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ export function TrendInsight({ view, context, embedded }: Props): JSX.Element {
const { insightProps, showPersonsModal: insightLogicShowPersonsModal } = useValues(insightLogic)
const showPersonsModal = insightLogicShowPersonsModal && !embedded

const { display, series, breakdownFilter, loadMoreBreakdownUrl, hasBreakdownOther, breakdownValuesLoading } =
useValues(trendsDataLogic(insightProps))
const { loadMoreBreakdownValues, updateBreakdownFilter } = useActions(trendsDataLogic(insightProps))
const { display, series, breakdownFilter, hasBreakdownMore, breakdownValuesLoading } = useValues(
trendsDataLogic(insightProps)
)
const { updateBreakdownFilter } = useActions(trendsDataLogic(insightProps))

const renderViz = (): JSX.Element | undefined => {
if (
Expand Down Expand Up @@ -72,27 +73,25 @@ export function TrendInsight({ view, context, embedded }: Props): JSX.Element {
{!embedded &&
display !== ChartDisplayType.WorldMap && // the world map doesn't need this cta
breakdownFilter &&
(hasBreakdownOther || loadMoreBreakdownUrl) && (
<div className="my-4 flex flex-col items-center px-2">
<div className="text-muted text-center mb-2">
For readability, <b>not all breakdown values are displayed</b>. Click below to load more.
hasBreakdownMore && (
<div className="p-4">
<div className="text-muted">
Breakdown limited to {breakdownFilter.breakdown_limit || 25} - more available
<LemonButton
onClick={() =>
updateBreakdownFilter({
...breakdownFilter,
breakdown_limit: (breakdownFilter.breakdown_limit || 25) * 2,
})
}
loading={breakdownValuesLoading}
size="xsmall"
type="secondary"
className="inline-block ml-2"
>
Set to {(breakdownFilter.breakdown_limit || 25) * 2}
</LemonButton>
</div>
<LemonButton
onClick={
hasBreakdownOther
? () =>
updateBreakdownFilter({
...breakdownFilter,
breakdown_limit: (breakdownFilter.breakdown_limit || 25) * 2,
})
: loadMoreBreakdownValues
}
loading={breakdownValuesLoading}
size="small"
type="secondary"
>
Load more breakdown values
</LemonButton>
</div>
)}
</>
Expand Down
30 changes: 2 additions & 28 deletions frontend/src/scenes/trends/trendsDataLogic.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { actions, connect, kea, key, listeners, path, props, reducers, selectors } from 'kea'
import api from 'lib/api'
import { dayjs } from 'lib/dayjs'
import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic'
import { keyForInsightLogicProps } from 'scenes/insights/sharedUtils'
Expand All @@ -8,7 +7,6 @@ import {
BREAKDOWN_NULL_STRING_LABEL,
BREAKDOWN_OTHER_NUMERIC_LABEL,
BREAKDOWN_OTHER_STRING_LABEL,
isOtherBreakdown,
} from 'scenes/insights/utils'

import { LifecycleQuery, MathType, TrendsFilter } from '~/queries/schema'
Expand Down Expand Up @@ -103,21 +101,13 @@ export const trendsDataLogic = kea<trendsDataLogicType>([
},
],

loadMoreBreakdownUrl: [
(s) => [s.insightData, s.isTrends],
(insightData, isTrends) => {
return isTrends ? insightData?.next : null
},
],

hasBreakdownOther: [
hasBreakdownMore: [
(s) => [s.insightData, s.isTrends],
(insightData, isTrends) => {
if (!isTrends) {
return false
}
const results = insightData.result ?? insightData.results
return !!(Array.isArray(results) && results.find((r) => isOtherBreakdown(r.breakdown_value)))
return !!insightData.hasMore
},
],

Expand Down Expand Up @@ -258,21 +248,5 @@ export const trendsDataLogic = kea<trendsDataLogicType>([
])
}
},
loadMoreBreakdownValues: async () => {
if (!values.loadMoreBreakdownUrl) {
return
}
actions.setBreakdownValuesLoading(true)

const response = await api.get(values.loadMoreBreakdownUrl)

actions.setInsightData({
...values.insightData,
result: [...values.insightData.result, ...(response.result ? response.result : [])],
next: response.next,
})

actions.setBreakdownValuesLoading(false)
},
})),
])
6 changes: 3 additions & 3 deletions posthog/hogql_queries/insights/trends/trends_query_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -809,9 +809,9 @@ def _trends_display(self) -> TrendsDisplay:

def _breakdown_outer_query_select(self, breakdown: Breakdown, breakdown_limit: int | None = None) -> ast.Expr:
breakdown_limit_expr = ast.Constant(value=breakdown_limit or self._get_breakdown_limit())
other_label_expr = ast.Constant(
value=None if breakdown.hide_other_aggregation else BREAKDOWN_OTHER_STRING_LABEL
)
# We always add the "other" aggregation to tell if we truncated the results
# It is then removed later
other_label_expr = ast.Constant(value=BREAKDOWN_OTHER_STRING_LABEL)

if breakdown.is_multiple_breakdown:
return parse_expr(
Expand Down
7 changes: 7 additions & 0 deletions posthog/hogql_queries/insights/trends/trends_query_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,8 +422,15 @@ def run(
if isinstance(timing, list):
timings.extend(timing)

has_more = False
if self.breakdown_enabled and any(item["label"] == BREAKDOWN_OTHER_STRING_LABEL for item in final_result):
if self.query.breakdownFilter and self.query.breakdownFilter.breakdown_hide_other_aggregation:
final_result = [item for item in final_result if item["label"] != BREAKDOWN_OTHER_STRING_LABEL]
has_more = True

return TrendsQueryResponse(
results=final_result,
hasMore=has_more,
timings=timings,
hogql=response_hogql,
modifiers=self.modifiers,
Expand Down
3 changes: 3 additions & 0 deletions posthog/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -1220,6 +1220,7 @@ class TrendsQueryResponse(BaseModel):
default=None,
description="Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
)
hasMore: Optional[bool] = Field(default=None, description="Wether more breakdown values are available.")
hogql: Optional[str] = Field(default=None, description="Generated HogQL query.")
modifiers: Optional[HogQLQueryModifiers] = Field(
default=None, description="Modifiers used when performing the query"
Expand Down Expand Up @@ -1799,6 +1800,7 @@ class CachedTrendsQueryResponse(BaseModel):
default=None,
description="Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
)
hasMore: Optional[bool] = Field(default=None, description="Wether more breakdown values are available.")
hogql: Optional[str] = Field(default=None, description="Generated HogQL query.")
is_cached: bool
last_refresh: AwareDatetime
Expand Down Expand Up @@ -2998,6 +3000,7 @@ class QueryResponseAlternative22(BaseModel):
default=None,
description="Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
)
hasMore: Optional[bool] = Field(default=None, description="Wether more breakdown values are available.")
hogql: Optional[str] = Field(default=None, description="Generated HogQL query.")
modifiers: Optional[HogQLQueryModifiers] = Field(
default=None, description="Modifiers used when performing the query"
Expand Down
Loading