Skip to content

Commit

Permalink
test(e2e): Treat HogQL insights as fully rolled out (#21722)
Browse files Browse the repository at this point in the history
* test(e2e): Treat HogQL insights as fully rolled out

* Only intercept /query in `interceptInsightLoad()`

* Remove long-broken `*Premium.cy.ts` tests

* Update e2e.ts

* Don't assume that untyped properties are boolean

* Update test_property.py
  • Loading branch information
Twixes authored Apr 23, 2024
1 parent c0b3406 commit 3604a1c
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 150 deletions.
38 changes: 0 additions & 38 deletions cypress/e2e/dashboardPremium.cy.ts

This file was deleted.

6 changes: 0 additions & 6 deletions cypress/e2e/insights.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,6 @@ describe('Insights', () => {
savedInsights.checkInsightIsInListView(insightName)
})

it('Shows not found error with invalid short URL', () => {
cy.visit('/i/i_dont_exist')
cy.location('pathname').should('contain', '/insights/i_dont_exist')
cy.get('.LemonSkeleton').should('exist')
})

it('Stickiness graph', () => {
cy.get('[role=tab]').contains('Stickiness').click()
cy.get('[data-attr=add-action-event-button]').click()
Expand Down
31 changes: 0 additions & 31 deletions cypress/e2e/insightsPremium.cy.ts

This file was deleted.

6 changes: 3 additions & 3 deletions cypress/e2e/trends.cy.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import { insight, interceptInsightLoad } from '../productAnalytics'
import { insight } from '../productAnalytics'

describe('Trends', () => {
beforeEach(() => {
insight.newInsight()
})

it('Can load a graph from a URL directly', () => {
const networkInterceptAlias = interceptInsightLoad('TRENDS')
cy.intercept('POST', /api\/projects\/\d+\/query\//).as('loadNewQueryInsight')

// regression test, the graph wouldn't load when going directly to a URL
cy.visit(
'/insights/new?insight=TRENDS&interval=day&display=ActionsLineGraph&events=%5B%7B"id"%3A"%24pageview"%2C"name"%3A"%24pageview"%2C"type"%3A"events"%2C"order"%3A0%7D%5D&filter_test_accounts=false&breakdown=%24referrer&breakdown_type=event&properties=%5B%7B"key"%3A"%24current_url"%2C"value"%3A"http%3A%2F%2Fhogflix.com"%2C"operator"%3A"icontains"%2C"type"%3A"event"%7D%5D'
)

cy.wait(`@${networkInterceptAlias}`)
cy.wait(`@loadNewQueryInsight`)

cy.get('[data-attr=trend-line-graph]').should('exist')
})
Expand Down
45 changes: 4 additions & 41 deletions cypress/productAnalytics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,43 +16,6 @@ export const savedInsights = {
},
}

export function interceptInsightLoad(insightType: string): string {
cy.intercept('POST', /api\/projects\/\d+\/insights\/trend\//).as('loadNewTrendsInsight')
cy.intercept('POST', /api\/projects\/\d+\/insights\/funnel\//).as('loadNewFunnelInsight')
cy.intercept('POST', /api\/projects\/\d+\/insights\/retention\//).as('loadNewRetentionInsight')
cy.intercept('POST', /api\/projects\/\d+\/insights\/path\//).as('loadNewPathsInsight')
cy.intercept('POST', /api\/projects\/\d+\/query\//).as('loadNewQueryInsight')

let networkInterceptAlias: string = ''
switch (insightType) {
case 'TRENDS':
case 'STICKINESS':
case 'LIFECYCLE':
networkInterceptAlias = 'loadNewTrendsInsight'
break
case 'FUNNELS':
networkInterceptAlias = 'loadNewFunnelInsight'
break
case 'RETENTION':
networkInterceptAlias = 'loadNewRetentionInsight'
break
case 'PATH':
case 'PATHS':
networkInterceptAlias = 'loadNewPathsInsight'
break
case 'SQL':
case 'JSON':
networkInterceptAlias = 'loadNewQueryInsight'
break
}

if (networkInterceptAlias === '') {
throw new Error('Unknown insight type: ' + insightType)
}

return networkInterceptAlias
}

export const insight = {
applyFilter: (): void => {
cy.get('[data-attr$=add-filter-group]').click()
Expand All @@ -75,16 +38,16 @@ export const insight = {
cy.url().should('not.include', '/new')
},
clickTab: (tabName: string): void => {
const networkInterceptAlias = interceptInsightLoad(tabName)
cy.intercept('POST', /api\/projects\/\d+\/query\//).as('loadNewQueryInsight')

cy.get(`[data-attr="insight-${(tabName === 'PATHS' ? 'PATH' : tabName).toLowerCase()}-tab"]`).click()
if (tabName !== 'FUNNELS') {
// funnel insights require two steps before making an api call
cy.wait(`@${networkInterceptAlias}`)
cy.wait(`@loadNewQueryInsight`)
}
},
newInsight: (insightType: string = 'TRENDS'): void => {
const networkInterceptAlias = interceptInsightLoad(insightType)
cy.intercept('POST', /api\/projects\/\d+\/query\//).as('loadNewQueryInsight')

if (insightType === 'JSON') {
cy.clickNavMenu('savedinsights')
Expand All @@ -99,7 +62,7 @@ export const insight = {

if (insightType !== 'FUNNELS') {
// funnel insights require two steps before making an api call
cy.wait(`@${networkInterceptAlias}`)
cy.wait(`@loadNewQueryInsight`)
}
},
visitInsight: (insightName: string): void => {
Expand Down
38 changes: 12 additions & 26 deletions cypress/support/e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,9 @@ beforeEach(() => {
cy.intercept('**/decide/*', (req) =>
req.reply(
decideResponse({
// set feature flags here e.g.
// Feature flag to be treated as rolled out in E2E tests, e.g.:
// 'toolbar-launch-side-action': true,
'surveys-new-creation-flow': true,
'auto-redirect': true,
hogql: true,
'data-exploration-insights': true,
notebooks: true,
'hogql-insights-preview': true,
})
)
)
Expand All @@ -46,30 +42,20 @@ beforeEach(() => {
req.reply({ statusCode: 404, body: 'Cypress forced 404' })
)

if (Cypress.spec.name.includes('Premium')) {
cy.intercept('/api/users/@me/', { fixture: 'api/user-enterprise' })
cy.intercept('GET', /\/api\/projects\/\d+\/insights\/?\?/).as('getInsights')

cy.request('POST', '/api/login/', {
email: '[email protected]',
password: '12345678',
})
cy.request('POST', '/api/login/', {
email: '[email protected]',
password: '12345678',
})

if (Cypress.spec.name.includes('before-onboarding')) {
cy.visit('/?no-preloaded-app-context=true')
} else {
cy.intercept('GET', /\/api\/projects\/\d+\/insights\/?\?/).as('getInsights')

cy.request('POST', '/api/login/', {
email: '[email protected]',
password: '12345678',
cy.visit('/insights')
cy.wait('@getInsights').then(() => {
cy.get('.saved-insights tr').should('exist')
})

if (Cypress.spec.name.includes('before-onboarding')) {
cy.visit('/?no-preloaded-app-context=true')
} else {
cy.visit('/insights')
cy.wait('@getInsights').then(() => {
cy.get('.saved-insights tr').should('exist')
})
}
}
})

Expand Down
2 changes: 1 addition & 1 deletion posthog/hogql/property.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ def property_to_expr(
)[0:1].values_list("property_type", flat=True)
property_type = property_types[0] if property_types else None

if not property_type or property_type == PropertyType.Boolean:
if property_type == PropertyType.Boolean:
if value == "true":
value = True
if value == "false":
Expand Down
10 changes: 6 additions & 4 deletions posthog/hogql/test/test_property.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,17 +185,19 @@ def test_property_to_expr_boolean(self):
)
self.assertEqual(
self._property_to_expr(
{"type": "event", "key": "unknown_prop", "value": "true"},
{"type": "event", "key": "boolean_prop", "value": "false"},
team=self.team,
),
self._parse_expr("properties.unknown_prop = true"),
self._parse_expr("properties.boolean_prop = false"),
)
self.assertEqual(
self._property_to_expr(
{"type": "event", "key": "boolean_prop", "value": "false"},
{"type": "event", "key": "unknown_prop", "value": "true"},
team=self.team,
),
self._parse_expr("properties.boolean_prop = false"),
self._parse_expr(
"properties.unknown_prop = 'true'" # We don't have a type for unknown_prop, so string comparison it is
),
)

def test_property_to_expr_event_list(self):
Expand Down

0 comments on commit 3604a1c

Please sign in to comment.