Merge branch 'master' into feat/never-drop-data

.storybook/test-runner.ts

@@ -63,17 +63,22 @@ const LOADER_SELECTORS = [
 
 const customSnapshotsDir = `${process.cwd()}/frontend/__snapshots__`
 
+const TEST_TIMEOUT_MS = 10000
+const BROWSER_DEFAULT_TIMEOUT_MS = 9000 // Reduce the default timeout down from 30s, to pre-empt Jest timeouts
+const SCREENSHOT_TIMEOUT_MS = 9000
+
 module.exports = {
     setup() {
         expect.extend({ toMatchImageSnapshot })
         jest.retryTimes(RETRY_TIMES, { logErrorsBeforeRetry: true })
+        jest.setTimeout(TEST_TIMEOUT_MS)
     },
     async postRender(page, context) {
         const browserContext = page.context()
         const storyContext = (await getStoryContext(page, context)) as StoryContext
         const { skip = false, snapshotBrowsers = ['chromium'] } = storyContext.parameters?.testOptions ?? {}
 
-        browserContext.setDefaultTimeout(5000) // Reduce the default timeout from 30 s to 5 s to pre-empt Jest timeouts
+        browserContext.setDefaultTimeout(BROWSER_DEFAULT_TIMEOUT_MS)
         if (!skip) {
             const currentBrowser = browserContext.browser()!.browserType().name() as SupportedBrowserName
             if (snapshotBrowsers.includes(currentBrowser)) {
@@ -198,7 +203,7 @@ async function expectLocatorToMatchStorySnapshot(
     browser: SupportedBrowserName,
     options?: LocatorScreenshotOptions
 ): Promise<void> {
-    const image = await locator.screenshot({ timeout: 3000, ...options })
+    const image = await locator.screenshot({ timeout: SCREENSHOT_TIMEOUT_MS, ...options })
     let customSnapshotIdentifier = context.id
     if (browser !== 'chromium') {
         customSnapshotIdentifier += `--${browser}`

cypress/fixtures/api/session-recordings/recording.json

@@ -31,6 +31,5 @@
         "created_at": "2023-07-11T14:21:33.883000Z",
         "uuid": "01894554-925a-0000-11d9-a44d69b426d7"
     },
-    "storage": "clickhouse",
-    "pinned_count": 0
+    "storage": "object_storage"
 }

ee/session_recordings/test/test_session_recording_playlist.py

@@ -207,7 +207,6 @@ def test_get_pinned_recordings_for_playlist(self):
         ).json()
         assert len(result["results"]) == 2
         assert {x["id"] for x in result["results"]} == {session_one, session_two}
-        assert {x["pinned_count"] for x in result["results"]} == {1, 1}
 
     @patch("ee.session_recordings.session_recording_extensions.object_storage.list_objects")
     @patch("ee.session_recordings.session_recording_extensions.object_storage.copy_objects")
@@ -313,11 +312,9 @@ def test_add_remove_static_playlist_items(self):
 
         session_recording_obj_1 = SessionRecording.get_or_build(team=self.team, session_id=recording1_session_id)
         assert session_recording_obj_1
-        assert session_recording_obj_1.pinned_count == 1
 
         session_recording_obj_2 = SessionRecording.get_or_build(team=self.team, session_id=recording2_session_id)
         assert session_recording_obj_2
-        assert session_recording_obj_2.pinned_count == 2
 
         # Delete playlist items
         result = self.client.delete(

frontend/src/lib/api.ts

@@ -1293,14 +1293,21 @@ const api = {
     },
 
     recordings: {
-        async list(params: string): Promise<SessionRecordingsResponse> {
-            return await new ApiRequest().recordings().withQueryString(params).get()
+        async list(params: Record<string, any>): Promise<SessionRecordingsResponse> {
+            return await new ApiRequest().recordings().withQueryString(toParams(params)).get()
         },
         async getMatchingEvents(params: string): Promise<{ results: string[] }> {
             return await new ApiRequest().recordingMatchingEvents().withQueryString(params).get()
         },
-        async get(recordingId: SessionRecordingType['id'], params: string): Promise<SessionRecordingType> {
-            return await new ApiRequest().recording(recordingId).withQueryString(params).get()
+        async get(
+            recordingId: SessionRecordingType['id'],
+            params: Record<string, any> = {}
+        ): Promise<SessionRecordingType> {
+            return await new ApiRequest().recording(recordingId).withQueryString(toParams(params)).get()
+        },
+
+        async persist(recordingId: SessionRecordingType['id']): Promise<{ success: boolean }> {
+            return await new ApiRequest().recording(recordingId).withAction('persist').create()
         },
 
         async delete(recordingId: SessionRecordingType['id']): Promise<{ success: boolean }> {
@@ -1360,12 +1367,12 @@ const api = {
 
         async listPlaylistRecordings(
             playlistId: SessionRecordingPlaylistType['short_id'],
-            params: string
+            params: Record<string, any> = {}
         ): Promise<SessionRecordingsResponse> {
             return await new ApiRequest()
                 .recordingPlaylist(playlistId)
                 .withAction('recordings')
-                .withQueryString(params)
+                .withQueryString(toParams(params))
                 .get()
         },
 

frontend/src/lib/components/Cards/InsightCard/InsightCard.tsx

@@ -45,13 +45,14 @@ import {
 import { Resizeable } from 'lib/components/Cards/CardMeta'
 import { Query } from '~/queries/Query/Query'
 import { QueriesUnsupportedHere } from 'lib/components/Cards/InsightCard/QueriesUnsupportedHere'
-import { QueryContext } from '~/queries/schema'
+import { InsightQueryNode, QueryContext } from '~/queries/schema'
 import { InsightMeta } from './InsightMeta'
 import { dataNodeLogic, DataNodeLogicProps } from '~/queries/nodes/DataNode/dataNodeLogic'
 import { filtersToQueryNode } from '~/queries/nodes/InsightQuery/utils/filtersToQueryNode'
 import { insightVizDataNodeKey } from '~/queries/nodes/InsightViz/InsightViz'
 import { getCachedResults } from '~/queries/nodes/InsightViz/utils'
 import { funnelDataLogic } from 'scenes/funnels/funnelDataLogic'
+import { insightDataLogic } from 'scenes/insights/insightDataLogic'
 
 type DisplayedType = ChartDisplayType | 'RetentionContainer' | 'FunnelContainer' | 'PathsContainer'
 
@@ -186,7 +187,7 @@ export function FilterBasedCardContent({
 }: FilterBasedCardContentProps): JSX.Element {
     const displayedType = getDisplayedType(insight.filters)
     const VizComponent = displayMap[displayedType]?.element || VizComponentFallback
-    const query = filtersToQueryNode(insight.filters)
+    const query: InsightQueryNode = filtersToQueryNode(insight.filters)
     const dataNodeLogicProps: DataNodeLogicProps = {
         query,
         key: insightVizDataNodeKey(insightProps),
@@ -271,10 +272,10 @@ function InsightCardInternal(
         dashboardItemId: insight.short_id,
         dashboardId: dashboardId,
         cachedInsight: insight,
-        doNotLoad: true,
     }
 
     const { insightLoading } = useValues(insightLogic(insightLogicProps))
+    const { insightDataLoading } = useValues(insightDataLogic(insightLogicProps))
     const { isFunnelWithEnoughSteps, hasFunnelResults, areExclusionFiltersValid } = useValues(
         funnelDataLogic(insightLogicProps)
     )
@@ -292,7 +293,7 @@ function InsightCardInternal(
             empty = true
         }
     }
-    if (insightLoading) {
+    if (insightLoading || insightDataLoading) {
         loading = true
     }
 
@@ -351,7 +352,7 @@ function InsightCardInternal(
                             <QueriesUnsupportedHere />
                         )}
                     </div>
-                ) : (
+                ) : insight.filters?.insight ? (
                     <FilterBasedCardContent
                         insight={insight}
                         insightProps={insightLogicProps}
@@ -370,6 +371,13 @@ function InsightCardInternal(
                         }
                         setAreDetailsShown={setAreDetailsShown}
                     />
+                ) : (
+                    <div className="flex justify-between items-center h-full">
+                        <InsightErrorState
+                            excludeDetail
+                            title="Missing 'filters.insight' property, can't display insight"
+                        />
+                    </div>
                 )}
             </BindLogic>
             {showResizeHandles && (

frontend/src/lib/components/PropertyIcon.tsx

@@ -20,7 +20,7 @@ import {
 import clsx from 'clsx'
 import { Tooltip } from 'lib/lemon-ui/Tooltip'
 import { countryCodeToFlag } from 'scenes/insights/views/WorldMap'
-import { ReactNode } from 'react'
+import { HTMLAttributes, ReactNode } from 'react'
 
 export const PROPERTIES_ICON_MAP = {
     $browser: {
@@ -61,7 +61,7 @@ interface PropertyIconProps {
     value?: string
     className?: string
     noTooltip?: boolean
-    onClick?: (property: string, value?: string) => void
+    onClick?: HTMLAttributes<HTMLDivElement>['onClick']
     tooltipTitle?: (property: string, value?: string) => ReactNode // Tooltip title will default to `value`
 }
 
@@ -87,15 +87,7 @@ export function PropertyIcon({
     }
 
     const content = (
-        <div
-            onClick={(e) => {
-                if (onClick) {
-                    e.stopPropagation()
-                    onClick(property, value)
-                }
-            }}
-            className={clsx('inline-flex items-center', className)}
-        >
+        <div onClick={onClick} className={clsx('inline-flex items-center', className)}>
             {icon}
         </div>
     )

frontend/src/lib/components/TZLabel/index.tsx

@@ -7,13 +7,13 @@ import { teamLogic } from '../../../scenes/teamLogic'
 import { dayjs } from 'lib/dayjs'
 import clsx from 'clsx'
 import React, { useCallback, useEffect, useMemo, useState } from 'react'
-import { LemonButton, LemonDivider, LemonDropdown } from '@posthog/lemon-ui'
+import { LemonButton, LemonDivider, LemonDropdown, LemonDropdownProps } from '@posthog/lemon-ui'
 import { IconSettings } from 'lib/lemon-ui/icons'
 import { urls } from 'scenes/urls'
 
 const BASE_OUTPUT_FORMAT = 'ddd, MMM D, YYYY h:mm A'
 
-interface TZLabelRawProps {
+export type TZLabelProps = Omit<LemonDropdownProps, 'overlay' | 'trigger' | 'children'> & {
     time: string | dayjs.Dayjs
     showSeconds?: boolean
     formatDate?: string
@@ -26,7 +26,7 @@ interface TZLabelRawProps {
 const TZLabelPopoverContent = React.memo(function TZLabelPopoverContent({
     showSeconds,
     time,
-}: Pick<TZLabelRawProps, 'showSeconds'> & { time: dayjs.Dayjs }): JSX.Element {
+}: Pick<TZLabelProps, 'showSeconds'> & { time: dayjs.Dayjs }): JSX.Element {
     const DATE_OUTPUT_FORMAT = !showSeconds ? BASE_OUTPUT_FORMAT : `${BASE_OUTPUT_FORMAT}:ss`
     const { currentTeam } = useValues(teamLogic)
     const { reportTimezoneComponentViewed } = useActions(eventUsageLogic)
@@ -86,7 +86,8 @@ function TZLabelRaw({
     showPopover = true,
     noStyles = false,
     className,
-}: TZLabelRawProps): JSX.Element {
+    ...dropdownProps
+}: TZLabelProps): JSX.Element {
     const parsedTime = useMemo(() => (dayjs.isDayjs(time) ? time : dayjs(time)), [time])
 
     const format = useCallback(() => {
@@ -120,9 +121,10 @@ function TZLabelRaw({
     if (showPopover) {
         return (
             <LemonDropdown
-                trigger="hover"
                 placement="top"
                 showArrow
+                {...dropdownProps}
+                trigger="hover"
                 overlay={<TZLabelPopoverContent time={parsedTime} showSeconds={showSeconds} />}
             >
                 {innerContent}

frontend/src/lib/lemon-ui/LemonWidget/LemonWidget.tsx

@@ -1,47 +1,26 @@
-import { useState } from 'react'
 import { LemonButton } from '../LemonButton'
-import { IconClose, IconUnfoldLess, IconUnfoldMore } from '../icons'
+import { IconClose } from '../icons'
 import './LemonWidget.scss'
 import clsx from 'clsx'
 
 export interface LemonWidgetProps {
     title: string
-    collapsible?: boolean
     onClose?: () => void
     actions?: React.ReactNode
-    children: React.ReactChild
+    children: React.ReactNode
+    className?: string
 }
 
-export function LemonWidget({ title, collapsible = true, onClose, actions, children }: LemonWidgetProps): JSX.Element {
-    const [isExpanded, setIsExpanded] = useState<boolean>(true)
-
+export function LemonWidget({ title, onClose, actions, children, className }: LemonWidgetProps): JSX.Element {
     return (
-        <Widget>
+        <Widget className={className}>
             <Header>
-                {collapsible ? (
-                    <>
-                        <LemonButton
-                            onClick={() => setIsExpanded(!isExpanded)}
-                            size="small"
-                            status="primary-alt"
-                            className="flex-1"
-                        >
-                            <span className="flex-1 cursor-pointer">{title}</span>
-                        </LemonButton>
-                        <LemonButton
-                            onClick={() => setIsExpanded(!isExpanded)}
-                            size="small"
-                            icon={isExpanded ? <IconUnfoldLess /> : <IconUnfoldMore />}
-                        />
-                    </>
-                ) : (
-                    <span className="flex-1 text-primary-alt px-2">{title}</span>
-                )}
+                <span className="flex-1 text-primary-alt px-2">{title}</span>
                 {actions}
 
                 {onClose && <LemonButton status="danger" onClick={onClose} size="small" icon={<IconClose />} />}
             </Header>
-            {isExpanded && <Content>{children}</Content>}
+            <Content>{children}</Content>
         </Widget>
     )
 }
@@ -55,5 +34,5 @@ const Header = ({ children, className }: { children: React.ReactNode; className?
 }
 
 const Content = ({ children }: { children: React.ReactNode }): JSX.Element => {
-    return <div className="border-t border-border">{children}</div>
+    return <div className="LemonWidget__content border-t border-border">{children}</div>
 }

frontend/src/lib/utils.tsx

@@ -432,31 +432,36 @@ export function objectClean<T extends Record<string | number | symbol, unknown>>
     })
     return response
 }
-export function objectCleanWithEmpty<T extends Record<string | number | symbol, unknown>>(obj: T): T {
+export function objectCleanWithEmpty<T extends Record<string | number | symbol, unknown>>(
+    obj: T,
+    ignoredKeys: string[] = []
+): T {
     const response = { ...obj }
-    Object.keys(response).forEach((key) => {
-        // remove undefined values
-        if (response[key] === undefined) {
-            delete response[key]
-        }
-        // remove empty arrays i.e. []
-        if (
-            typeof response[key] === 'object' &&
-            Array.isArray(response[key]) &&
-            (response[key] as unknown[]).length === 0
-        ) {
-            delete response[key]
-        }
-        // remove empty objects i.e. {}
-        if (
-            typeof response[key] === 'object' &&
-            !Array.isArray(response[key]) &&
-            response[key] !== null &&
-            Object.keys(response[key] as Record<string | number | symbol, unknown>).length === 0
-        ) {
-            delete response[key]
-        }
-    })
+    Object.keys(response)
+        .filter((key) => !ignoredKeys.includes(key))
+        .forEach((key) => {
+            // remove undefined values
+            if (response[key] === undefined) {
+                delete response[key]
+            }
+            // remove empty arrays i.e. []
+            if (
+                typeof response[key] === 'object' &&
+                Array.isArray(response[key]) &&
+                (response[key] as unknown[]).length === 0
+            ) {
+                delete response[key]
+            }
+            // remove empty objects i.e. {}
+            if (
+                typeof response[key] === 'object' &&
+                !Array.isArray(response[key]) &&
+                response[key] !== null &&
+                Object.keys(response[key] as Record<string | number | symbol, unknown>).length === 0
+            ) {
+                delete response[key]
+            }
+        })
     return response
 }
 

frontend/src/queries/nodes/DataTable/DataTableExport.tsx

@@ -212,7 +212,7 @@ export function DataTableExport({ query }: DataTableExportProps): JSX.Element |
                             ? [
                                   <ExportWithConfirmation
                                       key={0}
-                                      placement={'bottomRight'}
+                                      placement={'topRight'}
                                       onConfirm={() => startDownload(query, false)}
                                       actor={isPersonsNode(query.source) ? 'persons' : 'events'}
                                       limit={EXPORT_MAX_LIMIT}

frontend/src/queries/nodes/DataTable/ExportWithConfirmation.tsx

@@ -18,6 +18,7 @@ export function ExportWithConfirmation({
 }: ExportWithConfirmationProps): JSX.Element {
     return (
         <Popconfirm
+            className="ant-popconfirm"
             placement={placement}
             title={
                 <>