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

Insight URL cleanup #7201

Merged
merged 46 commits into from
Nov 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
1e46c5a
insight route refactor, part 1
mariusandra Nov 18, 2021
0923905
add fromItem to get redirects
mariusandra Nov 18, 2021
e628ea1
fix some tests
mariusandra Nov 18, 2021
09e1b1e
adjust many more paths
mariusandra Nov 18, 2021
6ea8afb
fix test
mariusandra Nov 18, 2021
84d9a91
move new insight creation into insight logic
mariusandra Nov 18, 2021
02608d8
fix a noisy test
mariusandra Nov 18, 2021
6c20554
simplify one test
mariusandra Nov 18, 2021
f32128b
open the url with the right filters
mariusandra Nov 18, 2021
c9407e4
null fix
mariusandra Nov 18, 2021
9bccaf0
fix some more noisy tests
mariusandra Nov 18, 2021
ca63825
move saved insights to `/insights`, fix logic tests
mariusandra Nov 18, 2021
ebaa346
fix cypress urls
mariusandra Nov 18, 2021
a294b40
fix some tests
mariusandra Nov 18, 2021
0f6c0a7
fix even more insight urls
mariusandra Nov 18, 2021
85110b6
wait a bit longer
mariusandra Nov 18, 2021
2a47ae0
add old saved_insights redirect
mariusandra Nov 18, 2021
c9965f9
Merge branch 'master' into insight-urls-one-more-time
mariusandra Nov 19, 2021
e5db7e3
this might not be there yet
mariusandra Nov 19, 2021
e6be27c
Merge branch 'master' into insight-urls-one-more-time
Twixes Nov 19, 2021
b7296ca
rename newInsight -> insightNew
mariusandra Nov 19, 2021
b384c37
rename Scene.Insights -> Scene.Insight
mariusandra Nov 19, 2021
be6d211
also redirect old searches without fromItem
mariusandra Nov 19, 2021
23bbbfc
Merge branch 'master' into insight-urls-one-more-time
mariusandra Nov 19, 2021
1040fb2
fix link
mariusandra Nov 19, 2021
237ca61
Merge branch 'master' into insight-urls-one-more-time
mariusandra Nov 22, 2021
080b035
Merge branch 'master' into insight-urls-one-more-time
mariusandra Nov 22, 2021
b2fbfc8
fix TS merge bugs
mariusandra Nov 22, 2021
c33f9aa
fix import
mariusandra Nov 22, 2021
94749b3
fix imports
mariusandra Nov 22, 2021
76b5d91
fix tests
mariusandra Nov 22, 2021
46bc51b
fix test
mariusandra Nov 22, 2021
817aa6f
Merge branch 'master' into insight-urls-one-more-time
Twixes Nov 22, 2021
4bae9b5
Merge branch 'master' into insight-urls-one-more-time
Twixes Nov 22, 2021
e26802d
Run prettier
Twixes Nov 22, 2021
51dbde4
Merge branch 'master' into insight-urls-one-more-time
mariusandra Nov 23, 2021
f00c491
Merge branch 'master' into insight-urls-one-more-time
mariusandra Nov 23, 2021
d47b11b
fix changes after merge
mariusandra Nov 23, 2021
fb304f0
Merge branch 'master' into insight-urls-one-more-time
mariusandra Nov 24, 2021
c3ebe1f
switch to a simpler scene
mariusandra Nov 24, 2021
7bcdfe1
fix another test
mariusandra Nov 24, 2021
1b8c8dd
Merge branch 'master' into insight-urls-one-more-time
mariusandra Nov 24, 2021
e73b3c1
fix "save as" reset
mariusandra Nov 25, 2021
a6d210b
Merge branch 'master' into insight-urls-one-more-time
mariusandra Nov 25, 2021
7ae3ced
rerun tests
mariusandra Nov 25, 2021
638592c
Insight Short URLs (#7259)
mariusandra Nov 25, 2021
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
2 changes: 1 addition & 1 deletion cypress/integration/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('Actions', () => {
createAction(actionName)

// Test the action is immediately available
cy.clickNavMenu('insights')
cy.clickNavMenu('insight')

cy.contains('Add graph series').click()
cy.get('[data-attr=trend-element-subject-1]').click()
Expand Down
2 changes: 1 addition & 1 deletion cypress/integration/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('Auth', () => {
cy.location('pathname').should('include', '/login')

cy.visit(
'/insights?insight=TRENDS&interval=day&display=ActionsLineGraph&actions=%5B%5D&events=%5B%7B"id"%3A"%24pageview"%2C"name"%3A"%24pageview"%2C"type"%3A"events"%2C"order"%3A0%7D%2C%7B"id"%3A"%24autocapture"%2C"name"%3A"%24autocapture"%2C"type"%3A"events"%2C"order"%3A1%7D%5D&properties=%5B%5D&filter_test_accounts=false&new_entity=%5B%5D'
'/insights/new?insight=TRENDS&interval=day&display=ActionsLineGraph&actions=%5B%5D&events=%5B%7B"id"%3A"%24pageview"%2C"name"%3A"%24pageview"%2C"type"%3A"events"%2C"order"%3A0%7D%2C%7B"id"%3A"%24autocapture"%2C"name"%3A"%24autocapture"%2C"type"%3A"events"%2C"order"%3A1%7D%5D&properties=%5B%5D&filter_test_accounts=false&new_entity=%5B%5D'
)
cy.location('pathname').should('include', '/login') // Should be redirected to login because we're now logged out

Expand Down
8 changes: 5 additions & 3 deletions cypress/integration/insights.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { urls } from 'scenes/urls'

// For tests related to trends please check trendsElements.js
describe('Insights', () => {
beforeEach(() => {
cy.visit('/insights')
cy.visit(urls.insightNew())
})

it('Opens insight with short URL', () => {
cy.visit('/i/TEST1234') // Insight `TEST1234` is created in demo data (revenue_data_generator.py)
cy.location('pathname').should('eq', '/insights') // User is taken to the insights page
cy.location('pathname').should('eq', '/insights/TEST1234') // User is taken to the insights page
cy.get('[data-attr=insight-edit-button]').click()
cy.get('[data-attr=trend-element-subject-0]').contains('Entered Free Trial').should('exist') // Funnel is properly loaded
cy.get('[data-attr=trend-element-subject-1]').contains('Purchase').should('exist')
Expand Down Expand Up @@ -74,7 +76,7 @@ describe('Insights', () => {
cy.visit('/events') // Test that default params are set correctly even if the app doesn't start on insights
cy.reload()

cy.clickNavMenu('insights')
cy.clickNavMenu('insight')
cy.get('[data-attr=trend-element-subject-0] span').should('contain', 'Pageview')
cy.get('[data-attr=trend-line-graph]').should('exist')
cy.contains('Add graph series').click()
Expand Down
2 changes: 1 addition & 1 deletion cypress/integration/insightsPremium.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
describe('Insights Premium Features', () => {
beforeEach(() => {
cy.clickNavMenu('insights')
cy.clickNavMenu('insight')
cy.location('pathname').should('include', '/insights')
})

Expand Down
4 changes: 3 additions & 1 deletion cypress/integration/paths.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { urls } from 'scenes/urls'

describe('Paths', () => {
beforeEach(() => {
cy.visit('/insights/new')
cy.visit(urls.insightNew())
cy.get('[data-attr=insight-path-tab]').click()
})

Expand Down
2 changes: 1 addition & 1 deletion cypress/integration/person.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe('Merge person', () => {
cy.contains('Merge persons').click()

cy.contains('Automatically load new events').click()
cy.contains('$create_alias').should('exist')
cy.contains('$create_alias', { timeout: 20000 }).should('exist')
cy.get('span:contains(Pageview)').should('have.length', 2)
cy.get('span:contains(clicked)').should('have.length', 2)
})
Expand Down
4 changes: 3 additions & 1 deletion cypress/integration/retention.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { urls } from 'scenes/urls'

describe('Retention', () => {
beforeEach(() => {
cy.visit('/insights/new')
cy.visit(urls.insightNew())
cy.get('[data-attr=insight-retention-tab]').click()
})

Expand Down
7 changes: 5 additions & 2 deletions cypress/integration/trends.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { urls } from 'scenes/urls'

describe('Trends', () => {
beforeEach(() => {
cy.visit('/insights')
cy.visit(urls.insightNew())
cy.location('pathname').should('include', '/edit')
})

it('Can load a graph from a URL directly', () => {
// regression test, the graph wouldn't load when going directly to a URL
cy.visit(
'/insights?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'
'/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.get('[data-attr=trend-line-graph]').should('exist')
Expand Down
4 changes: 3 additions & 1 deletion cypress/integration/trendsSessions.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { urls } from 'scenes/urls'

describe('Trends sessions', () => {
beforeEach(() => {
// given
cy.visit('/insights')
cy.visit(urls.insightNew())
cy.get('[id="rc-tabs-0-tab-SESSIONS"]').click()
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export const breadcrumbsLogic = kea<breadcrumbsLogicType<Breadcrumb>>({
here: true,
})
break
case Scene.Insights:
case Scene.Insight:
breadcrumbs.push({
name: 'Insights',
path: urls.savedInsights(),
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/layout/navigation/SideBar/SideBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,9 @@ function Pages(): JSX.Element {
to={urls.savedInsights()}
sideAction={{
icon: <IconPlus />,
to: urls.newInsight(InsightType.TRENDS),
to: urls.insightNew({ insight: InsightType.TRENDS }),
tooltip: 'New insight',
identifier: Scene.Insights,
identifier: Scene.Insight,
}}
/>
<PageButton icon={<IconRecording />} identifier={Scene.SessionRecordings} to={urls.sessionRecordings()} />
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/lib/api.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,11 @@ export function defaultAPIMocks(
} else if (
[
`api/projects/${MOCK_TEAM_ID}/actions/`,
`api/projects/${MOCK_TEAM_ID}/annotations/`,
`api/projects/${MOCK_TEAM_ID}/event_definitions/`,
`api/projects/${MOCK_TEAM_ID}/dashboards/`,
`api/projects/${MOCK_TEAM_ID}/dashboards`,
`api/projects/${MOCK_TEAM_ID}/groups/`,
`api/projects/${MOCK_TEAM_ID}/insights/`,
`api/projects/${MOCK_TEAM_ID}/annotations/`,
'api/projects/@current/event_definitions/',
Expand Down
16 changes: 6 additions & 10 deletions frontend/src/lib/components/Annotations/AnnotationMarker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import { dashboardColors } from 'lib/colors'
import { eventUsageLogic } from 'lib/utils/eventUsageLogic'
import { Tooltip } from 'lib/components/Tooltip'
import { AnnotationScope, AnnotationType } from '~/types'
import { styles } from '../../../vars'
import { teamLogic } from '../../../scenes/teamLogic'
import { organizationLogic } from '../../../scenes/organizationLogic'
import { styles } from '~/vars'
import { teamLogic } from 'scenes/teamLogic'
import { organizationLogic } from 'scenes/organizationLogic'
import { dayjs } from 'lib/dayjs'

const { TextArea } = Input
Expand Down Expand Up @@ -44,7 +44,7 @@ interface AnnotationMarkerProps {
size?: number
color: string | null
accessoryColor: string | null
dashboardItemId?: number
insightId?: number
currentDateMarker: string
dynamic?: boolean
graphColor: string | null
Expand All @@ -64,7 +64,7 @@ export function AnnotationMarker({
size = 25,
color,
accessoryColor,
dashboardItemId,
insightId,
currentDateMarker,
onClose,
dynamic,
Expand Down Expand Up @@ -96,11 +96,7 @@ export function AnnotationMarker({
const { currentTeam } = useValues(teamLogic)
const { currentOrganization } = useValues(organizationLogic)

const { diffType, groupedAnnotations } = useValues(
annotationsLogic({
pageKey: dashboardItemId ? dashboardItemId : null,
})
)
const { diffType, groupedAnnotations } = useValues(annotationsLogic({ insightId: insightId }))

function closePopup(): void {
setFocused(false)
Expand Down
20 changes: 6 additions & 14 deletions frontend/src/lib/components/Annotations/Annotations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ interface AnnotationsProps {
leftExtent: number
interval: number
topExtent: number
dashboardItemId?: number
insightId?: number
color: string | null
graphColor: string
accessoryColor: string | null
Expand All @@ -24,26 +24,18 @@ export function Annotations({
leftExtent,
interval,
topExtent,
dashboardItemId,
insightId,
onClick,
color,
accessoryColor,
onClose,
graphColor,
currentDateMarker,
}: AnnotationsProps): JSX.Element[] {
const { diffType, groupedAnnotations } = useValues(
annotationsLogic({
pageKey: dashboardItemId ? dashboardItemId : null,
})
)
const { diffType, groupedAnnotations } = useValues(annotationsLogic({ insightId }))

const { createAnnotation, createAnnotationNow, deleteAnnotation, deleteGlobalAnnotation, createGlobalAnnotation } =
useActions(
annotationsLogic({
pageKey: dashboardItemId ? dashboardItemId : null,
})
)
useActions(annotationsLogic({ insightId }))

const markers: JSX.Element[] = []

Expand All @@ -57,8 +49,8 @@ export function Annotations({
annotations={annotationsToMark}
onCreate={(input: string, applyAll: boolean) => {
if (applyAll) {
createGlobalAnnotation(input, date, dashboardItemId)
} else if (dashboardItemId) {
createGlobalAnnotation(input, date, insightId)
} else if (insightId) {
createAnnotationNow(input, date)
} else {
createAnnotation(input, date)
Expand Down
12 changes: 6 additions & 6 deletions frontend/src/lib/components/Annotations/annotationsLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ import { annotationsModel } from '~/models/annotationsModel'
import { getNextKey } from './utils'
import { annotationsLogicType } from './annotationsLogicType'
import { AnnotationScope, AnnotationType } from '~/types'
import { teamLogic } from '../../../scenes/teamLogic'
import { teamLogic } from 'scenes/teamLogic'

interface AnnotationsLogicProps {
pageKey?: string | number | null
insightId?: number
}

export const annotationsLogic = kea<annotationsLogicType<AnnotationsLogicProps>>({
path: (key) => ['lib', 'components', 'Annotations', 'annotationsLogic', key],
props: {} as AnnotationsLogicProps,
key: (props) => (props.pageKey ? `${props.pageKey}_annotations` : 'annotations_default'),
key: (props) => String(props.insightId || 'default'),
connect: {
actions: [annotationsModel, ['deleteGlobalAnnotation', 'createGlobalAnnotation']],
values: [annotationsModel, ['activeGlobalAnnotations']],
Expand Down Expand Up @@ -51,7 +51,7 @@ export const annotationsLogic = kea<annotationsLogicType<AnnotationsLogicProps>>
__default: [] as AnnotationType[],
loadAnnotations: async () => {
const params = {
...(props.pageKey ? { dashboardItemId: props.pageKey } : {}),
...(props.insightId ? { dashboardItemId: props.insightId } : {}),
scope: AnnotationScope.DashboardItem,
deleted: false,
}
Expand Down Expand Up @@ -139,7 +139,7 @@ export const annotationsLogic = kea<annotationsLogicType<AnnotationsLogicProps>>
content,
date_marker: dayjs(date_marker),
created_at,
dashboard_item: props.pageKey,
dashboard_item: props.insightId,
scope,
})
actions.loadAnnotations()
Expand All @@ -157,6 +157,6 @@ export const annotationsLogic = kea<annotationsLogicType<AnnotationsLogicProps>>
},
}),
events: ({ actions, props }) => ({
afterMount: () => props.pageKey && actions.loadAnnotations(),
afterMount: () => props.insightId && actions.loadAnnotations(),
}),
})
Original file line number Diff line number Diff line change
Expand Up @@ -422,47 +422,47 @@ export const commandPaletteLogic = kea<
icon: RiseOutlined,
display: 'Go to Insights',
executor: () => {
push(urls.insights())
push(urls.savedInsights())
},
},
{
icon: RiseOutlined,
display: 'Go to Trends',
executor: () => {
// TODO: Don't reset insight on change
push(urls.newInsight(InsightType.TRENDS))
push(urls.insightNew({ insight: InsightType.TRENDS }))
},
},
{
icon: ClockCircleOutlined,
display: 'Go to Sessions',
executor: () => {
// TODO: Don't reset insight on change
push(urls.newInsight(InsightType.SESSIONS))
push(urls.insightNew({ insight: InsightType.SESSIONS }))
},
},
{
icon: FunnelPlotOutlined,
display: 'Go to Funnels',
executor: () => {
// TODO: Don't reset insight on change
push(urls.newInsight(InsightType.FUNNELS))
push(urls.insightNew({ insight: InsightType.FUNNELS }))
},
},
{
icon: GatewayOutlined,
display: 'Go to Retention',
executor: () => {
// TODO: Don't reset insight on change
push(urls.newInsight(InsightType.RETENTION))
push(urls.insightNew({ insight: InsightType.RETENTION }))
},
},
{
icon: InteractionOutlined,
display: 'Go to Paths',
executor: () => {
// TODO: Don't reset insight on change
push(urls.newInsight(InsightType.PATHS))
push(urls.insightNew({ insight: InsightType.PATHS }))
},
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { useValues } from 'kea'
import { LinkButton } from '../LinkButton'
import { urls } from '../../../scenes/urls'
import { Tooltip } from '../Tooltip'
import { combineUrl } from 'kea-router'

interface Props {
insight: Partial<DashboardItemType>
Expand All @@ -24,7 +25,7 @@ export function SaveToDashboard({ insight }: Props): JSX.Element {
{dashboard ? (
<Tooltip title={`Go to dashboard "${dashboard?.name}"`} placement="bottom">
<LinkButton
to={`${urls.dashboard(dashboard.id)}?highlightInsightId=${insight.id}`}
to={combineUrl(urls.dashboard(dashboard.id), { highlightInsightId: insight.short_id }).url}
type="default"
style={{ color: 'var(--primary)' }}
icon={<CheckSquareOutlined />}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ interface SaveToDashboardModalProps {
}

export function SaveToDashboardModal({ visible, closeModal, insight }: SaveToDashboardModalProps): JSX.Element {
const logic = saveToDashboardModalLogic({ id: insight.id, fromDashboard: insight.dashboard || undefined })
const logic = saveToDashboardModalLogic({ id: insight.short_id, fromDashboard: insight.dashboard || undefined })
const { nameSortedDashboards } = useValues(dashboardsModel)
const { dashboardId } = useValues(logic)
const { addNewDashboard, setDashboardId } = useActions(logic)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { saveToDashboardModalLogicType } from './saveToDashboardModalLogicType'
export const saveToDashboardModalLogic = kea<saveToDashboardModalLogicType>({
path: (key) => ['lib', 'components', 'SaveToDashboard', 'saveToDashboardModalLogic', key],
props: {} as {
id?: number
id?: string
fromDashboard?: number
},
key: ({ id }) => id || 'none',
Expand Down
Loading