Skip to content

Commit

Permalink
feat: Add 'viewing as visitor' label (#3445)
Browse files Browse the repository at this point in the history
  • Loading branch information
suejung-sentry authored Oct 29, 2024
1 parent a8a4b15 commit b7968a4
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 24 deletions.
59 changes: 57 additions & 2 deletions src/layouts/Header/components/Navigator/Navigator.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,16 @@ const mockDetailOwner = {
},
}

const mockDetailOwnerNotMyOrg = {
owner: {
ownerid: 1,
username: 'not-codecov',
avatarUrl: 'http://127.0.0.1/avatar-url',
isCurrentUserPartOfOrg: false,
isAdmin: false,
},
}

const mockOwnerPageData = {
owner: {
username: 'codecov',
Expand Down Expand Up @@ -174,6 +184,10 @@ describe('Header Navigator', () => {
return HttpResponse.json({ data: mockMyContexts })
}),
graphql.query('DetailOwner', (info) => {
if (!isMyOrg) {
return HttpResponse.json({ data: mockDetailOwnerNotMyOrg })
}

return HttpResponse.json({ data: mockDetailOwner })
}),
graphql.query('OwnerPageData', (info) => {
Expand Down Expand Up @@ -203,6 +217,19 @@ describe('Header Navigator', () => {
const repo = await screen.findByText('test-repo')
expect(repo).toBeInTheDocument()
})

it('should show Viewing as Visitor if appropriate', async () => {
setup({ isMyOrg: false })
render(<Navigator currentUser={mockUser} />, {
wrapper: wrapper('/gh/not-codecov/test-repo'),
})

const org = await screen.findByText('not-codecov')
expect(org).toBeInTheDocument()

const text = await screen.findByText('Viewing as visitor')
expect(text).toBeInTheDocument()
})
})

describe('when on self-hosted admin settings page', () => {
Expand All @@ -222,7 +249,7 @@ describe('Header Navigator', () => {

describe('when viewing owner page', () => {
describe('and user is not part of the org', () => {
it('should render non-ContextSwitcher owner page variant', async () => {
it('should still render the user orgs dropdown', async () => {
const { user } = setup({ isMyOrg: false })
render(<Navigator currentUser={mockUser} />, {
wrapper: wrapper('/gh/not-codecov'),
Expand All @@ -234,8 +261,23 @@ describe('Header Navigator', () => {
await user.click(org)

const sentryOrg = screen.queryByRole('link', { name: 'sentry' })
expect(sentryOrg).not.toBeInTheDocument()
expect(sentryOrg).toBeInTheDocument()
})
})

it('should show the fallback if not logged in', async () => {
const { user } = setup({ isMyOrg: false })
render(<Navigator currentUser={undefined} />, {
wrapper: wrapper('/gh/not-codecov'),
})

const org = await screen.findByText('not-codecov')
expect(org).toBeInTheDocument()

await user.click(org)

const dropdownText = screen.queryByText('Install Codecov GitHub app')
expect(dropdownText).not.toBeInTheDocument()
})
})

Expand Down Expand Up @@ -327,5 +369,18 @@ describe('Header Navigator', () => {
expect(sentryOrg).toBeInTheDocument()
expect(sentryOrg).toHaveAttribute('href', '/gh/sentry')
})

it('should show Viewing as Visitor if appropriate', async () => {
setup({ isMyOrg: false })
render(<Navigator currentUser={mockUser} />, {
wrapper: wrapper('/gh/not-codecov'),
})

const org = await screen.findByText('not-codecov')
expect(org).toBeInTheDocument()

const text = await screen.findByText('Viewing as visitor')
expect(text).toBeInTheDocument()
})
})
})
38 changes: 33 additions & 5 deletions src/layouts/Header/components/Navigator/Navigator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useCrumbs } from 'pages/RepoPage/context'
import { Me } from 'services/user'
import Avatar from 'ui/Avatar'
import Breadcrumb from 'ui/Breadcrumb'
import Label from 'ui/Label'

import MyContextSwitcher from './MyContextSwitcher'

Expand All @@ -18,9 +19,22 @@ function Navigator({ currentUser }: NavigatorProps) {
const { path } = useRouteMatch()
const { breadcrumbs } = useCrumbs()

const isCurrentUserPartOfOrg = ownerData?.isCurrentUserPartOfOrg

// Repo page
if (path.startsWith('/:provider/:owner/:repo')) {
return <Breadcrumb paths={breadcrumbs} largeFont />
return (
<div className="flex items-center">
<span className="inline-block">
<Breadcrumb paths={breadcrumbs} largeFont />{' '}
</span>
{isCurrentUserPartOfOrg === false ? (
<Label variant="plain" className="ml-2 hidden sm:block">
Viewing as visitor
</Label>
) : null}
</div>
)
}

// Self-hosted admin settings
Expand All @@ -42,16 +56,21 @@ function Navigator({ currentUser }: NavigatorProps) {
)
}

if (ownerData && !ownerData.isCurrentUserPartOfOrg) {
// Fallback instead of MyContextSwitcher if not logged in
if (!currentUser) {
return (
<div className="flex items-center">
<Avatar user={ownerData} />
<h2 className="mx-2 text-xl font-semibold">{ownerData.username}</h2>
<h2 className="mx-2 text-xl font-semibold">{ownerData?.username}</h2>
{isCurrentUserPartOfOrg === false ? (
<Label variant="plain" className="ml-2 hidden sm:block">
Viewing as visitor
</Label>
) : null}
</div>
)
}

// Everything else uses MyContextSwitcher
let pageName = 'owner'
if (path.startsWith('/analytics/:provider/:owner')) {
pageName = 'analytics'
Expand All @@ -63,7 +82,16 @@ function Navigator({ currentUser }: NavigatorProps) {
pageName = 'accountAdmin'
}

return <MyContextSwitcher pageName={pageName} />
return (
<div className="flex items-center">
<MyContextSwitcher pageName={pageName} />
{isCurrentUserPartOfOrg === false ? (
<Label variant="plain" className="ml-2 hidden sm:block">
Viewing as visitor
</Label>
) : null}
</div>
)
}

export default Navigator
39 changes: 26 additions & 13 deletions src/shared/ListRepo/RepoTitleLink/RepoTitleLink.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,7 @@ import PropTypes from 'prop-types'

import AppLink from 'shared/AppLink'
import Icon from 'ui/Icon'

function Badge({ children }) {
return (
<span className="ml-2 h-min rounded border border-ds-gray-tertiary px-1 py-0.5 text-xs text-ds-gray-senary dark:bg-ds-gray-tertiary dark:text-ds-secondary-text">
{children}
</span>
)
}
import Label from 'ui/Label'

const getRepoIconName = ({ activated, isRepoPrivate, active }) =>
!activated && active ? 'ban' : isRepoPrivate ? 'lock-closed' : 'globe-alt'
Expand All @@ -36,8 +29,16 @@ function RepoTitleLink({ repo, showRepoOwner, pageName, disabledLink }) {
<span className="font-semibold">{repo.name}</span>
</span>
</div>
{isRepoPrivate && <Badge>Private</Badge>}
{active && !activated && <Badge>Deactivated</Badge>}
{isRepoPrivate && (
<Label variant="plain" className="ml-2">
Private
</Label>
)}
{active && !activated && (
<Label variant="plain" className="ml-2">
Deactivated
</Label>
)}
</div>
)
}
Expand All @@ -59,9 +60,21 @@ function RepoTitleLink({ repo, showRepoOwner, pageName, disabledLink }) {
<span className="font-semibold">{repo.name}</span>
</span>
</AppLink>
{isRepoPrivate && <Badge>Private</Badge>}
{active && !activated && <Badge>Deactivated</Badge>}
{repo?.isDemo && <Badge>System generated</Badge>}
{isRepoPrivate && (
<Label variant="plain" className="ml-2">
Private
</Label>
)}
{active && !activated && (
<Label variant="plain" className="ml-2">
Deactivated
</Label>
)}
{repo?.isDemo && (
<Label variant="plain" className="ml-2">
System generated
</Label>
)}
</div>
)
}
Expand Down
2 changes: 1 addition & 1 deletion src/ui/ContextSwitcher/ContextSwitcher.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ function ContextSwitcher({
</button>
<ul
className={cs(
'absolute z-10 max-h-64 w-full overflow-y-auto rounded-md bg-ds-background shadow-md ring-1 ring-black ring-opacity-5 focus:outline-none dark:bg-ds-container dark:shadow-lg dark:ring-1 dark:ring-ds-gray-tertiary',
'absolute z-10 max-h-64 w-screen max-w-[500px] overflow-y-auto rounded-md bg-ds-background shadow-md ring-1 ring-black ring-opacity-5 focus:outline-none dark:bg-ds-container dark:shadow-lg dark:ring-1 dark:ring-ds-gray-tertiary',
{ hidden: !toggle }
)}
tabIndex="-1"
Expand Down
14 changes: 13 additions & 1 deletion src/ui/Label/Label.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const meta: Meta<typeof Label> = {
component: Label,
argTypes: {
variant: {
options: ['default', 'subtle'],
options: ['default', 'subtle', 'plain'],
control: { type: 'select' },
},
},
Expand Down Expand Up @@ -43,6 +43,18 @@ export const SubtleLabel: Story = {
),
}

export const PlainLabel: Story = {
args: { variant: 'plain' },
render: (args) => (
<div className="flex gap-2">
<Label {...args}>Label in light mode</Label>
<div className="dark">
<Label {...args}>Label in dark mode</Label>
</div>
</div>
),
}

export const LabelInherits: Story = {
args: {
variant: 'default',
Expand Down
9 changes: 7 additions & 2 deletions src/ui/Label/Label.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const label = cva(
variant: {
default: 'border-current',
subtle: 'border-ds-border-line text-ds-gray-senary bg-ds-gray-primary',
plain:
'py-0.5 border-ds-gray-tertiary text-ds-gray-senary dark:bg-ds-gray-tertiary dark:text-ds-secondary-text',
},
},
defaultVariants: {
Expand All @@ -17,13 +19,16 @@ const label = cva(
}
)

interface LabelProps extends VariantProps<typeof label> {}
interface LabelProps extends VariantProps<typeof label> {
className?: string
}

const Label: React.FC<React.PropsWithChildren<LabelProps>> = ({
children,
variant,
className,
}) => {
return <span className={cn(label({ variant }))}>{children}</span>
return <span className={cn(label({ variant, className }))}>{children}</span>
}

export default Label

0 comments on commit b7968a4

Please sign in to comment.