Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(e2e): Treat HogQL insights as fully rolled out #21722

Merged
merged 6 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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:
Copy link
Member Author

@Twixes Twixes Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to change this assumption for retention.cy.ts to pass, as the legacy demo data generator used by E2E tests doesn't add a property definition for is_demo, hence the type is null – and we don't make this type assumption in the spot where we wrap the JSONExtractRaw output in bool parsing, so a type mismatch was guaranteed! @mariusandra

if property_type == PropertyType.Boolean:
if value == "true":
value = True
if value == "false":
Expand Down
Loading