diff --git a/packages/types/Resources.ts b/packages/types/Resources.ts index 4aac4f210..0ab1f06ad 100644 --- a/packages/types/Resources.ts +++ b/packages/types/Resources.ts @@ -74,8 +74,8 @@ export type FlatResource = { lastUpdated: string; stringAttributes: (ReferrableResourceTranslatableAttribute & { key: string })[]; referenceStringAttributes: (ReferrableResourceTranslatableAttribute & { - key: string; list: string; + key: string; })[]; booleanAttributes: (ReferrableResourceAttribute & { key: string })[]; numberAttributes: (ReferrableResourceAttribute & { key: string })[]; diff --git a/packages/types/ResourcesImport.ts b/packages/types/ResourcesImport.ts index 4e2c29361..65f3d557f 100644 --- a/packages/types/ResourcesImport.ts +++ b/packages/types/ResourcesImport.ts @@ -13,7 +13,7 @@ * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see https://www.gnu.org/licenses/. */ -import { FlatResource } from './Resources'; +import { FlatResource, ReferrableResourceTranslatableAttribute } from './Resources'; export type InlineAttributeProperty = | 'stringAttributes' @@ -40,7 +40,21 @@ export type ImportProgress = ImportBatch & { lastProcessedId: string; }; +/** + * Type that allows reference attributes to be specified by ID rathger than value when importing + */ +export type ReferrableResourceReferenceAttribute = ( + | ReferrableResourceTranslatableAttribute + | (Omit & { id: string }) +) & { + list: string; +}; +export type ImportFlatResource = Omit & { + referenceStringAttributes: (ReferrableResourceReferenceAttribute & { + key: string; + })[]; +}; export type ImportRequestBody = { - importedResources: FlatResource[]; + importedResources: ImportFlatResource[]; batch: ImportBatch; }; diff --git a/resources-domain/resources-import-consumer/tests/unit/handler.test.ts b/resources-domain/resources-import-consumer/tests/unit/handler.test.ts index 9a50f2e40..efff590e6 100644 --- a/resources-domain/resources-import-consumer/tests/unit/handler.test.ts +++ b/resources-domain/resources-import-consumer/tests/unit/handler.test.ts @@ -18,7 +18,7 @@ import parseISO from 'date-fns/parseISO'; import { handler } from '../../index'; import each from 'jest-each'; // eslint-disable-next-line prettier/prettier -import type { FlatResource, ImportRequestBody } from '@tech-matters/types'; +import type { ImportFlatResource, ImportRequestBody } from '@tech-matters/types'; import type { SQSEvent } from 'aws-lambda'; const mockFetch = jest.fn(); @@ -41,8 +41,8 @@ const baselineDate = parseISO('2020-01-01T00:00:00.000Z'); const generateImportResource = ( resourceIdSuffix: string, lastUpdated: Date, - additionalAttributes: Partial = {}, -): FlatResource => ({ + additionalAttributes: Partial = {}, +): ImportFlatResource => ({ accountSid, id: `RESOURCE_${resourceIdSuffix}`, name: `Resource ${resourceIdSuffix}`, diff --git a/resources-domain/resources-import-producer/src/khpMappings.ts b/resources-domain/resources-import-producer/src/khpMappings.ts index 254a53896..10e1b8fa3 100644 --- a/resources-domain/resources-import-producer/src/khpMappings.ts +++ b/resources-domain/resources-import-producer/src/khpMappings.ts @@ -529,7 +529,7 @@ export const KHP_MAPPING_NODE: MappingNode = { 'howIsServiceOffered/{howIsServiceOfferedIndex}', 'khp-how-is-service-offered', { - value: ctx => ctx.parentValue.en, + value: ctx => ctx.parentValue.objectId, language: ctx => ctx.captures.language, // value: ctx => ctx.currentValue.en || ctx.currentValue.fr, }, @@ -540,7 +540,7 @@ export const KHP_MAPPING_NODE: MappingNode = { }, accessibility: referenceAttributeMapping('accessibility', 'khp-accessibility', { // We use objectId or the name for this referrable resources? - value: ctx => ctx.currentValue.objectId, + id: ctx => ctx.currentValue.objectId, // value: ctx => ctx.currentValue.en || ctx.currentValue.fr, }), volunteer: { diff --git a/resources-domain/resources-import-producer/src/mappers.ts b/resources-domain/resources-import-producer/src/mappers.ts index 4b9669965..74b45e9e9 100644 --- a/resources-domain/resources-import-producer/src/mappers.ts +++ b/resources-domain/resources-import-producer/src/mappers.ts @@ -71,11 +71,17 @@ export type TranslatableAttributeMapping = AttributeMapping<'stringAttributes'> export type ReferenceAttributeMapping = Omit< AttributeMapping<'referenceStringAttributes'>, - 'infoGenerator' + 'infoGenerator' | 'valueGenerator' > & { list: string; languageGenerator: ContextConsumerFunc; -}; +} & ( + { + valueGenerator: ContextConsumerFunc>; + } | { + idGenerator: ContextConsumerFunc; + } +); export const isResourceFieldMapping = (mapping: any): mapping is ResourceFieldMapping => { return mapping && mapping.field; @@ -100,7 +106,7 @@ export const isReferenceAttributeMapping = ( mapping && mapping.property === 'referenceStringAttributes' && typeof mapping.keyGenerator === 'function' && - typeof mapping.valueGenerator === 'function' && + (typeof mapping.valueGenerator === 'function' || typeof mapping.idGenerator === 'function') && typeof mapping.list === 'string' && mapping.list ); @@ -205,23 +211,32 @@ export const referenceAttributeMapping = ( key: ValueOrContextConsumerFunc, list: string, data: { - value: ValueOrContextConsumerFunc>; language?: ValueOrContextConsumerFunc; - }, + } & ({ + value: ValueOrContextConsumerFunc>; + } | { + id: ValueOrContextConsumerFunc; + }), ): ReferenceAttributeMapping => { - const { infoGenerator, ...mappingResult } = attributeMapping( - 'referenceStringAttributes', - key, - data, - ); - - const mapping = { - ...mappingResult, + + const mapping: Omit = { + property: 'referenceStringAttributes', + keyGenerator: typeof key === 'function' ? key : context => substituteCaptureTokens(key, context), list, + ...( + 'value' in data ? { + valueGenerator: + typeof data.value === 'function' + ? data.value + : () => data.value, + } : { + idGenerator: data.id === 'function' ? data.id : () => data.id, + } + ), }; // This case should be impossible but we gotta help TS if (!isReferenceAttributeMapping(mapping)) { - throw new Error(`Panic! mappingResult is not ReferenceAttributeMapping: ${mappingResult}`); + throw new Error(`Panic! mapping is not ReferenceAttributeMapping: ${mapping}`); } if (isContextConsumerFunc(data.language)) { diff --git a/resources-domain/resources-import-producer/src/transformExternalResourceToApiResource.ts b/resources-domain/resources-import-producer/src/transformExternalResourceToApiResource.ts index 6d328b868..69b433db1 100644 --- a/resources-domain/resources-import-producer/src/transformExternalResourceToApiResource.ts +++ b/resources-domain/resources-import-producer/src/transformExternalResourceToApiResource.ts @@ -15,7 +15,7 @@ */ // eslint-disable-next-line prettier/prettier -import type { AccountSID, FlatResource, InlineAttributeProperty } from '@tech-matters/types'; +import type { AccountSID, ImportFlatResource, InlineAttributeProperty } from '@tech-matters/types'; import { KhpApiResource } from '.'; import { FieldMappingContext, @@ -33,41 +33,58 @@ import { isValid, parseISO } from 'date-fns'; const pushResourceFieldMapping = ({ aseloResource, context, mapping }: { mapping: Omit, - aseloResource: FlatResource, + aseloResource: ImportFlatResource, context: FieldMappingContext }): void => { aseloResource[mapping.field] = mapping.valueGenerator(context); }; const pushReferenceAttributeMapping = ({ aseloResource, context, mapping }: { - mapping: Omit, - aseloResource: FlatResource, + mapping:ReferenceAttributeMapping, + aseloResource: ImportFlatResource, context: FieldMappingContext }): void => { - const value = mapping.valueGenerator(context); - const key = mapping.keyGenerator(context); - - if (value === null || value === undefined) { - console.debug(`No value provided to referenceStringAttributes: key ${key} and value ${value} - omitting attribute`); - return; - } - if (typeof value !== 'string') { - console.info(`Wrong value provided to referenceStringAttributes: key ${key} and value ${value} - omitting attribute`); - return; - } - - aseloResource.referenceStringAttributes.push({ + const attribute = { key: mapping.keyGenerator(context), - value: mapping.valueGenerator(context) ?? '', language: mapping.languageGenerator(context), list: mapping.list, - }); + }; + if ('valueGenerator' in mapping) { + const value = mapping.valueGenerator(context); + + if (value === null || value === undefined) { + console.debug(`No value provided to referenceStringAttributes: key ${attribute.key} and value ${value} - omitting attribute`); + return; + } + if (typeof value !== 'string') { + console.info(`Wrong value provided to referenceStringAttributes: key ${attribute.key} and value ${value} - omitting attribute`); + return; + } + aseloResource.referenceStringAttributes.push({ + ...attribute, + value, + }); + } else if ('idGenerator' in mapping) { + const id = mapping.idGenerator(context); + if (id === null || id === undefined) { + console.debug(`No id provided to referenceStringAttributes: key ${attribute.key} and id ${id} - omitting attribute`); + return; + } + if (typeof id !== 'string') { + console.info(`Wrong id type provided to referenceNumberAttributes: key ${attribute.key} and id ${id} - omitting attribute`); + return; + } + aseloResource.referenceStringAttributes.push({ + ...attribute, + id, + }); + } }; const pushInlineAttributeMapping = ({ aseloResource, context, mapping }: { mapping: Omit, 'children'>, - aseloResource: FlatResource, + aseloResource: ImportFlatResource, context: FieldMappingContext }): void => { const value = mapping.valueGenerator(context); @@ -148,8 +165,8 @@ const mapNode = ( mappingNode: MappingNode, dataNode: any, parentContext: FieldMappingContext, - aseloResource: FlatResource, -): FlatResource => { + aseloResource: ImportFlatResource, +): ImportFlatResource => { Object.entries(mappingNode).forEach(([property, { children, ...mapping }]) => { const captureProperty = property.match(/{(?.*)}/)?.groups?.captureProperty; @@ -204,8 +221,8 @@ export const transformExternalResourceToApiResource = ( resourceMapping: MappingNode, accountSid: AccountSID, khpResource: T, -): FlatResource => { - const resource: FlatResource = { +): ImportFlatResource => { + const resource: ImportFlatResource = { accountSid, id: '', lastUpdated: '', @@ -228,5 +245,5 @@ export const transformExternalResourceToApiResource = ( export const transformKhpResourceToApiResource = ( accountSid: AccountSID, khpResource: KhpApiResource, -): FlatResource => +): ImportFlatResource => transformExternalResourceToApiResource(khp.KHP_MAPPING_NODE, accountSid, khpResource); diff --git a/resources-domain/resources-service/src/import/importDataAccess.ts b/resources-domain/resources-service/src/import/importDataAccess.ts index 106afa25c..a3dc5380b 100644 --- a/resources-domain/resources-service/src/import/importDataAccess.ts +++ b/resources-domain/resources-service/src/import/importDataAccess.ts @@ -14,7 +14,7 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ -import { AccountSID, FlatResource, ImportProgress } from '@tech-matters/types'; +import { AccountSID, FlatResource, ImportFlatResource, ImportProgress } from '@tech-matters/types'; import { generateUpdateImportProgressSql, generateUpsertSqlFromImportResource, @@ -22,6 +22,7 @@ import { } from './sql'; import { ITask } from 'pg-promise'; import { db } from '../connection-pool'; +import { SELECT_RESOURCE_IN_IDS } from '../resource/sql/resourceGetSql'; const txIfNotInOne = async ( task: ITask<{}> | undefined, @@ -33,19 +34,35 @@ const txIfNotInOne = async ( return db.tx(work); }; -export type UpsertImportedResourceResult = { - id: string; - success: boolean; - error?: Error; -}; +export type UpsertImportedResourceResult = + | { + success: true; + resource: FlatResource; + } + | { + id: string; + success: false; + error: Error; + }; export const upsertImportedResource = (task?: ITask<{}>) => async ( accountSid: AccountSID, - resource: FlatResource, + resource: ImportFlatResource, ): Promise => { return txIfNotInOne(task, async tx => { await tx.none(generateUpsertSqlFromImportResource(accountSid, resource)); - return { id: resource.id, success: true }; + const savedResource: FlatResource | null = await tx.oneOrNone(SELECT_RESOURCE_IN_IDS, { + accountSid, + resourceIds: [resource.id], + }); + if (savedResource) { + return { resource: savedResource, success: true }; + } + return { + id: resource.id, + success: false, + error: new Error('Upserted resource not found in DB after saving'), + }; }); }; diff --git a/resources-domain/resources-service/src/import/importService.ts b/resources-domain/resources-service/src/import/importService.ts index 61dda2973..3bf66bc16 100644 --- a/resources-domain/resources-service/src/import/importService.ts +++ b/resources-domain/resources-service/src/import/importService.ts @@ -14,7 +14,7 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ -import { AccountSID, FlatResource, ImportBatch, ImportProgress } from '@tech-matters/types'; +import { AccountSID, ImportFlatResource, ImportBatch, ImportProgress } from '@tech-matters/types'; import { db } from '../connection-pool'; import { getImportState, @@ -27,7 +27,13 @@ import { publishSearchIndexJob } from '../resource-jobs/client-sqs'; export type ValidationFailure = { reason: 'missing field'; fields: string[]; - resource: FlatResource; + resource: ImportFlatResource; +}; + +type ImportedResourceResult = { + id: string; + success: boolean; + error?: Error; }; export const isValidationFailure = (result: any): result is ValidationFailure => { @@ -40,9 +46,9 @@ const importService = () => { return { upsertResources: async ( accountSid: AccountSID, - resources: FlatResource[], + resources: ImportFlatResource[], batch: ImportBatch, - ): Promise => { + ): Promise => { if (!resources?.length) return []; try { return await db.tx(async t => { @@ -68,7 +74,7 @@ const importService = () => { } results.push(result); try { - await publishSearchIndexJob(resource.accountSid, resource); + await publishSearchIndexJob(resource.accountSid, result.resource); } catch (e) { console.error( `Failed to publish search index job for ${resource.accountSid}/${resource.id}`, @@ -90,7 +96,18 @@ const importService = () => { lastProcessedId: id, }); - return results; + return results.map(result => + result.success + ? { + id: result.resource.id, + success: result.success, + } + : { + id: result.id, + success: result.success, + error: result.error, + }, + ); }); } catch (e) { const error = e as any; diff --git a/resources-domain/resources-service/src/import/sql.ts b/resources-domain/resources-service/src/import/sql.ts index 1384a09aa..494003ff6 100644 --- a/resources-domain/resources-service/src/import/sql.ts +++ b/resources-domain/resources-service/src/import/sql.ts @@ -15,11 +15,11 @@ */ import { pgp } from '../connection-pool'; -import { AccountSID, ImportProgress, FlatResource } from '@tech-matters/types'; +import { AccountSID, ImportProgress, ImportFlatResource } from '@tech-matters/types'; export const generateUpsertSqlFromImportResource = ( accountSid: string, - { stringAttributes, referenceStringAttributes, ...resourceRecord }: FlatResource, + { stringAttributes, referenceStringAttributes, ...resourceRecord }: ImportFlatResource, ): string => { const sqlBatch: string[] = []; @@ -99,14 +99,26 @@ export const generateUpsertSqlFromImportResource = ( sqlBatch.push( ...referenceStringAttributes.map(attribute => { const { language, ...queryValues } = attribute; - return pgp.as.format( - `INSERT INTO resources."ResourceReferenceStringAttributes" - ("accountSid", "resourceId", "key", "list", "referenceId") - SELECT $, $, $, $, "id" - FROM resources."ResourceReferenceStringAttributeValues" - WHERE "accountSid" = $ AND "list" = $ AND "value" = $`, - { ...queryValues, accountSid, resourceId: resourceRecord.id }, - ); + if ('value' in queryValues) { + return pgp.as.format( + `INSERT INTO resources."ResourceReferenceStringAttributes" + ("accountSid", "resourceId", "key", "list", "referenceId") + SELECT $, $, $, $, "id" + FROM resources."ResourceReferenceStringAttributeValues" + WHERE "accountSid" = $ AND "list" = $ AND "value" = $`, + { ...queryValues, accountSid, resourceId: resourceRecord.id }, + ); + } else { + // We still do the sub select to ensure we omit any orphaned reference values rather than causing a FK violation + return pgp.as.format( + `INSERT INTO resources."ResourceReferenceStringAttributes" + ("accountSid", "resourceId", "key", "list", "referenceId") + SELECT $, $, $, $, "id" + FROM resources."ResourceReferenceStringAttributeValues" + WHERE "accountSid" = $ AND "list" = $ AND "id" = $`, + { ...queryValues, accountSid, resourceId: resourceRecord.id }, + ); + } }), ); return sqlBatch.join(';\n');