From c8209581f99f7a4158ba870d8f3fa383abbe017a Mon Sep 17 00:00:00 2001 From: nicholas-codecov Date: Mon, 17 Oct 2022 09:37:52 -0300 Subject: [PATCH 1/2] remove flag and hide setup repo link for non org members --- src/shared/ListRepo/ReposTable/ReposTable.jsx | 50 +++++++++++-------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/src/shared/ListRepo/ReposTable/ReposTable.jsx b/src/shared/ListRepo/ReposTable/ReposTable.jsx index a414e05ab7..4ac3d658f8 100644 --- a/src/shared/ListRepo/ReposTable/ReposTable.jsx +++ b/src/shared/ListRepo/ReposTable/ReposTable.jsx @@ -2,9 +2,9 @@ import PropTypes from 'prop-types' import { useContext } from 'react' import { useRepos } from 'services/repos' +import { useOwner, useUser } from 'services/user' import AppLink from 'shared/AppLink' import { ActiveContext } from 'shared/context' -import { useFlags } from 'shared/featureFlags' import { formatTimeToNow } from 'shared/utils/dates' import Button from 'ui/Button' import Progress from 'ui/Progress' @@ -58,11 +58,11 @@ function transformRepoToTable({ repos, owner, searchValue, - linkToOnboardingWithGazebo, isSetup, + isCurrentUserPartOfOrg, }) { // if there are no repos show empty message - if (repos.length <= 0) { + if (!repos?.length || repos?.length <= 0) { return [ { title: ( @@ -75,9 +75,8 @@ function transformRepoToTable({ ] } - const repoPageName = !isSetup && linkToOnboardingWithGazebo ? 'new' : 'repo' - - return repos.map((repo) => ({ + const repoPageName = !isSetup ? 'new' : 'repo' + return repos?.map((repo) => ({ title: ( - {repo.latestCommitAt ? formatTimeToNow(repo.latestCommitAt) : ''} + {repo?.latestCommitAt ? formatTimeToNow(repo?.latestCommitAt) : ''} ), coverage: - typeof repo.coverage === 'number' ? ( + typeof repo?.coverage === 'number' ? (
- +
) : ( No data available ), notEnabled: ( - + Not yet enabled{' '} - - setup repo - + {isCurrentUserPartOfOrg && ( + + setup repo + + )} ), })) } +// eslint-disable-next-line complexity function ReposTable({ searchValue, owner, sortItem, filterValues = [] }) { const active = useContext(ActiveContext) + const { data: userData } = useUser() + const { data: ownerData } = useOwner({ + username: owner || userData?.user?.username, + }) + const { data, fetchNextPage, hasNextPage, isFetchingNextPage } = useRepos({ active, sortItem, @@ -126,13 +133,12 @@ function ReposTable({ searchValue, owner, sortItem, filterValues = [] }) { owner, }) - const { newRepoSetupLink } = useFlags({ newRepoSetupLink: false }) const dataTable = transformRepoToTable({ repos: data.repos, owner, searchValue, - linkToOnboardingWithGazebo: newRepoSetupLink, isSetup: active, + isCurrentUserPartOfOrg: ownerData?.isCurrentUserPartOfOrg, }) return ( From 8526dcee30cadb5858d8b93ba7cb82895561503c Mon Sep 17 00:00:00 2001 From: nicholas-codecov Date: Mon, 17 Oct 2022 09:38:09 -0300 Subject: [PATCH 2/2] update repotables tests --- .../ListRepo/ReposTable/ReposTable.spec.jsx | 760 ++++++++++++------ 1 file changed, 522 insertions(+), 238 deletions(-) diff --git a/src/shared/ListRepo/ReposTable/ReposTable.spec.jsx b/src/shared/ListRepo/ReposTable/ReposTable.spec.jsx index dee92b35bd..dbe8c030a5 100644 --- a/src/shared/ListRepo/ReposTable/ReposTable.spec.jsx +++ b/src/shared/ListRepo/ReposTable/ReposTable.spec.jsx @@ -1,119 +1,213 @@ -import { render, screen } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { subDays } from 'date-fns' +import { graphql } from 'msw' +import { setupServer } from 'msw/node' import { MemoryRouter, Route } from 'react-router-dom' -import { orderingOptions, useRepos } from 'services/repos' +import { orderingOptions } from 'services/repos' import { ActiveContext } from 'shared/context' -import { useFlags } from 'shared/featureFlags' import ReposTable from './ReposTable' -jest.mock('services/repos/useRepos') -jest.mock('shared/featureFlags') +const queryClient = new QueryClient() +const server = setupServer() + +beforeAll(() => server.listen()) +afterEach(() => { + queryClient.clear() + server.resetHandlers() +}) +afterAll(() => server.close) describe('ReposTable', () => { + let active let props - let fetchNextPage = jest.fn(() => {}) + function setup({ - active, - repos, - hasNextPage, + activePassed, propObj = {}, - newRepoSetupLink, + edges = [], + isCurrentUserPartOfOrg = true, }) { - useRepos.mockReturnValue({ - data: { - repos, - }, - hasNextPage, - fetchNextPage, - }) - useFlags.mockReturnValue({ newRepoSetupLink }) + server.use( + graphql.query('CurrentUser', (req, res, ctx) => { + return res( + ctx.status(200), + ctx.data({ me: { user: { userName: 'codecov-user' } } }) + ) + }), + graphql.query('DetailOwner', (req, res, ctx) => { + return res( + ctx.status(200), + ctx.data({ owner: { isCurrentUserPartOfOrg } }) + ) + }), + graphql.query('ReposForOwner', (req, res, ctx) => { + return res( + ctx.status(200), + ctx.data({ + owner: { + repositories: { + edges, + pageInfo: { + hasNextPage: true, + endCursor: '2', + }, + }, + }, + }) + ) + }), + graphql.query('MyRepos', (req, res, ctx) => { + if (req?.variables?.after === '2') { + return res( + ctx.status(200), + ctx.data({ + me: { + viewableRepositories: { + edges: [ + { + node: { + private: false, + activated: true, + author: { + username: 'owner2', + }, + name: 'Repo name extra', + latestCommitAt: subDays(new Date(), 5).toISOString(), + coverage: 50, + active: true, + }, + }, + ], + pageInfo: { + hasNextPage: false, + endCursor: '3', + }, + }, + }, + }) + ) + } + + return res( + ctx.status(200), + ctx.data({ + me: { + viewableRepositories: { + edges, + pageInfo: { + hasNextPage: true, + endCursor: '2', + }, + }, + }, + }) + ) + }) + ) + props = { searchValue: '', sortItem: orderingOptions[0], ...propObj, } - render( - - - - - - - - ) + + active = activePassed } describe('when rendered with active true', () => { beforeEach(() => { setup({ - active: true, - repos: [ + activePassed: true, + edges: [ { - private: false, - activated: true, - author: { - username: 'owner1', + node: { + private: false, + activated: true, + author: { + username: 'owner1', + }, + name: 'Repo name 1', + latestCommitAt: subDays(new Date(), 3).toISOString(), + coverage: 43, + active: true, }, - name: 'Repo name 1', - latestCommitAt: subDays(new Date(), 3).toISOString(), - coverage: 43, - active: true, }, { - private: true, - activated: true, - author: { - username: 'owner1', + node: { + private: true, + activated: true, + author: { + username: 'owner1', + }, + name: 'Repo name 2', + latestCommitAt: subDays(new Date(), 2).toISOString(), + coverage: 100, + active: true, }, - name: 'Repo name 2', - latestCommitAt: subDays(new Date(), 2).toISOString(), - coverage: 100, - active: true, }, { - private: true, - activated: true, - author: { - username: 'owner1', + node: { + private: true, + activated: true, + author: { + username: 'owner1', + }, + name: 'Repo name 3', + latestCommitAt: null, + active: true, }, - name: 'Repo name 3', - latestCommitAt: null, - active: true, }, ], }) }) - it('calls useRepos with the right data', () => { - expect(useRepos).toHaveBeenCalledWith({ - active: true, - owner: undefined, - repoNames: [], - sortItem: { - direction: 'DESC', - ordering: 'COMMIT_DATE', - text: 'Most recent commit', - }, - term: '', - }) - }) + it('renders table repo name', async () => { + render( + + + + + + + + + + ) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) - it('renders table repo name', () => { - const buttons = screen.getAllByText(/Repo name/) + const buttons = await screen.findAllByText(/Repo name/) expect(buttons.length).toBe(3) }) - it('links to /:organization/:owner/:repo', () => { - const repo1 = screen.getByRole('link', { + it('links to /:organization/:owner/:repo', async () => { + render( + + + + + + + + + + ) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) + + const repo1 = await screen.findByRole('link', { name: 'globe-alt.svg owner1 / Repo name 1', }) - const repo2 = screen.getByRole('link', { + const repo2 = await screen.findByRole('link', { name: 'lock-closed.svg owner1 / Repo name 2', }) - const repo3 = screen.getByRole('link', { + const repo3 = await screen.findByRole('link', { name: 'lock-closed.svg owner1 / Repo name 3', }) @@ -122,199 +216,336 @@ describe('ReposTable', () => { expect(repo3).toHaveAttribute('href', '/gh/owner1/Repo name 3') }) - it('renders second column', () => { - const lastseen1 = screen.getByText(/3 days ago/) - const lastseen2 = screen.getByText(/2 days ago/) - expect(lastseen1).toBeInTheDocument() - expect(lastseen2).toBeInTheDocument() + it('renders second column', async () => { + render( + + + + + + + + + + ) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) + + const lastSeen1 = await screen.findByText(/3 days ago/) + expect(lastSeen1).toBeInTheDocument() + + const lastSeen2 = await screen.findByText(/2 days ago/) + expect(lastSeen2).toBeInTheDocument() }) - it('renders third column', () => { - const bars = screen.getAllByTestId('org-progress-bar') + it('renders third column', async () => { + render( + + + + + + + + + + ) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) + + const bars = await screen.findAllByTestId('org-progress-bar') expect(bars.length).toBe(2) }) - it('renders handles null coverage', () => { - const noData = screen.getByText(/No data available/) + it('renders handles null coverage', async () => { + render( + + + + + + + + + + ) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) + + const noData = await screen.findByText(/No data available/) expect(noData).toBeInTheDocument() }) }) describe('when rendered with active false', () => { - beforeEach(() => { - setup({ - active: false, - repos: [ - { - private: false, - activated: true, - author: { - username: 'owner1', + describe('user belongs to org', () => { + beforeEach(() => { + setup({ + activePassed: false, + edges: [ + { + node: { + private: false, + activated: true, + author: { + username: 'owner1', + }, + name: 'Repo name 1', + latestCommitAt: subDays(new Date(), 3).toISOString(), + coverage: 43, + active: false, + }, }, - name: 'Repo name 1', - latestCommitAt: subDays(new Date(), 3).toISOString(), - coverage: 43, - active: false, - }, - { - private: true, - activated: true, - author: { - username: 'owner1', + { + node: { + private: true, + activated: true, + author: { + username: 'owner1', + }, + name: 'Repo name 2', + latestCommitAt: subDays(new Date(), 2).toISOString(), + coverage: 100, + active: false, + }, }, - name: 'Repo name 2', - latestCommitAt: subDays(new Date(), 2).toISOString(), - coverage: 100, - active: false, - }, - { - private: true, - activated: true, - author: { - username: 'owner1', + { + node: { + private: true, + activated: true, + author: { + username: 'owner1', + }, + name: 'Repo name 3', + latestCommitAt: subDays(new Date(), 5).toISOString(), + coverage: 0, + active: false, + }, }, - name: 'Repo name 3', - latestCommitAt: subDays(new Date(), 5).toISOString(), - coverage: 0, - active: false, - }, - ], + ], + }) }) - }) - it('calls useRepos with the right data', () => { - expect(useRepos).toHaveBeenCalledWith({ - active: false, - owner: undefined, - repoNames: [], - sortItem: { - direction: 'DESC', - ordering: 'COMMIT_DATE', - text: 'Most recent commit', - }, - term: '', - }) - }) + it('links to /:organization/:owner/:repo/new', async () => { + render( + + + + + + + + + + ) - it('renders table repo name', () => { - const buttons = screen.getAllByText(/Repo name/) - expect(buttons.length).toBe(3) - }) + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) - it('links to /:organization/:owner/:repo', () => { - const repo1 = screen.getByRole('link', { - name: 'globe-alt.svg owner1 / Repo name 1', - }) - const repo2 = screen.getByRole('link', { - name: 'lock-closed.svg owner1 / Repo name 2', - }) - const repo3 = screen.getByRole('link', { - name: 'lock-closed.svg owner1 / Repo name 3', - }) + const repo1 = await screen.findByRole('link', { + name: 'globe-alt.svg owner1 / Repo name 1', + }) + expect(repo1).toHaveAttribute('href', '/gh/owner1/Repo name 1/new') - expect(repo1).toHaveAttribute('href', '/gh/owner1/Repo name 1') - expect(repo2).toHaveAttribute('href', '/gh/owner1/Repo name 2') - expect(repo3).toHaveAttribute('href', '/gh/owner1/Repo name 3') - }) + const repo2 = await screen.findByRole('link', { + name: 'lock-closed.svg owner1 / Repo name 2', + }) + expect(repo2).toHaveAttribute('href', '/gh/owner1/Repo name 2/new') - it('renders second column', () => { - const notActiveRepos = screen.getAllByText(/Not yet enabled/) - expect(notActiveRepos.length).toBe(3) - }) - it('does not render next page button', () => { - const button = screen.queryByText(/Load More/) - expect(button).not.toBeInTheDocument() + const repo3 = await screen.findByRole('link', { + name: 'lock-closed.svg owner1 / Repo name 3', + }) + expect(repo3).toHaveAttribute('href', '/gh/owner1/Repo name 3/new') + }) }) - }) - describe('when rendered with active false and targets for new experience', () => { - beforeEach(() => { - setup({ - newRepoSetupLink: true, - active: false, - repos: [ - { - private: false, - activated: true, - author: { - username: 'owner1', + describe('user does not belongs to org', () => { + beforeEach(() => { + setup({ + activePassed: false, + isCurrentUserPartOfOrg: false, + edges: [ + { + node: { + private: false, + activated: true, + author: { + username: 'owner1', + }, + name: 'Repo name 1', + latestCommitAt: subDays(new Date(), 3).toISOString(), + coverage: 43, + active: false, + }, }, - name: 'Repo name 1', - latestCommitAt: subDays(new Date(), 3).toISOString(), - coverage: 43, - active: false, - }, - { - private: true, - activated: true, - author: { - username: 'owner1', + { + node: { + private: true, + activated: true, + author: { + username: 'owner1', + }, + name: 'Repo name 2', + latestCommitAt: subDays(new Date(), 2).toISOString(), + coverage: 100, + active: false, + }, }, - name: 'Repo name 2', - latestCommitAt: subDays(new Date(), 2).toISOString(), - coverage: 100, - active: false, - }, - { - private: true, - activated: true, - author: { - username: 'owner1', + { + node: { + private: true, + activated: true, + author: { + username: 'owner1', + }, + name: 'Repo name 3', + latestCommitAt: subDays(new Date(), 5).toISOString(), + coverage: 0, + active: false, + }, }, - name: 'Repo name 3', - latestCommitAt: subDays(new Date(), 5).toISOString(), - coverage: 0, - active: false, - }, - ], + ], + }) }) - }) - it('links to /:organization/:owner/:repo/new', () => { - const repo1 = screen.getByRole('link', { - name: 'globe-alt.svg owner1 / Repo name 1', - }) - const repo2 = screen.getByRole('link', { - name: 'lock-closed.svg owner1 / Repo name 2', - }) - const repo3 = screen.getByRole('link', { - name: 'lock-closed.svg owner1 / Repo name 3', - }) + it('does not show setup repo link', async () => { + render( + + + + + + + + + + ) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) - expect(repo1).toHaveAttribute('href', '/gh/owner1/Repo name 1/new') - expect(repo2).toHaveAttribute('href', '/gh/owner1/Repo name 2/new') - expect(repo3).toHaveAttribute('href', '/gh/owner1/Repo name 3/new') + const notEnabled = await screen.findAllByText('Not yet enabled') + expect(notEnabled.length).toBe(3) + + const repo1 = screen.queryByRole('link', { + name: '/gh/owner1/Repo name 1/new', + }) + expect(repo1).not.toBeInTheDocument() + }) }) }) describe('when rendered empty repos', () => { beforeEach(() => { - setup({ active: true, repos: [] }) + setup({ activePassed: true, edges: [] }) }) - it('renders no repos detected', () => { - const buttons = screen.getAllByText(/No repos setup yet/) + + it('renders no repos detected', async () => { + render( + + + + + + + + + + ) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) + + const buttons = await screen.findAllByText(/No repos setup yet/) expect(buttons.length).toBe(1) }) - it('renders the select the repo link', () => { - const link = screen.getByRole('link', { name: 'Select the repo' }) + it('renders the select the repo link', async () => { + render( + + + + + + + + + + ) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) + + const link = await screen.findByRole('link', { + name: 'Select the repo', + }) expect(link).toBeInTheDocument() }) - it('renders the select the repo to have the right link', () => { - const link = screen.getByRole('link', { name: 'Select the repo' }) + it('renders the select the repo to have the right link', async () => { + render( + + + + + + + + + + ) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) + + const link = await screen.findByRole('link', { name: 'Select the repo' }) expect(link).toHaveAttribute('href', '/gh/+') }) - it('renders the quick start guide link', () => { - const link = screen.getByRole('link', { + it('renders the quick start guide link', async () => { + render( + + + + + + + + + + ) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) + + const link = await screen.findByRole('link', { name: 'quick start guide. external-link.svg', }) expect(link).toBeInTheDocument() }) - it('renders the view repos for setup button', () => { - const btn = screen.getByRole('link', { name: 'View repos for setup' }) + it('renders the view repos for setup button', async () => { + render( + + + + + + + + + + ) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) + + const btn = await screen.findByRole('link', { + name: 'View repos for setup', + }) expect(btn).toBeInTheDocument() }) }) @@ -322,14 +553,28 @@ describe('ReposTable', () => { describe('when rendered empty search', () => { beforeEach(() => { setup({ - active: true, - repos: [], - hasNextPage: false, + activePassed: true, + edges: [], propObj: { searchValue: 'something' }, }) }) - it('renders no results found', () => { - const buttons = screen.getAllByText(/no results found/) + it('renders no results found', async () => { + render( + + + + + + + + + + ) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) + + const buttons = await screen.findAllByText(/no results found/) expect(buttons.length).toBe(1) }) }) @@ -337,30 +582,69 @@ describe('ReposTable', () => { describe('render next page button', () => { beforeEach(() => { setup({ - active: true, - repos: [ + activePassed: true, + edges: [ { - private: false, - activated: true, - author: { - username: 'owner1', + node: { + private: false, + activated: true, + author: { + username: 'owner1', + }, + name: 'Repo name 1', + latestCommitAt: subDays(new Date(), 3).toISOString(), + coverage: 43, + active: false, }, - name: 'Repo name 1', - latestCommitAt: subDays(new Date(), 3).toISOString(), - coverage: 43, - active: false, }, ], - hasNextPage: true, }) }) - it('renders button', () => { - const button = screen.getByText(/Load More/) + + it('renders button', async () => { + render( + + + + + + + + + + ) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) + + const button = await screen.findByText(/Load More/) expect(button).toBeInTheDocument() }) - it('fires next page button click', () => { - userEvent.click(screen.getByText(/Load More/)) - expect(fetchNextPage).toHaveBeenCalled() + + it('loads next page of data', async () => { + render( + + + + + + + + + + ) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) + + const loadMore = await screen.findByText(/Load More/) + userEvent.click(loadMore) + + await waitFor(() => queryClient.isFetching()) + await waitFor(() => !queryClient.isFetching()) + + const newlyLoadedRepo = await screen.findByText('Repo name extra') + expect(newlyLoadedRepo).toBeInTheDocument() }) }) })