From 9f87aa0ded738eea8aade7ca07b5ab76abcd75fd Mon Sep 17 00:00:00 2001 From: Robert Kenny Date: Wed, 27 Nov 2024 16:20:35 +0000 Subject: [PATCH 1/5] concept pages link to works search by label --- content/webapp/pages/concepts/[conceptId].tsx | 236 ++++++++++++++---- 1 file changed, 182 insertions(+), 54 deletions(-) diff --git a/content/webapp/pages/concepts/[conceptId].tsx b/content/webapp/pages/concepts/[conceptId].tsx index 46ef811d1a..fee98a4e64 100644 --- a/content/webapp/pages/concepts/[conceptId].tsx +++ b/content/webapp/pages/concepts/[conceptId].tsx @@ -149,21 +149,25 @@ type ImagesTabPanelProps = { id: string; link: LinkProps; results: ReturnedResults; + totalResults: number; }; const ImagesTabPanel: FunctionComponent = ({ id, link, results, + totalResults, }) => { return (
- + {totalResults > 0 && ( + + )}
); @@ -173,22 +177,26 @@ type WorksTabPanelProps = { id: string; link: LinkProps; results: ReturnedResults; + totalResults: number; }; const WorksTabPanel: FunctionComponent = ({ id, link, results, + totalResults, }) => { return (
- + {totalResults > 0 && ( + + )}
@@ -206,12 +214,14 @@ type PageSectionDefinition = { id: string; link: LinkProps; results: ReturnedResults; + totalResults: number; }; }; type PageSectionDefinitionProps = { tabId: string; resultsGroup: ReturnedResults | undefined; tabLabelText: string; + totalResults: number; link: LinkProps; }; @@ -219,6 +229,7 @@ function toPageSectionDefinition({ tabId, resultsGroup, tabLabelText, + totalResults, link, }: PageSectionDefinitionProps): PageSectionDefinition | undefined { return resultsGroup?.totalResults @@ -231,7 +242,7 @@ function toPageSectionDefinition({ totalResults: resultsGroup.totalResults, }), }, - panel: { id: tabId, link, results: resultsGroup }, + panel: { id: tabId, link, results: resultsGroup, totalResults }, } : undefined; } @@ -240,6 +251,7 @@ type SectionData = { label: string; works: ReturnedResults | undefined; images: ReturnedResults | undefined; + totalResults: { works: number; images: number }; }; type SectionsData = { @@ -275,6 +287,7 @@ export const ConceptPage: NextPage = ({ tabId, resultsGroup: data.works, tabLabelText: data.label, + totalResults: data.totalResults.works, link: toWorksLink( linkParams(tabId, conceptResponse), linkSources[tabId] @@ -291,10 +304,12 @@ export const ConceptPage: NextPage = ({ const tabId = `images${relationship .charAt(0) .toUpperCase()}${relationship.slice(1)}`; + return toPageSectionDefinition({ tabId, resultsGroup: sectionsData[relationship].images, tabLabelText: sectionsData[relationship].label, + totalResults: sectionsData[relationship].totalResults.works, link: toImagesLink( linkParams(tabId, conceptResponse), `${linkSources[tabId]}_${pathname}` as ImagesLinkSource @@ -454,37 +469,67 @@ export const getServerSideProps: GetServerSideProps< ); } - const filterByConceptId = serverData.toggles?.conceptsById?.value; - const queryParams = filterByConceptId ? queryParamsById : queryParamsByLabel; - - const getConceptWorks = (sectionName: string) => - getWorks({ - params: queryParams(sectionName, conceptResponse), - toggles: serverData.toggles, - pageSize: 5, - }); + const conceptsById = serverData.toggles?.conceptsById?.value; + + const getConceptDocs = { + works: { + byId: (sectionName: string) => + getWorks({ + params: queryParamsById(sectionName, conceptResponse), + toggles: serverData.toggles, + pageSize: 5, + }), + byLabel: (sectionName: string) => + getWorks({ + params: queryParamsByLabel(sectionName, conceptResponse), + toggles: serverData.toggles, + pageSize: 5, + }), + }, + images: { + byId: (sectionName: string) => + getImages({ + params: queryParamsById(sectionName, conceptResponse), + toggles: serverData.toggles, + pageSize: 5, + }), + byLabel: (sectionName: string) => + getImages({ + params: queryParamsByLabel(sectionName, conceptResponse), + toggles: serverData.toggles, + pageSize: 5, + }), + }, + }; - const getConceptImages = (sectionName: string) => - getImages({ - params: queryParams(sectionName, conceptResponse), - toggles: serverData.toggles, - pageSize: 10, - }); + const worksAboutPromiseById = getConceptDocs.works.byId('worksAbout'); + const imagesAboutPromiseById = getConceptDocs.images.byId('imagesAbout'); - const worksAboutPromise = getConceptWorks('worksAbout'); - const imagesAboutPromise = getConceptImages('imagesAbout'); + const worksAboutPromiseByLabel = getConceptDocs.works.byLabel('worksAbout'); + const imagesAboutPromiseByLabel = + getConceptDocs.images.byLabel('imagesAbout'); // Only Genres can have works or images "in" them // so don't bother executing this query for other types // just pretend that we have. - const worksInPromise = + const worksInPromiseById = + conceptResponse.type === 'Genre' + ? getConceptDocs.works.byId('worksIn') + : Promise.resolve(emptyWorkResults); + + const imagesInPromiseById = conceptResponse.type === 'Genre' - ? getConceptWorks('worksIn') + ? getConceptDocs.images.byId('imagesIn') + : Promise.resolve(emptyImageResults); + + const worksInPromiseByLabel = + conceptResponse.type === 'Genre' + ? getConceptDocs.works.byLabel('worksIn') : Promise.resolve(emptyWorkResults); - const imagesInPromise = + const imagesInPromiseByLabel = conceptResponse.type === 'Genre' - ? getConceptImages('imagesIn') + ? getConceptDocs.images.byLabel('imagesIn') : Promise.resolve(emptyImageResults); // Genres cannot be creators of works or images. @@ -494,57 +539,128 @@ export const getServerSideProps: GetServerSideProps< // the contributor fields. // Therefore, we can only really be certain that a Concept // is not (and should never be) a contributor when it is a genre. - const worksByPromise = + const worksByPromiseById = + conceptResponse.type !== 'Genre' + ? getConceptDocs.works.byId('worksBy') + : Promise.resolve(emptyWorkResults); + + const imagesByPromiseById = + conceptResponse.type !== 'Genre' + ? getConceptDocs.images.byId('imagesBy') + : Promise.resolve(emptyImageResults); + + const worksByPromiseByLabel = conceptResponse.type !== 'Genre' - ? getConceptWorks('worksBy') + ? getConceptDocs.works.byLabel('worksBy') : Promise.resolve(emptyWorkResults); - const imagesByPromise = + const imagesByPromiseByLabel = conceptResponse.type !== 'Genre' - ? getConceptImages('imagesBy') + ? getConceptDocs.images.byLabel('imagesBy') : Promise.resolve(emptyImageResults); const [ - worksAboutResponse, - worksByResponse, - worksInResponse, - imagesAboutResponse, - imagesByResponse, - imagesInResponse, + worksAboutResponseById, + worksByResponseById, + worksInResponseById, + imagesAboutResponseById, + imagesByResponseById, + imagesInResponseById, + + worksAboutResponseByLabel, + worksByResponseByLabel, + worksInResponseByLabel, + imagesAboutResponseByLabel, + imagesByResponseByLabel, + imagesInResponseByLabel, ] = await Promise.all([ - worksAboutPromise, - worksByPromise, - worksInPromise, - imagesAboutPromise, - imagesByPromise, - imagesInPromise, + worksAboutPromiseById, + worksByPromiseById, + worksInPromiseById, + imagesAboutPromiseById, + imagesByPromiseById, + imagesInPromiseById, + + worksAboutPromiseByLabel, + worksByPromiseByLabel, + worksInPromiseByLabel, + imagesAboutPromiseByLabel, + imagesByPromiseByLabel, + imagesInPromiseByLabel, ]); const worksAbout = getQueryResults({ categoryName: 'works about', - queryResults: worksAboutResponse, + queryResults: worksAboutResponseById, }); const worksBy = getQueryResults({ categoryName: 'works by', - queryResults: worksByResponse, + queryResults: worksByResponseById, }); const imagesAbout = getQueryResults({ categoryName: 'images about', - queryResults: imagesAboutResponse, + queryResults: imagesAboutResponseById, }); const imagesBy = getQueryResults({ categoryName: 'images by', - queryResults: imagesByResponse, + queryResults: imagesByResponseById, }); const worksIn = getQueryResults({ categoryName: 'works in', - queryResults: worksInResponse, + queryResults: worksInResponseById, }); const imagesIn = getQueryResults({ categoryName: 'images in', - queryResults: imagesInResponse, + queryResults: imagesInResponseById, }); + const getLabelTotals = () => { + const worksAboutByLabelTotalResults = getQueryResults({ + categoryName: 'works about', + queryResults: worksAboutResponseByLabel, + })?.totalResults; + const worksByByLabelTotalResults = getQueryResults({ + categoryName: 'works by', + queryResults: worksByResponseByLabel, + })?.totalResults; + const imagesAboutByLabelTotalResults = getQueryResults({ + categoryName: 'images about', + queryResults: imagesAboutResponseByLabel, + })?.totalResults; + const imagesByByLabelTotalResults = getQueryResults({ + categoryName: 'images by', + queryResults: imagesByResponseByLabel, + })?.totalResults; + const worksInByLabelTotalResults = getQueryResults({ + categoryName: 'works in', + queryResults: worksInResponseByLabel, + })?.totalResults; + const imagesInByLabelTotalResults = getQueryResults({ + categoryName: 'images in', + queryResults: imagesInResponseByLabel, + })?.totalResults; + + return { + worksAbout: worksAboutByLabelTotalResults, + worksBy: worksByByLabelTotalResults, + worksIn: worksInByLabelTotalResults, + imagesAbout: imagesAboutByLabelTotalResults, + imagesBy: imagesByByLabelTotalResults, + imagesIn: imagesInByLabelTotalResults, + }; + }; + + const totalResults = conceptsById + ? { + worksAbout: worksAbout?.totalResults, + worksBy: worksBy?.totalResults, + worksIn: worksIn?.totalResults, + imagesAbout: imagesAbout?.totalResults, + imagesBy: imagesBy?.totalResults, + imagesIn: imagesIn?.totalResults, + } + : getLabelTotals(); + const apiToolbarLinks = createApiToolbarLinks(conceptResponse); const conceptTypeName = conceptTypeDisplayName(conceptResponse).toLowerCase(); @@ -557,6 +673,10 @@ export const getServerSideProps: GetServerSideProps< pageResults: worksAbout.pageResults.map(toWorkBasic), }, images: imagesAbout, + totalResults: { + works: totalResults.worksAbout, + images: totalResults.imagesAbout, + }, }, by: { label: `By this ${conceptTypeName}`, @@ -565,6 +685,10 @@ export const getServerSideProps: GetServerSideProps< pageResults: worksBy.pageResults.map(toWorkBasic), }, images: imagesBy, + totalResults: { + works: totalResults.worksBy, + images: totalResults.imagesBy, + }, }, in: { label: `Using this ${conceptTypeName}`, @@ -573,6 +697,10 @@ export const getServerSideProps: GetServerSideProps< pageResults: worksIn.pageResults.map(toWorkBasic), }, images: imagesIn, + totalResults: { + works: totalResults.worksIn, + images: totalResults.imagesIn, + }, }, }; From a3e093c4b2abb9ad76705f99b4849bff749961b6 Mon Sep 17 00:00:00 2001 From: Robert Kenny Date: Thu, 28 Nov 2024 15:54:44 +0000 Subject: [PATCH 2/5] oops, use image count in images page section --- content/webapp/pages/concepts/[conceptId].tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content/webapp/pages/concepts/[conceptId].tsx b/content/webapp/pages/concepts/[conceptId].tsx index fee98a4e64..9650a92c71 100644 --- a/content/webapp/pages/concepts/[conceptId].tsx +++ b/content/webapp/pages/concepts/[conceptId].tsx @@ -309,7 +309,7 @@ export const ConceptPage: NextPage = ({ tabId, resultsGroup: sectionsData[relationship].images, tabLabelText: sectionsData[relationship].label, - totalResults: sectionsData[relationship].totalResults.works, + totalResults: sectionsData[relationship].totalResults.images, link: toImagesLink( linkParams(tabId, conceptResponse), `${linkSources[tabId]}_${pathname}` as ImagesLinkSource From 79ab21d23e7f99c3fdd3829b53b570cca487e8c0 Mon Sep 17 00:00:00 2001 From: Robert Kenny Date: Thu, 28 Nov 2024 16:04:07 +0000 Subject: [PATCH 3/5] please tsc --- content/webapp/pages/concepts/[conceptId].tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/content/webapp/pages/concepts/[conceptId].tsx b/content/webapp/pages/concepts/[conceptId].tsx index 9650a92c71..9355e81a59 100644 --- a/content/webapp/pages/concepts/[conceptId].tsx +++ b/content/webapp/pages/concepts/[conceptId].tsx @@ -149,7 +149,7 @@ type ImagesTabPanelProps = { id: string; link: LinkProps; results: ReturnedResults; - totalResults: number; + totalResults: number | undefined; }; const ImagesTabPanel: FunctionComponent = ({ id, @@ -161,7 +161,7 @@ const ImagesTabPanel: FunctionComponent = ({
- {totalResults > 0 && ( + {totalResults && ( ; - totalResults: number; + totalResults: number | undefined; }; const WorksTabPanel: FunctionComponent = ({ id, @@ -190,7 +190,7 @@ const WorksTabPanel: FunctionComponent = ({
- {totalResults > 0 && ( + {totalResults && ( = { id: string; link: LinkProps; results: ReturnedResults; - totalResults: number; + totalResults: number | undefined; }; }; type PageSectionDefinitionProps = { tabId: string; resultsGroup: ReturnedResults | undefined; tabLabelText: string; - totalResults: number; + totalResults: number | undefined; link: LinkProps; }; @@ -251,7 +251,7 @@ type SectionData = { label: string; works: ReturnedResults | undefined; images: ReturnedResults | undefined; - totalResults: { works: number; images: number }; + totalResults: { works: number | undefined; images: number | undefined }; }; type SectionsData = { From 649a18e33f30a0597acc828a7cd73b678f522981 Mon Sep 17 00:00:00 2001 From: Robert Kenny Date: Fri, 29 Nov 2024 12:36:13 +0000 Subject: [PATCH 4/5] fix e2es and displaying 0 accidentally --- content/webapp/pages/concepts/[conceptId].tsx | 8 +++- playwright/test/concept.test.ts | 37 +++++++++---------- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/content/webapp/pages/concepts/[conceptId].tsx b/content/webapp/pages/concepts/[conceptId].tsx index 9355e81a59..3951548d5f 100644 --- a/content/webapp/pages/concepts/[conceptId].tsx +++ b/content/webapp/pages/concepts/[conceptId].tsx @@ -157,11 +157,13 @@ const ImagesTabPanel: FunctionComponent = ({ results, totalResults, }) => { + const showSeeMore = totalResults !== undefined && totalResults > 0; + return (
- {totalResults && ( + {showSeeMore && ( = ({ results, totalResults, }) => { + const showSeeMore = totalResults !== undefined && totalResults > 0; + return (
- {totalResults && ( + {showSeeMore && ( ({ @@ -16,11 +16,11 @@ const test = base.extend<{ await use(armyPage); }, - lithographsPage: async ({ context, page }, use) => { + mohPage: async ({ context, page }, use) => { // A Genre with both works and images using it, but nothing about it. - await concept('fmydsuw2', context, page); - const lithographsPage = new ConceptPage(page, 'type/technique'); - await use(lithographsPage); + await concept('rasp7aye', context, page); + const mohPage = new ConceptPage(page, 'type/technique'); + await use(mohPage); }, songsPage: async ({ context, page }, use) => { @@ -165,28 +165,27 @@ test.describe('a Concept representing a Genre with works and images both about a test.describe('a Concept representing a Genre that is only used as a genre for both works and images', () => { test('has both works and image sections showing records in that genre', async ({ - lithographsPage, + mohPage, }) => { // Both images and works sections exist - await expect(lithographsPage.imagesHeader).toBeVisible(); - await expect(lithographsPage.worksHeader).toBeVisible(); + await expect(mohPage.imagesHeader).toBeVisible(); + await expect(mohPage.worksHeader).toBeVisible(); + // There are no tabs, because there is only one group within each of the two sections - await expect(lithographsPage.worksAboutTab).not.toBeVisible(); - await expect(lithographsPage.worksInTab).not.toBeVisible(); - await expect(lithographsPage.imagesAboutTab).not.toBeVisible(); - await expect(lithographsPage.imagesInTab).not.toBeVisible(); + await expect(mohPage.worksAboutTab).not.toBeVisible(); + await expect(mohPage.worksInTab).not.toBeVisible(); + await expect(mohPage.imagesAboutTab).not.toBeVisible(); + await expect(mohPage.imagesInTab).not.toBeVisible(); - // It has links to filtered searches - await expect(lithographsPage.allWorksLink).toHaveAttribute( + // It has links to filtered searches, (not using encodeURIComponent because the genre includes '+'") + await expect(mohPage.allWorksLink).toHaveAttribute( 'href', - `/search/works?genres.label=${encodeURIComponent('"Lithographs"')}` + `/search/works?genres.label=%22MOH+reports%22` ); - await expect(lithographsPage.allImagesLink).toHaveAttribute( + await expect(mohPage.allImagesLink).toHaveAttribute( 'href', - `/search/images?source.genres.label=${encodeURIComponent( - '"Lithographs"' - )}` + `/search/images?source.genres.label=%22MOH+reports%22` ); }); }); From d9b60e5e4672e745dfbbfb8807259475fd5da8c8 Mon Sep 17 00:00:00 2001 From: Robert Kenny Date: Mon, 2 Dec 2024 08:58:29 +0000 Subject: [PATCH 5/5] Use !! to coerce value to boolean Co-Authored-By: Gareth Estchild --- content/webapp/pages/concepts/[conceptId].tsx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/content/webapp/pages/concepts/[conceptId].tsx b/content/webapp/pages/concepts/[conceptId].tsx index 3951548d5f..48ba416805 100644 --- a/content/webapp/pages/concepts/[conceptId].tsx +++ b/content/webapp/pages/concepts/[conceptId].tsx @@ -157,13 +157,11 @@ const ImagesTabPanel: FunctionComponent = ({ results, totalResults, }) => { - const showSeeMore = totalResults !== undefined && totalResults > 0; - return (
- {showSeeMore && ( + {!!totalResults && ( = ({ results, totalResults, }) => { - const showSeeMore = totalResults !== undefined && totalResults > 0; - return (
- {showSeeMore && ( + {!!totalResults && (