diff --git a/apps/portals/src/configurations/nf/routesConfig.ts b/apps/portals/src/configurations/nf/routesConfig.ts index 18df85b755..060d48f6ff 100644 --- a/apps/portals/src/configurations/nf/routesConfig.ts +++ b/apps/portals/src/configurations/nf/routesConfig.ts @@ -67,7 +67,7 @@ const routes: GenericRoute[] = [ outsideContainerClassName: 'home-spacer home-bg-dark', link: '/Explore/Studies', props: { - limit, + initialLimit: limit, columnAliases, sql: newStudiesSql, ...studyCardConfiguration, diff --git a/apps/portals/src/configurations/nf/synapseConfigs/organizations.ts b/apps/portals/src/configurations/nf/synapseConfigs/organizations.ts index fa68fef8d0..e872975580 100644 --- a/apps/portals/src/configurations/nf/synapseConfigs/organizations.ts +++ b/apps/portals/src/configurations/nf/synapseConfigs/organizations.ts @@ -33,11 +33,10 @@ export const organizationDetailsPageConfig: DetailsPageProps = { sql: studiesSql, visibleColumnCount: 7, sqlOperator: ColumnSingleValueFilterOperator.LIKE, - cardConfiguration: studyCardConfiguration, + cardConfiguration: { ...studyCardConfiguration, initialLimit: 2 }, name: 'Funded Studies', columnAliases, searchConfiguration, - limit: 2, }, tableSqlKeys: ['fundingAgency'], columnName: 'fundingAgency', @@ -46,7 +45,7 @@ export const organizationDetailsPageConfig: DetailsPageProps = { name: 'CardContainerLogic', props: { sql: publicationsSql, - limit: 3, + initialLimit: 3, ...publicationsCardConfiguration, sqlOperator: ColumnSingleValueFilterOperator.LIKE, }, @@ -96,7 +95,7 @@ export const organizationDetailsPageConfig: DetailsPageProps = { props: { ...datasetCardConfiguration, sql: datasetsSql, - limit: 3, + initialLimit: 3, }, columnName: 'fundingAgency', title: 'Datasets', diff --git a/apps/portals/src/configurations/nf/synapseConfigs/studies.ts b/apps/portals/src/configurations/nf/synapseConfigs/studies.ts index 24434af1e0..411b136ff1 100644 --- a/apps/portals/src/configurations/nf/synapseConfigs/studies.ts +++ b/apps/portals/src/configurations/nf/synapseConfigs/studies.ts @@ -168,7 +168,7 @@ export const studiesDetailPage: DetailsPageProps = { tableSqlKeys: ['studyId'], props: { sql: toolStudySql, - limit: 3, + initialLimit: 3, ...toolsCardConfiguration, }, }, diff --git a/apps/portals/src/configurations/nf/synapseConfigs/tools.ts b/apps/portals/src/configurations/nf/synapseConfigs/tools.ts index 646f28e056..7f233fc854 100644 --- a/apps/portals/src/configurations/nf/synapseConfigs/tools.ts +++ b/apps/portals/src/configurations/nf/synapseConfigs/tools.ts @@ -172,7 +172,7 @@ export const toolDetailsPageConfig: DetailsPageProps = { title: 'Development Publication', props: { ...publicationsV2CardConfiguration, - limit: 3, + initialLimit: 3, columnAliases, sql: developmentPublicationSql, secondaryLabelLimit: 4, @@ -221,7 +221,7 @@ export const toolDetailsPageConfig: DetailsPageProps = { title: 'Publications', props: { ...publicationsV2CardConfiguration, - limit: 3, + initialLimit: 3, columnAliases, sql: publicationsV2Sql, }, @@ -272,7 +272,7 @@ export const toolDetailsPageConfig: DetailsPageProps = { props: { sql: `${observationsSql} WHERE observationTime IS NULL`, type: SynapseConstants.OBSERVATION_CARD, - limit: 3, + initialLimit: 3, }, title: 'Community Observations', tableSqlKeys: ['resourceId'], diff --git a/packages/synapse-react-client/src/components/CardContainer/CardContainer.integration.test.tsx b/packages/synapse-react-client/src/components/CardContainer/CardContainer.integration.test.tsx index e905fbd5b0..c77ad8c188 100644 --- a/packages/synapse-react-client/src/components/CardContainer/CardContainer.integration.test.tsx +++ b/packages/synapse-react-client/src/components/CardContainer/CardContainer.integration.test.tsx @@ -1,7 +1,6 @@ -import { render, screen, waitFor } from '@testing-library/react' +import { render, screen, waitFor, within } from '@testing-library/react' import userEvent from '@testing-library/user-event' import React from 'react' -import { SynapseConstants } from '../../utils' import CardContainer, { CardContainerProps } from './CardContainer' import { QueryVisualizationWrapper } from '../QueryVisualizationWrapper/QueryVisualizationWrapper' import { createWrapper } from '../../testutils/TestingLibraryUtils' @@ -9,8 +8,11 @@ import { QueryBundleRequest } from '@sage-bionetworks/synapse-types' import syn16787123Json from '../../mocks/query/syn16787123' import { DEFAULT_PAGE_SIZE, + GENERIC_CARD, MEDIUM_USER_CARD, + STUDY, } from '../../utils/SynapseConstants' +import { mockTableEntity } from '../../mocks/entity/mockTableEntity' import mockUserCardTableQueryResultBundle from '../../mocks/query/mockUserCardTableQueryResultBundle' import { server } from '../../mocks/msw/server' import { mockUserProfileData } from '../../mocks/user/mock_user_profile' @@ -19,12 +21,22 @@ import { getHandlersForTableQuery } from '../../mocks/msw/handlers/tableQueryHan import { cloneDeep } from 'lodash-es' import { QueryContextConsumer } from '../QueryContext' import { InfiniteQueryContextType } from '../QueryContext/QueryContext' +import SynapseClient from '../../synapse-client' -const sql = 'SELECT * FROM syn16787123' +const tableId = 'syn16787123' +const sql = `SELECT * FROM ${tableId}` + +jest + .spyOn(SynapseClient, 'getEntity') + .mockResolvedValue({ ...mockTableEntity, id: tableId }) +const getQueryTableAsyncJobResultsSpy = jest.spyOn( + SynapseClient, + 'getQueryTableAsyncJobResults', +) const lastQueryRequest: QueryBundleRequest = { concreteType: 'org.sagebionetworks.repo.model.table.QueryBundleRequest', - entityId: 'syn16787123', + entityId: tableId, partMask: 255, query: { sql, @@ -38,13 +50,33 @@ dataWithOnePage.queryCount = dataWithOnePage.queryResult!.queryResults.rows.length dataWithOnePage.queryResult!.nextPageToken = undefined -const dataWithMultiplePages = cloneDeep(syn16787123Json) -dataWithMultiplePages.queryCount = - dataWithMultiplePages.queryResult!.queryResults.rows.length + 1 -dataWithMultiplePages.queryResult!.nextPageToken = { +const dataWithMultiplePagesFirstPage = cloneDeep(syn16787123Json) +dataWithMultiplePagesFirstPage.queryCount = + dataWithMultiplePagesFirstPage.queryResult!.queryResults.rows.length * 2 + 1 +dataWithMultiplePagesFirstPage.queryResult!.nextPageToken = { token: 'abcd', } +const dataWithMultiplePagesSecondPage = cloneDeep( + dataWithMultiplePagesFirstPage, +) +dataWithMultiplePagesSecondPage.queryResult!.nextPageToken!.token = 'efgh' +dataWithMultiplePagesSecondPage.queryResult!.queryResults.rows = [ + ...dataWithMultiplePagesFirstPage.queryResult!.queryResults.rows.map(row => ({ + ...row, + rowId: row.rowId! + DEFAULT_PAGE_SIZE, + })), +] + +const createDataWithOnePageWithQueryCount = (queryCount: number) => { + const page = cloneDeep(dataWithOnePage) + page.queryCount = queryCount + page.queryResult!.queryResults.rows = cloneDeep( + dataWithOnePage.queryResult!.queryResults.rows.slice(0, queryCount), + ) + return page +} + const unitDescription = 'study' let capturedQueryContext: InfiniteQueryContextType | undefined @@ -69,19 +101,36 @@ const renderComponent = (props: CardContainerProps) => { ) } +const setUp = (props: CardContainerProps) => { + const user = userEvent.setup() + const component = renderComponent(props) + return { component, user } +} + +const waitForCardCount = async (cardText: string, nCards: number) => { + await waitFor(() => { + expect( + within(screen.getByRole('list')).getAllByText(cardText), + ).toHaveLength(nCards) + }) +} + describe('CardContainer tests', () => { beforeAll(() => server.listen()) beforeEach(() => { - server.use(...getHandlersForTableQuery(dataWithMultiplePages)) + jest.clearAllMocks() + server.use(...getHandlersForTableQuery(dataWithMultiplePagesFirstPage)) }) afterEach(() => server.restoreHandlers()) afterAll(() => server.close()) - const type = SynapseConstants.STUDY - // cast the data to ignore ts warning const props: CardContainerProps = { unitDescription, - type, + type: GENERIC_CARD, + genericCardSchema: { + type: STUDY, + title: 'title', + }, } it('renders without crashing', () => { @@ -103,8 +152,11 @@ describe('CardContainer tests', () => { await screen.findByText(title) }) - it('handleViewMore works', async () => { - renderComponent(props) + it('ViewMore button shows full pages on each click when initialLimit is not set', async () => { + const { user } = setUp(props) + await waitForCardCount(STUDY, DEFAULT_PAGE_SIZE) + expect(getQueryTableAsyncJobResultsSpy).toHaveBeenCalledTimes(1) + // go through calling handle view more const viewMoreButton = await screen.findByRole('button', { name: 'View More', @@ -115,25 +167,128 @@ describe('CardContainer tests', () => { 'appendNextPageToResults', ) - await userEvent.click(viewMoreButton) + server.use(...getHandlersForTableQuery(dataWithMultiplePagesSecondPage)) + await user.click(viewMoreButton) // Verify that the next page was appended await waitFor(() => { expect(appendNextPageSpy).toHaveBeenCalled() }) + await waitForCardCount(STUDY, DEFAULT_PAGE_SIZE * 2) + expect(getQueryTableAsyncJobResultsSpy).toHaveBeenCalledTimes(2) }) - it('show ViewMore does not render when hasNextPage is false', () => { + it('ViewMore does not render when hasNextPage is false and no initialLimit is not set', async () => { server.use(...getHandlersForTableQuery(dataWithOnePage)) renderComponent(props) + await waitForCardCount(STUDY, DEFAULT_PAGE_SIZE) expect( screen.queryByRole('button', { name: 'View More' }), ).not.toBeInTheDocument() }) + it('ViewMore button shows rest of first page on first click and full page on next click when initialLimit is set', async () => { + const initialLimit = 3 + const queryCount = dataWithMultiplePagesFirstPage.queryCount! + expect(initialLimit).toBeLessThan(queryCount) + expect(DEFAULT_PAGE_SIZE * 2).toBeLessThan(queryCount) + + const { user } = setUp({ + ...props, + initialLimit: initialLimit, + unitDescription: undefined, + }) + + const viewMoreButton = await screen.findByRole('button', { + name: 'View More', + }) + + // initial load - limited to initialLimit + await waitForCardCount(STUDY, initialLimit) + expect(viewMoreButton).toBeInTheDocument() + expect(getQueryTableAsyncJobResultsSpy).toHaveBeenCalledTimes(1) // initial call to get data + + server.use(...getHandlersForTableQuery(dataWithMultiplePagesSecondPage)) + await user.click(viewMoreButton) + + // first showMore click - full first page + await waitForCardCount(STUDY, DEFAULT_PAGE_SIZE) + expect(viewMoreButton).toBeInTheDocument() + expect(getQueryTableAsyncJobResultsSpy).toHaveBeenCalledTimes(1) // no call needed - data already loaded + + await user.click(viewMoreButton) + + // second showMore click - full second page + await waitForCardCount(STUDY, DEFAULT_PAGE_SIZE * 2) + expect(viewMoreButton).toBeInTheDocument() + expect(getQueryTableAsyncJobResultsSpy).toHaveBeenCalledTimes(2) // call to get second page + }) + + it('ViewMore button does not render when initialLimit is greater than or equal to queryCount', async () => { + const initialLimit = 4 + const queryCount = 3 + expect(initialLimit).toBeGreaterThanOrEqual(queryCount) + + server.use( + ...getHandlersForTableQuery( + createDataWithOnePageWithQueryCount(queryCount), + ), + ) + + setUp({ + ...props, + initialLimit: initialLimit, + unitDescription: undefined, + }) + + // initial load - limited to queryCount + await waitForCardCount(STUDY, queryCount) + const viewMoreButton = screen.queryByRole('button', { + name: 'View More', + }) + expect(viewMoreButton).not.toBeInTheDocument() + expect(getQueryTableAsyncJobResultsSpy).toHaveBeenCalledTimes(1) // initial call to get data + }) + + it('ViewMore button renders when hasNextPage is false, but initialLimit is less than queryCount', async () => { + const initialLimit = 3 + const queryCount = 5 + expect(initialLimit).toBeLessThan(queryCount) + server.use( + ...getHandlersForTableQuery( + createDataWithOnePageWithQueryCount(queryCount), + ), + ) + + const { user } = setUp({ + ...props, + initialLimit: initialLimit, + unitDescription: undefined, + }) + + // initial load - limited to initialLimit + const viewMoreButton = await screen.findByRole('button', { + name: 'View More', + }) + expect(viewMoreButton).toBeInTheDocument() + await waitForCardCount(STUDY, initialLimit) + expect(getQueryTableAsyncJobResultsSpy).toHaveBeenCalledTimes(1) // initial call to get data + + await user.click(viewMoreButton) + + // first showMore click - remaining results + await waitForCardCount(STUDY, queryCount) + expect(viewMoreButton).not.toBeInTheDocument() + expect(getQueryTableAsyncJobResultsSpy).toHaveBeenCalledTimes(1) // no call needed - data already loaded + }) + it('Does not filter null IDs when rendering user cards (PORTALS-2430)', async () => { server.use(...getHandlersForTableQuery(mockUserCardTableQueryResultBundle)) - renderComponent({ ...props, type: MEDIUM_USER_CARD }) + renderComponent({ + ...props, + type: MEDIUM_USER_CARD, + genericCardSchema: undefined, + }) // Since the first user in the mock data has a user ID, their profile information will be fetched, ignoring the table data. await screen.findByText( diff --git a/packages/synapse-react-client/src/components/CardContainer/CardContainer.tsx b/packages/synapse-react-client/src/components/CardContainer/CardContainer.tsx index a01b13c1fc..1294c80dbb 100644 --- a/packages/synapse-react-client/src/components/CardContainer/CardContainer.tsx +++ b/packages/synapse-react-client/src/components/CardContainer/CardContainer.tsx @@ -61,6 +61,7 @@ export function CardContainer(props: CardContainerProps) { isLoading, secondaryLabelLimit = 3, title, + initialLimit, ...rest } = props const infiniteQueryContext = useInfiniteQueryContext() @@ -68,6 +69,20 @@ export function CardContainer(props: CardContainerProps) { const { appendNextPageToResults, hasNextPage } = infiniteQueryContext const data = useAtomValue(tableQueryDataAtom) const queryVisualizationContext = useQueryVisualizationContext() + const [hasAppliedInitialLimit, setHasAppliedInitialLimit] = + React.useState(false) + + const queryCount = data?.queryCount + const applyInitialLimit = + initialLimit !== undefined && + queryCount !== undefined && + initialLimit < queryCount && + !hasAppliedInitialLimit + + let dataRows: Row[] = (data && data.queryResult?.queryResults.rows) || [] + if (applyInitialLimit) { + dataRows = dataRows.slice(0, initialLimit) + } const ids = data?.queryResult!.queryResults.tableId ? [data?.queryResult.queryResults.tableId] @@ -84,7 +99,7 @@ export function CardContainer(props: CardContainerProps) { {isLoading && type !== OBSERVATION_CARD && loadingScreen} ) - } else if (data && data.queryResult!.queryResults.rows.length === 0) { + } else if (dataRows.length === 0) { // Show "no results" UI (see PORTALS-1497) return } @@ -92,14 +107,18 @@ export function CardContainer(props: CardContainerProps) { data.queryResult!.queryResults.headers.forEach((element, index) => { schema[element.name] = index }) - const showViewMoreButton = hasNextPage && ( + const showViewMoreButton = (applyInitialLimit || hasNextPage) && ( { - appendNextPageToResults() + if (applyInitialLimit && !hasAppliedInitialLimit) { + setHasAppliedInitialLimit(true) + } else { + appendNextPageToResults() + } }} > View More @@ -118,15 +137,12 @@ export function CardContainer(props: CardContainerProps) { 'Type MEDIUM_USER_CARD specified but no columnType USERID found', ) } - const listIds = data.queryResult!.queryResults.rows.map( - el => el.values[userIdColumnIndex], - ) + const listIds = dataRows.map(el => el.values[userIdColumnIndex]) cards = } else { // render the cards - const cardsData = data.queryResult!.queryResults.rows - cards = cardsData.length ? ( - cardsData.map((rowData: Row, index) => { + cards = dataRows.length ? ( + dataRows.map((rowData: Row, index) => { const key = JSON.stringify(rowData.values) const propsForCard = { key, diff --git a/packages/synapse-react-client/src/components/CardContainerLogic/CardContainerLogic.stories.ts b/packages/synapse-react-client/src/components/CardContainerLogic/CardContainerLogic.stories.ts index 0bca27428a..acff974757 100644 --- a/packages/synapse-react-client/src/components/CardContainerLogic/CardContainerLogic.stories.ts +++ b/packages/synapse-react-client/src/components/CardContainerLogic/CardContainerLogic.stories.ts @@ -21,7 +21,8 @@ type Story = StoryObj export const GenericCard: Story = { args: { sql: 'SELECT * FROM syn22095937.4 order by authors asc', - limit: 2, + initialLimit: 2, + limit: 5, type: GENERIC_CARD, genericCardSchema: { type: PUBLICATION, diff --git a/packages/synapse-react-client/src/components/CardContainerLogic/CardContainerLogic.tsx b/packages/synapse-react-client/src/components/CardContainerLogic/CardContainerLogic.tsx index 810795336c..ae354f6501 100644 --- a/packages/synapse-react-client/src/components/CardContainerLogic/CardContainerLogic.tsx +++ b/packages/synapse-react-client/src/components/CardContainerLogic/CardContainerLogic.tsx @@ -135,6 +135,7 @@ export type CardConfiguration = { type: string hasInternalLink?: boolean iconOptions?: IconOptions + initialLimit?: number } & CommonCardProps export type CardContainerLogicProps = {