Skip to content

Commit

Permalink
Merge branch 'master' into feat/heatmaps-opt-in
Browse files Browse the repository at this point in the history
  • Loading branch information
benjackwhite authored Apr 24, 2024
2 parents 3aaee8b + b4cea12 commit b6b3da9
Show file tree
Hide file tree
Showing 41 changed files with 682 additions and 304 deletions.
1 change: 1 addition & 0 deletions .github/workflows/ci-backend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ jobs:
echo running_time_run_id=${run_id} >> $GITHUB_ENV
echo running_time_run_started_at=${run_started_at} >> $GITHUB_ENV
- name: Capture running time to PostHog
if: github.repository == 'PostHog/posthog'
uses: PostHog/posthog-github-action@v0.1
with:
posthog-token: ${{secrets.POSTHOG_API_TOKEN}}
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/ci-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ jobs:
echo running_time_run_id=${run_id} >> $GITHUB_ENV
echo running_time_run_started_at=${run_started_at} >> $GITHUB_ENV
- name: Capture running time to PostHog
if: github.repository == 'PostHog/posthog'
uses: PostHog/posthog-github-action@v0.1
with:
posthog-token: ${{secrets.POSTHOG_API_TOKEN}}
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/report-pr-age.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ jobs:
echo is_revert=false >> $GITHUB_ENV
fi
- name: Capture PR age to PostHog
if: github.repository == 'PostHog/posthog'
uses: PostHog/posthog-github-action@v0.1
with:
posthog-token: ${{secrets.POSTHOG_API_TOKEN}}
Expand Down
34 changes: 34 additions & 0 deletions cypress/e2e/dashboard.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,40 @@ describe('Dashboard', () => {
cy.get('[data-attr^="breadcrumb-Dashboard:"]').should('have.text', TEST_DASHBOARD_NAME + 'UnnamedCancelSave')
})

const assertVariablesConfigurationScreenIsShown = (): void => {
cy.get('[data-attr="new-dashboard-chooser"]').contains('Unique variable name').should('exist')
}

it('Allow reselecting a dashboard after pressing back', () => {
cy.intercept('GET', /\/api\/projects\/\d+\/dashboard_templates/, (req) => {
req.reply((response) => {
response.body.results[0].variables = [
{
id: 'id',
name: 'Unique variable name',
type: 'event',
default: {},
required: true,
description: 'description',
},
]
return response
})
})

// Request templates again.
cy.clickNavMenu('dashboards')

cy.get('[data-attr="new-dashboard"]').click()
cy.get('[data-attr="create-dashboard-from-template"]').click()
assertVariablesConfigurationScreenIsShown()

cy.contains('.LemonButton', 'Back').click()

cy.get('[data-attr="create-dashboard-from-template"]').click()
assertVariablesConfigurationScreenIsShown()
})

it('Click on a dashboard item dropdown and view graph', () => {
cy.get('[data-attr=dashboard-name]').contains('Web Analytics').click()
cy.get('.InsightCard [data-attr=more-button]').first().click()
Expand Down
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.
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ ShowTime.args = {
showTime: true,
}

export const FromToday: Story = BasicTemplate.bind({})
FromToday.args = {
fromToday: true,
export const OnlyAllowUpcoming: Story = BasicTemplate.bind({})
OnlyAllowUpcoming.args = {
onlyAllowUpcoming: true,
}
4 changes: 2 additions & 2 deletions frontend/src/lib/lemon-ui/LemonCalendar/LemonCalendar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export interface LemonCalendarProps {
/** Show a time picker */
showTime?: boolean
/** Only allow upcoming dates */
fromToday?: boolean
onlyAllowUpcoming?: boolean
}

export interface GetLemonButtonPropsOpts {
Expand Down Expand Up @@ -137,7 +137,7 @@ export const LemonCalendar = forwardRef(function LemonCalendar(
LemonCalendar__today: date.isSame(today, 'd'),
}),
disabledReason:
props.fromToday && pastDate
props.onlyAllowUpcoming && pastDate
? 'Cannot select dates in the past'
: undefined,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,66 @@ describe('LemonCalendarSelect', () => {
// only changes the hour
expect(onChange).toHaveBeenCalledWith(dayjs('2023-01-15T08:42:00.000Z'))
})

test('onlyAllowUpcoming', async () => {
const onClose = jest.fn()
const onChange = jest.fn()
window.HTMLElement.prototype.scrollIntoView = jest.fn()

jest.useFakeTimers().setSystemTime(new Date('2023-01-10 17:22:08'))

function TestSelect(): JSX.Element {
const [value, setValue] = useState<dayjs.Dayjs | null>(null)
return (
<LemonCalendarSelect
months={1}
value={value}
onClose={onClose}
onChange={(value) => {
setValue(value)
onChange(value)
}}
showTime
onlyAllowUpcoming
/>
)
}
const { container } = render(<TestSelect />)

async function clickOnDate(day: string): Promise<void> {
const element = container.querySelector('.LemonCalendar__month') as HTMLElement
if (element) {
userEvent.click(await within(element).findByText(day))
userEvent.click(getByDataAttr(container, 'lemon-calendar-select-apply'))
}
}

async function clickOnTime(props: GetLemonButtonTimePropsOpts): Promise<void> {
const element = getTimeElement(container.querySelector('.LemonCalendar__time'), props)
if (element) {
userEvent.click(element)
userEvent.click(getByDataAttr(container, 'lemon-calendar-select-apply'))
}
}

// click on minute
await clickOnTime({ unit: 'm', value: 42 })
// time is disabled until a date is clicked
expect(onChange).not.toHaveBeenCalled()

// click on current date
await clickOnDate('9')
// cannot select a date in the past
expect(onChange).not.toHaveBeenCalled()

// click on current date
await clickOnDate('10')
// chooses the current date and sets the time to the current hour and minute
expect(onChange).toHaveBeenCalledWith(dayjs('2023-01-10T17:22:00.000Z'))

// click on an earlier hour
await clickOnTime({ unit: 'a', value: 'am' })
// does not update the date because it is in the past
expect(onChange).lastCalledWith(dayjs('2023-01-10T17:22:00.000Z'))
})
})
64 changes: 45 additions & 19 deletions frontend/src/lib/lemon-ui/LemonCalendar/LemonCalendarSelect.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { IconX } from '@posthog/icons'
import { dayjs } from 'lib/dayjs'
import { LemonButton, LemonButtonWithSideActionProps, SideAction } from 'lib/lemon-ui/LemonButton'
import { LemonButton, LemonButtonProps, LemonButtonWithSideActionProps, SideAction } from 'lib/lemon-ui/LemonButton'
import { GetLemonButtonTimePropsOpts, LemonCalendar } from 'lib/lemon-ui/LemonCalendar/LemonCalendar'
import { useEffect, useMemo, useRef, useState } from 'react'

Expand Down Expand Up @@ -28,13 +28,27 @@ function scrollToTimeElement(
})
}

function proposedDate(target: dayjs.Dayjs | null, { value, unit }: GetLemonButtonTimePropsOpts): dayjs.Dayjs {
let date = target || dayjs().startOf('day')
if (value != date.format(unit)) {
if (unit === 'h') {
date = date.hour(date.format('a') === 'am' || value === 12 ? Number(value) : Number(value) + 12)
} else if (unit === 'm') {
date = date.minute(Number(value))
} else if (unit === 'a') {
date = value === 'am' ? date.subtract(12, 'hour') : date.add(12, 'hour')
}
}
return date
}

export interface LemonCalendarSelectProps {
value?: dayjs.Dayjs | null
onChange: (date: dayjs.Dayjs) => void
months?: number
onClose?: () => void
showTime?: boolean
fromToday?: boolean
onlyAllowUpcoming?: boolean
}

export function LemonCalendarSelect({
Expand All @@ -43,13 +57,14 @@ export function LemonCalendarSelect({
months,
onClose,
showTime,
fromToday,
onlyAllowUpcoming,
}: LemonCalendarSelectProps): JSX.Element {
const calendarRef = useRef<HTMLDivElement | null>(null)
const [selectValue, setSelectValue] = useState<dayjs.Dayjs | null>(
value ? (showTime ? value : value.startOf('day')) : null
)

const now = dayjs()
const isAM = useMemo(() => selectValue?.format('a') === 'am', [selectValue])

const scrollToTime = (date: dayjs.Dayjs, skipAnimation: boolean): void => {
Expand All @@ -62,8 +77,6 @@ export function LemonCalendarSelect({
}

const onDateClick = (date: dayjs.Dayjs | null): void => {
const now = dayjs()

if (date) {
date = showTime ? date.hour(selectValue === null ? now.hour() : selectValue.hour()) : date.startOf('hour')
date = showTime
Expand All @@ -82,17 +95,7 @@ export function LemonCalendarSelect({
}, [])

const onTimeClick = (props: GetLemonButtonTimePropsOpts): void => {
const { value, unit } = props

let date = selectValue || dayjs().startOf('day')
if (unit === 'h') {
date = date.hour(date.format('a') === 'am' ? Number(value) : Number(value) + 12)
} else if (unit === 'm') {
date = date.minute(Number(value))
} else if (unit === 'a') {
date = value === 'am' ? date.subtract(12, 'hour') : date.add(12, 'hour')
}

const date = proposedDate(selectValue, props)
scrollToTime(date, false)
setSelectValue(date)
}
Expand All @@ -111,17 +114,40 @@ export function LemonCalendarSelect({
leftmostMonth={selectValue?.startOf('month')}
months={months}
getLemonButtonProps={({ date, props }) => {
const modifiedProps: LemonButtonProps = { ...props }
const isDisabled =
onlyAllowUpcoming &&
selectValue &&
date.isSame(now.tz('utc'), 'date') &&
(selectValue.hour() < now.hour() ||
(selectValue.hour() === now.hour() && selectValue.minute() <= now.minute()))

if (isDisabled) {
modifiedProps.disabledReason = 'Pick a time in the future first'
}

if (date.isSame(selectValue, 'd')) {
return { ...props, status: 'default', type: 'primary' }
return { ...modifiedProps, status: 'default', type: 'primary' }
}
return props
return modifiedProps
}}
getLemonButtonTimeProps={(props) => {
const selected = selectValue ? selectValue.format(props.unit) : null
const newDate = proposedDate(selectValue, props)

const disabledReason = onlyAllowUpcoming
? selectValue
? newDate.isBefore(now)
? 'Cannot choose a time in the past'
: undefined
: 'Choose a date first'
: undefined

return {
active: selected === String(props.value),
className: 'rounded-none',
'data-attr': timeDataAttr(props),
disabledReason: disabledReason,
onClick: () => {
if (selected != props.value) {
onTimeClick(props)
Expand All @@ -130,7 +156,7 @@ export function LemonCalendarSelect({
}
}}
showTime={showTime}
fromToday={fromToday}
onlyAllowUpcoming={onlyAllowUpcoming}
/>
<div className="flex space-x-2 justify-end items-center border-t p-2 pt-4">
<LemonButton type="secondary" onClick={onClose} data-attr="lemon-calendar-select-cancel">
Expand Down
1 change: 1 addition & 0 deletions frontend/src/scenes/dashboard/NewDashboardModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export function NewDashboardModal(): JSX.Element {
onClose={hideNewDashboardModal}
isOpen={newDashboardModalVisible}
title={activeDashboardTemplate ? 'Choose your events' : 'Create a dashboard'}
data-attr="new-dashboard-chooser"
description={
activeDashboardTemplate ? (
<p>
Expand Down
1 change: 1 addition & 0 deletions frontend/src/scenes/dashboard/newDashboardLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ export const newDashboardLogic = kea<newDashboardLogicType>([
hideNewDashboardModal: () => false,
submitNewDashboardSuccess: () => false,
submitNewDashboardFailure: () => false,
clearActiveDashboardTemplate: () => false,
},
],
newDashboardModalVisible: [
Expand Down
7 changes: 6 additions & 1 deletion frontend/src/scenes/experiments/ExperimentView/Goal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ export function ExposureMetric({ experimentId }: { experimentId: Experiment['id'
}

export function ExperimentGoalModal({ experimentId }: { experimentId: Experiment['id'] }): JSX.Element {
const { experiment, isExperimentGoalModalOpen, experimentLoading } = useValues(experimentLogic({ experimentId }))
const { experiment, isExperimentGoalModalOpen, experimentLoading, goalInsightDataLoading } = useValues(
experimentLogic({ experimentId })
)
const { closeExperimentGoalModal, updateExperimentGoal, setNewExperimentInsight } = useActions(
experimentLogic({ experimentId })
)
Expand All @@ -108,6 +110,9 @@ export function ExperimentGoalModal({ experimentId }: { experimentId: Experiment
Cancel
</LemonButton>
<LemonButton
disabledReason={
goalInsightDataLoading && 'The insight needs to be loaded before saving the goal'
}
form="edit-experiment-goal-form"
onClick={() => {
updateExperimentGoal(experiment.filters)
Expand Down
24 changes: 21 additions & 3 deletions frontend/src/scenes/experiments/experimentLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ export const experimentLogic = kea<experimentLogicType>([
['conversionMetrics'],
trendsDataLogic({ dashboardItemId: EXPERIMENT_INSIGHT_ID }),
['results as trendResults'],
insightDataLogic({ dashboardItemId: EXPERIMENT_INSIGHT_ID }),
['insightDataLoading as goalInsightDataLoading'],
],
actions: [
experimentsLogic,
Expand Down Expand Up @@ -309,7 +311,7 @@ export const experimentLogic = kea<experimentLogicType>([
// Terminate if the insight did not manage to load in time
if (!minimumDetectableChange) {
eventUsageLogic.actions.reportExperimentInsightLoadFailed()
lemonToast.error(
return lemonToast.error(
'Failed to load insight. Experiment cannot be saved without this value. Try changing the experiment goal.'
)
}
Expand Down Expand Up @@ -481,10 +483,26 @@ export const experimentLogic = kea<experimentLogicType>([
values.experiment && actions.reportExperimentArchived(values.experiment)
},
updateExperimentGoal: async ({ filters }) => {
// We never want to update global properties in the experiment
const { recommendedRunningTime, recommendedSampleSize, minimumDetectableChange } = values
if (!minimumDetectableChange) {
eventUsageLogic.actions.reportExperimentInsightLoadFailed()
return lemonToast.error(
'Failed to load insight. Experiment cannot be saved without this value. Try changing the experiment goal.'
)
}

const filtersToUpdate = { ...filters }
delete filtersToUpdate.properties
actions.updateExperiment({ filters: filtersToUpdate })

actions.updateExperiment({
filters: filtersToUpdate,
parameters: {
...values.experiment?.parameters,
recommended_running_time: recommendedRunningTime,
recommended_sample_size: recommendedSampleSize,
minimum_detectable_effect: minimumDetectableChange,
},
})
actions.closeExperimentGoalModal()
},
updateExperimentExposure: async ({ filters }) => {
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/feature-flags/FeatureFlagSchedule.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ export default function FeatureFlagSchedule(): JSX.Element {
value={scheduleDateMarker}
onChange={(value) => setScheduleDateMarker(value)}
placeholder="Select date"
onlyAllowUpcoming
showTime
fromToday
/>
</div>
</div>
Expand Down
Loading

0 comments on commit b6b3da9

Please sign in to comment.