diff --git a/cypress/e2e/dashboard.cy.ts b/cypress/e2e/dashboard.cy.ts index 3af5333100eef..7919c3dded56a 100644 --- a/cypress/e2e/dashboard.cy.ts +++ b/cypress/e2e/dashboard.cy.ts @@ -95,7 +95,7 @@ describe('Dashboard', () => { // refresh the dashboard by changing date range cy.get('[data-attr="date-filter"]').click() cy.contains('span', 'Last 14 days').click() - cy.contains('span', 'Apply and save dashboard').click() + cy.contains('span', 'Save').click() cy.contains('span[class="text-primary text-sm font-medium"]', 'Refreshing').should('not.exist') cy.get('span').contains('Refreshing').should('not.exist') @@ -163,7 +163,7 @@ describe('Dashboard', () => { cy.get('[data-attr=date-filter]').contains('No date range override').click() cy.get('div').contains('Yesterday').should('exist').click() cy.get('[data-attr=date-filter]').contains('Yesterday') - cy.get('button').contains('Apply and save dashboard').click() + cy.get('button').contains('Save').click() cy.get('.InsightCard h5').should('have.length', 1).contains('Yesterday') // Cool, now back to A and make sure the insight is still using the original range there, not the one from B cy.clickNavMenu('dashboards') diff --git a/cypress/productAnalytics/index.ts b/cypress/productAnalytics/index.ts index cedfa970c825f..af1a0c3f32e85 100644 --- a/cypress/productAnalytics/index.ts +++ b/cypress/productAnalytics/index.ts @@ -176,7 +176,7 @@ export const dashboard = { cy.get('[data-attr="prop-filter-event_properties-0"]').click({ force: true }).wait(1000) cy.get('.LemonInput').type(value) cy.contains('.LemonButton__content', value).click({ force: true }) - cy.get('button').contains('Apply and save dashboard').click() + cy.get('button').contains('Save').click() }, addAnyFilter(): void { cy.get('.PropertyFilterButton').should('have.length', 0) @@ -188,7 +188,7 @@ export const dashboard = { // click .dashboard to blur cy.get('.dashboard').click({ force: true }) cy.get('.PropertyFilterButton').should('have.length', 1) - cy.get('button').contains('Apply and save dashboard').click() + cy.get('button').contains('Save').click() }, } diff --git a/frontend/__snapshots__/posthog-3000-navigation--navigation-3000--dark.png b/frontend/__snapshots__/posthog-3000-navigation--navigation-3000--dark.png index 030f1724e0f3e..4086ce4b6a27e 100644 Binary files a/frontend/__snapshots__/posthog-3000-navigation--navigation-3000--dark.png and b/frontend/__snapshots__/posthog-3000-navigation--navigation-3000--dark.png differ diff --git a/frontend/__snapshots__/posthog-3000-navigation--navigation-3000--light.png b/frontend/__snapshots__/posthog-3000-navigation--navigation-3000--light.png index bcf05c9abf360..648af90308438 100644 Binary files a/frontend/__snapshots__/posthog-3000-navigation--navigation-3000--light.png and b/frontend/__snapshots__/posthog-3000-navigation--navigation-3000--light.png differ diff --git a/frontend/__snapshots__/posthog-3000-navigation--navigation-base--dark.png b/frontend/__snapshots__/posthog-3000-navigation--navigation-base--dark.png index f981e52556b18..4037a2ef634ce 100644 Binary files a/frontend/__snapshots__/posthog-3000-navigation--navigation-base--dark.png and b/frontend/__snapshots__/posthog-3000-navigation--navigation-base--dark.png differ diff --git a/frontend/__snapshots__/posthog-3000-navigation--navigation-base--light.png b/frontend/__snapshots__/posthog-3000-navigation--navigation-base--light.png index 1581d4a4edae4..cab4754531e0a 100644 Binary files a/frontend/__snapshots__/posthog-3000-navigation--navigation-base--light.png and b/frontend/__snapshots__/posthog-3000-navigation--navigation-base--light.png differ diff --git a/frontend/__snapshots__/scenes-app-dashboards--edit--dark.png b/frontend/__snapshots__/scenes-app-dashboards--edit--dark.png index 02241c05b0510..fb7b9107acb31 100644 Binary files a/frontend/__snapshots__/scenes-app-dashboards--edit--dark.png and b/frontend/__snapshots__/scenes-app-dashboards--edit--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-dashboards--show--dark.png b/frontend/__snapshots__/scenes-app-dashboards--show--dark.png index 17acdf6a3854d..2c51c1dcbc24a 100644 Binary files a/frontend/__snapshots__/scenes-app-dashboards--show--dark.png and b/frontend/__snapshots__/scenes-app-dashboards--show--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-dashboards--show--light.png b/frontend/__snapshots__/scenes-app-dashboards--show--light.png index 7b2cf09f9f27c..0a4ea86b18c4a 100644 Binary files a/frontend/__snapshots__/scenes-app-dashboards--show--light.png and b/frontend/__snapshots__/scenes-app-dashboards--show--light.png differ diff --git a/frontend/__snapshots__/scenes-app-project-homepage--project-homepage--dark.png b/frontend/__snapshots__/scenes-app-project-homepage--project-homepage--dark.png index 6f0a54565311f..68560cb0bc84a 100644 Binary files a/frontend/__snapshots__/scenes-app-project-homepage--project-homepage--dark.png and b/frontend/__snapshots__/scenes-app-project-homepage--project-homepage--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-project-homepage--project-homepage--light.png b/frontend/__snapshots__/scenes-app-project-homepage--project-homepage--light.png index 9f5a2404bb23d..e125144a00562 100644 Binary files a/frontend/__snapshots__/scenes-app-project-homepage--project-homepage--light.png and b/frontend/__snapshots__/scenes-app-project-homepage--project-homepage--light.png differ diff --git a/frontend/src/lib/components/Cards/InsightCard/InsightCard.tsx b/frontend/src/lib/components/Cards/InsightCard/InsightCard.tsx index 5cd43a5317597..833795d4f24b5 100644 --- a/frontend/src/lib/components/Cards/InsightCard/InsightCard.tsx +++ b/frontend/src/lib/components/Cards/InsightCard/InsightCard.tsx @@ -33,8 +33,6 @@ export interface InsightCardProps extends Resizeable, React.HTMLAttributes diff --git a/frontend/src/lib/utils/eventUsageLogic.ts b/frontend/src/lib/utils/eventUsageLogic.ts index 63b62be00ee67..c72f45b329486 100644 --- a/frontend/src/lib/utils/eventUsageLogic.ts +++ b/frontend/src/lib/utils/eventUsageLogic.ts @@ -71,7 +71,9 @@ import type { eventUsageLogicType } from './eventUsageLogicType' export enum DashboardEventSource { LongPress = 'long_press', MoreDropdown = 'more_dropdown', - DashboardHeader = 'dashboard_header', + DashboardHeaderSaveDashboard = 'dashboard_header_save_dashboard', + DashboardHeaderDiscardChanges = 'dashboard_header_discard_changes', + DashboardHeaderExitFullscreen = 'dashboard_header_exit_fullscreen', Hotkey = 'hotkey', InputEnter = 'input_enter', Toast = 'toast', diff --git a/frontend/src/queries/Query/Query.tsx b/frontend/src/queries/Query/Query.tsx index 3828f98f2e431..d5a4bb5755a99 100644 --- a/frontend/src/queries/Query/Query.tsx +++ b/frontend/src/queries/Query/Query.tsx @@ -1,5 +1,4 @@ import { LemonDivider } from 'lib/lemon-ui/LemonDivider' -import { SpinnerOverlay } from 'lib/lemon-ui/Spinner' import { useEffect, useState } from 'react' import { HogDebug } from 'scenes/debug/HogDebug' @@ -38,14 +37,12 @@ export interface QueryProps { cachedResults?: AnyResponseType /** Disable any changes to the query */ readOnly?: boolean - /** Show a stale overlay */ - stale?: boolean /** Reduce UI elements to only show data */ embedded?: boolean } export function Query(props: QueryProps): JSX.Element | null { - const { query: propsQuery, setQuery: propsSetQuery, readOnly, stale, embedded } = props + const { query: propsQuery, setQuery: propsSetQuery, readOnly, embedded } = props const [localQuery, localSetQuery] = useState(propsQuery) useEffect(() => { @@ -139,7 +136,6 @@ export function Query(props: QueryProps): JSX.Element | null ) : null} - {stale && } {component} diff --git a/frontend/src/scenes/dashboard/Dashboard.tsx b/frontend/src/scenes/dashboard/Dashboard.tsx index 4eee95898ee97..d6576eb4fe20b 100644 --- a/frontend/src/scenes/dashboard/Dashboard.tsx +++ b/frontend/src/scenes/dashboard/Dashboard.tsx @@ -117,9 +117,9 @@ function DashboardScene(): JSX.Element { > {[DashboardPlacement.Public].includes(placement) ? ( - ) : ( + ) : !(dashboardMode === DashboardMode.Edit) ? ( - )} + ) : null} )} diff --git a/frontend/src/scenes/dashboard/DashboardEditBar.tsx b/frontend/src/scenes/dashboard/DashboardEditBar.tsx index 2dc6c481030d9..2fdf7c1c8767e 100644 --- a/frontend/src/scenes/dashboard/DashboardEditBar.tsx +++ b/frontend/src/scenes/dashboard/DashboardEditBar.tsx @@ -1,6 +1,4 @@ import { IconCalendar } from '@posthog/icons' -import { LemonButton, Popover } from '@posthog/lemon-ui' -import clsx from 'clsx' import { useActions, useValues } from 'kea' import { DateFilter } from 'lib/components/DateFilter/DateFilter' import { PropertyFilters } from 'lib/components/PropertyFilters/PropertyFilters' @@ -8,72 +6,55 @@ import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types' import { dashboardLogic } from 'scenes/dashboard/dashboardLogic' import { groupsModel } from '~/models/groupsModel' +import { DashboardMode } from '~/types' export function DashboardEditBar(): JSX.Element { - const { dashboard, canEditDashboard, temporaryFilters, stale } = useValues(dashboardLogic) - const { setDates, setProperties, cancelTemporary, applyTemporary } = useActions(dashboardLogic) + const { dashboard, canEditDashboard, temporaryFilters, dashboardMode } = useValues(dashboardLogic) + const { setDates, setProperties, setDashboardMode } = useActions(dashboardLogic) const { groupsTaxonomicTypes } = useValues(groupsModel) - const isEditInProgress: boolean = canEditDashboard && stale const disabledReason = !canEditDashboard ? "You don't have permission to edit this dashboard" : undefined return ( - - - Cancel changes - - - Apply and save dashboard - - - } - placement="right" - showArrow - > -
+ { + if (dashboardMode !== DashboardMode.Edit) { + setDashboardMode(DashboardMode.Edit, null) + } + setDates(from_date, to_date) + }} + disabledReason={disabledReason} + makeLabel={(key) => ( + <> + + {key} + )} - > - ( - <> - - {key} - - )} - /> - -
- + /> + { + if (dashboardMode !== DashboardMode.Edit) { + setDashboardMode(DashboardMode.Edit, null) + } + setProperties(properties) + }} + pageKey={'dashboard_' + dashboard?.id} + propertyFilters={temporaryFilters.properties} + taxonomicGroupTypes={[ + TaxonomicFilterGroupType.EventProperties, + TaxonomicFilterGroupType.PersonProperties, + TaxonomicFilterGroupType.EventFeatureFlags, + ...groupsTaxonomicTypes, + TaxonomicFilterGroupType.Cohorts, + TaxonomicFilterGroupType.Elements, + TaxonomicFilterGroupType.HogQLExpression, + ]} + /> + ) } diff --git a/frontend/src/scenes/dashboard/DashboardHeader.tsx b/frontend/src/scenes/dashboard/DashboardHeader.tsx index 65ce4cdaeeb74..95bbb0c480e6a 100644 --- a/frontend/src/scenes/dashboard/DashboardHeader.tsx +++ b/frontend/src/scenes/dashboard/DashboardHeader.tsx @@ -120,19 +120,33 @@ export function DashboardHeader(): JSX.Element | null { setDashboardMode(null, DashboardEventSource.DashboardHeader)} - tabIndex={10} - disabled={dashboardLoading} - > - Done editing - + <> + + setDashboardMode(null, DashboardEventSource.DashboardHeaderDiscardChanges) + } + tabIndex={9} + > + Cancel + + + setDashboardMode(null, DashboardEventSource.DashboardHeaderSaveDashboard) + } + tabIndex={10} + disabled={dashboardLoading} + > + Save + + ) : dashboardMode === DashboardMode.Fullscreen ? ( setDashboardMode(null, DashboardEventSource.DashboardHeader)} + onClick={() => setDashboardMode(null, DashboardEventSource.DashboardHeaderExitFullscreen)} data-attr="dashboard-exit-presentation-mode" disabled={dashboardLoading} > diff --git a/frontend/src/scenes/dashboard/DashboardItems.tsx b/frontend/src/scenes/dashboard/DashboardItems.tsx index 552809a5e3214..fccd51a9885f5 100644 --- a/frontend/src/scenes/dashboard/DashboardItems.tsx +++ b/frontend/src/scenes/dashboard/DashboardItems.tsx @@ -26,7 +26,6 @@ export function DashboardItems(): JSX.Element { highlightedInsightId, refreshStatus, canEditDashboard, - stale, itemsLoading, } = useValues(dashboardLogic) const { @@ -139,7 +138,6 @@ export function DashboardItems(): JSX.Element { { logic.mount() }) - it('saving layouts with no provided tiles updates all tiles', async () => { + it('saving layouts creates api call with all tiles', async () => { + await expectLogic(logic).toFinishAllListeners() + jest.spyOn(api, 'update') await expectLogic(logic, () => { - logic.actions.saveLayouts() + logic.actions.updateFiltersAndLayouts() }).toFinishAllListeners() expect(api.update).toHaveBeenCalledWith(`api/projects/${MOCK_TEAM_ID}/dashboards/5`, { @@ -322,23 +324,11 @@ describe('dashboardLogic', () => { layouts: {}, }, ], - }) - }) - - it('saving layouts with provided tiles updates only those tiles', async () => { - jest.spyOn(api, 'update') - - await expectLogic(logic, () => { - logic.actions.saveLayouts([{ id: 1, layouts: { sm: {} as TileLayout, xs: {} as TileLayout } }]) - }).toFinishAllListeners() - - expect(api.update).toHaveBeenCalledWith(`api/projects/${MOCK_TEAM_ID}/dashboards/5`, { - tiles: [ - { - id: 1, - layouts: { sm: {} as TileLayout, xs: {} as TileLayout }, - }, - ], + filters: { + date_from: null, + date_to: null, + properties: [], + }, }) }) }) diff --git a/frontend/src/scenes/dashboard/dashboardLogic.tsx b/frontend/src/scenes/dashboard/dashboardLogic.tsx index 84f9e3ad5aa63..908b778ebb311 100644 --- a/frontend/src/scenes/dashboard/dashboardLogic.tsx +++ b/frontend/src/scenes/dashboard/dashboardLogic.tsx @@ -21,7 +21,7 @@ import { captureTimeToSeeData, currentSessionId, TimeToSeeDataPayload } from 'li import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast' import { Link } from 'lib/lemon-ui/Link' import { featureFlagLogic } from 'lib/logic/featureFlagLogic' -import { clearDOMTextSelection, isAbortedRequest, isUserLoggedIn, shouldCancelQuery, toParams, uuid } from 'lib/utils' +import { clearDOMTextSelection, isAbortedRequest, shouldCancelQuery, toParams, uuid } from 'lib/utils' import { DashboardEventSource, eventUsageLogic } from 'lib/utils/eventUsageLogic' import { Layout, Layouts } from 'react-grid-layout' import { calculateLayouts } from 'scenes/dashboard/tileLayouts' @@ -137,13 +137,15 @@ async function getSingleInsight( dashboardId: number, queryId: string, refresh: RefreshType, - methodOptions?: ApiMethodOptions + methodOptions?: ApiMethodOptions, + filtersOverride?: DashboardFilter ): Promise { const apiUrl = `api/projects/${currentTeamId}/insights/${insight.id}/?${toParams({ refresh, from_dashboard: dashboardId, // needed to load insight in correct context client_query_id: queryId, session_id: currentSessionId(), + ...(filtersOverride ? { filters_override: filtersOverride } : {}), })}` const insightResponse: Response = await api.getResponse(apiUrl, methodOptions) const legacyInsight: InsightModel | null = await getJSONOrNull(insightResponse) @@ -169,12 +171,17 @@ export const dashboardLogic = kea([ actions({ loadDashboard: (payload: { refresh?: RefreshType - action: 'initial_load' | 'update' | 'refresh' | 'load_missing' | 'refresh_insights_on_filters_updated' + action: + | 'initial_load' + | 'update' + | 'refresh' + | 'load_missing' + | 'refresh_insights_on_filters_updated' + | 'preview' }) => payload, triggerDashboardUpdate: (payload) => ({ payload }), /** The current state in which the dashboard is being viewed, see DashboardMode. */ setDashboardMode: (mode: DashboardMode | null, source: DashboardEventSource | null) => ({ mode, source }), - saveLayouts: (tilesToSave: DashboardTileLayoutUpdatePayload[] = []) => ({ tilesToSave }), updateLayouts: (layouts: Layouts) => ({ layouts }), updateContainerWidth: (containerWidth: number, columns: number) => ({ containerWidth, columns }), updateTileColor: (tileId: number, color: string | null) => ({ tileId, color }), @@ -194,7 +201,7 @@ export const dashboardLogic = kea([ date_to, }), setProperties: (properties: AnyPropertyFilter[] | null) => ({ properties }), - setFilters: (filters: DashboardFilter) => ({ filters }), + setFiltersAndLayouts: (filters: DashboardFilter) => ({ filters }), setAutoRefresh: (enabled: boolean, interval: number) => ({ enabled, interval }), setRefreshStatus: (shortId: InsightShortId, loading = false, queued = false) => ({ shortId, loading, queued }), setRefreshStatuses: (shortIds: InsightShortId[], loading = false, queued = false) => ({ @@ -226,8 +233,7 @@ export const dashboardLogic = kea([ setInitialLoadResponseBytes: (responseBytes: number) => ({ responseBytes }), abortQuery: (payload: { dashboardQueryId: string; queryId: string; queryStartTime: number }) => payload, abortAnyRunningQuery: true, - applyTemporary: true, - cancelTemporary: true, + updateFiltersAndLayouts: true, }), loaders(({ actions, props, values }) => ({ @@ -240,12 +246,34 @@ export const dashboardLogic = kea([ await breakpoint(200) try { - const apiUrl = values.apiUrl(refresh || 'async') + const apiUrl = values.apiUrl( + refresh || 'async', + action === 'preview' ? values.temporaryFilters : undefined + ) const dashboardResponse: Response = await api.getResponse(apiUrl) const dashboard: DashboardType | null = await getJSONOrNull(dashboardResponse) actions.setInitialLoadResponseBytes(getResponseBytes(dashboardResponse)) + // don't update dashboard tile layouts if we're previewing + // we want to retain what the user has temporarily set + if (action === 'preview' && dashboard) { + const editModeTileLayouts: Record = {} + values.dashboard?.tiles.forEach((tile: DashboardTile) => { + editModeTileLayouts[tile.id] = tile.layouts + }) + + const tilesWithPreviousLayouts = dashboard.tiles.map((tile) => ({ + ...tile, + layouts: editModeTileLayouts?.[tile.id], + })) + + return getQueryBasedDashboard({ + ...dashboard, + tiles: tilesWithPreviousLayouts, + }) + } + return getQueryBasedDashboard(dashboard) } catch (error: any) { if (error.status === 404) { @@ -254,19 +282,27 @@ export const dashboardLogic = kea([ throw error } }, - updateFilters: async () => { + updateFiltersAndLayouts: async (_, breakpoint) => { actions.abortAnyRunningQuery() try { + const layoutsToUpdate = (values.dashboard?.tiles || []).map((tile) => ({ + id: tile.id, + layouts: tile.layouts, + })) + + breakpoint() + const dashboard: DashboardType = await api.update( `api/projects/${values.currentTeamId}/dashboards/${props.id}`, { filters: values.filters, + tiles: layoutsToUpdate, } ) return getQueryBasedDashboard(dashboard) } catch (e) { - lemonToast.error('Could not update dashboardFilters: ' + String(e)) + lemonToast.error('Could not update dashboard: ' + String(e)) return values.dashboard } }, @@ -299,6 +335,23 @@ export const dashboardLogic = kea([ return values.dashboard } }, + setDashboardMode: async ({ mode, source }) => { + if ( + mode === null && + source === DashboardEventSource.DashboardHeaderDiscardChanges && + values.dashboard?.tiles + ) { + // layout changes were discarded so need to reset to original state + const restoredTiles = values.dashboard?.tiles?.map((tile) => ({ + ...tile, + layouts: values.dashboardLayouts?.[tile.id], + })) + + values.dashboard.tiles = restoredTiles + } + + return values.dashboard + }, duplicateTile: async ({ tile }) => { try { const newTile = { ...tile } as Partial> @@ -340,6 +393,14 @@ export const dashboardLogic = kea([ ], })), reducers(({ props }) => ({ + _dashboardLoading: [ + false, + { + loadDashboard: () => true, + loadDashboardSuccess: () => false, + loadDashboardFailure: () => false, + }, + ], pageVisibility: [ true, { @@ -353,6 +414,24 @@ export const dashboardLogic = kea([ loadDashboardFailure: () => true, }, ], + dashboardLayouts: [ + {} as Record, + { + loadDashboardSuccess: (state, { dashboard, payload }) => { + // don't update dashboardLayouts if we're previewing + if (payload?.action === 'preview') { + return state + } + + const tileIdToLayouts: Record = {} + dashboard?.tiles.forEach((tile: DashboardTile) => { + tileIdToLayouts[tile.id] = tile.layouts + }) + + return tileIdToLayouts + }, + }, + ], temporaryFilters: [ { date_from: null, @@ -375,7 +454,7 @@ export const dashboardLogic = kea([ ...state, date_from: dashboard?.filters.date_from || null, date_to: dashboard?.filters.date_to || null, - properties: dashboard?.filters.properties || null, + properties: dashboard?.filters.properties || [], } : state, }, @@ -387,17 +466,22 @@ export const dashboardLogic = kea([ properties: null, } as DashboardFilter, { - setFilters: (state, { filters }) => ({ + setFiltersAndLayouts: (state, { filters }) => ({ ...state, ...filters, }), - loadDashboardSuccess: (state, { dashboard }) => + loadDashboardSuccess: (state, { dashboard, payload }) => dashboard ? { ...state, - date_from: dashboard?.filters.date_from || null, - date_to: dashboard?.filters.date_to || null, - properties: dashboard?.filters.properties || [], + // don't update filters if we're previewing + ...(payload?.action === 'preview' + ? {} + : { + date_from: dashboard?.filters.date_from || null, + date_to: dashboard?.filters.date_to || null, + properties: dashboard?.filters.properties || [], + }), } : state, }, @@ -647,9 +731,10 @@ export const dashboardLogic = kea([ apiUrl: [ () => [(_, props) => props.id], (id) => { - return (refresh?: RefreshType) => + return (refresh?: RefreshType, filtersOverride?: DashboardFilter) => `api/projects/${teamLogic.values.currentTeamId}/dashboards/${id}/?${toParams({ refresh, + filters_override: filtersOverride, })}` }, ], @@ -660,7 +745,7 @@ export const dashboardLogic = kea([ ], textTiles: [(s) => [s.tiles], (tiles) => tiles.filter((t) => !!t.text)], itemsLoading: [ - (s) => [s.dashboardLoading, s.refreshStatus], + (s) => [s._dashboardLoading, s.refreshStatus], (dashboardLoading, refreshStatus) => { return dashboardLoading || Object.values(refreshStatus).some((s) => s.loading || s.queued) }, @@ -777,7 +862,7 @@ export const dashboardLogic = kea([ }, ], breadcrumbs: [ - (s) => [s.dashboard, s.dashboardLoading, s.dashboardFailedToLoad], + (s) => [s.dashboard, s._dashboardLoading, s.dashboardFailedToLoad], (dashboard, dashboardLoading, dashboardFailedToLoad): Breadcrumb[] => [ { key: Scene.Dashboards, @@ -822,22 +907,6 @@ export const dashboardLogic = kea([ }) }, ], - stale: [ - (s) => [s.temporaryFilters, s.dashboard], - (temporaryFilters, dashboard) => { - const isDateFromStale = - !!(temporaryFilters.date_from || dashboard?.filters.date_from) && - temporaryFilters.date_from !== dashboard?.filters.date_from - const isDateToStale = - !!(temporaryFilters.date_to || dashboard?.filters.date_to) && - temporaryFilters.date_to !== dashboard?.filters.date_to - const isPropertiesStale = - !!(temporaryFilters.properties || dashboard?.filters.properties) && - JSON.stringify(temporaryFilters.properties) !== JSON.stringify(dashboard?.filters.properties) - - return isDateFromStale || isDateToStale || isPropertiesStale - }, - ], })), events(({ actions, cache, props }) => ({ afterMount: () => { @@ -878,7 +947,7 @@ export const dashboardLogic = kea([ }, })), listeners(({ actions, values, cache, props, sharedListeners }) => ({ - updateFiltersSuccess: () => { + updateFiltersAndLayoutsSuccess: () => { actions.loadDashboard({ action: 'update' }) }, setRefreshError: sharedListeners.reportRefreshTiming, @@ -913,25 +982,6 @@ export const dashboardLogic = kea([ actions.loadDashboard({ action: 'update' }) } }, - updateLayouts: () => { - actions.saveLayouts() - }, - saveLayouts: async ({ tilesToSave }, breakpoint) => { - await breakpoint(300) - if (!isUserLoggedIn()) { - // If user is anonymous (i.e. viewing a shared dashboard logged out), we don't save any layout changes. - return - } - const layoutsToUpdate = tilesToSave.length - ? tilesToSave - : (values.dashboard?.tiles || []).map((tile) => ({ id: tile.id, layouts: tile.layouts })) - - breakpoint() - - return await api.update(`api/projects/${values.currentTeamId}/dashboards/${props.id}`, { - tiles: layoutsToUpdate, - }) - }, moveToDashboardSuccess: ({ payload }) => { if (payload?.toDashboard === undefined || payload?.tile === undefined) { return @@ -1030,15 +1080,17 @@ export const dashboardLogic = kea([ let refreshesFinished = 0 const totalResponseBytes = 0 - // array of functions that reload each item + // array of functions that reload each insight const fetchItemFunctions = insightsToRefresh.map((insight) => async () => { - const queryId = `${dashboardQueryId}::${uuid()}` + // dashboard refresh or insight refresh will have been triggered first + // so we should have a query_id to poll for + const queryId = insight?.query_status?.id const queryStartTime = performance.now() try { breakpoint() - if (insight.query_status) { - await pollForResults(insight.query_status.id, false, methodOptions) + if (queryId) { + await pollForResults(queryId, false, methodOptions) const currentTeamId = values.currentTeamId // TODO: Check and remove - We get the insight again here to get everything in the right format (e.g. because of result vs results) const polledInsight = await getSingleInsight( @@ -1047,8 +1099,14 @@ export const dashboardLogic = kea([ dashboardId, queryId, 'force_cache', - methodOptions + methodOptions, + action === 'preview' ? values.temporaryFilters : undefined ) + + if (action === 'preview' && polledInsight!.dashboard_tiles) { + // if we're previewing, only update the insight on this dashboard + polledInsight!.dashboards = [dashboardId] + } dashboardsModel.actions.updateDashboardInsight(polledInsight!) actions.setRefreshStatus(insight.short_id) } @@ -1056,9 +1114,9 @@ export const dashboardLogic = kea([ if (isBreakpoint(e)) { cancelled = true } else if (shouldCancelQuery(e)) { - if (!cancelled) { + if (!cancelled && queryId) { // cancel all insight requests for this query in one go - actions.abortQuery({ dashboardQueryId: dashboardQueryId, queryId: queryId, queryStartTime }) + actions.abortQuery({ dashboardQueryId: dashboardQueryId, queryId, queryStartTime }) } if (isAbortedRequest(e)) { cancelled = true @@ -1101,14 +1159,29 @@ export const dashboardLogic = kea([ eventUsageLogic.actions.reportDashboardRefreshed(dashboardId, values.newestRefreshed) }, - setFilters: ({ filters: { date_from, date_to } }) => { - actions.updateFilters() + setFiltersAndLayouts: ({ filters: { date_from, date_to } }) => { + actions.updateFiltersAndLayouts() eventUsageLogic.actions.reportDashboardDateRangeChanged(date_from, date_to) eventUsageLogic.actions.reportDashboardPropertiesChanged() }, setDashboardMode: async ({ mode, source }) => { if (mode === DashboardMode.Edit) { clearDOMTextSelection() + lemonToast.info('Now editing the dashboard – save to persist changes') + } else if (mode === null) { + if (source === DashboardEventSource.DashboardHeaderDiscardChanges) { + // cancel edit mode changes + + // reset filters to that before previewing + actions.setDates(values.filters.date_from ?? null, values.filters.date_to ?? null) + actions.setProperties(values.filters.properties ?? null) + + // also reset layout to that we stored in dashboardLayouts + // this is done in the reducer for dashboard + } else if (source === DashboardEventSource.DashboardHeaderSaveDashboard) { + // save edit mode changes + actions.setFiltersAndLayouts(values.temporaryFilters) + } } if (mode) { @@ -1152,7 +1225,7 @@ export const dashboardLogic = kea([ const initialLoad = action === 'initial_load' const allLoaded = false // TODO: Check this - actions.refreshAllDashboardItems({ action: 'refresh', initialLoad, dashboardQueryId }) + actions.refreshAllDashboardItems({ action, initialLoad, dashboardQueryId }) const payload: TimeToSeeDataPayload = { type: 'dashboard_load', @@ -1226,12 +1299,11 @@ export const dashboardLogic = kea([ insights_fetched_cached: 0, }) }, - applyTemporary: () => { - actions.setFilters(values.temporaryFilters) + setProperties: () => { + actions.loadDashboard({ action: 'preview' }) }, - cancelTemporary: () => { - actions.setDates(values.dashboard?.filters.date_from ?? null, values.dashboard?.filters.date_to ?? null) - actions.setProperties(values.dashboard?.filters.properties ?? null) + setDates: () => { + actions.loadDashboard({ action: 'preview' }) }, })), diff --git a/posthog/api/dashboards/dashboard.py b/posthog/api/dashboards/dashboard.py index 8462aec010f45..b0b3a8084db99 100644 --- a/posthog/api/dashboards/dashboard.py +++ b/posthog/api/dashboards/dashboard.py @@ -18,6 +18,7 @@ ) from posthog.api.forbid_destroy_model import ForbidDestroyModel from posthog.api.insight import InsightSerializer, InsightViewSet +from posthog.api.monitoring import monitor, Feature from posthog.api.routing import TeamAndOrgViewSetMixin from posthog.api.shared import UserBasicSerializer from posthog.api.tagged_item import TaggedItemSerializerMixin, TaggedItemViewSetMixin @@ -29,7 +30,7 @@ from posthog.models.tagged_item import TaggedItem from posthog.models.user import User from posthog.user_permissions import UserPermissionsSerializerMixin -from posthog.api.monitoring import monitor, Feature +from posthog.utils import filters_override_requested_by_client logger = structlog.get_logger(__name__) @@ -124,6 +125,7 @@ def get_effective_privilege_level(self, dashboard: Dashboard) -> Dashboard.Privi class DashboardSerializer(DashboardBasicSerializer): tiles = serializers.SerializerMethodField() + filters = serializers.SerializerMethodField() created_by = UserBasicSerializer(read_only=True) use_template = serializers.CharField(write_only=True, allow_blank=True, required=False) use_dashboard = serializers.IntegerField(write_only=True, allow_null=True, required=False) @@ -172,7 +174,15 @@ def create(self, validated_data: dict, *args: Any, **kwargs: Any) -> Dashboard: validated_data.pop("delete_insights", None) # not used during creation validated_data = self._update_creation_mode(validated_data, use_template, use_dashboard) tags = validated_data.pop("tags", None) # tags are created separately below as global tag relationships - dashboard = Dashboard.objects.create(team_id=team_id, **validated_data) + + request_filters = request.data.get("filters") + if request_filters: + if not isinstance(request_filters, dict): + raise serializers.ValidationError("Filters must be a dictionary") + filters = request_filters + else: + filters = {} + dashboard = Dashboard.objects.create(team_id=team_id, filters=filters, **validated_data) if use_template: try: @@ -281,6 +291,12 @@ def update(self, instance: Dashboard, validated_data: dict, *args: Any, **kwargs if validated_data.get("deleted", False): self._delete_related_tiles(instance, self.validated_data.get("delete_insights", False)) + request_filters = initial_data.get("filters") + if request_filters: + if not isinstance(request_filters, dict): + raise serializers.ValidationError("Filters must be a dictionary") + instance.filters = request_filters + instance = super().update(instance, validated_data) user = cast(User, self.context["request"].user) @@ -380,6 +396,16 @@ def get_tiles(self, dashboard: Dashboard) -> Optional[list[ReturnDict]]: return serialized_tiles + def get_filters(self, dashboard: Dashboard) -> dict: + request = self.context.get("request") + if request: + filters_override = filters_override_requested_by_client(request) + + if filters_override is not None: + return filters_override + + return dashboard.filters + def validate(self, data): if data.get("use_dashboard", None) and data.get("use_template", None): raise serializers.ValidationError("`use_dashboard` and `use_template` cannot be used together") diff --git a/posthog/api/insight.py b/posthog/api/insight.py index 0406cd6e1abce..f4ecb8e1ab49e 100644 --- a/posthog/api/insight.py +++ b/posthog/api/insight.py @@ -21,6 +21,7 @@ from rest_framework.response import Response from rest_framework.settings import api_settings from rest_framework_csv import renderers as csvrenderers +from rest_framework.request import Request from posthog import schema from posthog.api.documentation import extend_schema @@ -102,6 +103,7 @@ refresh_requested_by_client, relative_date_parse, str_to_bool, + filters_override_requested_by_client, ) from posthog.api.monitoring import monitor, Feature @@ -550,6 +552,9 @@ def to_representation(self, instance: Insight): representation["dashboards"] = [tile["dashboard_id"] for tile in representation["dashboard_tiles"]] dashboard: Optional[Dashboard] = self.context.get("dashboard") + request: Optional[Request] = self.context.get("request") + dashboard_filters_override = filters_override_requested_by_client(request) if request else None + if hogql_insights_replace_filters(instance.team) and ( instance.query is not None or instance.query_from_filters is not None ): @@ -559,12 +564,20 @@ def to_representation(self, instance: Insight): query = instance.query or instance.query_from_filters if dashboard: - query = apply_dashboard_filters_to_dict(query, dashboard.filters, instance.team) + query = apply_dashboard_filters_to_dict( + query, + dashboard_filters_override if dashboard_filters_override is not None else dashboard.filters, + instance.team, + ) representation["filters"] = {} representation["query"] = query else: - representation["filters"] = instance.dashboard_filters(dashboard=dashboard) - representation["query"] = instance.get_effective_query(dashboard=dashboard) + representation["filters"] = instance.dashboard_filters( + dashboard=dashboard, dashboard_filters_override=dashboard_filters_override + ) + representation["query"] = instance.get_effective_query( + dashboard=dashboard, dashboard_filters_override=dashboard_filters_override + ) if "insight" not in representation["filters"] and not representation["query"]: representation["filters"]["insight"] = "TRENDS" @@ -583,6 +596,7 @@ def insight_result(self, insight: Insight) -> InsightResult: try: refresh_requested = refresh_requested_by_client(self.context["request"]) execution_mode = execution_mode_from_refresh(refresh_requested) + filters_override = filters_override_requested_by_client(self.context["request"]) if self.context.get("is_shared", False): execution_mode = shared_insights_execution_mode(execution_mode) @@ -592,6 +606,7 @@ def insight_result(self, insight: Insight) -> InsightResult: dashboard=dashboard, execution_mode=execution_mode, user=self.context["request"].user, + filters_override=filters_override, ) except ExposedHogQLError as e: raise ValidationError(str(e)) diff --git a/posthog/api/test/dashboards/test_dashboard.py b/posthog/api/test/dashboards/test_dashboard.py index ae4840b60550c..a23edc4ccc9c8 100644 --- a/posthog/api/test/dashboards/test_dashboard.py +++ b/posthog/api/test/dashboards/test_dashboard.py @@ -553,7 +553,9 @@ def test_dashboard_insights_out_of_synch_with_tiles_are_not_shown(self): dashboard = Dashboard.objects.get(id=dashboard_id) mock_view = MagicMock() mock_view.action = "retrieve" - dashboard_data = DashboardSerializer(dashboard, context={"view": mock_view, "request": MagicMock()}).data + mock_request = MagicMock() + mock_request.query_params.get.return_value = None + dashboard_data = DashboardSerializer(dashboard, context={"view": mock_view, "request": mock_request}).data assert len(dashboard_data["tiles"]) == 1 def test_dashboard_insight_tiles_can_be_loaded_correct_context(self): @@ -568,7 +570,6 @@ def test_dashboard_insight_tiles_can_be_loaded_correct_context(self): response = self.dashboard_api.get_dashboard(dashboard_id) self.assertEqual(len(response["tiles"]), 1) - self.assertEqual(len(response["tiles"]), 1) tile = response["tiles"][0] assert tile["insight"]["id"] == insight_id diff --git a/posthog/api/test/test_insight.py b/posthog/api/test/test_insight.py index 9962db4023311..0ce66a4bb6182 100644 --- a/posthog/api/test/test_insight.py +++ b/posthog/api/test/test_insight.py @@ -311,6 +311,7 @@ def test_get_insight_in_shared_context(self) -> None: dashboard=mock.ANY, execution_mode=ExecutionMode.EXTENDED_CACHE_CALCULATE_ASYNC_IF_STALE, user=mock.ANY, + filters_override=None, ) with patch( @@ -322,6 +323,7 @@ def test_get_insight_in_shared_context(self) -> None: dashboard=mock.ANY, execution_mode=ExecutionMode.RECENT_CACHE_CALCULATE_BLOCKING_IF_STALE, user=mock.ANY, + filters_override=None, ) def test_get_insight_by_short_id(self) -> None: diff --git a/posthog/caching/calculate_results.py b/posthog/caching/calculate_results.py index f1b4d50fc4bc8..7da32bb9e88cd 100644 --- a/posthog/caching/calculate_results.py +++ b/posthog/caching/calculate_results.py @@ -123,7 +123,12 @@ def get_cache_type(cacheable: Optional[FilterType] | Optional[dict]) -> CacheTyp def calculate_for_query_based_insight( - insight: Insight, *, dashboard: Optional[Dashboard] = None, execution_mode: ExecutionMode, user: Optional[User] + insight: Insight, + *, + dashboard: Optional[Dashboard] = None, + execution_mode: ExecutionMode, + user: Optional[User], + filters_override: Optional[dict] = None, ) -> "InsightResult": from posthog.caching.fetch_from_cache import InsightResult, NothingInCacheResult from posthog.caching.insight_cache import update_cached_state @@ -135,7 +140,9 @@ def calculate_for_query_based_insight( response = process_response = process_query_dict( insight.team, insight.query, - dashboard_filters_json=dashboard.filters if dashboard is not None else None, + dashboard_filters_json=( + filters_override if filters_override is not None else dashboard.filters if dashboard is not None else None + ), execution_mode=execution_mode, user=user, insight_id=insight.pk, diff --git a/posthog/models/insight.py b/posthog/models/insight.py index 1b7818bbb2fb8..48d0179417006 100644 --- a/posthog/models/insight.py +++ b/posthog/models/insight.py @@ -128,10 +128,14 @@ def query_from_filters(self): except Exception as e: capture_exception(e) - def dashboard_filters(self, dashboard: Optional[Dashboard] = None): + def dashboard_filters( + self, dashboard: Optional[Dashboard] = None, dashboard_filters_override: Optional[dict] = None + ): # query date range is set in a different function, see dashboard_query if dashboard and not self.query: - dashboard_filters = {**dashboard.filters} + dashboard_filters = { + **(dashboard_filters_override if dashboard_filters_override is not None else dashboard.filters) + } dashboard_properties = dashboard_filters.pop("properties") if dashboard_filters.get("properties") else None insight_date_from = self.filters.get("date_from", None) insight_date_to = self.filters.get("date_to", None) @@ -181,13 +185,19 @@ def dashboard_filters(self, dashboard: Optional[Dashboard] = None): else: return self.filters - def get_effective_query(self, *, dashboard: Optional[Dashboard]) -> Optional[dict]: + def get_effective_query( + self, *, dashboard: Optional[Dashboard], dashboard_filters_override: Optional[dict] = None + ) -> Optional[dict]: from posthog.hogql_queries.apply_dashboard_filters import apply_dashboard_filters_to_dict if not dashboard or not self.query: return self.query - return apply_dashboard_filters_to_dict(self.query, dashboard.filters, self.team) + return apply_dashboard_filters_to_dict( + self.query, + dashboard_filters_override if dashboard_filters_override is not None else dashboard.filters, + self.team, + ) @property def url(self): diff --git a/posthog/utils.py b/posthog/utils.py index 478c149b7ddc2..39bf6d606982f 100644 --- a/posthog/utils.py +++ b/posthog/utils.py @@ -20,6 +20,7 @@ from typing import TYPE_CHECKING, Any, Optional, Union, cast from urllib.parse import unquote, urljoin, urlparse from zoneinfo import ZoneInfo +from rest_framework import serializers import lzstring import posthoganalytics @@ -68,7 +69,6 @@ DEFAULT_DATE_FROM_DAYS = 7 - logger = structlog.get_logger(__name__) # https://stackoverflow.com/questions/4060221/how-to-reliably-open-a-file-in-the-same-directory-as-a-python-script @@ -1042,6 +1042,18 @@ def cache_requested_by_client(request: Request) -> bool | str: return _request_has_key_set("use_cache", request) +def filters_override_requested_by_client(request: Request) -> Optional[dict]: + raw_filters = request.query_params.get("filters_override") + + if raw_filters is not None: + try: + return json.loads(raw_filters) + except Exception: + raise serializers.ValidationError({"filters_override": "Invalid JSON passed in filters_override parameter"}) + + return None + + def _request_has_key_set(key: str, request: Request, allowed_values: Optional[list[str]] = None) -> bool | str: query_param = request.query_params.get(key) data_value = request.data.get(key)