-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat: prog #37075
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
base: master
Are you sure you want to change the base?
feat: prog #37075
Changes from all commits
ac1d480
db7af06
c7fa057
477fb7f
f8c33b2
4dfa7b3
86a862a
c6aa517
2714b98
993a1d3
7eb311c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ import { Link } from 'lib/lemon-ui/Link' | |
import { featureFlagLogic } from 'lib/logic/featureFlagLogic' | ||
import { clearDOMTextSelection, getJSHeapMemory, shouldCancelQuery, toParams, uuid } from 'lib/utils' | ||
import { DashboardEventSource, eventUsageLogic } from 'lib/utils/eventUsageLogic' | ||
import { BREAKPOINTS } from 'scenes/dashboard/dashboardUtils' | ||
import { calculateLayouts } from 'scenes/dashboard/tileLayouts' | ||
import { dataThemeLogic } from 'scenes/dataThemeLogic' | ||
import { MaxContextInput, createMaxContextHelpers } from 'scenes/max/maxTypes' | ||
|
@@ -155,7 +156,12 @@ export const dashboardLogic = kea<dashboardLogicType>([ | |
/** | ||
* Dashboard loading and dashboard tile refreshes. | ||
*/ | ||
loadDashboard: (payload: { action: DashboardLoadAction }) => payload, | ||
loadDashboard: (payload: { | ||
action: DashboardLoadAction | ||
limitTiles?: number // optional limit for progressive loading | ||
}) => payload, | ||
/** Load remaining tiles after partial dashboard load. */ | ||
loadRemainingTiles: true, | ||
/** Expose additional information about the current dashboard load in dashboardLoadData. */ | ||
loadingDashboardItemsStarted: (action: DashboardLoadAction) => ({ action }), | ||
/** Expose response size information about the current dashboard load in dashboardLoadData. */ | ||
|
@@ -266,13 +272,19 @@ export const dashboardLogic = kea<dashboardLogicType>([ | |
dashboard: [ | ||
null as DashboardType<QueryBasedInsightModel> | null, | ||
{ | ||
loadDashboard: async ({ action }, breakpoint) => { | ||
loadDashboard: async ({ action, limitTiles }, breakpoint) => { | ||
actions.loadingDashboardItemsStarted(action) | ||
|
||
await breakpoint(200) | ||
|
||
try { | ||
const apiUrl = values.apiUrl('force_cache', values.urlFilters, values.urlVariables) | ||
const apiUrl = values.apiUrl( | ||
'force_cache', | ||
values.urlFilters, | ||
values.urlVariables, | ||
values.currentLayoutSize, | ||
limitTiles | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could put this behind a feature flag. |
||
) | ||
const dashboardResponse: Response = await api.getResponse(apiUrl) | ||
const dashboard: DashboardType<InsightModel> | null = await getJSONOrNull(dashboardResponse) | ||
|
||
|
@@ -289,6 +301,29 @@ export const dashboardLogic = kea<dashboardLogicType>([ | |
throw error | ||
} | ||
}, | ||
loadRemainingTiles: async (_, breakpoint) => { | ||
if (!values.dashboard) { | ||
return values.dashboard | ||
} | ||
|
||
await breakpoint(200) | ||
|
||
try { | ||
const apiUrl = values.apiUrl( | ||
'force_cache', | ||
values.urlFilters, | ||
values.urlVariables, | ||
values.currentLayoutSize | ||
) | ||
const dashboardResponse: Response = await api.getResponse(apiUrl) | ||
const dashboard: DashboardType<InsightModel> | null = await getJSONOrNull(dashboardResponse) | ||
return getQueryBasedDashboard(dashboard) | ||
} catch (error: any) { | ||
// If loading remaining tiles fails, just return current dashboard | ||
console.warn('Failed to load remaining tiles:', error) | ||
return values.dashboard | ||
} | ||
}, | ||
saveEditModeChanges: async (_, breakpoint) => { | ||
actions.abortAnyRunningQuery() | ||
|
||
|
@@ -724,6 +759,14 @@ export const dashboardLogic = kea<dashboardLogicType>([ | |
loadDashboardFailure: () => false, | ||
}, | ||
], | ||
loadingRemainingTiles: [ | ||
false, | ||
{ | ||
loadRemainingTiles: () => true, | ||
loadRemainingTilesSuccess: () => false, | ||
loadRemainingTilesFailure: () => false, | ||
}, | ||
], | ||
/** Dashboard variables */ | ||
initialVariablesLoaded: [ | ||
false, | ||
|
@@ -924,15 +967,32 @@ export const dashboardLogic = kea<dashboardLogicType>([ | |
return ( | ||
refresh?: RefreshType, | ||
filtersOverride?: DashboardFilter, | ||
variablesOverride?: Record<string, HogQLVariable> | ||
variablesOverride?: Record<string, HogQLVariable>, | ||
layoutSize?: 'sm' | 'xs', | ||
limitTiles?: number | ||
) => | ||
`api/environments/${teamLogic.values.currentTeamId}/dashboards/${id}/?${toParams({ | ||
refresh, | ||
filters_override: filtersOverride, | ||
variables_override: variablesOverride, | ||
layout_size: layoutSize, | ||
limit_tiles: limitTiles, | ||
})}` | ||
}, | ||
], | ||
currentLayoutSize: [ | ||
(s) => [s.containerWidth], | ||
(containerWidth): 'sm' | 'xs' => { | ||
// Use precise container width when available, otherwise estimate from window width | ||
if (containerWidth !== null) { | ||
return containerWidth > BREAKPOINTS.sm ? 'sm' : 'xs' | ||
} | ||
// Estimate from window width, accounting for ~300px of sidebars | ||
const windowWidth = typeof window !== 'undefined' ? window.innerWidth : 1200 | ||
const estimatedContainerWidth = windowWidth - 300 | ||
return estimatedContainerWidth > BREAKPOINTS.sm ? 'sm' : 'xs' | ||
}, | ||
], | ||
tiles: [(s) => [s.dashboard], (dashboard) => dashboard?.tiles?.filter((t) => !t.deleted) || []], | ||
insightTiles: [ | ||
(s) => [s.tiles], | ||
|
@@ -1139,7 +1199,7 @@ export const dashboardLogic = kea<dashboardLogicType>([ | |
actions.loadDashboardSuccess(props.dashboard) | ||
} else { | ||
if (!(SEARCH_PARAM_QUERY_VARIABLES_KEY in router.values.searchParams)) { | ||
actions.loadDashboard({ action: DashboardLoadAction.InitialLoad }) | ||
actions.loadDashboard({ action: DashboardLoadAction.InitialLoad, limitTiles: 4 }) | ||
} | ||
} | ||
} | ||
|
@@ -1487,6 +1547,10 @@ export const dashboardLogic = kea<dashboardLogicType>([ | |
return // We hit a 404 | ||
} | ||
|
||
if (values.dashboard.has_more_tiles) { | ||
actions.loadRemainingTiles() | ||
} | ||
|
||
if (values.placement !== DashboardPlacement.Export) { | ||
// access stored values from dashboardLoadData | ||
// as we can't pass them down to this listener | ||
|
@@ -1499,6 +1563,13 @@ export const dashboardLogic = kea<dashboardLogicType>([ | |
actions.reportDashboardViewed() | ||
} | ||
}, | ||
loadRemainingTilesSuccess: () => { | ||
// Refresh all tiles using the same logic as initial dashboard load | ||
if (values.placement !== DashboardPlacement.Export) { | ||
const action = values.dashboardLoadData.action! | ||
actions.refreshDashboardItems({ action, forceRefresh: false }) | ||
} | ||
}, | ||
reportDashboardViewed: async (_, breakpoint) => { | ||
// Caching `dashboard`, as the dashboard might have unmounted after the breakpoint, | ||
// and "values.dashboard" will then fail | ||
|
@@ -1591,7 +1662,7 @@ export const dashboardLogic = kea<dashboardLogicType>([ | |
} | ||
|
||
if (SEARCH_PARAM_QUERY_VARIABLES_KEY in router.values.searchParams) { | ||
actions.loadDashboard({ action: DashboardLoadAction.InitialLoadWithVariables }) | ||
actions.loadDashboard({ action: DashboardLoadAction.InitialLoadWithVariables, limitTiles: 4 }) | ||
} | ||
|
||
actions.setInitialVariablesLoaded(true) | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -179,6 +179,7 @@ def get_access_control_version(self, dashboard: Dashboard) -> str: | |||||||||||||||||||||
|
||||||||||||||||||||||
class DashboardSerializer(DashboardBasicSerializer): | ||||||||||||||||||||||
tiles = serializers.SerializerMethodField() | ||||||||||||||||||||||
has_more_tiles = serializers.SerializerMethodField() | ||||||||||||||||||||||
filters = serializers.SerializerMethodField() | ||||||||||||||||||||||
variables = serializers.SerializerMethodField() | ||||||||||||||||||||||
created_by = UserBasicSerializer(read_only=True) | ||||||||||||||||||||||
|
@@ -216,6 +217,7 @@ class Meta: | |||||||||||||||||||||
"data_color_theme_id", | ||||||||||||||||||||||
"tags", | ||||||||||||||||||||||
"tiles", | ||||||||||||||||||||||
"has_more_tiles", | ||||||||||||||||||||||
"restriction_level", | ||||||||||||||||||||||
"effective_restriction_level", | ||||||||||||||||||||||
"effective_privilege_level", | ||||||||||||||||||||||
|
@@ -472,7 +474,7 @@ def get_tiles(self, dashboard: Dashboard) -> Optional[list[ReturnDict]]: | |||||||||||||||||||||
|
||||||||||||||||||||||
serialized_tiles: list[ReturnDict] = [] | ||||||||||||||||||||||
|
||||||||||||||||||||||
tiles = DashboardTile.dashboard_queryset(dashboard.tiles).prefetch_related( | ||||||||||||||||||||||
tiles = DashboardTile.dashboard_queryset(dashboard.tiles.all()).prefetch_related( | ||||||||||||||||||||||
Prefetch( | ||||||||||||||||||||||
"insight__tagged_items", | ||||||||||||||||||||||
queryset=TaggedItem.objects.select_related("tag"), | ||||||||||||||||||||||
|
@@ -489,16 +491,25 @@ def get_tiles(self, dashboard: Dashboard) -> Optional[list[ReturnDict]]: | |||||||||||||||||||||
group_properties={"organization": {"id": str(team.organization_id)}}, | ||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Get layout size and limit parameters for progressive loading | ||||||||||||||||||||||
limit_tiles, layout_size = self._get_progressive_loading_params() | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Sort tiles by layout to ensure insights are computed in order of appearance on dashboard | ||||||||||||||||||||||
# sm more common than xs, so we sort by sm | ||||||||||||||||||||||
# Use the specified layout size to get the correct order for the current viewport | ||||||||||||||||||||||
sorted_tiles = sorted( | ||||||||||||||||||||||
tiles, | ||||||||||||||||||||||
key=lambda tile: ( | ||||||||||||||||||||||
tile.layouts.get("sm", {}).get("y", 100), | ||||||||||||||||||||||
tile.layouts.get("sm", {}).get("x", 100), | ||||||||||||||||||||||
tile.layouts.get(layout_size, {}).get("y", 100), | ||||||||||||||||||||||
tile.layouts.get(layout_size, {}).get("x", 100), | ||||||||||||||||||||||
), | ||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Apply tile limit if specified, but only if there are enough tiles to make it worthwhile | ||||||||||||||||||||||
# For dashboards with < 10 tiles, return all tiles even if limit_tiles is specified | ||||||||||||||||||||||
total_tiles = len(sorted_tiles) | ||||||||||||||||||||||
if limit_tiles is not None and limit_tiles > 0 and total_tiles >= 10: | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need/want this? We could just request a limit of 10 insights from the frontend. |
||||||||||||||||||||||
sorted_tiles = sorted_tiles[:limit_tiles] | ||||||||||||||||||||||
Comment on lines
+510
to
+511
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: The 10-tile threshold is hardcoded. Consider using a constant at the top of the file for better maintainability
Suggested change
Context Used: Context - When defining constants, such as template IDs, use a constants file or define them at the top of the file for better maintainability. (link) |
||||||||||||||||||||||
|
||||||||||||||||||||||
with task_chain_context() if chained_tile_refresh_enabled else nullcontext(): | ||||||||||||||||||||||
# Handle case where there are no tiles | ||||||||||||||||||||||
if not sorted_tiles: | ||||||||||||||||||||||
|
@@ -510,6 +521,42 @@ def get_tiles(self, dashboard: Dashboard) -> Optional[list[ReturnDict]]: | |||||||||||||||||||||
|
||||||||||||||||||||||
return serialized_tiles | ||||||||||||||||||||||
|
||||||||||||||||||||||
def _get_progressive_loading_params(self): | ||||||||||||||||||||||
"""Extract progressive loading parameters from request.""" | ||||||||||||||||||||||
request = self.context.get("request") | ||||||||||||||||||||||
limit_tiles = None | ||||||||||||||||||||||
layout_size = "sm" | ||||||||||||||||||||||
|
||||||||||||||||||||||
if request and hasattr(request, "query_params"): | ||||||||||||||||||||||
try: | ||||||||||||||||||||||
limit_tiles = int(request.query_params.get("limit_tiles", "")) | ||||||||||||||||||||||
except (ValueError, TypeError): | ||||||||||||||||||||||
limit_tiles = None | ||||||||||||||||||||||
|
||||||||||||||||||||||
layout_size = request.query_params.get("layout_size", "sm") | ||||||||||||||||||||||
if layout_size not in ["sm", "xs"]: | ||||||||||||||||||||||
layout_size = "sm" # fallback to sm if invalid value | ||||||||||||||||||||||
|
||||||||||||||||||||||
return limit_tiles, layout_size | ||||||||||||||||||||||
|
||||||||||||||||||||||
def get_has_more_tiles(self, dashboard: Dashboard) -> bool: | ||||||||||||||||||||||
if self.context["view"].action == "list": | ||||||||||||||||||||||
return False | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Check if tiles were limited | ||||||||||||||||||||||
limit_tiles, _ = self._get_progressive_loading_params() | ||||||||||||||||||||||
|
||||||||||||||||||||||
if limit_tiles is None or limit_tiles <= 0: | ||||||||||||||||||||||
return False | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Get total number of tiles | ||||||||||||||||||||||
tiles = DashboardTile.dashboard_queryset(dashboard.tiles.all()) | ||||||||||||||||||||||
total_tiles = tiles.count() | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Only indicate more tiles if there are >= 10 total tiles and more than the limit | ||||||||||||||||||||||
# For dashboards with < 10 tiles, we return all tiles regardless of limit_tiles | ||||||||||||||||||||||
return total_tiles >= 10 and total_tiles > limit_tiles | ||||||||||||||||||||||
|
||||||||||||||||||||||
def get_filters(self, dashboard: Dashboard) -> dict: | ||||||||||||||||||||||
request = self.context.get("request") | ||||||||||||||||||||||
return filters_override_requested_by_client(request, dashboard) | ||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider using a consistent loading spinner component instead of inline styles for better maintainability