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

feat(frontend): implement project-based routing #13474

Merged
merged 52 commits into from
Jan 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
864b814
feat routs
mariusandra Dec 23, 2022
a4e24d9
transform path in actions
mariusandra Dec 23, 2022
23ed95e
pnpm
mariusandra Apr 3, 2023
17fab78
exclude /me, /instance, /organization
mariusandra Apr 3, 2023
404f4de
make it all work better
mariusandra Apr 3, 2023
68f02af
Update UI snapshots for `chromium` (2)
github-actions[bot] Apr 3, 2023
a02fe3b
Update UI snapshots for `webkit` (2)
github-actions[bot] Apr 3, 2023
2417cbc
Update UI snapshots for `chromium` (2)
github-actions[bot] Apr 3, 2023
3c4bf81
Update UI snapshots for `webkit` (2)
github-actions[bot] Apr 3, 2023
d2dc7a7
Update UI snapshots for `webkit` (2)
github-actions[bot] Apr 3, 2023
a4d8d8e
Update UI snapshots for `webkit` (2)
github-actions[bot] Apr 3, 2023
2b28588
Merge branch 'master' into kea-router-projects
mariusandra Jun 19, 2023
3ba1d0f
Merge branch 'master' into kea-router-projects
mariusandra Aug 7, 2023
e13c5aa
pnpm
mariusandra Aug 7, 2023
b93fd09
frontend updates
mariusandra Aug 7, 2023
5efac89
update team from url
mariusandra Aug 7, 2023
350a8bd
fixes
mariusandra Aug 7, 2023
8bc52cd
tests
mariusandra Aug 7, 2023
e77d213
mypy
mariusandra Aug 7, 2023
8608566
Update UI snapshots for `chromium` (2)
github-actions[bot] Aug 7, 2023
991f1eb
Merge branch 'master' into kea-router-projects
mariusandra Aug 7, 2023
1eacf51
Merge branch 'kea-router-projects' of github.com:PostHog/posthog into…
mariusandra Aug 7, 2023
802bf0d
Update UI snapshots for `chromium` (2)
github-actions[bot] Aug 7, 2023
9729550
Merge master
benjackwhite Jan 3, 2024
f11a0fd
Update UI snapshots for `chromium` (1)
github-actions[bot] Jan 3, 2024
2333cbb
Fixed api routes
benjackwhite Jan 4, 2024
82c5288
Fixes
benjackwhite Jan 4, 2024
ce321f4
Update query snapshots
github-actions[bot] Jan 4, 2024
4fd446e
Update query snapshots
github-actions[bot] Jan 4, 2024
809599f
Fixed some more issues around switching back
benjackwhite Jan 4, 2024
ad14a5f
Update query snapshots
github-actions[bot] Jan 4, 2024
4bb7b52
Simplify path checker
benjackwhite Jan 4, 2024
34f84b6
Merge branch 'kea-router-projects' of github.com:PostHog/posthog into…
benjackwhite Jan 4, 2024
508e446
Update query snapshots
github-actions[bot] Jan 4, 2024
7de8f8b
Fix
benjackwhite Jan 4, 2024
9b011a8
Moved to utils
benjackwhite Jan 4, 2024
8f50e4f
Fix issues with circular imports
benjackwhite Jan 4, 2024
c73aba9
Fix test
benjackwhite Jan 4, 2024
5bf7460
More fixes
benjackwhite Jan 4, 2024
11e0ffa
Fix
benjackwhite Jan 4, 2024
7cd6cf1
Update UI snapshots for `chromium` (1)
github-actions[bot] Jan 4, 2024
b2d9d58
Fix project switcher url
benjackwhite Jan 4, 2024
d527946
Merge branch 'kea-router-projects' of github.com:PostHog/posthog into…
benjackwhite Jan 4, 2024
4e5e53c
Update UI snapshots for `chromium` (1)
github-actions[bot] Jan 4, 2024
dd6d5c7
Merge branch 'kea-router-projects' of github.com:PostHog/posthog into…
benjackwhite Jan 4, 2024
ddbba41
Fix tests
benjackwhite Jan 4, 2024
decc3ac
Fixes
benjackwhite Jan 4, 2024
22a30a5
Fix
benjackwhite Jan 4, 2024
05cbfaf
Update query snapshots
github-actions[bot] Jan 4, 2024
516633f
Update query snapshots
github-actions[bot] Jan 4, 2024
56129a0
Fixed middleware for missing team
benjackwhite Jan 8, 2024
4534531
Merge branch 'master' into kea-router-projects
benjackwhite Jan 15, 2024
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
8 changes: 3 additions & 5 deletions cypress/e2e/auth.cy.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { urls } from 'scenes/urls'

describe('Auth', () => {
beforeEach(() => {
cy.get('[data-attr=menu-item-me]').click()
Expand All @@ -20,7 +18,7 @@ describe('Auth', () => {

cy.get('[type=submit]').click()
// Login should have succeeded
cy.location('pathname').should('eq', '/home')
cy.location('pathname').should('eq', '/')
})

it('Logout and verify that Google login button has correct link', () => {
Expand Down Expand Up @@ -48,7 +46,7 @@ describe('Auth', () => {
cy.get('[data-attr=password]').clear().type('12345678')
cy.get('[type=submit]').click()
// Login should have succeeded
cy.location('pathname').should('eq', '/home')
cy.location('pathname').should('eq', '/')
})

it('Redirect to appropriate place after login', () => {
Expand Down Expand Up @@ -84,6 +82,6 @@ describe('Auth', () => {

it('Cannot access signup page if authenticated', () => {
cy.visit('/signup')
cy.location('pathname').should('eq', urls.projectHomepage())
cy.location('pathname').should('eq', '/project/1')
})
})
2 changes: 1 addition & 1 deletion cypress/e2e/insights.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ describe('Insights', () => {

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

Expand Down
2 changes: 1 addition & 1 deletion cypress/e2e/invites.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe('Invite Signup', () => {
cy.get('[data-attr=menu-item-me]').click()
cy.get('[data-attr=top-menu-item-org-settings]').click()

cy.location('pathname').should('eq', '/settings/organization')
cy.location('pathname').should('contain', '/settings/organization')
cy.get('[id="invites"]').should('exist')
cy.contains('Pending Invites').should('exist')

Expand Down
2 changes: 1 addition & 1 deletion cypress/e2e/person.cy.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
describe('Person Visualization Check', () => {
beforeEach(() => {
cy.clickNavMenu('personsmanagement')
cy.location('pathname').should('eq', '/persons')
cy.location('pathname').should('contain', '/persons')
cy.wait(1000)
cy.get('[data-attr=persons-search]').type('deb').should('have.value', 'deb')
cy.contains('[email protected]').should('not.exist')
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.
10 changes: 10 additions & 0 deletions frontend/src/initKea.ts
benjackwhite marked this conversation as resolved.
Show resolved Hide resolved
benjackwhite marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { waitForPlugin } from 'kea-waitfor'
import { windowValuesPlugin } from 'kea-window-values'
import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast'
import { identifierToHuman } from 'lib/utils'
import { addProjectIdIfMissing, removeProjectIdIfPresent } from 'lib/utils/router-utils'

/*
Actions for which we don't want to show error alerts,
Expand Down Expand Up @@ -74,6 +75,15 @@ export function initKea({ routerHistory, routerLocation, beforePlugins }: InitKe
// in "/url/:key". Default: "a-zA-Z0-9-_~ %".
segmentValueCharset: "a-zA-Z0-9-_~ %.@()!'|",
},
pathFromRoutesToWindow: (path) => {
return addProjectIdIfMissing(path)
},
transformPathInActions: (path) => {
return addProjectIdIfMissing(path)
},
pathFromWindowToRoutes: (path) => {
benjackwhite marked this conversation as resolved.
Show resolved Hide resolved
return removeProjectIdIfPresent(path)
},
}),
formsPlugin,
loadersPlugin({
Expand Down
6 changes: 2 additions & 4 deletions frontend/src/layout/navigation/ProjectNotice.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ interface ProjectNoticeBlueprint {
export function ProjectNotice(): JSX.Element | null {
const { projectNoticeVariantWithClosability } = useValues(navigationLogic)
const { currentOrganization } = useValues(organizationLogic)
const { updateCurrentTeam, logout } = useActions(userLogic)
const { logout } = useActions(userLogic)
const { user } = useValues(userLogic)
const { closeProjectNotice } = useActions(navigationLogic)
const { showInviteModal } = useActions(inviteLogic)
Expand All @@ -46,9 +46,7 @@ export function ProjectNotice(): JSX.Element | null {
{' '}
When you're ready, head on over to the{' '}
<Link
onClick={() => {
updateCurrentTeam(altTeamForIngestion?.id, urls.products())
}}
to={urls.project(altTeamForIngestion.id, urls.products())}
data-attr="demo-project-alt-team-ingestion_link"
>
onboarding wizard
Expand Down
26 changes: 15 additions & 11 deletions frontend/src/layout/navigation/ProjectSwitcher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import { IconPlus, IconSettings } from 'lib/lemon-ui/icons'
import { LemonButton } from 'lib/lemon-ui/LemonButton'
import { LemonDivider } from 'lib/lemon-ui/LemonDivider'
import { LemonSnack } from 'lib/lemon-ui/LemonSnack/LemonSnack'
import { removeProjectIdIfPresent } from 'lib/utils/router-utils'
import { useMemo } from 'react'
import { organizationLogic } from 'scenes/organizationLogic'
import { sceneLogic } from 'scenes/sceneLogic'
import { isAuthenticatedTeam, teamLogic } from 'scenes/teamLogic'
import { urls } from 'scenes/urls'
import { userLogic } from 'scenes/userLogic'

import { AvailableFeature, TeamBasicType } from '~/types'

Expand Down Expand Up @@ -86,22 +87,25 @@ function CurrentProjectButton({ onClickInside }: { onClickInside?: () => void })
) : null
}

function OtherProjectButton({ team, onClickInside }: { team: TeamBasicType; onClickInside?: () => void }): JSX.Element {
const { updateCurrentTeam } = useActions(userLogic)
function OtherProjectButton({ team }: { team: TeamBasicType; onClickInside?: () => void }): JSX.Element {
const { location } = useValues(router)

const relativeOtherProjectPath = useMemo(() => {
// NOTE: There is a tradeoff here - because we choose keep the whole path it could be that the
// project switch lands on something like insight/abc that won't exist.
// On the other hand, if we remove the ID, it could be that someone opens a page, realizes they're in the wrong project
// and after switching is on a different page than before.
const route = removeProjectIdIfPresent(location.pathname)
return urls.project(team.id, route)
}, [location.pathname])

return (
<LemonButton
onClick={() => {
onClickInside?.()
updateCurrentTeam(team.id, '/')
}}
to={relativeOtherProjectPath}
sideAction={{
icon: <IconSettings className="text-muted-alt" />,
tooltip: `Go to ${team.name} settings`,
onClick: () => {
onClickInside?.()
updateCurrentTeam(team.id, '/settings')
},
to: urls.project(team.id, urls.settings()),
}}
title={`Switch to project ${team.name}`}
fullWidth
Expand Down
20 changes: 15 additions & 5 deletions frontend/src/lib/lemon-ui/Link/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import './Link.scss'
import clsx from 'clsx'
import { router } from 'kea-router'
import { isExternalLink } from 'lib/utils'
import { getCurrentTeamId } from 'lib/utils/getAppContext'
import { addProjectIdIfMissing } from 'lib/utils/router-utils'
import React from 'react'
import { useNotebookDrag } from 'scenes/notebooks/AddToNotebook/DraggableToNotebook'

Expand Down Expand Up @@ -36,14 +38,15 @@ export type LinkProps = Pick<React.HTMLProps<HTMLAnchorElement>, 'target' | 'cla
subtle?: boolean
}

// Some URLs we want to enforce a full reload such as billing which is redirected by Django
const FORCE_PAGE_LOAD = ['/billing/']

const shouldForcePageLoad = (input: any): boolean => {
if (!input || typeof input !== 'string') {
return false
}
return !!FORCE_PAGE_LOAD.find((x) => input.startsWith(x))

// If the link is to a different team, force a page load to ensure the proper team switch happens
const matches = input.match(/\/project\/(\d+)/)

return !!matches && matches[1] !== `${getCurrentTeamId()}`
}

const isPostHogDomain = (url: string): boolean => {
Expand Down Expand Up @@ -120,14 +123,21 @@ export const Link: React.FC<LinkProps & React.RefAttributes<HTMLElement>> = Reac
}

const rel = typeof to === 'string' && isPostHogDomain(to) ? 'noopener' : 'noopener noreferrer'
const href = to
? typeof to === 'string'
? to.includes('://')
? to
: addProjectIdIfMissing(to)
: '#'
: undefined

return to ? (
// eslint-disable-next-line react/forbid-elements
<a
ref={ref as any}
className={clsx('Link', subtle && 'Link--subtle', className)}
onClick={onClick}
href={typeof to === 'string' ? to : '#'}
href={href}
target={target}
rel={target === '_blank' ? rel : undefined}
{...props}
Expand Down
22 changes: 21 additions & 1 deletion frontend/src/lib/utils/getAppContext.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AppContext, PathType } from '~/types'
import { AppContext, OrganizationType, PathType, TeamType } from '~/types'

declare global {
export interface Window {
Expand All @@ -15,3 +15,23 @@ export function getAppContext(): AppContext | undefined {
export function getDefaultEventName(): string {
return getAppContext()?.default_event_name || PathType.PageView
}

// NOTE: Any changes to the teamId trigger a full page load so we don't use the logic
// This helps avoid circular imports
export function getCurrentTeamId(): TeamType['id'] {
const maybeTeamId = getAppContext()?.current_team?.id
if (!maybeTeamId) {
throw new Error(`Project ID is not known.${getAppContext()?.anonymous ? ' User is anonymous.' : ''}`)
}
return maybeTeamId
}

// NOTE: Any changes to the organizationId trigger a full page load so we don't use the logic
// This helps avoid circular imports
export function getCurrentOrganizationId(): OrganizationType['id'] {
const maybeOrgId = getAppContext()?.current_team?.organization
if (!maybeOrgId) {
throw new Error(`Organization ID is not known.${getAppContext()?.anonymous ? ' User is anonymous.' : ''}`)
}
return maybeOrgId
}
22 changes: 0 additions & 22 deletions frontend/src/lib/utils/logics.ts

This file was deleted.

39 changes: 39 additions & 0 deletions frontend/src/lib/utils/router-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { getCurrentTeamId } from './getAppContext'

const pathsWithoutProjectId = ['api', 'me', 'instance', 'organization', 'preflight', 'login', 'signup']

function isPathWithoutProjectId(path: string): boolean {
const firstPart = path.split('/')[1]
return pathsWithoutProjectId.includes(firstPart)
}

function addProjectIdUnlessPresent(path: string): string {
if (path.match(/^\/project\/\d+/)) {
return path
}

let prefix = ''
try {
prefix = `/project/${getCurrentTeamId()}`
if (path == '/') {
return prefix
}
} catch (e) {
// Not logged in
}
if (path === prefix || path.startsWith(prefix + '/')) {
return path
}
return `${prefix}/${path.startsWith('/') ? path.slice(1) : path}`
}

export function removeProjectIdIfPresent(path: string): string {
if (path.match(/^\/project\/\d+/)) {
return '/' + path.split('/').splice(3).join('/')
}
return path
}

export function addProjectIdIfMissing(path: string): string {
return isPathWithoutProjectId(path) ? removeProjectIdIfPresent(path) : addProjectIdUnlessPresent(path)
}
2 changes: 1 addition & 1 deletion frontend/src/queries/nodes/DataTable/EventRowActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { IconLink, IconPlayCircle } from 'lib/lemon-ui/icons'
import { LemonButton } from 'lib/lemon-ui/LemonButton'
import { More } from 'lib/lemon-ui/LemonButton/More'
import { copyToClipboard } from 'lib/utils/copyToClipboard'
import { getCurrentTeamId } from 'lib/utils/logics'
import { getCurrentTeamId } from 'lib/utils/getAppContext'
import { createActionFromEvent } from 'scenes/events/createActionFromEvent'
import { insightUrlForEvent } from 'scenes/insights/utils'
import { sessionPlayerModalLogic } from 'scenes/session-recordings/player/modal/sessionPlayerModalLogic'
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/queries/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { now } from 'lib/dayjs'
import { currentSessionId } from 'lib/internalMetrics'
import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
import { delay, flattenObject, toParams } from 'lib/utils'
import { getCurrentTeamId } from 'lib/utils/logics'
import { getCurrentTeamId } from 'lib/utils/getAppContext'
import posthog from 'posthog-js'
import {
filterTrendsClientSideParams,
Expand Down
26 changes: 13 additions & 13 deletions frontend/src/scenes/authentication/loginLogic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,19 @@ import { initKea } from '~/initKea'
import { initKeaTests } from '~/test/init'

describe('loginLogic', () => {
describe('redirect vulnerability', () => {
beforeEach(() => {
// Note, initKeaTests() is not called here because that uses a memory history, which doesn't throw on origin redirect
initKea({ beforePlugins: [testUtilsPlugin] })
})
it('should throw an exception on redirecting to a different origin', () => {
router.actions.push(`${origin}/login?next=//google.com`)
expect(() => {
handleLoginRedirect()
}).toThrow()
})
})

describe('parseLoginRedirectURL', () => {
let logic: ReturnType<typeof loginLogic.build>

Expand Down Expand Up @@ -48,17 +61,4 @@ describe('loginLogic', () => {
})
}
})

describe('redirect vulnerability', () => {
beforeEach(() => {
// Note, initKeaTests() is not called here because that uses a memory history, which doesn't throw on origin redirect
initKea({ beforePlugins: [testUtilsPlugin] })
})
it('should throw an exception on redirecting to a different origin', () => {
router.actions.push(`${origin}/login?next=//google.com`)
expect(() => {
handleLoginRedirect()
}).toThrow()
})
})
})
7 changes: 2 additions & 5 deletions frontend/src/scenes/feature-flags/FeatureFlagProjects.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { LemonTable, LemonTableColumn, LemonTableColumns } from 'lib/lemon-ui/Le
import { createdAtColumn, createdByColumn } from 'lib/lemon-ui/LemonTable/columnUtils'
import { useEffect } from 'react'
import { teamLogic } from 'scenes/teamLogic'
import { userLogic } from 'scenes/userLogic'
import { urls } from 'scenes/urls'

import { cohortsModel } from '~/models/cohortsModel'
import { groupsModel } from '~/models/groupsModel'
Expand Down Expand Up @@ -38,7 +38,6 @@ function checkHasStaticCohort(featureFlag: FeatureFlagType): boolean {
const getColumns = (): LemonTableColumns<OrganizationFeatureFlag> => {
const { currentTeamId } = useValues(teamLogic)
const { currentOrganization } = useValues(organizationLogic)
const { updateCurrentTeam } = useActions(userLogic)
const { aggregationLabel } = useValues(groupsModel)

return [
Expand All @@ -59,9 +58,7 @@ const getColumns = (): LemonTableColumns<OrganizationFeatureFlag> => {
) : (
<Link
className="row-name"
onClick={() => {
updateCurrentTeam(team.id, `/feature_flags/${record.flag_id}`)
}}
to={urls.project(team.id, record.flag_id ? urls.featureFlag(record.flag_id) : '')}
>
{linkText}
</Link>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/insights/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import api from 'lib/api'
import { dayjs } from 'lib/dayjs'
import { KEY_MAPPING } from 'lib/taxonomy'
import { ensureStringIsNotBlank, humanFriendlyNumber, objectsEqual } from 'lib/utils'
import { getCurrentTeamId } from 'lib/utils/logics'
import { getCurrentTeamId } from 'lib/utils/getAppContext'
import { ReactNode } from 'react'
import { dashboardLogic } from 'scenes/dashboard/dashboardLogic'
import { savedInsightsLogic } from 'scenes/saved-insights/savedInsightsLogic'
Expand Down
Loading
Loading