Skip to content

Commit

Permalink
feat(alerts): Alerts for trends with breakdowns (#26249)
Browse files Browse the repository at this point in the history
  • Loading branch information
anirudhpillai authored Nov 22, 2024
1 parent a73e03d commit e80e678
Show file tree
Hide file tree
Showing 11 changed files with 1,034 additions and 86 deletions.
23 changes: 22 additions & 1 deletion cypress/e2e/alerts.cy.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { decideResponse } from '../fixtures/api/decide'
import { createInsight } from '../productAnalytics'
import { createInsight, createInsightWithBreakdown } from '../productAnalytics'

describe('Alerts', () => {
beforeEach(() => {
Expand Down Expand Up @@ -115,4 +115,25 @@ describe('Alerts', () => {
cy.reload()
cy.contains('Alert name').should('not.exist')
})

it('Should allow creating alerts on trends with breakdowns', () => {
createInsightWithBreakdown('insight with breakdown')
setInsightDisplayTypeAndSave('Bar chart')

createAlert('Alert name', '10', '20', 'increases by')
cy.reload()

// Check the alert has the same values as when it was created
cy.contains('Alerts').click()
cy.get('[data-attr=alert-list-item]').contains('Alert name').click()
cy.contains('any breakdown value').should('exist')
cy.get('[data-attr=alertForm-name]').should('have.value', 'Alert name')
cy.get('[data-attr=alertForm-lower-threshold').should('have.value', '10')
cy.get('[data-attr=alertForm-upper-threshold').should('have.value', '20')
cy.contains('Delete alert').click()
cy.wait(2000)

cy.reload()
cy.contains('Alert name').should('not.exist')
})
})
16 changes: 16 additions & 0 deletions cypress/productAnalytics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ export const insight = {
}
})
},
applyBreakdown: (): void => {
cy.contains('Add breakdown').click()
cy.contains('Browser').click()
cy.wait(1000)
},
editName: (insightName: string): void => {
if (insightName) {
cy.get('[data-attr="top-bar-name"] button').click()
Expand Down Expand Up @@ -207,6 +212,17 @@ export function createInsight(insightName: string): Cypress.Chainable<string> {
})
}

export function createInsightWithBreakdown(insightName: string): Cypress.Chainable<string> {
savedInsights.createNewInsightOfType('TRENDS')
insight.applyBreakdown()
insight.editName(insightName)
insight.save()
// return insight id from the url
return cy.url().then((url) => {
return url.split('/').at(-1)
})
}

export function duplicateDashboardFromMenu(duplicateTiles = false): void {
cy.contains('.LemonButton', 'Duplicate').click()
if (duplicateTiles) {
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lib/components/Alerts/AlertsButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function AlertsButton({ insight, insightLogicProps, text, ...props }: Ale
onClick={() => push(urls.insightAlerts(insight.short_id!))}
disabledReason={
!areAlertsSupportedForInsight(insight.query)
? 'Alerts are only available for trends without breakdowns. Change the insight representation to add alerts.'
? 'Alerts are only available for trends. Change the insight representation to add alerts.'
: undefined
}
{...props}
Expand Down
10 changes: 2 additions & 8 deletions frontend/src/lib/components/Alerts/insightAlertsLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import api from 'lib/api'
import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic'

import { AlertConditionType, GoalLine, InsightThresholdType } from '~/queries/schema'
import { getBreakdown, isInsightVizNode, isTrendsQuery } from '~/queries/utils'
import { isInsightVizNode, isTrendsQuery } from '~/queries/utils'
import { InsightLogicProps } from '~/types'

import type { insightAlertsLogicType } from './insightAlertsLogicType'
Expand All @@ -16,13 +16,7 @@ export interface InsightAlertsLogicProps {
}

export const areAlertsSupportedForInsight = (query?: Record<string, any> | null): boolean => {
return (
!!query &&
isInsightVizNode(query) &&
isTrendsQuery(query.source) &&
query.source.trendsFilter !== null &&
!getBreakdown(query.source)
)
return !!query && isInsightVizNode(query) && isTrendsQuery(query.source) && query.source.trendsFilter !== null
}

export const insightAlertsLogic = kea<insightAlertsLogicType>([
Expand Down
26 changes: 23 additions & 3 deletions frontend/src/lib/components/Alerts/views/EditAlertModal.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { LemonCheckbox, LemonInput, LemonSegmentedButton, LemonSelect, SpinnerOverlay } from '@posthog/lemon-ui'
import {
LemonBanner,
LemonCheckbox,
LemonInput,
LemonSegmentedButton,
LemonSelect,
SpinnerOverlay,
} from '@posthog/lemon-ui'
import { useActions, useValues } from 'kea'
import { Form, Group } from 'kea-forms'
import { AlertStateIndicator } from 'lib/components/Alerts/views/ManageAlertsModal'
Expand Down Expand Up @@ -85,7 +92,7 @@ export function EditAlertModal({
const { setAlertFormValue } = useActions(formLogic)

const trendsLogic = trendsDataLogic({ dashboardItemId: insightShortId })
const { alertSeries, isNonTimeSeriesDisplay } = useValues(trendsLogic)
const { alertSeries, isNonTimeSeriesDisplay, isBreakdownValid } = useValues(trendsLogic)

const creatingNewAlert = alertForm.id === undefined

Expand Down Expand Up @@ -138,6 +145,12 @@ export function EditAlertModal({
<div className="space-y-6">
<h3>Definition</h3>
<div className="space-y-5">
{isBreakdownValid && (
<LemonBanner type="warning">
For trends with breakdown, the alert will fire if any of the breakdown
values breaches the threshold.
</LemonBanner>
)}
<div className="flex gap-4 items-center">
<div>When</div>
<Group name={['config']}>
Expand All @@ -146,9 +159,16 @@ export function EditAlertModal({
fullWidth
data-attr="alertForm-series-index"
options={alertSeries?.map(({ event }, index) => ({
label: `${alphabet[index]} - ${event}`,
label: isBreakdownValid
? 'any breakdown value'
: `${alphabet[index]} - ${event}`,
value: index,
}))}
disabledReason={
isBreakdownValid &&
`For trends with breakdown, the alert will fire if any of the breakdown
values breaches the threshold.`
}
/>
</LemonField>
</Group>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { IconCheck, IconX } from '@posthog/icons'
import { Link } from '@posthog/lemon-ui'
import { useActions, useValues } from 'kea'
import { router } from 'kea-router'
import { LemonButton } from 'lib/lemon-ui/LemonButton'
Expand Down Expand Up @@ -85,8 +86,12 @@ export function ManageAlertsModal(props: ManageAlertsModalProps): JSX.Element {
<div className="mb-4">
With alerts, PostHog will monitor your insight and notify you when certain conditions are met. We do
not evaluate alerts in real-time, but rather on a schedule (hourly, daily...). Please note that
alerts are in alpha and may not be fully reliable.
alerts are in alpha and may not be fully reliable. <br />
<Link to={urls.alerts()} target="_blank">
View all your alerts here
</Link>
</div>

{alerts.length ? (
<div className="space-y-2">
<div>
Expand Down
16 changes: 15 additions & 1 deletion frontend/src/scenes/trends/trendsDataLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,16 @@ import {
BREAKDOWN_OTHER_STRING_LABEL,
} from 'scenes/insights/utils'

import { EventsNode, InsightQueryNode, LifecycleQuery, MathType, TrendsFilter, TrendsQuery } from '~/queries/schema'
import {
BreakdownFilter,
EventsNode,
InsightQueryNode,
LifecycleQuery,
MathType,
TrendsFilter,
TrendsQuery,
} from '~/queries/schema'
import { isValidBreakdown } from '~/queries/utils'
import {
ChartDisplayType,
CountPerActorMathType,
Expand Down Expand Up @@ -124,6 +133,11 @@ export const trendsDataLogic = kea<trendsDataLogicType>([
},
],

isBreakdownValid: [
(s) => [s.breakdownFilter],
(breakdownFilter: BreakdownFilter | null) => isValidBreakdown(breakdownFilter),
],

indexedResults: [
(s) => [s.results, s.display, s.lifecycleFilter],
(results, display, lifecycleFilter): IndexedTrendResult[] => {
Expand Down
9 changes: 6 additions & 3 deletions posthog/tasks/alerts/test/test_alert_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,10 @@ def test_alert_is_triggered_for_values_above_higher_threshold(

anomalies_descriptions = self.get_breach_description(mock_send_notifications_for_breaches, call_index=0)
assert len(anomalies_descriptions) == 1
assert "The insight value for previous day (1) is more than upper threshold (0.0)" in anomalies_descriptions[0]
assert (
"The insight value ($pageview) for previous day (1) is more than upper threshold (0.0)"
in anomalies_descriptions[0]
)

def test_alert_is_not_triggered_for_events_beyond_interval(
self, mock_send_notifications_for_breaches: MagicMock, mock_send_errors: MagicMock
Expand Down Expand Up @@ -127,7 +130,7 @@ def test_alert_is_triggered_for_value_below_lower_threshold(

assert mock_send_notifications_for_breaches.call_count == 1
anomalies = self.get_breach_description(mock_send_notifications_for_breaches, call_index=0)
assert "The insight value for previous day (0) is less than lower threshold (1.0)" in anomalies
assert "The insight value ($pageview) for previous day (0) is less than lower threshold (1.0)" in anomalies

def test_alert_triggers_but_does_not_send_notification_during_firing(
self, mock_send_notifications_for_breaches: MagicMock, mock_send_errors: MagicMock
Expand Down Expand Up @@ -315,7 +318,7 @@ def test_alert_with_insight_with_filter(

assert mock_send_notifications_for_breaches.call_count == 1
anomalies = self.get_breach_description(mock_send_notifications_for_breaches, call_index=0)
assert "The insight value for previous day (0) is less than lower threshold (1.0)" in anomalies
assert "The insight value ($pageview) for previous day (0) is less than lower threshold (1.0)" in anomalies

@patch("posthog.tasks.alerts.utils.EmailMessage")
def test_send_emails(
Expand Down
Loading

0 comments on commit e80e678

Please sign in to comment.