-
Notifications
You must be signed in to change notification settings - Fork 1
Refactor of GetDigitalSpecimenComplete service and setup of data mapper to UI #244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
c24cf87
67759ea
8f8442c
e9b242a
f9bebf3
a1bca54
1eb622b
c48b3c5
53f3ba8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import { useQuery } from '@tanstack/react-query'; | ||
| import { getDigitalSpecimenComplete } from 'services/digitalSpecimenService/getDigitalSpecimenComplete'; | ||
| import { mapDigitalSpecimen } from 'utils/DataMappers/digitalSpecimenDataMapper'; | ||
|
|
||
| /* Base constants */ | ||
| const staleTime = 1000 * 60 * 5; // How long until the time is stale | ||
| const gcTime = 1000 * 60 * 10; // Cache time: How long to store it in the cache | ||
|
|
||
| /* useQuery hook to retrieve the complete Digital Specimen data object by calling the getDigitalSpecimenComplete service */ | ||
| export const useDigitalSpecimenComplete = ({ handle, version }: | ||
| { handle: string, version?: number }) => { | ||
| return useQuery({ | ||
| queryKey: ['digitalSpecimen', handle, version], | ||
| queryFn: () => getDigitalSpecimenComplete({ handle, version }), | ||
| select: (data) => { | ||
| return { | ||
| ...data, | ||
| ...mapDigitalSpecimen(data), | ||
| } | ||
| }, | ||
| staleTime, | ||
| gcTime | ||
| }); | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| import apiClient from '../apiClient'; | ||
|
|
||
| /** | ||
| * Service that retrieves the complete digital specimen details through the apiClient | ||
| * Takes handle and version as a parameter object | ||
| * @returns An array with complete digital specimen data on specimen, media and annotations | ||
| */ | ||
| export const getDigitalSpecimenComplete = async ({ handle, version }: | ||
| { handle: string, version?: number }) => { | ||
|
southeo marked this conversation as resolved.
Outdated
|
||
| let endPoint: string; | ||
|
|
||
| if (version) { | ||
| endPoint = `digital-specimen/v1/${handle}/${version}/full`; | ||
| } else { | ||
| endPoint = `digital-specimen/v1/${handle}/full`; | ||
| } | ||
| try { | ||
| /* Call service and wait for response */ | ||
| const response = await apiClient.get(endPoint, { | ||
| params: { | ||
| handle, | ||
| version | ||
| } | ||
| }); | ||
|
|
||
| /* Throw error if response is not as expected */ | ||
| if(!response.data?.data) { | ||
|
southeo marked this conversation as resolved.
|
||
| throw new Error('Incorrect response format'); | ||
| } | ||
|
|
||
| /* Return response data */ | ||
| return response.data; | ||
| } catch (error) { | ||
| /* If error, throw error with generic error message */ | ||
| console.error('Error fetching the digital specimen:', error); | ||
|
|
||
| /* Rethrow error for useQuery */ | ||
| throw error; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why catch here just to log it? why not just add the logging on line 28?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the future, this will be caught by some form of error notification system I still have to implement. The error in the catch can be any kind of error, 500s, 404s. The error on line 28 is only thrown if the status is OK (200, 201), but the response that we get does not match what we expect at all. The catch takes care of my error codes. So separate error instances. |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| /* Import schemas */ | ||
| import DIGITAL_SPECIMEN_SCHEMA_MAP from "./schemas/DigitalSpecimenSchema"; | ||
|
|
||
| /* Import types */ | ||
| import { DigitalSpecimenUIModel, SchemaMap, UIProperty } from "./types/dataMapperTypes"; | ||
|
|
||
| /** | ||
| * Function to get the accepted identification or the first one it can find | ||
| * @param ds The digital specimen object | ||
| * @returns Either the accepted identification or, if there is none, the first identification it can find | ||
| */ | ||
| const getAcceptedIdentification = (ds: any) => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can return
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I adjusted the function to also return a null in case of no identifications whatsoever. Probably a bit of an edge case, but my mapper works also if no identifications are found. |
||
| const identifications = ds["ods:hasIdentifications"]; | ||
| if (identifications) { | ||
| const availableIdentification = identifications.find((item: any) => item["ods:isVerifiedIdentification"]) || identifications[0]; | ||
| return availableIdentification?.["ods:hasTaxonIdentifications"]?.[0]; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Transforms raw Digital Specimen data into a UI-ready model | ||
| * based on the DIGITAL_SPECIMEN_SCHEMA_MAP definitions. | ||
| * It is executed in the useDigitalSpecimen hook immediately when the call is being done. | ||
| */ | ||
| export const mapDigitalSpecimen = (rawData: any): DigitalSpecimenUIModel | null => { | ||
| const ds = rawData?.data?.attributes?.digitalSpecimen; | ||
| if (!ds) return null; | ||
|
|
||
| /* Find acceptedIdentification or second best option */ | ||
| const acceptedIdentification = getAcceptedIdentification(ds); | ||
|
|
||
| return Object.entries(DIGITAL_SPECIMEN_SCHEMA_MAP as SchemaMap).reduce((accumulator, [groupKey, fields]) => { | ||
| const mappedFields: Record<string, UIProperty> = {}; | ||
|
|
||
| Object.entries(fields).forEach(([fieldKey, config]) => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could make your code a bit more compact and immutable, but this is optional and opinionated.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this approach! I addressed it as well as I could. Let me know what you think. |
||
| mappedFields[fieldKey] = { | ||
| label: config.label, | ||
| value: config.resolve(ds, acceptedIdentification), | ||
| isHtml: !!config.isHtml, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed it to Boolean(config.isHtml). Turned out to be the right one :) |
||
| type: config.type || 'base', | ||
| }; | ||
| }); | ||
|
|
||
| accumulator[groupKey as keyof DigitalSpecimenUIModel] = mappedFields; | ||
| return accumulator; | ||
| }, {} as DigitalSpecimenUIModel); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What it should return added. |
||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| import { FieldConfig } from "../types/dataMapperTypes"; | ||
|
|
||
| /* Data schema to map Digital Specimen data that we use in the UI, managed from one central data mapper */ | ||
| const DIGITAL_SPECIMEN_SCHEMA_MAP: Record<string, Record<string, FieldConfig>> = { | ||
| SPECIMEN_RECORD: { | ||
| doi: { | ||
| label: 'DOI', | ||
| resolve: (ds) => ds["@id"], | ||
| type: 'copy' | ||
| }, | ||
| catalogNumber: { | ||
| label: 'Catalog Number', | ||
| resolve: (ds) => { | ||
| const catalog = ds["ods:hasIdentifiers"]?.find((id: any) => id["dcterms:title"] === "dwc:catalogNumber"); | ||
| const fallBack = ds["ods:hasIdentifiers"]?.find((id: any) => id["dcterms:title"] === "dwca:ID"); | ||
| return catalog?.["dwc:catalogNumber"] || fallBack?.["dwca:id"]; | ||
| } | ||
| }, | ||
| specimenProvider: { | ||
| label: 'Specimen Provider', | ||
| resolve: (ds) => ds["ods:organisationName"] | ||
| }, | ||
| sourceSystem: { | ||
| label: 'Source System', | ||
| resolve: (ds) => ds["ods:sourceSystemName"] | ||
| }, | ||
| basisOfRecord: { | ||
| label: 'Basis of Record', | ||
| resolve: (ds) => ds["dwc:basisOfRecord"] | ||
| }, | ||
| discipline: { | ||
| label: 'Discipline', | ||
| resolve: (ds) => ds["dwc:topicDiscipline"] | ||
| } | ||
| }, | ||
|
|
||
| IDENTIFICATION: { | ||
| scientificName: { | ||
| label: 'Scientific Name', | ||
| isHtml: true, | ||
|
southeo marked this conversation as resolved.
Outdated
|
||
| resolve: (_, { acceptedIdentification }) => acceptedIdentification?.["ods:scientificNameHTMLLabel"] || acceptedIdentification?.["dwc:scientificName"] | ||
| }, | ||
| taxonomicStatus: { | ||
| label: 'Taxonomic Status', | ||
| type: 'url', | ||
| resolve: (_, { acceptedIdentification: acc }) => acc?.["@id"] | ||
| }, | ||
| verbatimName: { | ||
| label: 'Identification Verbatim', | ||
| type: 'verbatim', | ||
| resolve: (ds) => ds["ods:hasIdentifications"]?.[0]?.["dwc:verbatimIdentification"] | ||
| }, | ||
| kingdom: { | ||
| label: 'Kingdom', | ||
| resolve: (_, { acceptedIdentification }) => acceptedIdentification?.["dwc:kingdom"] | ||
| }, | ||
| phylum: { | ||
| label: 'Phylum', | ||
| resolve: (_, { acceptedIdentification }) => acceptedIdentification?.["dwc:phylum"] | ||
| }, | ||
| class: { | ||
| label: 'Class', | ||
| resolve: (_, { acceptedIdentification }) => acceptedIdentification?.["dwc:class"] | ||
| }, | ||
| order: { | ||
| label: 'Order', | ||
| resolve: (_, { acceptedIdentification }) => acceptedIdentification?.["dwc:order"] | ||
| }, | ||
| family: { | ||
| label: 'Family', | ||
| resolve: (_, { acceptedIdentification }) => acceptedIdentification?.["dwc:family"] | ||
| }, | ||
| subFamily: { | ||
| label: 'Sub-family', | ||
| resolve: (_, { acceptedIdentification }) => acceptedIdentification?.["dwc:subfamily"] | ||
| }, | ||
| genus: { | ||
| label: 'Genus', | ||
| resolve: (_, { acceptedIdentification }) => acceptedIdentification?.["dwc:genus"] | ||
| }, | ||
| species: { | ||
| label: 'Species', | ||
| resolve: (_, { acceptedIdentification }) => acceptedIdentification?.["dwc:species"] | ||
| }, | ||
| specificEpithet: { | ||
| label: 'Specific Epithet', | ||
| resolve: (_, { acceptedIdentification }) => acceptedIdentification?.["dwc:specificEpithet"] | ||
| }, | ||
| nomenClaturalCode: { | ||
| label: 'Nomen/Clatural Code', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a typo?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Haha yes. Sometimes I don't know what I am talking about |
||
| resolve: (_, { acceptedIdentification }) => acceptedIdentification?.["dwc:nomenclaturalCode"] | ||
| }, | ||
| scientificNameAuthorship: { | ||
| label: 'Scientific Name Authorship', | ||
| resolve: (_, { acceptedIdentification }) => acceptedIdentification?.["dwc:scientificNameAuthorship"] | ||
| }, | ||
| }, | ||
|
|
||
| LOCATION: { | ||
| country: { | ||
| label: 'Country', | ||
| resolve: (_, { primaryEvent: ev }) => ev?.["ods:hasLocation"]?.["dwc:country"] | ||
| }, | ||
| locality: { | ||
| label: 'Locality', | ||
| resolve: (_, { primaryEvent: ev }) => ev?.["ods:hasLocation"]?.["dwc:locality"] | ||
| }, | ||
| verbatimLocality: { | ||
| label: 'Locality Verbatim', | ||
| type: 'verbatim', | ||
| resolve: (_, { primaryEvent: ev }) => ev?.["ods:hasLocation"]?.["dwc:verbatimLocality"], | ||
| }, | ||
| geodeticDatum: { | ||
| label: 'Geodetic Datum', | ||
| resolve: (_, { primaryEvent: ev }) => ev?.["ods:hasLocation"]?.["dwc:geodeticDatum"] | ||
| } | ||
| }, | ||
|
|
||
| COLLECTING_EVENT: { | ||
| collector: { | ||
| label: 'Collector', | ||
| resolve: (_, { primaryEvent: ev }) => ev?.["ods:hasAgents"]?.[0]?.["schema:name"] | ||
| }, | ||
| date: { | ||
| label: 'Date', | ||
| resolve: (_, { primaryEvent: ev }) => ev?.["dwc:eventDate"] | ||
| }, | ||
| verbatimDate: { | ||
| label: 'Date verbatim', | ||
| resolve: (_, { primaryEvent: ev }) => ev?.["dwc:verbatimEventDate"], | ||
| type: 'verbatim' | ||
| } | ||
| }, | ||
|
|
||
| CITATION_LICENSE: { | ||
| license: { | ||
| label: 'License Agreement', | ||
| resolve: (ds) => ds["dcterms:license"] | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| export default DIGITAL_SPECIMEN_SCHEMA_MAP; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| /* MapperContext to set the context of the data, e.g. acceptedIdentification */ | ||
| interface MapperContext { | ||
| acceptedIdentification?: any; | ||
| primaryEvent?: any; | ||
| } | ||
|
|
||
| /* UI Property interface to map data to */ | ||
|
southeo marked this conversation as resolved.
Outdated
|
||
| interface UIProperty { | ||
| label: string; | ||
| value: any; | ||
| isHtml: boolean; | ||
| type: string; | ||
| } | ||
|
|
||
| /* Corresponding field config interface for DigitalSpecimen schema */ | ||
| interface FieldConfig { | ||
| label: string; | ||
| resolve: (ds: any, context: MapperContext) => any; | ||
| isHtml?: boolean; | ||
| type?: string; | ||
| } | ||
|
|
||
| /* Result of the Digital Specimen data mapper */ | ||
| interface DigitalSpecimenUIModel { | ||
| SPECIMEN_RECORD: Record<string, UIProperty>; | ||
| IDENTIFICATION: Record<string, UIProperty>; | ||
| LOCATION: Record<string, UIProperty>; | ||
| COLLECTING_EVENT: Record<string, UIProperty>; | ||
| CITATION_LICENSE: Record<string, UIProperty>; | ||
| } | ||
|
|
||
| /* Generic map to handle schemas for the digitalSpecimenUIModel */ | ||
| type SchemaMap = { | ||
| [Key in keyof DigitalSpecimenUIModel]: Record<string, FieldConfig>; | ||
| }; | ||
|
|
||
| export type { UIProperty, FieldConfig, DigitalSpecimenUIModel, SchemaMap }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you sure this cannot be changed with
overrides? Or are all the earlier packages vulnerable and new versions not compatible with formik ?https://docs.npmjs.com/cli/v8/configuring-npm/package-json#overrides
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I could. I have a follow up user story to address all these trivyignores, because the list is getting long and I'd like to clean them up.