Skip to content

feat: Org page UI improvements #3727

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

Merged
merged 3 commits into from
Feb 18, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 1 addition & 3 deletions src/services/repos/ReposQueryOpts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ const query = `query ReposForOwner(
interface ReposQueryArgs {
provider: string
owner: string
activated?: boolean
term?: string
sortItem?: {
ordering?: string
Expand All @@ -124,15 +123,14 @@ interface ReposQueryArgs {
function ReposQueryOpts({
provider,
owner,
activated,
term,
sortItem = orderingOptions[0],
first = 20,
repoNames,
isPublic = null, // by default, get both public and private repos
}: ReposQueryArgs) {
const variables = {
filters: { activated, term, repoNames, isPublic },
filters: { term, repoNames, isPublic },
ordering: sortItem?.ordering,
direction: sortItem?.direction,
first,
Expand Down
91 changes: 12 additions & 79 deletions src/shared/ListRepo/ListRepo.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -157,16 +157,16 @@ describe('ListRepo', () => {
describe('renders', () => {
it('renders the children', () => {
setup({})
render(<ListRepo canRefetch />, {
render(<ListRepo />, {
wrapper: wrapper({}),
})

expect(screen.getByText(/Not Configured/)).toBeInTheDocument()
expect(screen.getByText(/Search/)).toBeInTheDocument()
})

it('renders the repo table', () => {
setup({})
render(<ListRepo canRefetch />, {
render(<ListRepo />, {
wrapper: wrapper({}),
})

Expand All @@ -177,7 +177,7 @@ describe('ListRepo', () => {
describe('reads URL parameters', () => {
it('reads search parameter from URL', () => {
setup({})
render(<ListRepo canRefetch />, {
render(<ListRepo />, {
wrapper: wrapper({ url: '?search=thisisaquery' }),
})

Expand All @@ -186,77 +186,10 @@ describe('ListRepo', () => {
})
})

describe('switches Configured/Not Configured/All repos', () => {
it('switches to active repos', async () => {
const { user } = setup({})
render(<ListRepo canRefetch />, {
wrapper: wrapper({ url: '/gh', path: '/:provider' }),
})

const button = screen.getByRole('button', {
name: 'Configured',
})
await user.click(button)
expect(testLocation.state.repoDisplay).toEqual(
expect.stringMatching('Configured')
)
})

it('switches to Not Configured repos', async () => {
const { user } = setup({})
render(<ListRepo canRefetch />, {
wrapper: wrapper({ url: '/gh', path: '/:provider' }),
})

const button = screen.getByRole('button', {
name: /Not Configured/,
})
await user.click(button)
expect(testLocation.state.repoDisplay).toEqual(
expect.stringContaining('Not Configured')
)
})

it('switches to Configured repos owner page', async () => {
const { user } = setup({})
render(<ListRepo canRefetch />, {
wrapper: wrapper({
url: '/gh/hola',
path: '/:provider/:owner',
}),
})
const button = screen.getByRole('button', {
name: 'Configured',
})
await user.click(button)
expect(testLocation.state.repoDisplay).toEqual(
expect.stringMatching('Configured')
)
})

it('switches to all repos owner page', async () => {
const { user } = setup({})
render(<ListRepo canRefetch />, {
wrapper: wrapper({
url: '/gh/hola',
path: '/:provider/:owner',
}),
})

const button = screen.getByRole('button', {
name: /All/,
})
await user.click(button)
expect(testLocation.state.repoDisplay).toEqual(
expect.stringContaining('All')
)
})
})

describe('update params after typing', () => {
it('calls setSearchValue', async () => {
const { user } = setup({})
render(<ListRepo canRefetch />, {
render(<ListRepo />, {
wrapper: wrapper({}),
})

Expand All @@ -274,7 +207,7 @@ describe('ListRepo', () => {
describe('when rendered for team plan', () => {
it('renders the team table', async () => {
setup({ isTeamPlan: true })
render(<ListRepo canRefetch />, {
render(<ListRepo />, {
wrapper: wrapper({}),
})
const table = await screen.findByText(/ReposTableTeam/)
Expand All @@ -285,7 +218,7 @@ describe('ListRepo', () => {
describe('welcome demo alert banner', () => {
it('shows alert banner if it is my owner page and I came from onboarding', async () => {
const { me } = setup({})
render(<ListRepo canRefetch />, {
render(<ListRepo />, {
wrapper: wrapper({
url: '/gh/janedoe?source=onboarding',
path: '/:provider/:owner',
Expand All @@ -298,7 +231,7 @@ describe('ListRepo', () => {

it('does not show alert banner if I did not come from onboarding', async () => {
const { me } = setup({})
render(<ListRepo canRefetch />, {
render(<ListRepo />, {
wrapper: wrapper({
url: '/gh/janedoe',
path: '/:provider/:owner',
Expand All @@ -313,7 +246,7 @@ describe('ListRepo', () => {
describe('user does not have gh app installed', () => {
it('displays github app config banner if showDemoAlert is false', async () => {
setup({})
render(<ListRepo canRefetch hasGhApp={false} />, {
render(<ListRepo hasGhApp={false} />, {
wrapper: wrapper({
url: '/gh/janedoe',
path: '/:provider/:owner',
Expand All @@ -325,7 +258,7 @@ describe('ListRepo', () => {
})
it('does not display github app config banner if showDemoAlert is true', async () => {
setup({})
render(<ListRepo canRefetch hasGhApp={false} />, {
render(<ListRepo hasGhApp={false} />, {
wrapper: wrapper({
url: '/gh/janedoe?source=onboarding',
path: '/:provider/:owner',
Expand All @@ -339,7 +272,7 @@ describe('ListRepo', () => {
})
it('does not display github app config banner if isAdmin is false', async () => {
setup({ isAdmin: false })
render(<ListRepo canRefetch hasGhApp={false} />, {
render(<ListRepo hasGhApp={false} />, {
wrapper: wrapper({
url: '/gh/janedoe',
path: '/:provider/:owner',
Expand All @@ -352,7 +285,7 @@ describe('ListRepo', () => {
describe('user has gh app installed', () => {
it('does not display github app config banner if hasGhApp is true', async () => {
setup({})
render(<ListRepo canRefetch hasGhApp={true} />, {
render(<ListRepo hasGhApp={true} />, {
wrapper: wrapper({
url: '/gh/janedoe',
path: '/:provider/:owner',
Expand Down
21 changes: 2 additions & 19 deletions src/shared/ListRepo/ListRepo.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Suspense, useContext } from 'react'
import { Suspense } from 'react'
import { useParams } from 'react-router-dom'

import GithubConfigBanner from 'pages/OwnerPage/HeaderBanners/GithubConfigBanner'
Expand All @@ -8,7 +8,6 @@ import { orderingOptions } from 'services/repos/orderingOptions'
import { useIsTeamPlan } from 'services/useIsTeamPlan'
import { useIsCurrentUserAnAdmin, useUser } from 'services/user'
import { Provider } from 'shared/api/helpers'
import { ActiveContext } from 'shared/context'
import { Alert } from 'ui/Alert'
import Spinner from 'ui/Spinner'

Expand All @@ -23,14 +22,7 @@ const defaultQueryParams = {
repoDisplay: 'All',
}

export const repoDisplayOptions = Object.freeze({
CONFIGURED: { text: 'Configured', status: true },
NOT_CONFIGURED: { text: 'Not Configured', status: false },
ALL: { text: 'All', status: undefined },
})

interface ListRepoProps {
canRefetch: boolean
hasGhApp?: boolean
}

Expand All @@ -39,7 +31,7 @@ interface URLParams {
owner: string
}

function ListRepo({ canRefetch, hasGhApp }: ListRepoProps) {
function ListRepo({ hasGhApp }: ListRepoProps) {
const { provider, owner } = useParams<URLParams>()
const { params, updateParams } = useLocationParams(defaultQueryParams)
// @ts-expect-error useLocationParams needs to be typed
Expand All @@ -52,8 +44,6 @@ function ListRepo({ canRefetch, hasGhApp }: ListRepoProps) {
})
const isAdmin = useIsCurrentUserAnAdmin({ owner })

const repoDisplay = useContext(ActiveContext)

const loadingState = (
<div className="flex justify-center py-8">
<Spinner />
Expand All @@ -70,16 +60,9 @@ function ListRepo({ canRefetch, hasGhApp }: ListRepoProps) {
{isAdmin && !hasGhApp && !showDemoAlert ? <GithubConfigBanner /> : null}
<OrgControlTable
searchValue={search}
repoDisplay={repoDisplay}
setRepoDisplay={(repoDisplay) =>
updateParams({
repoDisplay,
})
}
setSearchValue={(search) => {
updateParams({ search })
}}
canRefetch={canRefetch}
/>

{showDemoAlert ? (
Expand Down
64 changes: 0 additions & 64 deletions src/shared/ListRepo/OrgControlTable/OrgControlTable.jsx

This file was deleted.

Loading