From 89fbb8690e1e84cf5f759d242fad783c0ea9440a Mon Sep 17 00:00:00 2001 From: Dani Pinyol Date: Fri, 21 Feb 2020 22:27:17 +0100 Subject: [PATCH 1/8] feat(contentful): export csv for translators --- .../botonic-plugin-contentful/package.json | 1 + .../src/tools/translators/csv-export.ts | 117 ++++++++++++++++++ .../tools/translators/csv-for-translator.ts | 35 ++++++ .../tools/translators/csv-export.test.ts | 70 +++++++++++ 4 files changed, 223 insertions(+) create mode 100644 packages/botonic-plugin-contentful/src/tools/translators/csv-export.ts create mode 100644 packages/botonic-plugin-contentful/src/tools/translators/csv-for-translator.ts create mode 100644 packages/botonic-plugin-contentful/tests/tools/translators/csv-export.test.ts diff --git a/packages/botonic-plugin-contentful/package.json b/packages/botonic-plugin-contentful/package.json index c512df4b02..a4d7081471 100644 --- a/packages/botonic-plugin-contentful/package.json +++ b/packages/botonic-plugin-contentful/package.json @@ -25,6 +25,7 @@ "dependencies": { "@babel/runtime": "^7.8.3", "contentful": "^7.13.1", + "csv-stringify": "^5.3.6", "escape-string-regexp": "^2.0.0", "memoizee": "^0.4.14", "moment": "^2.24.0", diff --git a/packages/botonic-plugin-contentful/src/tools/translators/csv-export.ts b/packages/botonic-plugin-contentful/src/tools/translators/csv-export.ts new file mode 100644 index 0000000000..29ceb35666 --- /dev/null +++ b/packages/botonic-plugin-contentful/src/tools/translators/csv-export.ts @@ -0,0 +1,117 @@ +import { + Button, + Carousel, + CMS, + CommonFields, + Content, + ContentType, + MESSAGE_TYPES, + StartUp, +} from '../../index' +import { Locale } from '../../nlp' +import { Text, TopContent } from '../../cms' +import stringify from 'csv-stringify' +import * as stream from 'stream' +import * as fs from 'fs' +import { promisify } from 'util' + +console.log(typeof stream.finished) +const finished = promisify(stream.finished) + +class I18nField { + constructor(readonly name: string, readonly value?: string) {} +} + +interface CsvExportOptions { + nameFilter?: (name: string) => boolean +} + +/*** + * Uses https://csv.js.org/stringify/api/ + */ +export class CsvExport { + private toFields = new ContentToI18nFields() + constructor(private readonly options: CsvExportOptions) {} + + create_stringifier() { + return stringify({ + escape: '"', + delimiter: ';', + quote: '"', + quoted: true, + record_delimiter: 'windows', + header: true, + columns: ['Model', 'Id', 'Code', 'Field', 'From', 'To'], + }) + } + + async write(fname: string, cms: CMS, locale: Locale): Promise { + const stringifier = this.create_stringifier() + const readable = stream.Readable.from(this.generate(cms, locale)) + const writable = readable + .pipe(stringifier) + .pipe(fs.createWriteStream(fname)) + return this.toPromise(writable) + } + + async toPromise(writable: stream.Writable): Promise { + return finished(writable) + } + + async *generate(cms: CMS, from: Locale): AsyncGenerator { + for (const model of [...MESSAGE_TYPES, ContentType.BUTTON]) { + const contents = await cms.contents(model, { locale: from }) + for (const content of contents) { + if (this.options.nameFilter && !this.options.nameFilter(content.name)) { + continue + } + console.log('Exporting content', content.name) + for (const field of this.getI18nFields(content)) { + const TO_COLUMN = '' + yield [...field, TO_COLUMN] + } + } + } + } + + // TODO create a text TextFieldVisitor + getI18nFields(content: Content): string[][] { + const columns = [content.contentType, content.id, content.name] + if (!(content instanceof TopContent)) { + if (content instanceof Button) { + return [[...columns, 'Text', content.text]] + } + } + const fields = this.toFields.getFields(content) + return fields.filter(f => f.value).map(f => [...columns, f.name, f.value!]) + } +} + +class ContentToI18nFields { + getFields(content: Content): I18nField[] { + if (content instanceof StartUp) { + return [ + ...this.getCommonFields(content.common), + new I18nField('Text', content.text), + ] + } else if (content instanceof Text) { + return [ + ...this.getCommonFields(content.common), + new I18nField('Text', content.text), + ] + } else if (content instanceof Carousel) { + const fields = this.getCommonFields(content.common) + return fields.concat( + ...content.elements.map(e => [ + new I18nField('Title', e.title), + new I18nField('Subtitle', e.subtitle), + ]) + ) + } + return [] + } + + getCommonFields(common: CommonFields): I18nField[] { + return [new I18nField('Short text', common.shortText)] + } +} diff --git a/packages/botonic-plugin-contentful/src/tools/translators/csv-for-translator.ts b/packages/botonic-plugin-contentful/src/tools/translators/csv-for-translator.ts new file mode 100644 index 0000000000..ed057e8b5d --- /dev/null +++ b/packages/botonic-plugin-contentful/src/tools/translators/csv-for-translator.ts @@ -0,0 +1,35 @@ +import { CsvExport } from './csv-export' +import { Locale } from '../../nlp' +import Contentful from '../../contentful' + +async function writeCsvForTranslators( + spaceId: string, + accessToken: string, + locales: Locale[] +) { + const cms = new Contentful({ + spaceId: spaceId, + accessToken: accessToken, + environment: 'master', + }) + const exporter = new CsvExport({ + // nameFilter: n => ['HOME_RETURN_URL'].includes(n), + }) + const promises = locales.map((from: string) => + exporter.write(`contentful_${from}.csv`, cms, from) + ) + return Promise.all(promises) +} + +const spaceId = process.argv[2] +const token = process.argv[3] +const locales = process.argv.slice(4) as Locale[] +console.log(process.execArgv) +if (process.argv.length < 5) { + console.error(`Usage: space_id access_token language`) + process.exit(1) +} + +writeCsvForTranslators(spaceId, token, locales).then(() => { + console.log('done') +}) diff --git a/packages/botonic-plugin-contentful/tests/tools/translators/csv-export.test.ts b/packages/botonic-plugin-contentful/tests/tools/translators/csv-export.test.ts new file mode 100644 index 0000000000..fd907db99c --- /dev/null +++ b/packages/botonic-plugin-contentful/tests/tools/translators/csv-export.test.ts @@ -0,0 +1,70 @@ +import { CsvExport } from '../../../src/tools/translators/csv-export' +import { testContentful } from '../../../tests/contentful/contentful.helper' +import { ENGLISH } from '../../../src/nlp' +import { TextBuilder } from '../../../src/cms/factories' +import sync from 'csv-stringify/lib/sync' +import { RndButtonsBuilder } from '../../../src/cms/test-helpers' + +test('integration test', async () => { + const cms = testContentful() + const exporter = new CsvExport({ + // nameFilter: n => ['HOME_RETURN_URL'].includes(n), + }) + const from = ENGLISH + await exporter.write(`contentful_${from}.csv`, cms, from) +}) + +test('getI18nFields Text', () => { + const exporter = new CsvExport({ + nameFilter: n => ['HOME_RETURN_URL'].includes(n), + }) + const fields = exporter.getI18nFields( + new TextBuilder('id1', 'name1', 'long text').withShortText('short1').build() + ) + expect(fields).toEqual([ + ['text', 'id1', 'name1', 'Short text', 'short1'], + ['text', 'id1', 'name1', 'Text', 'long text'], + ]) +}) + +test('getI18nFields Button', () => { + const exporter = new CsvExport({ + nameFilter: n => ['HOME_RETURN_URL'].includes(n), + }) + const button = new RndButtonsBuilder().withButton().build()[0] + const fields = exporter.getI18nFields(button) + expect(fields).toEqual([ + ['button', button.id, button.name, 'Text', button.text], + ]) +}) + +// returns "" because push callback within sync is never called. why? +test.skip('stringifier sync', () => { + const exporter = new CsvExport({ + nameFilter: n => ['HOME_RETURN_URL'].includes(n), + }) + const stringifier = exporter.create_stringifier() + const str = sync(['1 "2" 3', '4'], stringifier.options) + expect(str).toEqual('"1 ""2"" 3","4"') + // readable.pipe().pip +}) + +test('stringifier', () => { + const exporter = new CsvExport({ + nameFilter: n => ['HOME_RETURN_URL'].includes(n), + }) + const fields = [] + // does not work with Readable.from + // const readable = Readable.from(['1 "2" 3', '4']).pipe(exporter.stringifier) + const stringifier = exporter.create_stringifier() + stringifier.write(['1 "2" 3', '4']) + stringifier.end() + let row: Buffer + while ((row = stringifier.read())) { + fields.push(row.toString('utf8')) + } + + expect(fields).toEqual([ + '"Model";"Id";"Code";"Field";"From";"To"\r\n"1 ""2"" 3";"4"\r\n', + ]) +}) From 83b21173397b21aa0831144864b9abfeb06b171e Mon Sep 17 00:00:00 2001 From: Dani Pinyol Date: Fri, 28 Feb 2020 08:32:51 +0100 Subject: [PATCH 2/8] feat(contentful): add getContentType to ReducedClientApi --- .../src/contentful/delivery-api.ts | 6 ++++++ .../src/contentful/delivery/cache.ts | 4 +++- .../src/contentful/delivery/client-api.ts | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/botonic-plugin-contentful/src/contentful/delivery-api.ts b/packages/botonic-plugin-contentful/src/contentful/delivery-api.ts index a902e8d221..b8a3497e42 100644 --- a/packages/botonic-plugin-contentful/src/contentful/delivery-api.ts +++ b/packages/botonic-plugin-contentful/src/contentful/delivery-api.ts @@ -35,6 +35,8 @@ export interface DeliveryApi { context: Context, query?: any ): Promise> + + getContentType(id: string): Promise } /** @@ -67,6 +69,10 @@ export class AdaptorDeliveryApi implements DeliveryApi { ) } + async getContentType(id: string): Promise { + return this.client.getContentType(id) + } + private static queryFromContext(context: Context, query: any = {}): any { if (context.locale) { query['locale'] = context.locale diff --git a/packages/botonic-plugin-contentful/src/contentful/delivery/cache.ts b/packages/botonic-plugin-contentful/src/contentful/delivery/cache.ts index 461c0f26df..0a1eaa0782 100644 --- a/packages/botonic-plugin-contentful/src/contentful/delivery/cache.ts +++ b/packages/botonic-plugin-contentful/src/contentful/delivery/cache.ts @@ -1,5 +1,5 @@ import * as contentful from 'contentful/index' -import { Entry } from 'contentful/index' +import { ContentType, Entry } from 'contentful/index' import memoize from 'memoizee' import { ReducedClientApi } from './client-api' @@ -7,6 +7,7 @@ export class CachedClientApi implements ReducedClientApi { readonly getAsset: (id: string, query?: any) => Promise readonly getEntries: (query: any) => Promise> readonly getEntry: (id: string, query?: any) => Promise> + readonly getContentType: (id: string) => Promise constructor(readonly client: ReducedClientApi, readonly cacheTtlMs = 10000) { const options = (length: number) => @@ -24,5 +25,6 @@ export class CachedClientApi implements ReducedClientApi { this.getAsset = memoize(client.getAsset, options(2)) this.getEntries = memoize(client.getEntries, options(1)) this.getEntry = memoize(client.getEntry, options(2)) + this.getContentType = memoize(client.getContentType, options(1)) } } diff --git a/packages/botonic-plugin-contentful/src/contentful/delivery/client-api.ts b/packages/botonic-plugin-contentful/src/contentful/delivery/client-api.ts index d169f904bb..6c79d72262 100644 --- a/packages/botonic-plugin-contentful/src/contentful/delivery/client-api.ts +++ b/packages/botonic-plugin-contentful/src/contentful/delivery/client-api.ts @@ -2,5 +2,5 @@ import { ContentfulClientApi } from 'contentful' export type ReducedClientApi = Pick< ContentfulClientApi, - 'getAsset' | 'getEntries' | 'getEntry' + 'getAsset' | 'getEntries' | 'getEntry' | 'getContentType' > From bd25f38b0f187d7768dff9a45270061051575967 Mon Sep 17 00:00:00 2001 From: Dani Pinyol Date: Fri, 28 Feb 2020 16:51:13 +0100 Subject: [PATCH 3/8] chore(contentful): improvements in CMS enum types removed chitchat from enum value arrays because it's not a real content type --- .../src/cms/callback.ts | 9 ++++-- .../botonic-plugin-contentful/src/cms/cms.ts | 28 +++++++++++++------ .../src/contentful/delivery-api.ts | 7 ++++- .../src/contentful/index.ts | 26 +++++++++++------ .../src/tools/translators/csv-import.ts | 0 .../src/util/enums.ts | 3 ++ .../tests/cms/cms.test.ts | 6 ++-- .../tests/contentful/contentful.helper.ts | 24 ++++++++++++---- .../tests/contentful/contents.test.ts | 6 ++-- .../tests/contentful/delivery.test.ts | 8 ++++-- .../src/components/multichannel/index.d.ts | 4 +-- 11 files changed, 86 insertions(+), 35 deletions(-) create mode 100644 packages/botonic-plugin-contentful/src/tools/translators/csv-import.ts create mode 100644 packages/botonic-plugin-contentful/src/util/enums.ts diff --git a/packages/botonic-plugin-contentful/src/cms/callback.ts b/packages/botonic-plugin-contentful/src/cms/callback.ts index 1b452962e0..ecc34f93b0 100644 --- a/packages/botonic-plugin-contentful/src/cms/callback.ts +++ b/packages/botonic-plugin-contentful/src/cms/callback.ts @@ -1,4 +1,9 @@ -import { CMS, ContentType, MESSAGE_TYPES, MessageContentType } from './cms' +import { + CMS, + ContentType, + MESSAGE_CONTENT_TYPES, + MessageContentType, +} from './cms' import escapeStringRegexp from 'escape-string-regexp' import { Context } from './context' import { TopContent } from './contents' @@ -54,7 +59,7 @@ export class ContentCallback extends Callback { } private static checkDeliverableModel(modelType: string): MessageContentType { - if (MESSAGE_TYPES.includes(modelType as MessageContentType)) { + if (MESSAGE_CONTENT_TYPES.includes(modelType as MessageContentType)) { return modelType as MessageContentType } else { throw new Error( diff --git a/packages/botonic-plugin-contentful/src/cms/cms.ts b/packages/botonic-plugin-contentful/src/cms/cms.ts index 2819acbbdb..87cb4938a0 100644 --- a/packages/botonic-plugin-contentful/src/cms/cms.ts +++ b/packages/botonic-plugin-contentful/src/cms/cms.ts @@ -15,6 +15,7 @@ import { TopContent, Content, } from './contents' +import { enumValues } from '../util/enums' export enum MessageContentType { CAROUSEL = 'carousel', @@ -23,8 +24,10 @@ export enum MessageContentType { CHITCHAT = 'chitchat', //so far it's an alias for TEXT STARTUP = 'startUp', } -export const MESSAGE_TYPES = Object.values(MessageContentType).map( - m => m as MessageContentType + +// CHITCHAT removed because it's an alias for texts +export const MESSAGE_CONTENT_TYPES = enumValues(MessageContentType).filter( + m => m != MessageContentType.CHITCHAT ) export enum NonMessageTopContentType { @@ -39,9 +42,10 @@ export const TopContentType = { ...MessageContentType, ...NonMessageTopContentType, } -export const TOPCONTENT_TYPES = Object.values(TopContentType).map( - m => m as TopContentType -) +export const TOP_CONTENT_TYPES = [ + ...MESSAGE_CONTENT_TYPES, + ...enumValues(NonMessageTopContentType), +] export enum SubContentType { BUTTON = 'button', @@ -49,9 +53,17 @@ export enum SubContentType { } export type ContentType = TopContentType | SubContentType export const ContentType = { ...TopContentType, ...SubContentType } -export const CONTENT_TYPES = Object.values(ContentType).map( - m => m as ContentType -) +export const CONTENT_TYPES = [ + ...TOP_CONTENT_TYPES, + ...enumValues(SubContentType), +] + +export type BotonicContentType = MessageContentType | SubContentType +export const BotonicContentType = { ...MessageContentType, ...SubContentType } +export const BOTONIC_CONTENT_TYPES = [ + ...MESSAGE_CONTENT_TYPES, + ...enumValues(SubContentType), +] export function isSameModel(model1: ContentType, model2: ContentType): boolean { switch (model1) { diff --git a/packages/botonic-plugin-contentful/src/contentful/delivery-api.ts b/packages/botonic-plugin-contentful/src/contentful/delivery-api.ts index b8a3497e42..73de8fe52b 100644 --- a/packages/botonic-plugin-contentful/src/contentful/delivery-api.ts +++ b/packages/botonic-plugin-contentful/src/contentful/delivery-api.ts @@ -70,7 +70,12 @@ export class AdaptorDeliveryApi implements DeliveryApi { } async getContentType(id: string): Promise { - return this.client.getContentType(id) + try { + return this.client.getContentType(id) + } catch (e) { + console.error(`ERROR in getContentType for id ${id}: ${e}`) + throw e + } } private static queryFromContext(context: Context, query: any = {}): any { diff --git a/packages/botonic-plugin-contentful/src/contentful/index.ts b/packages/botonic-plugin-contentful/src/contentful/index.ts index 5b84c75c45..d387f2f831 100644 --- a/packages/botonic-plugin-contentful/src/contentful/index.ts +++ b/packages/botonic-plugin-contentful/src/contentful/index.ts @@ -32,6 +32,7 @@ import * as contentful from 'contentful' import { ContentfulOptions } from '../plugin' import { CachedClientApi } from './delivery/cache' import { CreateClientParams } from 'contentful' +import { IgnoreFallbackDecorator } from './ignore-fallback-decorator' export default class Contentful implements cms.CMS { private readonly _delivery: DeliveryApi @@ -56,15 +57,7 @@ export default class Contentful implements cms.CMS { * https://www.contentful.com/developers/docs/javascript/tutorials/using-js-cda-sdk/ */ constructor(options: ContentfulOptions) { - const params: CreateClientParams = { - space: options.spaceId, - accessToken: options.accessToken, - timeout: options.timeoutMs, - } - if (options.environment) { - params.environment = options.environment - } - const client = contentful.createClient(params) + const client = createContentfulClientApi(options) const deliveryApi = new AdaptorDeliveryApi( options.disableCache ? client @@ -212,5 +205,20 @@ export default class Contentful implements cms.CMS { } } +export function createContentfulClientApi( + options: ContentfulOptions +): contentful.ContentfulClientApi { + const params: CreateClientParams = { + space: options.spaceId, + accessToken: options.accessToken, + timeout: options.timeoutMs, + } + if (options.environment) { + params.environment = options.environment + } + const client = contentful.createClient(params) + return client +} + export { DeliveryApi } from './delivery-api' export { CarouselDelivery } from './contents/carousel' diff --git a/packages/botonic-plugin-contentful/src/tools/translators/csv-import.ts b/packages/botonic-plugin-contentful/src/tools/translators/csv-import.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/botonic-plugin-contentful/src/util/enums.ts b/packages/botonic-plugin-contentful/src/util/enums.ts new file mode 100644 index 0000000000..97714fc4f5 --- /dev/null +++ b/packages/botonic-plugin-contentful/src/util/enums.ts @@ -0,0 +1,3 @@ +export function enumValues(enumType: { [s: string]: T }): T[] { + return Object.values(enumType).map(m => m as T) +} diff --git a/packages/botonic-plugin-contentful/tests/cms/cms.test.ts b/packages/botonic-plugin-contentful/tests/cms/cms.test.ts index cfa5ef7f33..4ff6965c27 100644 --- a/packages/botonic-plugin-contentful/tests/cms/cms.test.ts +++ b/packages/botonic-plugin-contentful/tests/cms/cms.test.ts @@ -1,6 +1,6 @@ -import { TOPCONTENT_TYPES, ContentType } from '../../src/cms' +import { TOP_CONTENT_TYPES, ContentType } from '../../src/cms' test('TEST: ALL_TYPES', () => { - expect(TOPCONTENT_TYPES.length).toBeGreaterThanOrEqual(9) - expect(TOPCONTENT_TYPES).toContain(ContentType.TEXT) + expect(TOP_CONTENT_TYPES.length).toBeGreaterThanOrEqual(8) + expect(TOP_CONTENT_TYPES).toContain(ContentType.TEXT) }) diff --git a/packages/botonic-plugin-contentful/tests/contentful/contentful.helper.ts b/packages/botonic-plugin-contentful/tests/contentful/contentful.helper.ts index 353c77ea23..0f508d8dd9 100644 --- a/packages/botonic-plugin-contentful/tests/contentful/contentful.helper.ts +++ b/packages/botonic-plugin-contentful/tests/contentful/contentful.helper.ts @@ -1,17 +1,29 @@ import Contentful from '../../src/contentful' import { Context } from '../../src/cms' +import { ContentfulOptions } from '../../src' export function testSpaceId(): string { return process.env.CONTENTFUL_TEST_SPACE_ID! } -export function testContentful(): Contentful { +export function testContentful( + options: Partial = {} +): Contentful { + return new Contentful(testContentfulOptions(options)) +} + +export function testContentfulOptions( + options: Partial = {} +): ContentfulOptions { // useful to have long timeouts so that we can send many requests simultaneously - return new Contentful({ - spaceId: testSpaceId(), - accessToken: process.env.CONTENTFUL_TEST_TOKEN!, - environment: 'master', - }) + return { + ...{ + spaceId: testSpaceId(), + accessToken: process.env.CONTENTFUL_TEST_TOKEN!, + environment: 'master', + }, + ...options, + } } export function testContext( diff --git a/packages/botonic-plugin-contentful/tests/contentful/contents.test.ts b/packages/botonic-plugin-contentful/tests/contentful/contents.test.ts index 1d9cee4235..16c75d45b5 100644 --- a/packages/botonic-plugin-contentful/tests/contentful/contents.test.ts +++ b/packages/botonic-plugin-contentful/tests/contentful/contents.test.ts @@ -10,6 +10,8 @@ import { } from '../../src/cms' import { testContentful, testContext } from './contentful.helper' +const BUTTON_POST_FAQ3 = '40buQOqp9jbwoxmMZhFO16' + test('TEST: contentful contents buttons', async () => { const buttons = await testContentful().contents(ContentType.BUTTON, { locale: 'en', @@ -26,10 +28,10 @@ test('TEST: contentful contents buttons', async () => { Callback.ofPayload('RATING_1') ) ) - const empezar = buttons.filter(b => b.id == '40buQOqp9jbwoxmMZhFO16') + const empezar = buttons.filter(b => b.id == BUTTON_POST_FAQ3) expect(empezar[0]).toEqual( new Button( - '40buQOqp9jbwoxmMZhFO16', + BUTTON_POST_FAQ3, 'POST_FAQ3', 'Return an article', new ContentCallback(ContentType.TEXT, 'C39lEROUgJl9hHSXKOEXS') diff --git a/packages/botonic-plugin-contentful/tests/contentful/delivery.test.ts b/packages/botonic-plugin-contentful/tests/contentful/delivery.test.ts index 286eeb6207..997ba06149 100644 --- a/packages/botonic-plugin-contentful/tests/contentful/delivery.test.ts +++ b/packages/botonic-plugin-contentful/tests/contentful/delivery.test.ts @@ -1,4 +1,8 @@ -import { ContentCallback, ContentType, MESSAGE_TYPES } from '../../src/cms' +import { + ContentCallback, + ContentType, + MESSAGE_CONTENT_TYPES, +} from '../../src/cms' import { testContentful } from './contentful.helper' const TEST_IMAGE = '3xjvpC7d7PYBmiptEeygfd' @@ -6,7 +10,7 @@ const TEST_IMAGE = '3xjvpC7d7PYBmiptEeygfd' test('TEST: contentful delivery checks that we get the requested message type', async () => { const sut = testContentful() - for (const model of MESSAGE_TYPES) { + for (const model of MESSAGE_CONTENT_TYPES) { const callback = new ContentCallback(model, TEST_IMAGE) const content = callback.deliverPayloadContent(sut, { locale: 'es' }) if (model == ContentType.IMAGE) { diff --git a/packages/botonic-react/src/components/multichannel/index.d.ts b/packages/botonic-react/src/components/multichannel/index.d.ts index c492e0a15c..003e2e7ee6 100644 --- a/packages/botonic-react/src/components/multichannel/index.d.ts +++ b/packages/botonic-react/src/components/multichannel/index.d.ts @@ -1,8 +1,8 @@ import * as React from 'react' export const Multichannel: React.FunctionComponent<{ - firstIndex: number - boldIndex: boolean + firstIndex?: number + boldIndex?: boolean }> // Text From 06e096112b231b5568eb61e0bef7c3b2196e31b0 Mon Sep 17 00:00:00 2001 From: Dani Pinyol Date: Fri, 28 Feb 2020 20:36:58 +0100 Subject: [PATCH 4/8] chore(contentful): deepClone & tests for both clones --- .../src/util/objects.ts | 39 ++++++++++++- .../tests/util/objects.test.ts | 55 +++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 packages/botonic-plugin-contentful/tests/util/objects.test.ts diff --git a/packages/botonic-plugin-contentful/src/util/objects.ts b/packages/botonic-plugin-contentful/src/util/objects.ts index fea5ed280e..3641157cb2 100644 --- a/packages/botonic-plugin-contentful/src/util/objects.ts +++ b/packages/botonic-plugin-contentful/src/util/objects.ts @@ -13,4 +13,41 @@ export function shallowClone(obj: T): T { return clone as T } -// consider this deepClone in TS https://gist.github.com/erikvullings/ada7af09925082cbb89f40ed962d475e +/** + * Deep copy function for TypeScript. + * @param T Generic type of target/copied value. + * @param target Target value to be copied. + * @see Source project, ts-deepcopy https://github.com/ykdr2017/ts-deepcopy + * @see Code pen https://codepen.io/erikvullings/pen/ejyBYg + */ +export const deepClone = (target: T, alreadyCloned: object[] = []): T => { + // @ts-ignore + if (alreadyCloned.includes(target)) { + return target + } + // @ts-ignore + alreadyCloned.push(target) + if (target === undefined) { + return target + } + if (target instanceof Date) { + return new Date(target.getTime()) as any + } + if (target instanceof Array) { + const cp = [] as any[] + ;(target as any[]).forEach(v => { + cp.push(v) + }) + return cp.map((n: any) => deepClone(n, alreadyCloned)) as any + } + if (typeof target === 'object' && target !== {}) { + const cp = { ...(target as { [key: string]: any }) } as { + [key: string]: any + } + Object.keys(cp).forEach(k => { + cp[k] = deepClone(cp[k], alreadyCloned) + }) + return cp as T + } + return target +} diff --git a/packages/botonic-plugin-contentful/tests/util/objects.test.ts b/packages/botonic-plugin-contentful/tests/util/objects.test.ts new file mode 100644 index 0000000000..b165d7d365 --- /dev/null +++ b/packages/botonic-plugin-contentful/tests/util/objects.test.ts @@ -0,0 +1,55 @@ +import { deepClone, shallowClone } from '../../src/util/objects' + +class Subclass { + constructor(public field: number) {} +} +class Class { + constructor(public arr = [3, 4], public sub = new Subclass(1)) {} +} + +test('TEST: shallowClone', async () => { + const source = new Class() + + // act + const copy = shallowClone(source) + + // assert + expect(copy).toEqual(source) + expect(copy).not.toBe(source) + + source.sub = new Subclass(2) + expect(copy).not.toEqual(source) + + copy.sub.field = 3 + expect(source.sub.field).toEqual(2) +}) + +test('TEST: deepClone', async () => { + const source = new Class() + + // act + const copy = deepClone(source) + + // assert + expect(copy).toEqual(source) + expect(copy).not.toBe(source) + + source.sub = new Subclass(2) + expect(copy).not.toEqual(source) + + copy.sub.field = 3 + expect(source.sub.field).not.toEqual(3) +}) + +class RecursiveClass { + constructor(public rec?: RecursiveClass) {} +} + +test('TEST: deepClone no recursive call', async () => { + const source = new RecursiveClass() + source.rec = source + + // act + const copy = deepClone(source) + expect(copy).toEqual(source) +}) From ad20ed3561ca77c8c5fd11b097a4e31b01d6e35f Mon Sep 17 00:00:00 2001 From: Dani Pinyol Date: Fri, 28 Feb 2020 20:41:45 +0100 Subject: [PATCH 5/8] chore(contentful): CI also to build tests --- .github/workflows/botonic-plugin-contentful-tests.yml | 2 +- .github/workflows/botonic-plugin-dynamo-tests.yml | 2 +- packages/botonic-plugin-contentful/jest.setup.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/botonic-plugin-contentful-tests.yml b/.github/workflows/botonic-plugin-contentful-tests.yml index 4b2e674f45..c4e2ec839f 100644 --- a/.github/workflows/botonic-plugin-contentful-tests.yml +++ b/.github/workflows/botonic-plugin-contentful-tests.yml @@ -25,7 +25,7 @@ jobs: - name: Install dev dependencies run: (cd ./packages/$PACKAGE && npm install -D) - name: Build - run: (cd ./packages/$PACKAGE && npm run build) + run: (cd ./packages/$PACKAGE && npm run build_with_tests) - name: Run tests env: CONTENTFUL_TEST_SPACE_ID: ${{ secrets.CONTENTFUL_TEST_SPACE_ID }} diff --git a/.github/workflows/botonic-plugin-dynamo-tests.yml b/.github/workflows/botonic-plugin-dynamo-tests.yml index c32e86ce16..577656fde6 100644 --- a/.github/workflows/botonic-plugin-dynamo-tests.yml +++ b/.github/workflows/botonic-plugin-dynamo-tests.yml @@ -25,7 +25,7 @@ jobs: - name: Install dev dependencies run: (cd ./packages/$PACKAGE && npm install -D) - name: Build - run: (cd ./packages/$PACKAGE && npm run build) + run: (cd ./packages/$PACKAGE && npm run build_with_tests) - name: Run tests env: AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} diff --git a/packages/botonic-plugin-contentful/jest.setup.js b/packages/botonic-plugin-contentful/jest.setup.js index 4fd514ad16..96e13e6814 100644 --- a/packages/botonic-plugin-contentful/jest.setup.js +++ b/packages/botonic-plugin-contentful/jest.setup.js @@ -1,2 +1,2 @@ // uncomment to extend jest test timeout while debugging from IDE -// jest.setTimeout(300000); +// jest.setTimeout(3000000) From 1700fedb1cf6f0ae05d358b1617fa7ad9f1d9fcb Mon Sep 17 00:00:00 2001 From: Dani Pinyol Date: Fri, 28 Feb 2020 20:47:56 +0100 Subject: [PATCH 6/8] feat(contentful): improvements in CsvForTranslators trim and skip empty texts sort csv by left to right columns --- .../package-lock.json | 18 ++++ .../botonic-plugin-contentful/package.json | 4 +- .../src/cms/context.ts | 5 ++ .../src/contentful/contents/carousel.ts | 2 +- .../src/contentful/index.ts | 4 +- .../src/tools/translators/csv-export.ts | 82 +++++++++++-------- .../tools/translators/csv-for-translator.ts | 5 +- .../src/typings.d.ts | 5 ++ .../tools/translators/csv-export.test.ts | 59 +++++++------ 9 files changed, 121 insertions(+), 63 deletions(-) diff --git a/packages/botonic-plugin-contentful/package-lock.json b/packages/botonic-plugin-contentful/package-lock.json index 34fe67004b..51e8d3facf 100644 --- a/packages/botonic-plugin-contentful/package-lock.json +++ b/packages/botonic-plugin-contentful/package-lock.json @@ -1054,6 +1054,11 @@ "printj": "~1.1.0" } }, + "csv-parse": { + "version": "4.8.6", + "resolved": "https://registry.npmjs.org/csv-parse/-/csv-parse-4.8.6.tgz", + "integrity": "sha512-rSJlpgAjrB6pmlPaqiBAp3qVtQHN07VxI+ozs+knMsNvgh4bDQgENKLFYLFMvT+jn/wr/zvqsd7IVZ7Txdkr7w==" + }, "csv-stringify": { "version": "5.3.6", "resolved": "https://registry.npmjs.org/csv-stringify/-/csv-stringify-5.3.6.tgz", @@ -2365,6 +2370,14 @@ } } }, + "sort-stream": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/sort-stream/-/sort-stream-1.0.1.tgz", + "integrity": "sha1-owaEycKcozMGnBjWoKsPdor2HfI=", + "requires": { + "through": "~2.3.1" + } + }, "source-map": { "version": "0.6.1", "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.6.1.tgz", @@ -2442,6 +2455,11 @@ "has-flag": "^3.0.0" } }, + "through": { + "version": "2.3.8", + "resolved": "https://registry.npmjs.org/through/-/through-2.3.8.tgz", + "integrity": "sha1-DdTJ/6q8NXlgsbckEV1+Doai4fU=" + }, "timers-ext": { "version": "0.1.7", "resolved": "https://registry.npmjs.org/timers-ext/-/timers-ext-0.1.7.tgz", diff --git a/packages/botonic-plugin-contentful/package.json b/packages/botonic-plugin-contentful/package.json index a4d7081471..8dc572ec2a 100644 --- a/packages/botonic-plugin-contentful/package.json +++ b/packages/botonic-plugin-contentful/package.json @@ -25,12 +25,14 @@ "dependencies": { "@babel/runtime": "^7.8.3", "contentful": "^7.13.1", + "csv-parse": "^4.8.6", "csv-stringify": "^5.3.6", "escape-string-regexp": "^2.0.0", "memoizee": "^0.4.14", "moment": "^2.24.0", "moment-timezone": "^0.5.27", - "node-nlp": "^4.0.2" + "node-nlp": "^4.0.2", + "sort-stream": "^1.0.1" }, "devDependencies": { "@types/memoizee": "^0.4.3", diff --git a/packages/botonic-plugin-contentful/src/cms/context.ts b/packages/botonic-plugin-contentful/src/cms/context.ts index 9b853ef204..bbf5ed9d88 100644 --- a/packages/botonic-plugin-contentful/src/cms/context.ts +++ b/packages/botonic-plugin-contentful/src/cms/context.ts @@ -9,4 +9,9 @@ export const DEFAULT_CONTEXT: Context = {} export interface Context { locale?: Locale callbacks?: CallbackMap + /** + * When set, empty fields will be blank even if they have a value for the fallback locale + * NOT applying it so far for assets because cms.Asset does not support blank assets + */ + ignoreFallbackLocale?: boolean } diff --git a/packages/botonic-plugin-contentful/src/contentful/contents/carousel.ts b/packages/botonic-plugin-contentful/src/contentful/contents/carousel.ts index ce052f72f3..64d63eb898 100644 --- a/packages/botonic-plugin-contentful/src/contentful/contents/carousel.ts +++ b/packages/botonic-plugin-contentful/src/contentful/contents/carousel.ts @@ -36,7 +36,7 @@ export class CarouselDelivery extends DeliveryWithFollowUp { ) } - private async elementFromEntry( + public async elementFromEntry( entry: contentful.Entry, context: cms.Context ): Promise { diff --git a/packages/botonic-plugin-contentful/src/contentful/index.ts b/packages/botonic-plugin-contentful/src/contentful/index.ts index d387f2f831..b1cf9f7c4b 100644 --- a/packages/botonic-plugin-contentful/src/contentful/index.ts +++ b/packages/botonic-plugin-contentful/src/contentful/index.ts @@ -63,7 +63,7 @@ export default class Contentful implements cms.CMS { ? client : new CachedClientApi(client, options.cacheTtlMs) ) - const delivery = deliveryApi + const delivery = new IgnoreFallbackDecorator(deliveryApi) this._contents = new ContentsApi(delivery) this._delivery = delivery @@ -181,6 +181,8 @@ export default class Contentful implements cms.CMS { switch (model) { case ContentType.BUTTON: return this._button.fromEntry(entry, context) + case ContentType.ELEMENT: + return this._carousel.elementFromEntry(entry, context) default: return this.topContentFromEntry(entry, context) } diff --git a/packages/botonic-plugin-contentful/src/tools/translators/csv-export.ts b/packages/botonic-plugin-contentful/src/tools/translators/csv-export.ts index 29ceb35666..47f0a74088 100644 --- a/packages/botonic-plugin-contentful/src/tools/translators/csv-export.ts +++ b/packages/botonic-plugin-contentful/src/tools/translators/csv-export.ts @@ -1,37 +1,44 @@ import { + BOTONIC_CONTENT_TYPES, Button, - Carousel, CMS, CommonFields, Content, - ContentType, - MESSAGE_TYPES, + Element, StartUp, + TopContent, } from '../../index' import { Locale } from '../../nlp' -import { Text, TopContent } from '../../cms' +import { Text } from '../../cms' import stringify from 'csv-stringify' import * as stream from 'stream' import * as fs from 'fs' import { promisify } from 'util' +import sort from 'sort-stream' -console.log(typeof stream.finished) const finished = promisify(stream.finished) class I18nField { - constructor(readonly name: string, readonly value?: string) {} + constructor(readonly name: string, readonly value: string) {} } +type CsvLine = string[] + interface CsvExportOptions { - nameFilter?: (name: string) => boolean + readonly nameFilter?: (name: string) => boolean + readonly stringFilter?: (text: string) => boolean } +export const skipEmptyStrings = (str: string) => Boolean(str && str.trim()) /*** * Uses https://csv.js.org/stringify/api/ */ export class CsvExport { - private toFields = new ContentToI18nFields() - constructor(private readonly options: CsvExportOptions) {} + private toFields: ContentToCsvLines + + constructor(private readonly options: CsvExportOptions) { + this.toFields = new ContentToCsvLines(options) + } create_stringifier() { return stringify({ @@ -41,14 +48,25 @@ export class CsvExport { quoted: true, record_delimiter: 'windows', header: true, - columns: ['Model', 'Id', 'Code', 'Field', 'From', 'To'], + columns: ['Model', 'Code', 'Id', 'Field', 'From', 'To'], }) } + static sortRows(a: string[], b: string[]): number { + for (const i in a) { + const cmp = a[i].localeCompare(b[i]) + if (cmp != 0) { + return cmp + } + } + return 0 + } + async write(fname: string, cms: CMS, locale: Locale): Promise { const stringifier = this.create_stringifier() const readable = stream.Readable.from(this.generate(cms, locale)) const writable = readable + .pipe(sort(CsvExport.sortRows)) .pipe(stringifier) .pipe(fs.createWriteStream(fname)) return this.toPromise(writable) @@ -65,31 +83,32 @@ export class CsvExport { if (this.options.nameFilter && !this.options.nameFilter(content.name)) { continue } - console.log('Exporting content', content.name) - for (const field of this.getI18nFields(content)) { + console.log('Exporting content', content.name.trim() || content.id) + for (const field of this.toFields.getCsvLines(content)) { const TO_COLUMN = '' yield [...field, TO_COLUMN] } } } } +} - // TODO create a text TextFieldVisitor - getI18nFields(content: Content): string[][] { - const columns = [content.contentType, content.id, content.name] - if (!(content instanceof TopContent)) { - if (content instanceof Button) { - return [[...columns, 'Text', content.text]] - } +export class ContentToCsvLines { + constructor(private readonly options: CsvExportOptions) {} + + getCsvLines(content: Content): CsvLine[] { + const columns = [content.contentType, content.name, content.id] + let fields = this.getFields(content) + if (this.options.stringFilter) { + fields = fields.filter(f => this.options.stringFilter!(f.value)) } - const fields = this.toFields.getFields(content) - return fields.filter(f => f.value).map(f => [...columns, f.name, f.value!]) + return fields.map(f => [...columns, f.name, f.value!]) } -} -class ContentToI18nFields { getFields(content: Content): I18nField[] { - if (content instanceof StartUp) { + if (content instanceof Button) { + return [new I18nField('Text', content.text)] + } else if (content instanceof StartUp) { return [ ...this.getCommonFields(content.common), new I18nField('Text', content.text), @@ -99,14 +118,13 @@ class ContentToI18nFields { ...this.getCommonFields(content.common), new I18nField('Text', content.text), ] - } else if (content instanceof Carousel) { - const fields = this.getCommonFields(content.common) - return fields.concat( - ...content.elements.map(e => [ - new I18nField('Title', e.title), - new I18nField('Subtitle', e.subtitle), - ]) - ) + } else if (content instanceof Element) { + return [ + new I18nField('Title', content.title), + new I18nField('Subtitle', content.subtitle), + ] + } else if (content instanceof TopContent) { + return this.getCommonFields(content.common) } return [] } diff --git a/packages/botonic-plugin-contentful/src/tools/translators/csv-for-translator.ts b/packages/botonic-plugin-contentful/src/tools/translators/csv-for-translator.ts index ed057e8b5d..cbb5f54ef8 100644 --- a/packages/botonic-plugin-contentful/src/tools/translators/csv-for-translator.ts +++ b/packages/botonic-plugin-contentful/src/tools/translators/csv-for-translator.ts @@ -1,4 +1,4 @@ -import { CsvExport } from './csv-export' +import { CsvExport, skipEmptyStrings } from './csv-export' import { Locale } from '../../nlp' import Contentful from '../../contentful' @@ -11,9 +11,10 @@ async function writeCsvForTranslators( spaceId: spaceId, accessToken: accessToken, environment: 'master', + disableCache: true, }) const exporter = new CsvExport({ - // nameFilter: n => ['HOME_RETURN_URL'].includes(n), + stringFilter: skipEmptyStrings, }) const promises = locales.map((from: string) => exporter.write(`contentful_${from}.csv`, cms, from) diff --git a/packages/botonic-plugin-contentful/src/typings.d.ts b/packages/botonic-plugin-contentful/src/typings.d.ts index e1cf10f3ad..5b9f384e08 100644 --- a/packages/botonic-plugin-contentful/src/typings.d.ts +++ b/packages/botonic-plugin-contentful/src/typings.d.ts @@ -74,3 +74,8 @@ declare module '@nlpjs/lang-ca/src' { export class StemmerCa extends BaseStemmer {} } + +declare module 'sort-stream' { + function sort(func: (a: any, b: any) => number): any + export = sort +} diff --git a/packages/botonic-plugin-contentful/tests/tools/translators/csv-export.test.ts b/packages/botonic-plugin-contentful/tests/tools/translators/csv-export.test.ts index fd907db99c..d786514d88 100644 --- a/packages/botonic-plugin-contentful/tests/tools/translators/csv-export.test.ts +++ b/packages/botonic-plugin-contentful/tests/tools/translators/csv-export.test.ts @@ -1,55 +1,62 @@ -import { CsvExport } from '../../../src/tools/translators/csv-export' +import { + ContentToCsvLines, + CsvExport, + skipEmptyStrings, +} from '../../../src/tools/translators/csv-export' import { testContentful } from '../../../tests/contentful/contentful.helper' import { ENGLISH } from '../../../src/nlp' import { TextBuilder } from '../../../src/cms/factories' import sync from 'csv-stringify/lib/sync' import { RndButtonsBuilder } from '../../../src/cms/test-helpers' +import { OPTIONS_FOR_IGNORE_FALLBACK } from '../../../src/contentful/ignore-fallback-decorator' -test('integration test', async () => { - const cms = testContentful() - const exporter = new CsvExport({ - // nameFilter: n => ['HOME_RETURN_URL'].includes(n), - }) +test('TEST: CsvExport integration test', async () => { + const cms = testContentful(OPTIONS_FOR_IGNORE_FALLBACK) + + const exporter = new CsvExport({}) + // running for ENGLISH to test contents with empty fields const from = ENGLISH - await exporter.write(`contentful_${from}.csv`, cms, from) + await exporter.write(`/tmp/contentful_${from}.csv`, cms, from) }) -test('getI18nFields Text', () => { - const exporter = new CsvExport({ - nameFilter: n => ['HOME_RETURN_URL'].includes(n), - }) - const fields = exporter.getI18nFields( +test('TEST: ContentToCsvLines.getCsvLines Text', () => { + const exporter = new ContentToCsvLines({}) + const fields = exporter.getCsvLines( new TextBuilder('id1', 'name1', 'long text').withShortText('short1').build() ) expect(fields).toEqual([ - ['text', 'id1', 'name1', 'Short text', 'short1'], - ['text', 'id1', 'name1', 'Text', 'long text'], + ['text', 'name1', 'id1', 'Short text', 'short1'], + ['text', 'name1', 'id1', 'Text', 'long text'], ]) }) -test('getI18nFields Button', () => { - const exporter = new CsvExport({ - nameFilter: n => ['HOME_RETURN_URL'].includes(n), - }) - const button = new RndButtonsBuilder().withButton().build()[0] - const fields = exporter.getI18nFields(button) +test('TEST: ContentToCsvLines.getI18nFields skips blank and undefined strings', () => { + const exporter = new ContentToCsvLines({ stringFilter: skipEmptyStrings }) + const textWithoutShortText = new TextBuilder('id1', 'name1', ' ').build() + const fields = exporter.getCsvLines(textWithoutShortText) + expect(fields).toEqual([]) +}) + +test('TEST: ContentToCsvLines.getI18nFields Button', () => { + const exporter = new ContentToCsvLines({}) + const button = new RndButtonsBuilder().addButton().build()[0] + const fields = exporter.getCsvLines(button) expect(fields).toEqual([ - ['button', button.id, button.name, 'Text', button.text], + ['button', button.name, button.id, 'Text', button.text], ]) }) -// returns "" because push callback within sync is never called. why? -test.skip('stringifier sync', () => { +// Skipped because push callback within sync is never called (why?), and hence it returns "" +test.skip('TEST: CsvExport.stringifier sync', () => { const exporter = new CsvExport({ nameFilter: n => ['HOME_RETURN_URL'].includes(n), }) const stringifier = exporter.create_stringifier() const str = sync(['1 "2" 3', '4'], stringifier.options) expect(str).toEqual('"1 ""2"" 3","4"') - // readable.pipe().pip }) -test('stringifier', () => { +test('TEST: CsvExport.stringifier', () => { const exporter = new CsvExport({ nameFilter: n => ['HOME_RETURN_URL'].includes(n), }) @@ -65,6 +72,6 @@ test('stringifier', () => { } expect(fields).toEqual([ - '"Model";"Id";"Code";"Field";"From";"To"\r\n"1 ""2"" 3";"4"\r\n', + '"Model";"Code";"Id";"Field";"From";"To"\r\n"1 ""2"" 3";"4"\r\n', ]) }) From c4a32450a6d8328729110034cee548f38bfdb0af Mon Sep 17 00:00:00 2001 From: Dani Pinyol Date: Fri, 28 Feb 2020 20:49:12 +0100 Subject: [PATCH 7/8] fix(contentful): csv-Export to ignore contentful fallback language --- .../contentful/ignore-fallback-decorator.ts | 170 ++++++++++++++ .../src/contentful/traverser.ts | 207 ++++++++++++++++++ .../src/tools/translators/csv-export.ts | 10 +- .../ignore-fallback-decorator.test.ts | 106 +++++++++ 4 files changed, 490 insertions(+), 3 deletions(-) create mode 100644 packages/botonic-plugin-contentful/src/contentful/ignore-fallback-decorator.ts create mode 100644 packages/botonic-plugin-contentful/src/contentful/traverser.ts create mode 100644 packages/botonic-plugin-contentful/tests/contentful/ignore-fallback-decorator.test.ts diff --git a/packages/botonic-plugin-contentful/src/contentful/ignore-fallback-decorator.ts b/packages/botonic-plugin-contentful/src/contentful/ignore-fallback-decorator.ts new file mode 100644 index 0000000000..ed73ba4147 --- /dev/null +++ b/packages/botonic-plugin-contentful/src/contentful/ignore-fallback-decorator.ts @@ -0,0 +1,170 @@ +import { Asset, ContentType, Entry, EntryCollection, Field } from 'contentful' +import { DeliveryApi } from './delivery-api' +import { Context } from '../cms' +import { Locale } from '../nlp' +import { + ContentfulVisitor, + I18nEntryTraverser, + I18nValue, + LoggerContentfulVisitor, +} from './traverser' +import { ContentfulOptions } from '../plugin' + +/** + * Option required because IgnoreFallbackDecorator modifies the entries + * Consider using deepCopy in the decorator + */ +export const OPTIONS_FOR_IGNORE_FALLBACK: Partial = { + disableCache: true, +} + +export class IgnoreFallbackDecorator implements DeliveryApi { + constructor(private readonly api: DeliveryApi) {} + + getContentType(id: string): Promise { + return this.api.getContentType(id) + } + + async getEntries( + context: Context, + query: any = {} + ): Promise> { + if (!context.ignoreFallbackLocale) { + return this.api.getEntries(context, query) + } + const entries = await this.api.getEntries( + this.i18nContext(context), + query + ) + + await this.traverseEntries(context, entries.items) + return entries + } + + async getEntry( + id: string, + context: Context, + query: any = {} + ): Promise> { + if (!context.ignoreFallbackLocale) { + return this.api.getEntry(id, context, query) + } + const entry = await this.api.getEntry( + id, + this.i18nContext(context), + query + ) + await this.traverseEntries(context, [entry]) + return entry + } + + async traverseEntries( + context: Context, + entries: Entry[] + ): Promise { + const traverser = new I18nEntryTraverser(this.api) + const visitor = new IgnoreFallbackVisitor(context) + return Promise.all( + entries.map(item => traverser.traverse(item, visitor, context)) + ) + } + + getAsset(id: string, query?: any): Promise { + return this.api.getAsset(id, query) + } + + private i18nContext(context: Context) { + return { + ...context, + locale: '*', + } as Context + } +} + +class IgnoreFallbackVisitor implements ContentfulVisitor { + contextForContentful: Context + constructor(readonly context: Context) { + if (!context.locale) { + throw new Error( + 'Context.ignoreFallbackLocale set but Context.locale not set' + ) + } + this.contextForContentful = { + ...context, + locale: '*', + } + } + + visitEntry(entry: Entry): void {} + + visitName(entry: Entry, entryName: string): void { + entry.fields.name = entryName + } + + visitStringField( + entry: Entry, + field: Field, + values: I18nValue + ): void { + const value = this.valueForLocale(values, field) + this.setField(entry, field, value ?? '') + } + + visitMultipleStringField( + entry: Entry, + field: Field, + values: I18nValue + ): void { + const value = this.valueForLocale(values, field) + this.setField(entry, field, value ?? []) + } + + visitSingleReference( + entry: Entry, + field: Field, + values: I18nValue> + ): void { + const value = this.valueForLocale(values, field) + if (value == undefined) { + console.log( + `${LoggerContentfulVisitor.describeField( + entry, + field.id + )} has no value for locale ${this.locale()}. This might break the CMS model` + ) + } + this.setField(entry, field, value) + } + + visitMultipleReference( + entry: Entry, + field: Field, + values: I18nValue> + ): void { + const value = this.valueForLocale(values, field) + this.setField(entry, field, value ?? []) + } + + private setField(entry: Entry, field: Field, value: any): void { + entry.fields[field.id] = value + } + + private valueForLocale(value: I18nValue, field: Field): T | undefined { + if (!field.localized) { + return Object.values(value)[0] + } + if (!value['en'] && !value['es']) { + console.log('already translated') + return (value as any) as T + } + return value[this.locale()] + } + + private locale(): Locale { + return this.context.locale! + } + + name(): string { + return 'ignoreFallbackLocale' + } +} diff --git a/packages/botonic-plugin-contentful/src/contentful/traverser.ts b/packages/botonic-plugin-contentful/src/contentful/traverser.ts new file mode 100644 index 0000000000..1fa25ebce3 --- /dev/null +++ b/packages/botonic-plugin-contentful/src/contentful/traverser.ts @@ -0,0 +1,207 @@ +import * as cf from 'contentful' + +import { Context } from '../cms' +import { DeliveryApi } from './delivery-api' + +export type I18nValue = { [locale: string]: T } + +export interface ContentfulVisitor { + name(): string + visitEntry(entry: cf.Entry): void + visitName(entry: cf.Entry, entryName: string): void + visitStringField( + entry: cf.Entry, + field: cf.Field, + value: I18nValue + ): void + visitMultipleStringField( + entry: cf.Entry, + field: cf.Field, + value: I18nValue + ): void + visitSingleReference( + entry: cf.Entry, + field: cf.Field, + value: I18nValue> + ): void + visitMultipleReference( + entry: cf.Entry, + field: cf.Field, + value: I18nValue> + ): void +} + +export class LoggerContentfulVisitor implements ContentfulVisitor { + constructor(private readonly visitor: ContentfulVisitor) {} + name(): string { + return this.visitor.name() + } + + visitEntry(entry: cf.Entry): void { + this.log('visitEntry', entry) + return this.visitor.visitEntry(entry) + } + + visitMultipleReference( + entry: cf.Entry, + field: cf.Field, + value: I18nValue> + ): void { + this.log('visitMultipleReference', entry, field) + return this.visitor.visitMultipleReference(entry, field, value) + } + + visitMultipleStringField( + entry: cf.Entry, + field: cf.Field, + value: I18nValue + ): void { + this.log('visitMultipleStringField', entry, field) + return this.visitor.visitMultipleStringField(entry, field, value) + } + + visitName(entry: cf.Entry, entryName: string): void { + this.log('visitName', entry) + return this.visitor.visitName(entry, entryName) + } + + visitSingleReference( + entry: cf.Entry, + field: cf.Field, + value: I18nValue> + ): void { + this.log('visitSingleReference', entry, field) + return this.visitor.visitSingleReference(entry, field, value) + } + + visitStringField( + entry: cf.Entry, + field: cf.Field, + value: I18nValue + ): void { + this.log('visitStringField', entry, field) + return this.visitor.visitStringField(entry, field, value) + } + + log(method: string, entry: cf.Entry, field?: cf.Field): void { + const on = field + ? LoggerContentfulVisitor.describeField(entry, field.id) + : LoggerContentfulVisitor.describeEntry(entry) + console.log(`Visiting '${this.visitor.name()}.${method}' on ${on}`) + } + static describeEntry(entry: cf.Entry): string { + if (!entry.sys) { + console.error('no sys') + } + if (!entry.sys.contentType) { + return `entry with id ${entry.sys.id}` + } + return `entry of type ${entry.sys.contentType.sys.id} (id:${entry.sys.id})` + } + + static describeField(entry: cf.Entry, name: string): string { + return `field '${name}' of ${LoggerContentfulVisitor.describeEntry(entry)}` + // cannot stringify field values because they may contain circular references + // )} value:\n${JSON.stringify(entry.fields[name])}` + } +} + +/** + * Traverser a contentful Entry which has been requested for all locales. + * Limitations. It does not fetch entries from references which have not yet been delivered + */ +export class I18nEntryTraverser { + constructor(private readonly api: DeliveryApi) {} + + async traverse( + entry: cf.Entry, + visitor: ContentfulVisitor, + context: Context + ): Promise { + console.log(`Traversing ${LoggerContentfulVisitor.describeEntry(entry)}`) + console.assert(context.locale) + console.assert(context.ignoreFallbackLocale) + const locale = context.locale! + + const fields = (entry.fields as unknown) as { + [fieldName: string]: I18nValue + } + if (!entry.sys.contentType) { + console.log('no contentType') // it's a file + return + } + const contentType = await this.api.getContentType( + entry.sys.contentType.sys.id + ) + + for (const fieldName in fields) { + const field = contentType.fields.find(f => f.id == fieldName)! + const i18nValue = () => fields[fieldName] + // TODO remove when + if (this.alreadyVisited(i18nValue())) { + return + } + const value = () => { + if (field.localized) { + return i18nValue()[locale] + } + return this.getSingleValue(i18nValue()) + } + if (field.name == 'name') { + visitor.visitName(entry, name) + } + // const values = Object.values(i18nValue()) + // if (values.length == 0) { + // console.log( + // `${LoggerContentfulVisitor.describeField( + // entry, + // fieldName + // )} has no values` + // ) + // } + if (field.type === 'Symbol' || field.type === 'Text') { + visitor.visitStringField(entry, field, i18nValue()) + } else if (field.type == 'Link') { + if (value()) { + await this.traverse(value(), visitor, context) + } + visitor.visitSingleReference(entry, field, i18nValue()) + } else if (this.isArrayOfType(field, 'Link')) { + if (value()) { + const promises = value().map((item: cf.Entry) => + this.traverse(item, visitor, context) + ) + await Promise.all(promises) + } + visitor.visitMultipleReference(entry, field, i18nValue()) + } else if (this.isArrayOfType(field, 'Symbol')) { + visitor.visitMultipleStringField(entry, field, i18nValue()) + } else { + console.log( + `Not traversing ${LoggerContentfulVisitor.describeField( + entry, + fieldName + )}'}` + ) + } + } + visitor.visitEntry(entry) + } + + isArrayOfType(field: cf.Field, itemType: cf.FieldType): boolean { + return field.type == 'Array' && field.items?.type == itemType + } + + private getSingleValue(field: I18nValue): T { + const values = Object.values(field) + console.assert( + values.length == 1, + `Expecting a single value but got ${values.length}` + ) + return values[0] + } + + private alreadyVisited(value: any): boolean { + return !value['en'] && !value['es'] + } +} diff --git a/packages/botonic-plugin-contentful/src/tools/translators/csv-export.ts b/packages/botonic-plugin-contentful/src/tools/translators/csv-export.ts index 47f0a74088..79c9f63d98 100644 --- a/packages/botonic-plugin-contentful/src/tools/translators/csv-export.ts +++ b/packages/botonic-plugin-contentful/src/tools/translators/csv-export.ts @@ -76,9 +76,13 @@ export class CsvExport { return finished(writable) } - async *generate(cms: CMS, from: Locale): AsyncGenerator { - for (const model of [...MESSAGE_TYPES, ContentType.BUTTON]) { - const contents = await cms.contents(model, { locale: from }) + async *generate(cms: CMS, from: Locale): AsyncGenerator { + for (const model of BOTONIC_CONTENT_TYPES) { + console.log(`Exporting contents of type ${model}`) + const contents = await cms.contents(model, { + locale: from, + ignoreFallbackLocale: true, + }) for (const content of contents) { if (this.options.nameFilter && !this.options.nameFilter(content.name)) { continue diff --git a/packages/botonic-plugin-contentful/tests/contentful/ignore-fallback-decorator.test.ts b/packages/botonic-plugin-contentful/tests/contentful/ignore-fallback-decorator.test.ts new file mode 100644 index 0000000000..dfdd0dbb2c --- /dev/null +++ b/packages/botonic-plugin-contentful/tests/contentful/ignore-fallback-decorator.test.ts @@ -0,0 +1,106 @@ +import { testContentful, testContentfulOptions } from './contentful.helper' +import { AdaptorDeliveryApi } from '../../src/contentful/delivery-api' +import { createContentfulClientApi } from '../../src/contentful' +import { ButtonFields } from '../../src/contentful/contents/button' +import { ENGLISH, SPANISH } from '../../src/nlp' +import { ContentCallback, ContentType, Context } from '../../src/cms' +import { + IgnoreFallbackDecorator, + OPTIONS_FOR_IGNORE_FALLBACK, +} from '../../src/contentful/ignore-fallback-decorator' +import { TEST_POST_FAQ1_ID } from './contents/text.test' +import { TextFields } from '../../src/contentful/contents/text' +import { + TEST_CAROUSEL_MAIN_ID, + TEST_POST_MENU_CRSL, +} from './contents/carousel.test' + +const TEST_BUTTON_BLANK_SPANISH = '40buQOqp9jbwoxmMZhFO16' + +function createIgnoreFallbackDecorator() { + return new IgnoreFallbackDecorator( + new AdaptorDeliveryApi( + createContentfulClientApi( + testContentfulOptions(OPTIONS_FOR_IGNORE_FALLBACK) + ) + ) + ) +} + +const FALLBACK_TESTS = [ + [{ locale: ENGLISH, ignoreFallbackLocale: true }, 'Return an article'], + [{ locale: SPANISH, ignoreFallbackLocale: true }, ''], + [{}, 'Return an article'], + [{ locale: ENGLISH }, 'Return an article'], + [{ locale: SPANISH }, 'Return an article'], // fallback +] + +test.each(FALLBACK_TESTS)( + 'TEST: IgnoreFallbackDecorator.getEntry uses fallback locale', + async (context: Context, expectedText: string) => { + const sut = createIgnoreFallbackDecorator() + + // act + const entry = await sut.getEntry( + TEST_BUTTON_BLANK_SPANISH, + context + ) + + // assert + expect(entry.fields.text).toEqual(expectedText) + expect(entry.fields.name).toEqual('POST_FAQ3') + } +) + +test('TEST: ignoreFallbackLocale with a carousel with buttons to other carousels ', async () => { + const contentful = testContentful(OPTIONS_FOR_IGNORE_FALLBACK) + const carousel = await contentful.carousel(TEST_CAROUSEL_MAIN_ID, { + locale: SPANISH, + ignoreFallbackLocale: true, + }) + const elementPost = carousel.elements.find( + e => e.id == '6aOPY3UVh8M4lihW9xe17E' + ) + expect(elementPost).toBeDefined() + expect(elementPost!.buttons).toHaveLength(1) + expect(elementPost!.buttons[0].callback).toEqual( + new ContentCallback(ContentType.CAROUSEL, TEST_POST_MENU_CRSL) + ) +}) + +test('TEST: IgnoreFallbackDecorator.getEntry uses fallback locale ', async () => { + const sut = createIgnoreFallbackDecorator() + + // act + const entry = await sut.getEntry(TEST_POST_FAQ1_ID, { + locale: SPANISH, + ignoreFallbackLocale: true, + }) + + // assert + expect(entry.fields.text).toEqual('') + expect(entry.fields.name).toEqual('POST_FAQ1') + expect(entry.fields.buttons.length).toEqual(1) +}) + +test.each(FALLBACK_TESTS)( + 'TEST: IgnoreFallbackDecorator.getEntries uses fallback locale', + async (context: Context, expectedText: string) => { + const sut = createIgnoreFallbackDecorator() + + // acr + const entries = await sut.getEntries(context, { + content_type: ContentType.BUTTON, + }) + + //assert + const entry = entries.items.filter( + e => e.sys.id == TEST_BUTTON_BLANK_SPANISH + ) + expect(entry.length).toBe(1) + expect(entry[0].fields.text).toEqual(expectedText) + expect(entry[0].fields.name).toEqual('POST_FAQ3') + } +) + +test('hack because webstorm does not recognize test.skip.each', () => {}) From 1c498b870ae4158a86c8dd473af80228b505e6a2 Mon Sep 17 00:00:00 2001 From: Dani Pinyol Date: Sat, 29 Feb 2020 17:41:07 +0100 Subject: [PATCH 8/8] feat(contentful): decouple contentful traverser from visitor --- package-lock.json | 6 +- package.json | 2 +- .../src/contentful/contents/button.ts | 2 +- .../contentful/ignore-fallback-decorator.ts | 110 ++------ .../src/contentful/index.ts | 38 +-- .../src/contentful/traverser.ts | 262 +++++++++--------- .../tools/translators/csv-for-translator.ts | 15 +- .../tests/contentful/contents/text.test.ts | 2 +- .../ignore-fallback-decorator.test.ts | 37 ++- .../tools/translators/csv-export.test.ts | 3 +- .../tests/util/objects.test.ts | 6 +- 11 files changed, 230 insertions(+), 253 deletions(-) diff --git a/package-lock.json b/package-lock.json index 14d4bca6fc..4d0eec22e6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10898,9 +10898,9 @@ } }, "typescript": { - "version": "3.7.5", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.7.5.tgz", - "integrity": "sha512-/P5lkRXkWHNAbcJIiHPfRoKqyd7bsyCma1hZNUGfn20qm64T6ZBlrzprymeu918H+mB/0rIg2gGK/BXkhhYgBw==", + "version": "3.8.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.8.3.tgz", + "integrity": "sha512-MYlEfn5VrLNsgudQTVJeNaQFUAI7DkhnOjdpAp4T+ku1TfQClewlbSuTVHiA+8skNBgaf02TL/kLOvig4y3G8w==", "dev": true }, "uglify-js": { diff --git a/package.json b/package.json index f9e1c5fba3..a1f18270d5 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "ts-mockito": "^2.5.0", "ts-node": "^8.6.2", "tslib": "^1.10.0", - "typescript": "~3.7.5" + "typescript": "^3.8.3" }, "engines": { "node": ">=8.0.0" diff --git a/packages/botonic-plugin-contentful/src/contentful/contents/button.ts b/packages/botonic-plugin-contentful/src/contentful/contents/button.ts index a1e76e8372..7f623e074f 100644 --- a/packages/botonic-plugin-contentful/src/contentful/contents/button.ts +++ b/packages/botonic-plugin-contentful/src/contentful/contents/button.ts @@ -12,7 +12,7 @@ import { TextFields } from './text' import { UrlFields } from './url' export class ButtonDelivery { - private static BUTTON_CONTENT_TYPE = 'button' + public static BUTTON_CONTENT_TYPE = 'button' private static PAYLOAD_CONTENT_TYPE = 'payload' constructor(private readonly delivery: DeliveryApi) {} diff --git a/packages/botonic-plugin-contentful/src/contentful/ignore-fallback-decorator.ts b/packages/botonic-plugin-contentful/src/contentful/ignore-fallback-decorator.ts index ed73ba4147..e297e06ea8 100644 --- a/packages/botonic-plugin-contentful/src/contentful/ignore-fallback-decorator.ts +++ b/packages/botonic-plugin-contentful/src/contentful/ignore-fallback-decorator.ts @@ -1,22 +1,12 @@ -import { Asset, ContentType, Entry, EntryCollection, Field } from 'contentful' +import { Asset, ContentType, Entry, EntryCollection } from 'contentful' import { DeliveryApi } from './delivery-api' import { Context } from '../cms' -import { Locale } from '../nlp' import { ContentfulVisitor, I18nEntryTraverser, I18nValue, - LoggerContentfulVisitor, + VisitedField, } from './traverser' -import { ContentfulOptions } from '../plugin' - -/** - * Option required because IgnoreFallbackDecorator modifies the entries - * Consider using deepCopy in the decorator - */ -export const OPTIONS_FOR_IGNORE_FALLBACK: Partial = { - disableCache: true, -} export class IgnoreFallbackDecorator implements DeliveryApi { constructor(private readonly api: DeliveryApi) {} @@ -32,12 +22,10 @@ export class IgnoreFallbackDecorator implements DeliveryApi { if (!context.ignoreFallbackLocale) { return this.api.getEntries(context, query) } - const entries = await this.api.getEntries( - this.i18nContext(context), - query - ) + let entries = await this.api.getEntries(this.i18nContext(context), query) - await this.traverseEntries(context, entries.items) + entries = { ...entries } + entries.items = await this.traverseEntries(context, entries.items) return entries } @@ -54,18 +42,19 @@ export class IgnoreFallbackDecorator implements DeliveryApi { this.i18nContext(context), query ) - await this.traverseEntries(context, [entry]) - return entry + return (await this.traverseEntries(context, [entry]))[0] } - async traverseEntries( + async traverseEntries( context: Context, - entries: Entry[] - ): Promise { - const traverser = new I18nEntryTraverser(this.api) + entries: Entry[] + ): Promise[]> { const visitor = new IgnoreFallbackVisitor(context) return Promise.all( - entries.map(item => traverser.traverse(item, visitor, context)) + entries.map(async item => { + const traverser = new I18nEntryTraverser(this.api, visitor) + return await traverser.traverse(item, context) + }) ) } @@ -95,73 +84,32 @@ class IgnoreFallbackVisitor implements ContentfulVisitor { } } - visitEntry(entry: Entry): void {} - - visitName(entry: Entry, entryName: string): void { - entry.fields.name = entryName - } - - visitStringField( - entry: Entry, - field: Field, - values: I18nValue - ): void { - const value = this.valueForLocale(values, field) - this.setField(entry, field, value ?? '') + visitEntry(entry: Entry): Entry { + return entry } - visitMultipleStringField( - entry: Entry, - field: Field, - values: I18nValue - ): void { - const value = this.valueForLocale(values, field) - this.setField(entry, field, value ?? []) + visitStringField(vf: VisitedField): I18nValue { + return this.hackType(vf.value[vf.locale], '') } - - visitSingleReference( - entry: Entry, - field: Field, - values: I18nValue> - ): void { - const value = this.valueForLocale(values, field) - if (value == undefined) { - console.log( - `${LoggerContentfulVisitor.describeField( - entry, - field.id - )} has no value for locale ${this.locale()}. This might break the CMS model` - ) + hackType(t: T, defaultValue?: T): I18nValue { + if (defaultValue != undefined) { + t = t ?? defaultValue } - this.setField(entry, field, value) + return (t as any) as I18nValue } - visitMultipleReference( - entry: Entry, - field: Field, - values: I18nValue> - ): void { - const value = this.valueForLocale(values, field) - this.setField(entry, field, value ?? []) + visitMultipleStringField(vf: VisitedField): I18nValue { + return this.hackType(vf.value[vf.locale], []) } - private setField(entry: Entry, field: Field, value: any): void { - entry.fields[field.id] = value - } - - private valueForLocale(value: I18nValue, field: Field): T | undefined { - if (!field.localized) { - return Object.values(value)[0] - } - if (!value['en'] && !value['es']) { - console.log('already translated') - return (value as any) as T - } - return value[this.locale()] + visitSingleReference(vf: VisitedField>): I18nValue> { + return this.hackType(vf.value[vf.locale], (undefined as any) as Entry) } - private locale(): Locale { - return this.context.locale! + visitMultipleReference( + vf: VisitedField> + ): I18nValue> { + return this.hackType(vf.value[vf.locale]) } name(): string { diff --git a/packages/botonic-plugin-contentful/src/contentful/index.ts b/packages/botonic-plugin-contentful/src/contentful/index.ts index b1cf9f7c4b..e82658ba82 100644 --- a/packages/botonic-plugin-contentful/src/contentful/index.ts +++ b/packages/botonic-plugin-contentful/src/contentful/index.ts @@ -127,7 +127,6 @@ export default class Contentful implements cms.CMS { context = DEFAULT_CONTEXT, filter?: (cf: CommonFields) => boolean ): Promise { - console.log('getting contents for lang', context.locale) return this._contents.topContents( model, context, @@ -154,22 +153,27 @@ export default class Contentful implements cms.CMS { context: Context ): Promise { const model = ContentfulEntryUtils.getContentModel(entry) - switch (model) { - case ContentType.CAROUSEL: - return this._carousel.fromEntry(entry, context) - case ContentType.QUEUE: - return QueueDelivery.fromEntry(entry) - case ContentType.CHITCHAT: - case ContentType.TEXT: - return this._text.fromEntry(entry, context) - case ContentType.IMAGE: - return this._image.fromEntry(entry, context) - case ContentType.URL: - return this._url.fromEntry(entry, context) - case ContentType.STARTUP: - return this._startUp.fromEntry(entry, context) - default: - throw new Error(`${model} is not a Content type`) + try { + switch (model) { + case ContentType.CAROUSEL: + return await this._carousel.fromEntry(entry, context) + case ContentType.QUEUE: + return await QueueDelivery.fromEntry(entry) + case ContentType.CHITCHAT: + case ContentType.TEXT: + return await this._text.fromEntry(entry, context) + case ContentType.IMAGE: + return await this._image.fromEntry(entry, context) + case ContentType.URL: + return await this._url.fromEntry(entry, context) + case ContentType.STARTUP: + return await this._startUp.fromEntry(entry, context) + default: + throw new Error(`${model} is not a Content type`) + } + } catch (e) { + console.error(`Error creating ${model} with id: ${entry.sys.id}`) + throw e } } diff --git a/packages/botonic-plugin-contentful/src/contentful/traverser.ts b/packages/botonic-plugin-contentful/src/contentful/traverser.ts index 1fa25ebce3..626e413d92 100644 --- a/packages/botonic-plugin-contentful/src/contentful/traverser.ts +++ b/packages/botonic-plugin-contentful/src/contentful/traverser.ts @@ -2,33 +2,35 @@ import * as cf from 'contentful' import { Context } from '../cms' import { DeliveryApi } from './delivery-api' +import { Locale } from '../nlp' +import { ButtonDelivery } from './contents/button' export type I18nValue = { [locale: string]: T } +export class VisitedField { + constructor( + readonly entry: cf.Entry, + readonly locale: Locale, + readonly field: cf.Field, + readonly value: I18nValue + ) {} +} export interface ContentfulVisitor { name(): string - visitEntry(entry: cf.Entry): void - visitName(entry: cf.Entry, entryName: string): void - visitStringField( - entry: cf.Entry, - field: cf.Field, - value: I18nValue - ): void - visitMultipleStringField( - entry: cf.Entry, - field: cf.Field, - value: I18nValue - ): void + + visitEntry(entry: cf.Entry): cf.Entry + + visitStringField(field: VisitedField): I18nValue + + visitMultipleStringField(field: VisitedField): I18nValue + visitSingleReference( - entry: cf.Entry, - field: cf.Field, - value: I18nValue> - ): void - visitMultipleReference( - entry: cf.Entry, - field: cf.Field, - value: I18nValue> - ): void + field: VisitedField> + ): I18nValue> + + visitMultipleReference( + field: VisitedField> + ): I18nValue> } export class LoggerContentfulVisitor implements ContentfulVisitor { @@ -36,63 +38,45 @@ export class LoggerContentfulVisitor implements ContentfulVisitor { name(): string { return this.visitor.name() } - - visitEntry(entry: cf.Entry): void { + visitEntry(entry: cf.Entry): cf.Entry { this.log('visitEntry', entry) return this.visitor.visitEntry(entry) } - visitMultipleReference( - entry: cf.Entry, - field: cf.Field, - value: I18nValue> - ): void { - this.log('visitMultipleReference', entry, field) - return this.visitor.visitMultipleReference(entry, field, value) + visitStringField(field: VisitedField): I18nValue { + this.log('visitStringField', field.entry, field.field) + return this.visitor.visitStringField(field) } - visitMultipleStringField( - entry: cf.Entry, - field: cf.Field, - value: I18nValue - ): void { - this.log('visitMultipleStringField', entry, field) - return this.visitor.visitMultipleStringField(entry, field, value) + visitMultipleStringField( + field: VisitedField + ): I18nValue { + this.log('visitMultipleStringField', field.entry, field.field) + return this.visitor.visitMultipleStringField(field) } - visitName(entry: cf.Entry, entryName: string): void { - this.log('visitName', entry) - return this.visitor.visitName(entry, entryName) + visitSingleReference( + field: VisitedField> + ): I18nValue> { + this.log('visitSingleReference', field.entry, field.field) + return this.visitor.visitSingleReference(field) } - visitSingleReference( - entry: cf.Entry, - field: cf.Field, - value: I18nValue> - ): void { - this.log('visitSingleReference', entry, field) - return this.visitor.visitSingleReference(entry, field, value) - } - - visitStringField( - entry: cf.Entry, - field: cf.Field, - value: I18nValue - ): void { - this.log('visitStringField', entry, field) - return this.visitor.visitStringField(entry, field, value) + visitMultipleReference( + field: VisitedField> + ): I18nValue> { + this.log('visitMultipleReference', field.entry, field.field) + return this.visitor.visitMultipleReference(field) } - log(method: string, entry: cf.Entry, field?: cf.Field): void { + log(method: string, entry: cf.Entry, field?: cf.Field): void { const on = field ? LoggerContentfulVisitor.describeField(entry, field.id) : LoggerContentfulVisitor.describeEntry(entry) console.log(`Visiting '${this.visitor.name()}.${method}' on ${on}`) } + static describeEntry(entry: cf.Entry): string { - if (!entry.sys) { - console.error('no sys') - } if (!entry.sys.contentType) { return `entry with id ${entry.sys.id}` } @@ -100,9 +84,8 @@ export class LoggerContentfulVisitor implements ContentfulVisitor { } static describeField(entry: cf.Entry, name: string): string { - return `field '${name}' of ${LoggerContentfulVisitor.describeEntry(entry)}` // cannot stringify field values because they may contain circular references - // )} value:\n${JSON.stringify(entry.fields[name])}` + return `field '${name}' of ${LoggerContentfulVisitor.describeEntry(entry)}` } } @@ -111,97 +94,116 @@ export class LoggerContentfulVisitor implements ContentfulVisitor { * Limitations. It does not fetch entries from references which have not yet been delivered */ export class I18nEntryTraverser { - constructor(private readonly api: DeliveryApi) {} - - async traverse( - entry: cf.Entry, - visitor: ContentfulVisitor, + private visited = new Set() + constructor( + private readonly api: DeliveryApi, + readonly visitor: ContentfulVisitor + ) {} + + async traverse( + entry: cf.Entry, context: Context - ): Promise { - console.log(`Traversing ${LoggerContentfulVisitor.describeEntry(entry)}`) + ): Promise> { + //in the future we might extending to traverse all locales console.assert(context.locale) console.assert(context.ignoreFallbackLocale) - const locale = context.locale! + const promise = this.traverseCore(entry, context) + this.visited.add(entry.sys.id) + return promise + } + async traverseCore( + entry: cf.Entry, + context: Context + ): Promise> { + entry = { ...entry, fields: { ...entry.fields } } const fields = (entry.fields as unknown) as { [fieldName: string]: I18nValue } if (!entry.sys.contentType) { - console.log('no contentType') // it's a file - return + // it's a file or a dangling reference + return entry } const contentType = await this.api.getContentType( entry.sys.contentType.sys.id ) + for (const fieldId in fields) { + const field = contentType.fields.find(f => f.id == fieldId)! + const i18nValue = { ...fields[fieldId] } as I18nValue + const locale = field.localized + ? context.locale! + : Object.keys(i18nValue)[0] + const vf = new VisitedField(entry, locale, field, i18nValue) + fields[fieldId] = await this.traverseField(context, vf) + } + entry = this.visitor.visitEntry(entry) + return entry + } - for (const fieldName in fields) { - const field = contentType.fields.find(f => f.id == fieldName)! - const i18nValue = () => fields[fieldName] - // TODO remove when - if (this.alreadyVisited(i18nValue())) { - return - } - const value = () => { - if (field.localized) { - return i18nValue()[locale] - } - return this.getSingleValue(i18nValue()) + async traverseField( + context: Context, + vf: VisitedField + ): Promise> { + let val = vf.value[vf.locale] + + const visitOrTraverse = async (val: cf.Entry) => { + if (this.visited.has(val.sys.id) && val.sys.id >= vf.entry.sys.id) { + // break deadlock if contents have cyclic dependencies + return val } - if (field.name == 'name') { - visitor.visitName(entry, name) + return this.traverse(val, context) + } + if (vf.field.type === 'Symbol' || vf.field.type === 'Text') { + return this.visitor.visitStringField(vf) + } else if (vf.field.type == 'Link') { + if (val) { + val = this.stopRecursionOnButtonCallbacks(vf.field, val) + vf.value[vf.locale] = visitOrTraverse(val) } - // const values = Object.values(i18nValue()) - // if (values.length == 0) { - // console.log( - // `${LoggerContentfulVisitor.describeField( - // entry, - // fieldName - // )} has no values` - // ) - // } - if (field.type === 'Symbol' || field.type === 'Text') { - visitor.visitStringField(entry, field, i18nValue()) - } else if (field.type == 'Link') { - if (value()) { - await this.traverse(value(), visitor, context) - } - visitor.visitSingleReference(entry, field, i18nValue()) - } else if (this.isArrayOfType(field, 'Link')) { - if (value()) { - const promises = value().map((item: cf.Entry) => - this.traverse(item, visitor, context) - ) - await Promise.all(promises) - } - visitor.visitMultipleReference(entry, field, i18nValue()) - } else if (this.isArrayOfType(field, 'Symbol')) { - visitor.visitMultipleStringField(entry, field, i18nValue()) - } else { - console.log( - `Not traversing ${LoggerContentfulVisitor.describeField( - entry, - fieldName - )}'}` + return this.visitor.visitSingleReference(vf) + } else if (this.isArrayOfType(vf.field, 'Link')) { + if (val) { + val = await Promise.all( + (val as cf.Entry[]).map(v => visitOrTraverse(v)) ) + vf.value[vf.locale] = val } + return this.visitor.visitMultipleReference(vf) + } else if (this.isArrayOfType(vf.field, 'Symbol')) { + return this.visitor.visitMultipleStringField(vf) + } else { + console.log( + `Not traversing ${LoggerContentfulVisitor.describeField( + vf.entry, + vf.field.id + )}'}` + ) + return vf.value } - visitor.visitEntry(entry) } isArrayOfType(field: cf.Field, itemType: cf.FieldType): boolean { return field.type == 'Array' && field.items?.type == itemType } - private getSingleValue(field: I18nValue): T { - const values = Object.values(field) - console.assert( - values.length == 1, - `Expecting a single value but got ${values.length}` - ) - return values[0] - } - - private alreadyVisited(value: any): boolean { - return !value['en'] && !value['es'] + /** + * When a content has a button with another content reference, we just need the referered content id + * to create the content. Hence, we stop traversing. + */ + private stopRecursionOnButtonCallbacks( + field: cf.Field, + val: cf.Entry + ): cf.Entry { + if (field.id !== 'target') { + return val + } + if ( + !val.fields || + val.fields.payload || + val.sys.contentType.sys.id == ButtonDelivery.BUTTON_CONTENT_TYPE + ) { + return val + } + return { ...val, fields: {} } } } diff --git a/packages/botonic-plugin-contentful/src/tools/translators/csv-for-translator.ts b/packages/botonic-plugin-contentful/src/tools/translators/csv-for-translator.ts index cbb5f54ef8..2e2a013f77 100644 --- a/packages/botonic-plugin-contentful/src/tools/translators/csv-for-translator.ts +++ b/packages/botonic-plugin-contentful/src/tools/translators/csv-for-translator.ts @@ -11,7 +11,6 @@ async function writeCsvForTranslators( spaceId: spaceId, accessToken: accessToken, environment: 'master', - disableCache: true, }) const exporter = new CsvExport({ stringFilter: skipEmptyStrings, @@ -19,7 +18,7 @@ async function writeCsvForTranslators( const promises = locales.map((from: string) => exporter.write(`contentful_${from}.csv`, cms, from) ) - return Promise.all(promises) + await Promise.all(promises) } const spaceId = process.argv[2] @@ -31,6 +30,16 @@ if (process.argv.length < 5) { process.exit(1) } -writeCsvForTranslators(spaceId, token, locales).then(() => { +async function main() { + try { + await writeCsvForTranslators(spaceId, token, locales) + console.log('done') + } catch (e) { + console.error(e) + } +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +main().then(() => { console.log('done') }) diff --git a/packages/botonic-plugin-contentful/tests/contentful/contents/text.test.ts b/packages/botonic-plugin-contentful/tests/contentful/contents/text.test.ts index 199078ee5b..2f05e9c059 100644 --- a/packages/botonic-plugin-contentful/tests/contentful/contents/text.test.ts +++ b/packages/botonic-plugin-contentful/tests/contentful/contents/text.test.ts @@ -3,7 +3,7 @@ import { testContentful, testContext } from '../contentful.helper' import * as cms from '../../../src' export const TEST_POST_FAQ1_ID = 'djwHOFKknJ3AmyG6YKNip' -const TEST_POST_FAQ2_ID = '22h2Vba7v92MadcL5HeMrt' +export const TEST_POST_FAQ2_ID = '22h2Vba7v92MadcL5HeMrt' const TEST_FBK_MSG = '1U7XKJccDSsI3mP0yX04Mj' const TEST_FBK_OK_MSG = '63lakRZRu1AJ1DqlbZZb9O' const TEST_SORRY = '6ZjjdrKQbaLNc6JAhRnS8D' diff --git a/packages/botonic-plugin-contentful/tests/contentful/ignore-fallback-decorator.test.ts b/packages/botonic-plugin-contentful/tests/contentful/ignore-fallback-decorator.test.ts index dfdd0dbb2c..25bbb4cfa8 100644 --- a/packages/botonic-plugin-contentful/tests/contentful/ignore-fallback-decorator.test.ts +++ b/packages/botonic-plugin-contentful/tests/contentful/ignore-fallback-decorator.test.ts @@ -2,12 +2,14 @@ import { testContentful, testContentfulOptions } from './contentful.helper' import { AdaptorDeliveryApi } from '../../src/contentful/delivery-api' import { createContentfulClientApi } from '../../src/contentful' import { ButtonFields } from '../../src/contentful/contents/button' -import { ENGLISH, SPANISH } from '../../src/nlp' -import { ContentCallback, ContentType, Context } from '../../src/cms' +import { ENGLISH, Locale, SPANISH } from '../../src/nlp' import { - IgnoreFallbackDecorator, - OPTIONS_FOR_IGNORE_FALLBACK, -} from '../../src/contentful/ignore-fallback-decorator' + BOTONIC_CONTENT_TYPES, + ContentCallback, + ContentType, + Context, +} from '../../src/cms' +import { IgnoreFallbackDecorator } from '../../src/contentful/ignore-fallback-decorator' import { TEST_POST_FAQ1_ID } from './contents/text.test' import { TextFields } from '../../src/contentful/contents/text' import { @@ -19,11 +21,7 @@ const TEST_BUTTON_BLANK_SPANISH = '40buQOqp9jbwoxmMZhFO16' function createIgnoreFallbackDecorator() { return new IgnoreFallbackDecorator( - new AdaptorDeliveryApi( - createContentfulClientApi( - testContentfulOptions(OPTIONS_FOR_IGNORE_FALLBACK) - ) - ) + new AdaptorDeliveryApi(createContentfulClientApi(testContentfulOptions())) ) } @@ -53,7 +51,7 @@ test.each(FALLBACK_TESTS)( ) test('TEST: ignoreFallbackLocale with a carousel with buttons to other carousels ', async () => { - const contentful = testContentful(OPTIONS_FOR_IGNORE_FALLBACK) + const contentful = testContentful() const carousel = await contentful.carousel(TEST_CAROUSEL_MAIN_ID, { locale: SPANISH, ignoreFallbackLocale: true, @@ -68,6 +66,23 @@ test('TEST: ignoreFallbackLocale with a carousel with buttons to other carousels ) }) +test.each([ENGLISH, SPANISH])( + 'TEST: ignoreFallbackLocale all contents %s', + async (locale: Locale) => { + const contentful = testContentful() + for (const model of BOTONIC_CONTENT_TYPES) { + const ret = await contentful.contents(model, { + locale, + ignoreFallbackLocale: true, + }) + for (const content of ret) { + expect(content.id).toBeDefined() + expect(content.name).toBeDefined() + } + } + } +) + test('TEST: IgnoreFallbackDecorator.getEntry uses fallback locale ', async () => { const sut = createIgnoreFallbackDecorator() diff --git a/packages/botonic-plugin-contentful/tests/tools/translators/csv-export.test.ts b/packages/botonic-plugin-contentful/tests/tools/translators/csv-export.test.ts index d786514d88..920daedeb3 100644 --- a/packages/botonic-plugin-contentful/tests/tools/translators/csv-export.test.ts +++ b/packages/botonic-plugin-contentful/tests/tools/translators/csv-export.test.ts @@ -8,10 +8,9 @@ import { ENGLISH } from '../../../src/nlp' import { TextBuilder } from '../../../src/cms/factories' import sync from 'csv-stringify/lib/sync' import { RndButtonsBuilder } from '../../../src/cms/test-helpers' -import { OPTIONS_FOR_IGNORE_FALLBACK } from '../../../src/contentful/ignore-fallback-decorator' test('TEST: CsvExport integration test', async () => { - const cms = testContentful(OPTIONS_FOR_IGNORE_FALLBACK) + const cms = testContentful() const exporter = new CsvExport({}) // running for ENGLISH to test contents with empty fields diff --git a/packages/botonic-plugin-contentful/tests/util/objects.test.ts b/packages/botonic-plugin-contentful/tests/util/objects.test.ts index b165d7d365..1853eb9214 100644 --- a/packages/botonic-plugin-contentful/tests/util/objects.test.ts +++ b/packages/botonic-plugin-contentful/tests/util/objects.test.ts @@ -7,7 +7,7 @@ class Class { constructor(public arr = [3, 4], public sub = new Subclass(1)) {} } -test('TEST: shallowClone', async () => { +test('TEST: shallowClone', () => { const source = new Class() // act @@ -24,7 +24,7 @@ test('TEST: shallowClone', async () => { expect(source.sub.field).toEqual(2) }) -test('TEST: deepClone', async () => { +test('TEST: deepClone', () => { const source = new Class() // act @@ -45,7 +45,7 @@ class RecursiveClass { constructor(public rec?: RecursiveClass) {} } -test('TEST: deepClone no recursive call', async () => { +test('TEST: deepClone no recursive call', () => { const source = new RecursiveClass() source.rec = source