From b3e4ff72dd3ca847597cae49ab9622876a42b07b Mon Sep 17 00:00:00 2001 From: Austen Money Date: Thu, 3 Oct 2024 11:53:31 -0400 Subject: [PATCH 01/12] update combineMetadata param --- context/app/static/js/pages/Sample/Sample.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/context/app/static/js/pages/Sample/Sample.jsx b/context/app/static/js/pages/Sample/Sample.jsx index 914ba98f94..58c584c0fa 100644 --- a/context/app/static/js/pages/Sample/Sample.jsx +++ b/context/app/static/js/pages/Sample/Sample.jsx @@ -37,7 +37,7 @@ function SampleDetail() { const origin_sample = origin_samples[0]; const { mapped_organ } = origin_sample; - const combinedMetadata = combineMetadata(donor, undefined, metadata); + const combinedMetadata = combineMetadata(donor, origin_samples, metadata); const shouldDisplaySection = { summary: true, From aa520210d4f87aa1c3817ca6a6e3ca47a05f7c69 Mon Sep 17 00:00:00 2001 From: Austen Money Date: Thu, 3 Oct 2024 12:03:47 -0400 Subject: [PATCH 02/12] add changelog --- CHANGELOG-fix-metadata-table.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 CHANGELOG-fix-metadata-table.md diff --git a/CHANGELOG-fix-metadata-table.md b/CHANGELOG-fix-metadata-table.md new file mode 100644 index 0000000000..7adc1103fa --- /dev/null +++ b/CHANGELOG-fix-metadata-table.md @@ -0,0 +1 @@ +- Fix issue of sample metadata not showing up in the sample detail page metadata table. \ No newline at end of file From 474a4a8c52494d012f1de2e884f67652ffb2da29 Mon Sep 17 00:00:00 2001 From: Austen Money Date: Tue, 8 Oct 2024 11:08:56 -0400 Subject: [PATCH 03/12] append hubmap IDs to duplicate sample categories --- .../MetadataSection/MetadataSection.tsx | 78 +++++++++++++------ 1 file changed, 55 insertions(+), 23 deletions(-) diff --git a/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx b/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx index 09cd0fdb72..aba7f53578 100644 --- a/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx +++ b/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx @@ -7,11 +7,12 @@ import { useTrackEntityPageEvent } from 'js/components/detailPage/useTrackEntity import { tableToDelimitedString, createDownloadUrl } from 'js/helpers/functions'; import { useMetadataFieldDescriptions } from 'js/hooks/useUBKG'; import { getMetadata, hasMetadata } from 'js/helpers/metadata'; -import { ESEntityType, isDataset } from 'js/components/types'; -import { useProcessedDatasets } from 'js/pages/Dataset/hooks'; +import { Dataset, Donor, ESEntityType, Sample, isDataset, isSample } from 'js/components/types'; +import { ProcessedDatasetInfo, useProcessedDatasets } from 'js/pages/Dataset/hooks'; import { entityIconMap } from 'js/shared-styles/icons/entityIconMap'; import withShouldDisplay from 'js/helpers/withShouldDisplay'; import { sectionIconMap } from 'js/shared-styles/icons/sectionIconMap'; +import { SearchHit } from '@elastic/elasticsearch/lib/api/types'; import { DownloadIcon, StyledWhiteBackgroundIconButton } from '../MetadataTable/style'; import MetadataTabs from '../multi-assay/MultiAssayMetadataTabs'; import { Columns, defaultTSVColumns } from './columns'; @@ -147,6 +148,57 @@ function getEntityIcon(entity: { entity_type: ESEntityType; is_component?: boole return entityIconMap[entity.entity_type]; } +function getEntityLabel(entity: ProcessedDatasetInfo | Donor | Sample, sampleCategoryCounts: Record) { + if (isSample(entity)) { + // If samples have the same category, add the HuBMAP ID to the label + if (sampleCategoryCounts[entity.sample_category] > 1) { + return `${entity.sample_category} ${entity.hubmap_id}`; + } + return entity.sample_category; + } + if (isDataset(entity)) { + return entity.assay_display_name; + } + return entity.entity_type; +} + +function getTableEntities( + entity: Dataset, + datasetsWithMetadata: Required>[], + fieldDescriptions: Record, +) { + const { donor, source_samples } = entity; + + const entitiesWithMetadata = [entity, ...datasetsWithMetadata.map((d) => d._source), ...source_samples, donor].filter( + (e) => hasMetadata({ targetEntityType: e.entity_type, currentEntity: e }), + ); + + // Check whether there are multiple samples with the same sample category + const sampleCategoryCounts: Record = {}; + entitiesWithMetadata.forEach((e) => { + if (isSample(e)) { + sampleCategoryCounts[e.sample_category] = (sampleCategoryCounts[e.sample_category] || 0) + 1; + } + }); + + return entitiesWithMetadata.map((e) => { + const label = getEntityLabel(e, sampleCategoryCounts); + return { + uuid: e.uuid, + label, + icon: getEntityIcon(e), + tableRows: buildTableData( + getMetadata({ + targetEntityType: e.entity_type, + currentEntity: e, + }), + fieldDescriptions, + { hubmap_id: e.hubmap_id, label }, + ), + }; + }); +} + interface MetadataProps { metadata?: Record; } @@ -165,27 +217,7 @@ function Metadata({ metadata }: MetadataProps) { return null; } - const { donor, source_samples } = entity; - - const entities = [entity, ...datasetsWithMetadata.map((d) => d._source), ...source_samples, donor] - .filter((e) => hasMetadata({ targetEntityType: e.entity_type, currentEntity: e })) - .map((e) => { - const label = isDataset(e) ? e.assay_display_name : e.entity_type; - return { - uuid: e.uuid, - label, - icon: getEntityIcon(e), - tableRows: buildTableData( - getMetadata({ - targetEntityType: e.entity_type, - currentEntity: e, - }), - fieldDescriptions, - { hubmap_id: e.hubmap_id, label }, - ), - }; - }); - + const entities = getTableEntities(entity, datasetsWithMetadata, fieldDescriptions); const allTableRows = entities.map((d) => d.tableRows).flat(); return ( From e65cfea515df17ffad09f3643ae14608a180ebe6 Mon Sep 17 00:00:00 2001 From: Austen Money Date: Wed, 9 Oct 2024 17:22:31 -0400 Subject: [PATCH 04/12] use metadata from provenance table --- .../MetadataSection/MetadataSection.tsx | 76 +++++++++---------- .../MultiAssayMetadataTabs.tsx | 2 +- context/app/static/js/hooks/useEntityData.ts | 6 +- .../app/static/js/pages/Dataset/Dataset.tsx | 15 +++- context/app/static/js/pages/Donor/Donor.jsx | 29 ++++--- context/app/static/js/pages/Sample/Sample.jsx | 40 +++++----- 6 files changed, 91 insertions(+), 77 deletions(-) diff --git a/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx b/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx index aba7f53578..ba78691419 100644 --- a/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx +++ b/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx @@ -6,18 +6,16 @@ import { CollapsibleDetailPageSection } from 'js/components/detailPage/DetailPag import { useTrackEntityPageEvent } from 'js/components/detailPage/useTrackEntityPageEvent'; import { tableToDelimitedString, createDownloadUrl } from 'js/helpers/functions'; import { useMetadataFieldDescriptions } from 'js/hooks/useUBKG'; -import { getMetadata, hasMetadata } from 'js/helpers/metadata'; +import { getMetadata } from 'js/helpers/metadata'; import { Dataset, Donor, ESEntityType, Sample, isDataset, isSample } from 'js/components/types'; import { ProcessedDatasetInfo, useProcessedDatasets } from 'js/pages/Dataset/hooks'; import { entityIconMap } from 'js/shared-styles/icons/entityIconMap'; import withShouldDisplay from 'js/helpers/withShouldDisplay'; import { sectionIconMap } from 'js/shared-styles/icons/sectionIconMap'; -import { SearchHit } from '@elastic/elasticsearch/lib/api/types'; import { DownloadIcon, StyledWhiteBackgroundIconButton } from '../MetadataTable/style'; import MetadataTabs from '../multi-assay/MultiAssayMetadataTabs'; import { Columns, defaultTSVColumns } from './columns'; import { SectionDescription } from '../ProcessedData/ProcessedDataset/SectionDescription'; -import MetadataTable from '../MetadataTable'; import { nodeIcons } from '../DatasetRelationships/nodeTypes'; export function getDescription( @@ -63,11 +61,6 @@ export function buildTableData( ); } -function useTableData(tableData: Record) { - const { data: fieldDescriptions } = useMetadataFieldDescriptions(); - return buildTableData(tableData, fieldDescriptions); -} - interface TableRow { key: string; value: string; @@ -125,16 +118,6 @@ function MetadataWrapper({ allTableRows, tsvColumns = defaultTSVColumns, childre ); } -function SingleMetadata({ metadata }: { metadata: Record }) { - const tableRows = useTableData(metadata); - - return ( - - - - ); -} - function getEntityIcon(entity: { entity_type: ESEntityType; is_component?: boolean; processing?: string }) { if (isDataset(entity)) { if (entity.is_component) { @@ -163,29 +146,23 @@ function getEntityLabel(entity: ProcessedDatasetInfo | Donor | Sample, sampleCat } function getTableEntities( - entity: Dataset, - datasetsWithMetadata: Required>[], + entities: (Donor | Dataset | Sample)[], + uuid: string, fieldDescriptions: Record, ) { - const { donor, source_samples } = entity; - - const entitiesWithMetadata = [entity, ...datasetsWithMetadata.map((d) => d._source), ...source_samples, donor].filter( - (e) => hasMetadata({ targetEntityType: e.entity_type, currentEntity: e }), - ); - // Check whether there are multiple samples with the same sample category const sampleCategoryCounts: Record = {}; - entitiesWithMetadata.forEach((e) => { + entities.forEach((e) => { if (isSample(e)) { sampleCategoryCounts[e.sample_category] = (sampleCategoryCounts[e.sample_category] || 0) + 1; } }); - return entitiesWithMetadata.map((e) => { + const sortedEntities = entities.map((e) => { const label = getEntityLabel(e, sampleCategoryCounts); return { uuid: e.uuid, - label, + label: label ?? '', icon: getEntityIcon(e), tableRows: buildTableData( getMetadata({ @@ -195,37 +172,56 @@ function getTableEntities( fieldDescriptions, { hubmap_id: e.hubmap_id, label }, ), + entity_type: e.entity_type, + hubmap_id: e.hubmap_id, }; }); + + sortedEntities.sort((a, b) => { + // Current entity at the front + if (a.uuid === uuid) return -1; + if (b.uuid === uuid) return 1; + + // Then donors + if (a.entity_type === 'Donor' && b.entity_type !== 'Donor') return -1; + if (b.entity_type === 'Donor' && a.entity_type !== 'Donor') return 1; + + // Then samples, with unique categories first + const aIsSampleWithoutHubmapId = a.entity_type === 'Sample' && !a.label.includes(a.hubmap_id); + const bIsSampleWithoutHubmapId = b.entity_type === 'Sample' && !b.label.includes(b.hubmap_id); + if (aIsSampleWithoutHubmapId && !bIsSampleWithoutHubmapId) return -1; + if (bIsSampleWithoutHubmapId && !aIsSampleWithoutHubmapId) return 1; + + return a.label.localeCompare(b.label); + }); + + return sortedEntities; } interface MetadataProps { - metadata?: Record; + entities: (Donor | Dataset | Sample)[]; } -function Metadata({ metadata }: MetadataProps) { +function Metadata({ entities }: MetadataProps) { const { searchHits: datasetsWithMetadata, isLoading } = useProcessedDatasets(true); const { data: fieldDescriptions } = useMetadataFieldDescriptions(); - - const { entity } = useFlaskDataContext(); - - if (!isDataset(entity)) { - return ; - } + const { + entity: { uuid }, + } = useFlaskDataContext(); if (isLoading || !datasetsWithMetadata) { return null; } - const entities = getTableEntities(entity, datasetsWithMetadata, fieldDescriptions); - const allTableRows = entities.map((d) => d.tableRows).flat(); + const tableEntities = getTableEntities(entities, uuid, fieldDescriptions); + const allTableRows = tableEntities.map((d) => d.tableRows).flat(); return ( - + ); } diff --git a/context/app/static/js/components/detailPage/multi-assay/MultiAssayMetadataTabs/MultiAssayMetadataTabs.tsx b/context/app/static/js/components/detailPage/multi-assay/MultiAssayMetadataTabs/MultiAssayMetadataTabs.tsx index e0af51e4a7..5e176c90ed 100644 --- a/context/app/static/js/components/detailPage/multi-assay/MultiAssayMetadataTabs/MultiAssayMetadataTabs.tsx +++ b/context/app/static/js/components/detailPage/multi-assay/MultiAssayMetadataTabs/MultiAssayMetadataTabs.tsx @@ -38,7 +38,7 @@ function MetadataTabs({ entities }: { entities: MultiAssayEntityWithTableRows[] return ( - + {entities.map(({ label, uuid, icon }, index) => ( ))} diff --git a/context/app/static/js/hooks/useEntityData.ts b/context/app/static/js/hooks/useEntityData.ts index a08b686409..e49c2003b5 100644 --- a/context/app/static/js/hooks/useEntityData.ts +++ b/context/app/static/js/hooks/useEntityData.ts @@ -1,6 +1,6 @@ import { useMemo } from 'react'; import { useSearchHits } from 'js/hooks/useSearchData'; -import { Entity } from 'js/components/types'; +import { Dataset, Donor, Entity, Sample } from 'js/components/types'; export const useEntityQuery = (uuid: string | string[], source?: string[]) => { return useMemo( @@ -20,10 +20,10 @@ export function useEntityData(uuid: string, source?: string[]): [Entity, boolean return [searchHits[0]?._source, isLoading]; } -export function useEntitiesData(uuids: string[], source?: string[]): [Entity[], boolean] { +export function useEntitiesData(uuids: string[], source?: string[]): [(Donor | Sample | Dataset)[], boolean] { const query = useEntityQuery(uuids, source); - const { searchHits, isLoading } = useSearchHits(query); + const { searchHits, isLoading } = useSearchHits(query); return [searchHits.map((hit) => hit._source), isLoading]; } diff --git a/context/app/static/js/pages/Dataset/Dataset.tsx b/context/app/static/js/pages/Dataset/Dataset.tsx index 37a83738a2..24f9cf3f00 100644 --- a/context/app/static/js/pages/Dataset/Dataset.tsx +++ b/context/app/static/js/pages/Dataset/Dataset.tsx @@ -29,6 +29,8 @@ import useTrackID from 'js/hooks/useTrackID'; import { InternalLink } from 'js/shared-styles/Links'; import OrganIcon from 'js/shared-styles/icons/OrganIcon'; +import { useEntitiesData } from 'js/hooks/useEntityData'; +import { hasMetadata } from 'js/helpers/metadata'; import { useProcessedDatasets, useProcessedDatasetsSections, useRedirectAlert } from './hooks'; interface SummaryDataChildrenProps { @@ -94,8 +96,15 @@ function DatasetDetail({ assayMetadata }: EntityDetailProps) { is_component, assay_modality, processing, + ancestor_ids, } = assayMetadata; + const [entities, loadingEntities] = useEntitiesData([uuid, ...ancestor_ids]); + + const entitiesWithMetadata = entities.filter((e) => + hasMetadata({ targetEntityType: e.entity_type, currentEntity: e }), + ); + useRedirectAlert(); const origin_sample = origin_samples[0]; @@ -122,6 +131,10 @@ function DatasetDetail({ assayMetadata }: EntityDetailProps) { const { shouldDisplay: shouldDisplayRelationships } = useDatasetRelationships(uuid, processing); + if (loadingEntities) { + return null; + } + return ( ds._id) ?? []}> @@ -146,7 +159,7 @@ function DatasetDetail({ assayMetadata }: EntityDetailProps) { > - + diff --git a/context/app/static/js/pages/Donor/Donor.jsx b/context/app/static/js/pages/Donor/Donor.jsx index 8de24e9e90..0334ee6d80 100644 --- a/context/app/static/js/pages/Donor/Donor.jsx +++ b/context/app/static/js/pages/Donor/Donor.jsx @@ -12,21 +12,20 @@ import useTrackID from 'js/hooks/useTrackID'; import MetadataSection from 'js/components/detailPage/MetadataSection'; function DonorDetail() { + const { entity } = useFlaskDataContext(); const { - entity: { - uuid, - protocol_url, - hubmap_id, - entity_type, - mapped_metadata = {}, - created_timestamp, - last_modified_timestamp, - description, - group_name, - created_by_user_displayname, - created_by_user_email, - }, - } = useFlaskDataContext(); + uuid, + protocol_url, + hubmap_id, + entity_type, + mapped_metadata = {}, + created_timestamp, + last_modified_timestamp, + description, + group_name, + created_by_user_displayname, + created_by_user_email, + } = entity; const shouldDisplaySection = { summary: true, @@ -53,7 +52,7 @@ function DonorDetail() { description={description} group_name={group_name} /> - {shouldDisplaySection.metadata && } + {shouldDisplaySection.metadata && } {shouldDisplaySection.protocols && } diff --git a/context/app/static/js/pages/Sample/Sample.jsx b/context/app/static/js/pages/Sample/Sample.jsx index 58c584c0fa..17926c1dc5 100644 --- a/context/app/static/js/pages/Sample/Sample.jsx +++ b/context/app/static/js/pages/Sample/Sample.jsx @@ -11,41 +11,43 @@ import SummaryItem from 'js/components/detailPage/summary/SummaryItem'; import DetailLayout from 'js/components/detailPage/DetailLayout'; import SampleTissue from 'js/components/detailPage/SampleTissue'; import { DetailContext } from 'js/components/detailPage/DetailContext'; +import { hasMetadata } from 'js/helpers/metadata'; import DerivedDatasetsSection from 'js/components/detailPage/derivedEntities/DerivedDatasetsSection'; -import { combineMetadata } from 'js/pages/utils/entity-utils'; import useTrackID from 'js/hooks/useTrackID'; import MetadataSection from 'js/components/detailPage/MetadataSection'; +import { useEntitiesData } from 'js/hooks/useEntityData'; function SampleDetail() { + const { entity } = useFlaskDataContext(); const { - entity: { - uuid, - donor, - protocol_url, - sample_category, - origin_samples, - hubmap_id, - entity_type, - metadata, - descendant_counts, - }, - } = useFlaskDataContext(); + uuid, + protocol_url, + sample_category, + origin_samples, + hubmap_id, + entity_type, + descendant_counts, + ancestor_ids, + } = entity; + + const [entities, loadingEntities] = useEntitiesData([uuid, ...ancestor_ids]); + const entitiesWithMetadata = entities.filter((e) => + hasMetadata({ targetEntityType: e.entity_type, currentEntity: e }), + ); // TODO: Update design to reflect samples and datasets which have multiple origin samples with different organs. const origin_sample = origin_samples[0]; const { mapped_organ } = origin_sample; - const combinedMetadata = combineMetadata(donor, origin_samples, metadata); - const shouldDisplaySection = { summary: true, 'derived-data': Boolean(descendant_counts?.entity_type?.Dataset > 0), tissue: true, provenance: true, protocols: Boolean(protocol_url), - metadata: Boolean(Object.keys(combinedMetadata).length), + metadata: Boolean(entitiesWithMetadata.length), attribution: true, }; @@ -53,6 +55,10 @@ function SampleDetail() { const detailContext = useMemo(() => ({ hubmap_id, uuid }), [hubmap_id, uuid]); + if (loadingEntities) { + return null; + } + return ( @@ -70,7 +76,7 @@ function SampleDetail() { {shouldDisplaySection.protocols && } - + From 9503fd047c47b1d0f32c8b006aa10730408de17f Mon Sep 17 00:00:00 2001 From: Austen Money Date: Wed, 9 Oct 2024 17:46:26 -0400 Subject: [PATCH 05/12] fix tab widths --- .../MultiAssayMetadataTabs/MultiAssayMetadataTabs.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/context/app/static/js/components/detailPage/multi-assay/MultiAssayMetadataTabs/MultiAssayMetadataTabs.tsx b/context/app/static/js/components/detailPage/multi-assay/MultiAssayMetadataTabs/MultiAssayMetadataTabs.tsx index 5e176c90ed..8148676cb8 100644 --- a/context/app/static/js/components/detailPage/multi-assay/MultiAssayMetadataTabs/MultiAssayMetadataTabs.tsx +++ b/context/app/static/js/components/detailPage/multi-assay/MultiAssayMetadataTabs/MultiAssayMetadataTabs.tsx @@ -38,7 +38,7 @@ function MetadataTabs({ entities }: { entities: MultiAssayEntityWithTableRows[] return ( - + 4 ? 'scrollable' : 'fullWidth'}> {entities.map(({ label, uuid, icon }, index) => ( ))} From 0e4720d99e9bf0f746022076ddc1b5f0858038d8 Mon Sep 17 00:00:00 2001 From: Austen Money Date: Wed, 9 Oct 2024 17:51:57 -0400 Subject: [PATCH 06/12] update labels --- .../components/detailPage/MetadataSection/MetadataSection.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx b/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx index ba78691419..069a7b31d6 100644 --- a/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx +++ b/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx @@ -135,7 +135,7 @@ function getEntityLabel(entity: ProcessedDatasetInfo | Donor | Sample, sampleCat if (isSample(entity)) { // If samples have the same category, add the HuBMAP ID to the label if (sampleCategoryCounts[entity.sample_category] > 1) { - return `${entity.sample_category} ${entity.hubmap_id}`; + return `${entity.sample_category} (${entity.hubmap_id})`; } return entity.sample_category; } From a900ee9340246050d6c7acf2ecf323a4b16fe949 Mon Sep 17 00:00:00 2001 From: Austen Money Date: Thu, 10 Oct 2024 10:34:26 -0400 Subject: [PATCH 07/12] separate entity sorting --- .../MetadataSection/MetadataSection.tsx | 93 ++++++++++++------- 1 file changed, 57 insertions(+), 36 deletions(-) diff --git a/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx b/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx index 069a7b31d6..eac6d6502e 100644 --- a/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx +++ b/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx @@ -9,7 +9,7 @@ import { useMetadataFieldDescriptions } from 'js/hooks/useUBKG'; import { getMetadata } from 'js/helpers/metadata'; import { Dataset, Donor, ESEntityType, Sample, isDataset, isSample } from 'js/components/types'; import { ProcessedDatasetInfo, useProcessedDatasets } from 'js/pages/Dataset/hooks'; -import { entityIconMap } from 'js/shared-styles/icons/entityIconMap'; +import { MUIIcon, entityIconMap } from 'js/shared-styles/icons/entityIconMap'; import withShouldDisplay from 'js/helpers/withShouldDisplay'; import { sectionIconMap } from 'js/shared-styles/icons/sectionIconMap'; import { DownloadIcon, StyledWhiteBackgroundIconButton } from '../MetadataTable/style'; @@ -131,7 +131,44 @@ function getEntityIcon(entity: { entity_type: ESEntityType; is_component?: boole return entityIconMap[entity.entity_type]; } -function getEntityLabel(entity: ProcessedDatasetInfo | Donor | Sample, sampleCategoryCounts: Record) { +interface sortEntitiesProps { + tableEntities: { + uuid: string; + label: string; + icon: MUIIcon; + tableRows: TableRows; + entity_type: ESEntityType; + hubmap_id: string; + }[]; + uuid: string; +} + +function sortEntities({ tableEntities, uuid }: sortEntitiesProps) { + return tableEntities.sort((a, b) => { + // Current entity at the front + if (a.uuid === uuid) return -1; + if (b.uuid === uuid) return 1; + + // Then donors + if (a.entity_type === 'Donor' && b.entity_type !== 'Donor') return -1; + if (b.entity_type === 'Donor' && a.entity_type !== 'Donor') return 1; + + // Then samples, with unique categories first + const aIsSampleWithoutHubmapId = a.entity_type === 'Sample' && !a.label.includes(a.hubmap_id); + const bIsSampleWithoutHubmapId = b.entity_type === 'Sample' && !b.label.includes(b.hubmap_id); + if (aIsSampleWithoutHubmapId && !bIsSampleWithoutHubmapId) return -1; + if (bIsSampleWithoutHubmapId && !aIsSampleWithoutHubmapId) return 1; + + return a.label.localeCompare(b.label); + }); +} + +interface getEntityLabelProps { + entity: ProcessedDatasetInfo | Donor | Sample; + sampleCategoryCounts: Record; +} + +function getEntityLabel({ entity, sampleCategoryCounts }: getEntityLabelProps) { if (isSample(entity)) { // If samples have the same category, add the HuBMAP ID to the label if (sampleCategoryCounts[entity.sample_category] > 1) { @@ -145,11 +182,13 @@ function getEntityLabel(entity: ProcessedDatasetInfo | Donor | Sample, sampleCat return entity.entity_type; } -function getTableEntities( - entities: (Donor | Dataset | Sample)[], - uuid: string, - fieldDescriptions: Record, -) { +interface getTableEntitiesProps { + entities: (Donor | Dataset | Sample)[]; + uuid: string; + fieldDescriptions: Record; +} + +function getTableEntities({ entities, uuid, fieldDescriptions }: getTableEntitiesProps) { // Check whether there are multiple samples with the same sample category const sampleCategoryCounts: Record = {}; entities.forEach((e) => { @@ -158,44 +197,26 @@ function getTableEntities( } }); - const sortedEntities = entities.map((e) => { - const label = getEntityLabel(e, sampleCategoryCounts); + const tableEntities = entities.map((entity) => { + const label = getEntityLabel({ entity, sampleCategoryCounts }); return { - uuid: e.uuid, + uuid: entity.uuid, label: label ?? '', - icon: getEntityIcon(e), + icon: getEntityIcon(entity), tableRows: buildTableData( getMetadata({ - targetEntityType: e.entity_type, - currentEntity: e, + targetEntityType: entity.entity_type, + currentEntity: entity, }), fieldDescriptions, - { hubmap_id: e.hubmap_id, label }, + { hubmap_id: entity.hubmap_id, label }, ), - entity_type: e.entity_type, - hubmap_id: e.hubmap_id, + entity_type: entity.entity_type, + hubmap_id: entity.hubmap_id, }; }); - sortedEntities.sort((a, b) => { - // Current entity at the front - if (a.uuid === uuid) return -1; - if (b.uuid === uuid) return 1; - - // Then donors - if (a.entity_type === 'Donor' && b.entity_type !== 'Donor') return -1; - if (b.entity_type === 'Donor' && a.entity_type !== 'Donor') return 1; - - // Then samples, with unique categories first - const aIsSampleWithoutHubmapId = a.entity_type === 'Sample' && !a.label.includes(a.hubmap_id); - const bIsSampleWithoutHubmapId = b.entity_type === 'Sample' && !b.label.includes(b.hubmap_id); - if (aIsSampleWithoutHubmapId && !bIsSampleWithoutHubmapId) return -1; - if (bIsSampleWithoutHubmapId && !aIsSampleWithoutHubmapId) return 1; - - return a.label.localeCompare(b.label); - }); - - return sortedEntities; + return sortEntities({ tableEntities, uuid }); } interface MetadataProps { @@ -213,7 +234,7 @@ function Metadata({ entities }: MetadataProps) { return null; } - const tableEntities = getTableEntities(entities, uuid, fieldDescriptions); + const tableEntities = getTableEntities({ entities, uuid, fieldDescriptions }); const allTableRows = tableEntities.map((d) => d.tableRows).flat(); return ( From bfdf1b3088ee60a65cd4ff8b34dfd70ab42481fb Mon Sep 17 00:00:00 2001 From: Austen Money Date: Thu, 10 Oct 2024 11:42:18 -0400 Subject: [PATCH 08/12] add tests, move to utils file --- .../MetadataSection/MetadataSection.tsx | 154 +-------------- .../MetadataSection/MetadataTable.spec.ts | 79 -------- .../detailPage/MetadataSection/utils.spec.ts | 179 ++++++++++++++++++ .../detailPage/MetadataSection/utils.ts | 152 +++++++++++++++ 4 files changed, 335 insertions(+), 229 deletions(-) delete mode 100644 context/app/static/js/components/detailPage/MetadataSection/MetadataTable.spec.ts create mode 100644 context/app/static/js/components/detailPage/MetadataSection/utils.spec.ts create mode 100644 context/app/static/js/components/detailPage/MetadataSection/utils.ts diff --git a/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx b/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx index eac6d6502e..d5491814d2 100644 --- a/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx +++ b/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx @@ -6,60 +6,15 @@ import { CollapsibleDetailPageSection } from 'js/components/detailPage/DetailPag import { useTrackEntityPageEvent } from 'js/components/detailPage/useTrackEntityPageEvent'; import { tableToDelimitedString, createDownloadUrl } from 'js/helpers/functions'; import { useMetadataFieldDescriptions } from 'js/hooks/useUBKG'; -import { getMetadata } from 'js/helpers/metadata'; -import { Dataset, Donor, ESEntityType, Sample, isDataset, isSample } from 'js/components/types'; -import { ProcessedDatasetInfo, useProcessedDatasets } from 'js/pages/Dataset/hooks'; -import { MUIIcon, entityIconMap } from 'js/shared-styles/icons/entityIconMap'; +import { Dataset, Donor, Sample, isDataset } from 'js/components/types'; +import { useProcessedDatasets } from 'js/pages/Dataset/hooks'; import withShouldDisplay from 'js/helpers/withShouldDisplay'; import { sectionIconMap } from 'js/shared-styles/icons/sectionIconMap'; +import { getTableEntities } from 'js/components/detailPage/MetadataSection/utils'; import { DownloadIcon, StyledWhiteBackgroundIconButton } from '../MetadataTable/style'; import MetadataTabs from '../multi-assay/MultiAssayMetadataTabs'; -import { Columns, defaultTSVColumns } from './columns'; import { SectionDescription } from '../ProcessedData/ProcessedDataset/SectionDescription'; -import { nodeIcons } from '../DatasetRelationships/nodeTypes'; - -export function getDescription( - field: string, - metadataFieldDescriptions: Record | Record, -) { - const [prefix, stem] = field.split('.'); - if (!stem) { - return metadataFieldDescriptions?.[field]; - } - const description = metadataFieldDescriptions?.[stem]; - if (!description) { - return undefined; - } - if (prefix === 'donor') { - return `For the original donor: ${metadataFieldDescriptions?.[stem]}`; - } - if (prefix === 'sample') { - return `For the original sample: ${metadataFieldDescriptions?.[stem]}`; - } - throw new Error(`Unrecognized metadata field prefix: ${prefix}`); -} - -export function buildTableData( - tableData: Record, - metadataFieldDescriptions: Record | Record, - extraValues: Record = {}, -) { - return ( - Object.entries(tableData) - // Filter out nested objects, like nested "metadata" for Samples... - // but allow arrays. Remember, in JS: typeof [] === 'object' - .filter((entry) => typeof entry[1] !== 'object' || Array.isArray(entry[1])) - // Filter out fields from TSV that aren't really metadata: - .filter((entry) => !['contributors_path', 'antibodies_path', 'version'].includes(entry[0])) - .map((entry) => ({ - ...extraValues, - key: entry[0], - // eslint-disable-next-line @typescript-eslint/no-base-to-string - value: Array.isArray(entry[1]) ? entry[1].join(', ') : entry[1].toString(), - description: getDescription(entry[0], metadataFieldDescriptions), - })) - ); -} +import { Columns, defaultTSVColumns } from './columns'; interface TableRow { key: string; @@ -118,107 +73,6 @@ function MetadataWrapper({ allTableRows, tsvColumns = defaultTSVColumns, childre ); } -function getEntityIcon(entity: { entity_type: ESEntityType; is_component?: boolean; processing?: string }) { - if (isDataset(entity)) { - if (entity.is_component) { - return nodeIcons.componentDataset; - } - if (entity.processing === 'processed') { - return nodeIcons.processedDataset; - } - return nodeIcons.primaryDataset; - } - return entityIconMap[entity.entity_type]; -} - -interface sortEntitiesProps { - tableEntities: { - uuid: string; - label: string; - icon: MUIIcon; - tableRows: TableRows; - entity_type: ESEntityType; - hubmap_id: string; - }[]; - uuid: string; -} - -function sortEntities({ tableEntities, uuid }: sortEntitiesProps) { - return tableEntities.sort((a, b) => { - // Current entity at the front - if (a.uuid === uuid) return -1; - if (b.uuid === uuid) return 1; - - // Then donors - if (a.entity_type === 'Donor' && b.entity_type !== 'Donor') return -1; - if (b.entity_type === 'Donor' && a.entity_type !== 'Donor') return 1; - - // Then samples, with unique categories first - const aIsSampleWithoutHubmapId = a.entity_type === 'Sample' && !a.label.includes(a.hubmap_id); - const bIsSampleWithoutHubmapId = b.entity_type === 'Sample' && !b.label.includes(b.hubmap_id); - if (aIsSampleWithoutHubmapId && !bIsSampleWithoutHubmapId) return -1; - if (bIsSampleWithoutHubmapId && !aIsSampleWithoutHubmapId) return 1; - - return a.label.localeCompare(b.label); - }); -} - -interface getEntityLabelProps { - entity: ProcessedDatasetInfo | Donor | Sample; - sampleCategoryCounts: Record; -} - -function getEntityLabel({ entity, sampleCategoryCounts }: getEntityLabelProps) { - if (isSample(entity)) { - // If samples have the same category, add the HuBMAP ID to the label - if (sampleCategoryCounts[entity.sample_category] > 1) { - return `${entity.sample_category} (${entity.hubmap_id})`; - } - return entity.sample_category; - } - if (isDataset(entity)) { - return entity.assay_display_name; - } - return entity.entity_type; -} - -interface getTableEntitiesProps { - entities: (Donor | Dataset | Sample)[]; - uuid: string; - fieldDescriptions: Record; -} - -function getTableEntities({ entities, uuid, fieldDescriptions }: getTableEntitiesProps) { - // Check whether there are multiple samples with the same sample category - const sampleCategoryCounts: Record = {}; - entities.forEach((e) => { - if (isSample(e)) { - sampleCategoryCounts[e.sample_category] = (sampleCategoryCounts[e.sample_category] || 0) + 1; - } - }); - - const tableEntities = entities.map((entity) => { - const label = getEntityLabel({ entity, sampleCategoryCounts }); - return { - uuid: entity.uuid, - label: label ?? '', - icon: getEntityIcon(entity), - tableRows: buildTableData( - getMetadata({ - targetEntityType: entity.entity_type, - currentEntity: entity, - }), - fieldDescriptions, - { hubmap_id: entity.hubmap_id, label }, - ), - entity_type: entity.entity_type, - hubmap_id: entity.hubmap_id, - }; - }); - - return sortEntities({ tableEntities, uuid }); -} - interface MetadataProps { entities: (Donor | Dataset | Sample)[]; } diff --git a/context/app/static/js/components/detailPage/MetadataSection/MetadataTable.spec.ts b/context/app/static/js/components/detailPage/MetadataSection/MetadataTable.spec.ts deleted file mode 100644 index cad1593421..0000000000 --- a/context/app/static/js/components/detailPage/MetadataSection/MetadataTable.spec.ts +++ /dev/null @@ -1,79 +0,0 @@ -import { getDescription, buildTableData } from './MetadataSection'; - -test('should handle plain fields', () => { - // Wouldn't actually expect to see age_value anywhere except for donor, - // but shouldn't make a difference... and if it does, we want to know. - expect(getDescription('age_value', { age_value: 'The time elapsed since birth.' })).toEqual( - 'The time elapsed since birth.', - ); -}); - -test('should handle donor fields', () => { - expect(getDescription('donor.age_value', { age_value: 'The time elapsed since birth.' })).toEqual( - 'For the original donor: The time elapsed since birth.', - ); -}); - -test('should handle sample fields', () => { - expect(getDescription('sample.age_value', { age_value: 'The time elapsed since birth.' })).toEqual( - 'For the original sample: The time elapsed since birth.', - ); -}); - -test('should return undefined if there is not a definition', () => { - expect(getDescription('sample.no_such_stem', {})).toEqual(undefined); -}); - -test('should error if stem is known, but prefix is not', () => { - expect(() => getDescription('no_such_prefix.age_value', { age_value: 'The time elapsed since birth.' })).toThrow(); -}); - -test('should look up field descriptions', () => { - expect( - buildTableData( - { - assay_type: 'FAKE-seq', - }, - { - assay_type: 'The specific type of assay being executed.', - }, - ), - ).toEqual([ - { - description: 'The specific type of assay being executed.', - key: 'assay_type', - value: 'FAKE-seq', - }, - ]); -}); - -test('should remove nested objects, but concat nested lists', () => { - expect( - buildTableData( - { - object: { foo: 'bar' }, - list: ['foo', 'bar'], - }, - {}, - ), - ).toEqual([ - { - description: undefined, - key: 'list', - value: 'foo, bar', - }, - ]); -}); - -test('should remove keys that are not metadata', () => { - expect( - buildTableData( - { - contributors_path: '/local/path', - antibodies_path: '/local/path', - version: '42', - }, - {}, - ), - ).toEqual([]); -}); diff --git a/context/app/static/js/components/detailPage/MetadataSection/utils.spec.ts b/context/app/static/js/components/detailPage/MetadataSection/utils.spec.ts new file mode 100644 index 0000000000..94e9b00209 --- /dev/null +++ b/context/app/static/js/components/detailPage/MetadataSection/utils.spec.ts @@ -0,0 +1,179 @@ +import { DonorIcon } from 'js/shared-styles/icons'; +import { getDescription, buildTableData, sortEntities, TableEntity } from './utils'; + +/** + * ============================================================================= + * getDescription + * ============================================================================= + */ + +test('should handle plain fields', () => { + // Wouldn't actually expect to see age_value anywhere except for donor, + // but shouldn't make a difference... and if it does, we want to know. + expect(getDescription('age_value', { age_value: 'The time elapsed since birth.' })).toEqual( + 'The time elapsed since birth.', + ); +}); + +test('should handle donor fields', () => { + expect(getDescription('donor.age_value', { age_value: 'The time elapsed since birth.' })).toEqual( + 'For the original donor: The time elapsed since birth.', + ); +}); + +test('should handle sample fields', () => { + expect(getDescription('sample.age_value', { age_value: 'The time elapsed since birth.' })).toEqual( + 'For the original sample: The time elapsed since birth.', + ); +}); + +test('should return undefined if there is not a definition', () => { + expect(getDescription('sample.no_such_stem', {})).toEqual(undefined); +}); + +test('should error if stem is known, but prefix is not', () => { + expect(() => getDescription('no_such_prefix.age_value', { age_value: 'The time elapsed since birth.' })).toThrow(); +}); + +/** + * ============================================================================= + * buildTableData + * ============================================================================= + */ + +test('should look up field descriptions', () => { + expect( + buildTableData( + { + assay_type: 'FAKE-seq', + }, + { + assay_type: 'The specific type of assay being executed.', + }, + ), + ).toEqual([ + { + description: 'The specific type of assay being executed.', + key: 'assay_type', + value: 'FAKE-seq', + }, + ]); +}); + +test('should remove nested objects, but concat nested lists', () => { + expect( + buildTableData( + { + object: { foo: 'bar' }, + list: ['foo', 'bar'], + }, + {}, + ), + ).toEqual([ + { + description: undefined, + key: 'list', + value: 'foo, bar', + }, + ]); +}); + +test('should remove keys that are not metadata', () => { + expect( + buildTableData( + { + contributors_path: '/local/path', + antibodies_path: '/local/path', + version: '42', + }, + {}, + ), + ).toEqual([]); +}); + +/** + * ============================================================================= + * sortEntities + * ============================================================================= + */ + +const baseEntity = { + icon: DonorIcon, + tableRows: [], +}; + +const current: TableEntity = { + ...baseEntity, + uuid: 'current', + label: 'Current', + entity_type: 'Dataset', + hubmap_id: 'current', +}; + +const donor: TableEntity = { + ...baseEntity, + uuid: 'donor', + label: 'Donor', + entity_type: 'Donor', + hubmap_id: 'donor', +}; + +const uniqueSample1: TableEntity = { + ...baseEntity, + uuid: 'sample1', + label: 'Unique Category', + entity_type: 'Sample', + hubmap_id: 'sample1', +}; + +const duplicateSample2: TableEntity = { + ...baseEntity, + uuid: 'sample2', + label: 'Duplicate Category (sample2)', + entity_type: 'Sample', + hubmap_id: 'sample2', +}; + +const duplicateSample3: TableEntity = { + ...baseEntity, + uuid: 'sample3', + label: 'Duplicate Category (sample3)', + entity_type: 'Sample', + hubmap_id: 'sample3', +}; + +test('should sort by current -> donor -> samples without IDs in their label -> samples with IDs in their label', () => { + expect( + sortEntities({ + tableEntities: [duplicateSample2, duplicateSample3, donor, current, uniqueSample1], + uuid: 'current', + }), + ).toEqual([current, donor, uniqueSample1, duplicateSample2, duplicateSample3]); +}); + +test('should sort by current -> samples without IDs in their label -> samples with IDs in their label when donor is absent', () => { + expect( + sortEntities({ + tableEntities: [duplicateSample2, current, uniqueSample1, duplicateSample3], + uuid: 'current', + }), + ).toEqual([current, uniqueSample1, duplicateSample2, duplicateSample3]); +}); + +test('should sort by donor -> samples without IDs -> samples with IDs when current is absent', () => { + expect( + sortEntities({ + tableEntities: [duplicateSample3, duplicateSample2, donor, uniqueSample1], + uuid: 'current', + }), + ).toEqual([donor, uniqueSample1, duplicateSample2, duplicateSample3]); +}); + +test('should return only the current entity if no other entities are present', () => { + expect( + sortEntities({ + tableEntities: [current], + uuid: 'current', + }), + ).toEqual([current]); +}); diff --git a/context/app/static/js/components/detailPage/MetadataSection/utils.ts b/context/app/static/js/components/detailPage/MetadataSection/utils.ts new file mode 100644 index 0000000000..064fc34649 --- /dev/null +++ b/context/app/static/js/components/detailPage/MetadataSection/utils.ts @@ -0,0 +1,152 @@ +import { nodeIcons } from 'js/components/detailPage/DatasetRelationships/nodeTypes'; +import { TableRows } from 'js/components/detailPage/MetadataSection/MetadataSection'; +import { Dataset, Donor, ESEntityType, Sample, isDataset, isSample } from 'js/components/types'; +import { getMetadata } from 'js/helpers/metadata'; +import { ProcessedDatasetInfo } from 'js/pages/Dataset/hooks'; +import { MUIIcon, entityIconMap } from 'js/shared-styles/icons/entityIconMap'; + +function getDescription(field: string, metadataFieldDescriptions: Record | Record) { + const [prefix, stem] = field.split('.'); + if (!stem) { + return metadataFieldDescriptions?.[field]; + } + const description = metadataFieldDescriptions?.[stem]; + if (!description) { + return undefined; + } + if (prefix === 'donor') { + return `For the original donor: ${metadataFieldDescriptions?.[stem]}`; + } + if (prefix === 'sample') { + return `For the original sample: ${metadataFieldDescriptions?.[stem]}`; + } + throw new Error(`Unrecognized metadata field prefix: ${prefix}`); +} + +function buildTableData( + tableData: Record, + metadataFieldDescriptions: Record | Record, + extraValues: Record = {}, +) { + return ( + Object.entries(tableData) + // Filter out nested objects, like nested "metadata" for Samples... + // but allow arrays. Remember, in JS: typeof [] === 'object' + .filter((entry) => typeof entry[1] !== 'object' || Array.isArray(entry[1])) + // Filter out fields from TSV that aren't really metadata: + .filter((entry) => !['contributors_path', 'antibodies_path', 'version'].includes(entry[0])) + .map((entry) => ({ + ...extraValues, + key: entry[0], + // eslint-disable-next-line @typescript-eslint/no-base-to-string + value: Array.isArray(entry[1]) ? entry[1].join(', ') : entry[1].toString(), + description: getDescription(entry[0], metadataFieldDescriptions), + })) + ); +} + +function getEntityIcon(entity: { entity_type: ESEntityType; is_component?: boolean; processing?: string }) { + if (isDataset(entity)) { + if (entity.is_component) { + return nodeIcons.componentDataset; + } + if (entity.processing === 'processed') { + return nodeIcons.processedDataset; + } + return nodeIcons.primaryDataset; + } + return entityIconMap[entity.entity_type]; +} + +export interface TableEntity { + uuid: string; + label: string; + icon: MUIIcon; + tableRows: TableRows; + entity_type: ESEntityType; + hubmap_id: string; +} + +interface sortEntitiesProps { + tableEntities: TableEntity[]; + uuid: string; +} + +function sortEntities({ tableEntities, uuid }: sortEntitiesProps) { + return tableEntities.sort((a, b) => { + // Current entity at the front + if (a.uuid === uuid) return -1; + if (b.uuid === uuid) return 1; + + // Then donors + if (a.entity_type === 'Donor' && b.entity_type !== 'Donor') return -1; + if (b.entity_type === 'Donor' && a.entity_type !== 'Donor') return 1; + + // Then samples, with unique categories first + const aIsSampleWithoutHubmapId = a.entity_type === 'Sample' && !a.label.includes(a.hubmap_id); + const bIsSampleWithoutHubmapId = b.entity_type === 'Sample' && !b.label.includes(b.hubmap_id); + if (aIsSampleWithoutHubmapId && !bIsSampleWithoutHubmapId) return -1; + if (bIsSampleWithoutHubmapId && !aIsSampleWithoutHubmapId) return 1; + + return a.label.localeCompare(b.label); + }); +} + +interface getEntityLabelProps { + entity: ProcessedDatasetInfo | Donor | Sample; + sampleCategoryCounts: Record; +} + +function getEntityLabel({ entity, sampleCategoryCounts }: getEntityLabelProps) { + if (isSample(entity)) { + // If samples have the same category, add the HuBMAP ID to the label + if (sampleCategoryCounts[entity.sample_category] > 1) { + return `${entity.sample_category} (${entity.hubmap_id})`; + } + return entity.sample_category; + } + if (isDataset(entity)) { + return entity.assay_display_name; + } + return entity.entity_type; +} + +interface getTableEntitiesProps { + entities: (Donor | Dataset | Sample)[]; + uuid: string; + fieldDescriptions: Record; +} + +function getTableEntities({ entities, uuid, fieldDescriptions }: getTableEntitiesProps) { + // Keep track of whether there are multiple samples with the same category + const sampleCategoryCounts: Record = {}; + entities.forEach((e) => { + if (isSample(e)) { + sampleCategoryCounts[e.sample_category] = (sampleCategoryCounts[e.sample_category] || 0) + 1; + } + }); + + const tableEntities = entities.map((entity) => { + // Generate a label with the HuBMAP ID if there are multiple samples with the same category + const label = getEntityLabel({ entity, sampleCategoryCounts }); + return { + uuid: entity.uuid, + label: label ?? '', + icon: getEntityIcon(entity), + tableRows: buildTableData( + getMetadata({ + targetEntityType: entity.entity_type, + currentEntity: entity, + }), + fieldDescriptions, + { hubmap_id: entity.hubmap_id, label }, + ), + entity_type: entity.entity_type, + hubmap_id: entity.hubmap_id, + }; + }); + + return sortEntities({ tableEntities, uuid }); +} + +export { getTableEntities, getDescription, buildTableData, sortEntities }; From d3809aed3befa3c421c876505d8aaaccb5ffc9d9 Mon Sep 17 00:00:00 2001 From: Austen Money Date: Thu, 10 Oct 2024 12:03:13 -0400 Subject: [PATCH 09/12] clean up --- .../detailPage/MetadataSection/MetadataSection.tsx | 6 ------ context/app/static/js/hooks/useEntityData.ts | 6 +++--- context/app/static/js/pages/Dataset/Dataset.tsx | 6 ++---- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx b/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx index d5491814d2..1ec2ce4e3b 100644 --- a/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx +++ b/context/app/static/js/components/detailPage/MetadataSection/MetadataSection.tsx @@ -7,7 +7,6 @@ import { useTrackEntityPageEvent } from 'js/components/detailPage/useTrackEntity import { tableToDelimitedString, createDownloadUrl } from 'js/helpers/functions'; import { useMetadataFieldDescriptions } from 'js/hooks/useUBKG'; import { Dataset, Donor, Sample, isDataset } from 'js/components/types'; -import { useProcessedDatasets } from 'js/pages/Dataset/hooks'; import withShouldDisplay from 'js/helpers/withShouldDisplay'; import { sectionIconMap } from 'js/shared-styles/icons/sectionIconMap'; import { getTableEntities } from 'js/components/detailPage/MetadataSection/utils'; @@ -78,16 +77,11 @@ interface MetadataProps { } function Metadata({ entities }: MetadataProps) { - const { searchHits: datasetsWithMetadata, isLoading } = useProcessedDatasets(true); const { data: fieldDescriptions } = useMetadataFieldDescriptions(); const { entity: { uuid }, } = useFlaskDataContext(); - if (isLoading || !datasetsWithMetadata) { - return null; - } - const tableEntities = getTableEntities({ entities, uuid, fieldDescriptions }); const allTableRows = tableEntities.map((d) => d.tableRows).flat(); diff --git a/context/app/static/js/hooks/useEntityData.ts b/context/app/static/js/hooks/useEntityData.ts index e49c2003b5..d80436a254 100644 --- a/context/app/static/js/hooks/useEntityData.ts +++ b/context/app/static/js/hooks/useEntityData.ts @@ -1,6 +1,6 @@ import { useMemo } from 'react'; import { useSearchHits } from 'js/hooks/useSearchData'; -import { Dataset, Donor, Entity, Sample } from 'js/components/types'; +import { Entity } from 'js/components/types'; export const useEntityQuery = (uuid: string | string[], source?: string[]) => { return useMemo( @@ -20,10 +20,10 @@ export function useEntityData(uuid: string, source?: string[]): [Entity, boolean return [searchHits[0]?._source, isLoading]; } -export function useEntitiesData(uuids: string[], source?: string[]): [(Donor | Sample | Dataset)[], boolean] { +export function useEntitiesData(uuids: string[], source?: string[]): [T[], boolean] { const query = useEntityQuery(uuids, source); - const { searchHits, isLoading } = useSearchHits(query); + const { searchHits, isLoading } = useSearchHits(query); return [searchHits.map((hit) => hit._source), isLoading]; } diff --git a/context/app/static/js/pages/Dataset/Dataset.tsx b/context/app/static/js/pages/Dataset/Dataset.tsx index 24f9cf3f00..4f7c3be0fd 100644 --- a/context/app/static/js/pages/Dataset/Dataset.tsx +++ b/context/app/static/js/pages/Dataset/Dataset.tsx @@ -17,7 +17,7 @@ import { getCombinedDatasetStatus } from 'js/components/detailPage/utils'; import ComponentAlert from 'js/components/detailPage/multi-assay/ComponentAlert'; import MultiAssayRelationship from 'js/components/detailPage/multi-assay/MultiAssayRelationship'; import MetadataSection from 'js/components/detailPage/MetadataSection'; -import { Dataset, Entity, isDataset } from 'js/components/types'; +import { Dataset, Donor, Entity, Sample, isDataset } from 'js/components/types'; import DatasetRelationships from 'js/components/detailPage/DatasetRelationships'; import ProcessedDataSection from 'js/components/detailPage/ProcessedData'; import { SelectedVersionStoreProvider } from 'js/components/detailPage/VersionSelect/SelectedVersionStore'; @@ -28,7 +28,6 @@ import { useDatasetsCollections } from 'js/hooks/useDatasetsCollections'; import useTrackID from 'js/hooks/useTrackID'; import { InternalLink } from 'js/shared-styles/Links'; import OrganIcon from 'js/shared-styles/icons/OrganIcon'; - import { useEntitiesData } from 'js/hooks/useEntityData'; import { hasMetadata } from 'js/helpers/metadata'; import { useProcessedDatasets, useProcessedDatasetsSections, useRedirectAlert } from './hooks'; @@ -99,8 +98,7 @@ function DatasetDetail({ assayMetadata }: EntityDetailProps) { ancestor_ids, } = assayMetadata; - const [entities, loadingEntities] = useEntitiesData([uuid, ...ancestor_ids]); - + const [entities, loadingEntities] = useEntitiesData([uuid, ...ancestor_ids]); const entitiesWithMetadata = entities.filter((e) => hasMetadata({ targetEntityType: e.entity_type, currentEntity: e }), ); From aee97b4a1333960e31449a48c85ffc6515bf6df9 Mon Sep 17 00:00:00 2001 From: Austen Money Date: Thu, 10 Oct 2024 12:34:50 -0400 Subject: [PATCH 10/12] update changelog --- CHANGELOG-fix-metadata-table.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG-fix-metadata-table.md b/CHANGELOG-fix-metadata-table.md index 7adc1103fa..fe89cfa966 100644 --- a/CHANGELOG-fix-metadata-table.md +++ b/CHANGELOG-fix-metadata-table.md @@ -1 +1,3 @@ -- Fix issue of sample metadata not showing up in the sample detail page metadata table. \ No newline at end of file +- Switch to using metadata table with tabs component in the Sample and Donor pages. +- In the metadata sections of Dataset, Sample, and Donor pages, add tabs for any entities in the Provenance section with metadata. +- Update the metadata table component to show unique labels for each tab and to be scrollable when many tabs are present. \ No newline at end of file From fdfcb4831a9d973c346a00362633b9badc10542c Mon Sep 17 00:00:00 2001 From: Austen Money Date: Thu, 10 Oct 2024 14:48:19 -0400 Subject: [PATCH 11/12] remove extra utils functions --- .../detailPage/MetadataSection/utils.spec.ts | 36 +---------- .../detailPage/MetadataSection/utils.ts | 24 +------ .../js/pages/utils/entity-utils.spec.ts | 63 ------------------- .../app/static/js/pages/utils/entity-utils.ts | 33 ---------- 4 files changed, 4 insertions(+), 152 deletions(-) delete mode 100644 context/app/static/js/pages/utils/entity-utils.spec.ts delete mode 100644 context/app/static/js/pages/utils/entity-utils.ts diff --git a/context/app/static/js/components/detailPage/MetadataSection/utils.spec.ts b/context/app/static/js/components/detailPage/MetadataSection/utils.spec.ts index 94e9b00209..ffd24d4584 100644 --- a/context/app/static/js/components/detailPage/MetadataSection/utils.spec.ts +++ b/context/app/static/js/components/detailPage/MetadataSection/utils.spec.ts @@ -1,39 +1,5 @@ import { DonorIcon } from 'js/shared-styles/icons'; -import { getDescription, buildTableData, sortEntities, TableEntity } from './utils'; - -/** - * ============================================================================= - * getDescription - * ============================================================================= - */ - -test('should handle plain fields', () => { - // Wouldn't actually expect to see age_value anywhere except for donor, - // but shouldn't make a difference... and if it does, we want to know. - expect(getDescription('age_value', { age_value: 'The time elapsed since birth.' })).toEqual( - 'The time elapsed since birth.', - ); -}); - -test('should handle donor fields', () => { - expect(getDescription('donor.age_value', { age_value: 'The time elapsed since birth.' })).toEqual( - 'For the original donor: The time elapsed since birth.', - ); -}); - -test('should handle sample fields', () => { - expect(getDescription('sample.age_value', { age_value: 'The time elapsed since birth.' })).toEqual( - 'For the original sample: The time elapsed since birth.', - ); -}); - -test('should return undefined if there is not a definition', () => { - expect(getDescription('sample.no_such_stem', {})).toEqual(undefined); -}); - -test('should error if stem is known, but prefix is not', () => { - expect(() => getDescription('no_such_prefix.age_value', { age_value: 'The time elapsed since birth.' })).toThrow(); -}); +import { buildTableData, sortEntities, TableEntity } from './utils'; /** * ============================================================================= diff --git a/context/app/static/js/components/detailPage/MetadataSection/utils.ts b/context/app/static/js/components/detailPage/MetadataSection/utils.ts index 064fc34649..f5a5825294 100644 --- a/context/app/static/js/components/detailPage/MetadataSection/utils.ts +++ b/context/app/static/js/components/detailPage/MetadataSection/utils.ts @@ -5,24 +5,6 @@ import { getMetadata } from 'js/helpers/metadata'; import { ProcessedDatasetInfo } from 'js/pages/Dataset/hooks'; import { MUIIcon, entityIconMap } from 'js/shared-styles/icons/entityIconMap'; -function getDescription(field: string, metadataFieldDescriptions: Record | Record) { - const [prefix, stem] = field.split('.'); - if (!stem) { - return metadataFieldDescriptions?.[field]; - } - const description = metadataFieldDescriptions?.[stem]; - if (!description) { - return undefined; - } - if (prefix === 'donor') { - return `For the original donor: ${metadataFieldDescriptions?.[stem]}`; - } - if (prefix === 'sample') { - return `For the original sample: ${metadataFieldDescriptions?.[stem]}`; - } - throw new Error(`Unrecognized metadata field prefix: ${prefix}`); -} - function buildTableData( tableData: Record, metadataFieldDescriptions: Record | Record, @@ -40,7 +22,7 @@ function buildTableData( key: entry[0], // eslint-disable-next-line @typescript-eslint/no-base-to-string value: Array.isArray(entry[1]) ? entry[1].join(', ') : entry[1].toString(), - description: getDescription(entry[0], metadataFieldDescriptions), + description: metadataFieldDescriptions?.[entry[0]], })) ); } @@ -73,7 +55,7 @@ interface sortEntitiesProps { } function sortEntities({ tableEntities, uuid }: sortEntitiesProps) { - return tableEntities.sort((a, b) => { + return [...tableEntities].sort((a, b) => { // Current entity at the front if (a.uuid === uuid) return -1; if (b.uuid === uuid) return 1; @@ -149,4 +131,4 @@ function getTableEntities({ entities, uuid, fieldDescriptions }: getTableEntitie return sortEntities({ tableEntities, uuid }); } -export { getTableEntities, getDescription, buildTableData, sortEntities }; +export { getTableEntities, buildTableData, sortEntities }; diff --git a/context/app/static/js/pages/utils/entity-utils.spec.ts b/context/app/static/js/pages/utils/entity-utils.spec.ts deleted file mode 100644 index b51eb42418..0000000000 --- a/context/app/static/js/pages/utils/entity-utils.spec.ts +++ /dev/null @@ -1,63 +0,0 @@ -import { Donor, Sample } from 'js/components/types'; -import { combineMetadata } from './entity-utils'; - -test('robust against undefined data', () => { - const donor = undefined; - const source_samples = undefined; - const metadata = undefined; - // @ts-expect-error - This is a test case for bad data. - expect(combineMetadata(donor, source_samples, metadata)).toEqual({}); -}); - -test('robust against empty objects', () => { - const donor = {}; - const source_samples: Sample[] = []; - const metadata = {}; - // @ts-expect-error - This is a test case for bad data. - expect(combineMetadata(donor, source_samples, metadata)).toEqual({}); -}); - -test('combines appropriately structured metadata', () => { - // This information is also available in the "ancestors" list, - // but metadata is structured differently between Samples and Donors, - // so it wouldn't simplify things to use that. - const donor = { - created_by_user_displayname: 'John Doe', - mapped_metadata: { - age_unit: ['years'], - age_value: [40], - }, - metadata: { - // This is the source of the mapped_metadata. - living_donor_data: [], - }, - } as unknown as Donor; - const source_samples = [ - { - // mapped_metadata seems to be empty. - mapped_metadata: {}, - metadata: { - cold_ischemia_time_unit: 'minutes', - cold_ischemia_time_value: '100', - }, - }, - ] as unknown as Sample[]; - const metadata = { - dag_provenance_list: [], - extra_metadata: {}, - metadata: { - analyte_class: 'polysaccharides', - assay_category: 'imaging', - assay_type: 'PAS microscopy', - }, - }; - expect(combineMetadata(donor, source_samples, metadata)).toEqual({ - analyte_class: 'polysaccharides', - assay_category: 'imaging', - assay_type: 'PAS microscopy', - 'donor.age_unit': ['years'], - 'donor.age_value': [40], - 'sample.cold_ischemia_time_unit': 'minutes', - 'sample.cold_ischemia_time_value': '100', - }); -}); diff --git a/context/app/static/js/pages/utils/entity-utils.ts b/context/app/static/js/pages/utils/entity-utils.ts deleted file mode 100644 index 1161cfa2af..0000000000 --- a/context/app/static/js/pages/utils/entity-utils.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { Donor, Sample } from 'js/components/types'; - -function addPrefix(prefix: string, object: object) { - return Object.fromEntries(Object.entries(object).map(([key, value]) => [prefix + key, value])); -} - -function prefixDonorMetadata(donor: Donor | null | undefined) { - const donorMetadata = donor?.mapped_metadata ?? {}; - return addPrefix('donor.', donorMetadata); -} - -function prefixSampleMetadata(source_samples: Sample[] | null | undefined) { - const sampleMetadatas = (source_samples ?? []).filter((sample) => sample?.metadata).map((sample) => sample.metadata); - return sampleMetadatas.map((sampleMetadata) => addPrefix('sample.', sampleMetadata)); -} - -function combineMetadata( - donor: Donor, - source_samples: Sample[] = [], - metadata: Record | null | undefined = {}, -) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const combinedMetadata: Record = { - ...(metadata?.metadata ?? {}), - ...prefixDonorMetadata(donor), - }; - prefixSampleMetadata(source_samples).forEach((sampleMetadata) => { - Object.assign(combinedMetadata, sampleMetadata); - }); - - return combinedMetadata; -} -export { combineMetadata }; From 913e5596b4a643518f994d8aa7fbac210f16eea9 Mon Sep 17 00:00:00 2001 From: Austen Money Date: Fri, 11 Oct 2024 11:47:44 -0400 Subject: [PATCH 12/12] lift helper function --- .../detailPage/MetadataSection/utils.ts | 17 ++--------------- context/app/static/js/helpers/functions.ts | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/context/app/static/js/components/detailPage/MetadataSection/utils.ts b/context/app/static/js/components/detailPage/MetadataSection/utils.ts index f5a5825294..639bb791f8 100644 --- a/context/app/static/js/components/detailPage/MetadataSection/utils.ts +++ b/context/app/static/js/components/detailPage/MetadataSection/utils.ts @@ -1,9 +1,9 @@ -import { nodeIcons } from 'js/components/detailPage/DatasetRelationships/nodeTypes'; import { TableRows } from 'js/components/detailPage/MetadataSection/MetadataSection'; import { Dataset, Donor, ESEntityType, Sample, isDataset, isSample } from 'js/components/types'; +import { getEntityIcon } from 'js/helpers/functions'; import { getMetadata } from 'js/helpers/metadata'; import { ProcessedDatasetInfo } from 'js/pages/Dataset/hooks'; -import { MUIIcon, entityIconMap } from 'js/shared-styles/icons/entityIconMap'; +import { MUIIcon } from 'js/shared-styles/icons/entityIconMap'; function buildTableData( tableData: Record, @@ -27,19 +27,6 @@ function buildTableData( ); } -function getEntityIcon(entity: { entity_type: ESEntityType; is_component?: boolean; processing?: string }) { - if (isDataset(entity)) { - if (entity.is_component) { - return nodeIcons.componentDataset; - } - if (entity.processing === 'processed') { - return nodeIcons.processedDataset; - } - return nodeIcons.primaryDataset; - } - return entityIconMap[entity.entity_type]; -} - export interface TableEntity { uuid: string; label: string; diff --git a/context/app/static/js/helpers/functions.ts b/context/app/static/js/helpers/functions.ts index 79fdf59bc9..8a5e5a2d36 100644 --- a/context/app/static/js/helpers/functions.ts +++ b/context/app/static/js/helpers/functions.ts @@ -1,6 +1,9 @@ import { SearchRequest } from '@elastic/elasticsearch/lib/api/types'; +import { nodeIcons } from 'js/components/detailPage/DatasetRelationships/nodeTypes'; +import { ESEntityType, isDataset } from 'js/components/types'; import { MAX_NUMBER_OF_WORKSPACE_DATASETS } from 'js/components/workspaces/api'; import { MergedWorkspace } from 'js/components/workspaces/types'; +import { entityIconMap } from 'js/shared-styles/icons/entityIconMap'; export function isEmptyArrayOrObject(val: object | unknown[]) { if (val.constructor.name === 'Object') { @@ -222,3 +225,16 @@ export function isValidEmail(email: string) { const cleanedValue: string = email?.replace(/^\s+|\s+$/g, ''); return emailRegex.test(cleanedValue); } + +export function getEntityIcon(entity: { entity_type: ESEntityType; is_component?: boolean; processing?: string }) { + if (isDataset(entity)) { + if (entity.is_component) { + return nodeIcons.componentDataset; + } + if (entity.processing === 'processed') { + return nodeIcons.processedDataset; + } + return nodeIcons.primaryDataset; + } + return entityIconMap[entity.entity_type]; +}