From c6550263860dc79df672f0d58ccba77aced9f1d4 Mon Sep 17 00:00:00 2001 From: abdou6666 Date: Fri, 22 Nov 2024 11:25:50 +0100 Subject: [PATCH 1/4] fix: display translatable settings only in translations UI --- api/src/chat/schemas/types/message.ts | 3 +- api/src/extensions/channels/web/settings.ts | 2 + .../i18n/services/translation.service.spec.ts | 81 +++++++++++++++++-- api/src/i18n/services/translation.service.ts | 42 +++++++--- api/src/plugins/types.ts | 4 +- api/src/setting/dto/setting.dto.ts | 15 +++- api/src/setting/schemas/setting.schema.ts | 6 ++ api/src/setting/seeds/setting.seed-model.ts | 1 + api/src/utils/test/fixtures/setting.ts | 18 +++++ 9 files changed, 149 insertions(+), 23 deletions(-) diff --git a/api/src/chat/schemas/types/message.ts b/api/src/chat/schemas/types/message.ts index 5a49a4b..5c8d9be 100644 --- a/api/src/chat/schemas/types/message.ts +++ b/api/src/chat/schemas/types/message.ts @@ -7,6 +7,7 @@ */ import { Attachment } from '@/attachment/schemas/attachment.schema'; +import { PluginName } from '@/plugins/types'; import { Message } from '../message.schema'; @@ -107,7 +108,7 @@ export type StdOutgoingAttachmentMessage< }; export type StdPluginMessage = { - plugin: string; + plugin: PluginName; args: { [key: string]: any }; }; diff --git a/api/src/extensions/channels/web/settings.ts b/api/src/extensions/channels/web/settings.ts index 2ab0516..0713449 100644 --- a/api/src/extensions/channels/web/settings.ts +++ b/api/src/extensions/channels/web/settings.ts @@ -45,6 +45,7 @@ export default [ label: Web.SettingLabel.greeting_message, value: 'Welcome! Ready to start a conversation with our chatbot?', type: SettingType.textarea, + translatable: true, }, { group: WEB_CHANNEL_NAMESPACE, @@ -58,6 +59,7 @@ export default [ label: Web.SettingLabel.window_title, value: 'Widget Title', type: SettingType.text, + translatable: true, }, { group: WEB_CHANNEL_NAMESPACE, diff --git a/api/src/i18n/services/translation.service.spec.ts b/api/src/i18n/services/translation.service.spec.ts index 5396e89..fda9087 100644 --- a/api/src/i18n/services/translation.service.spec.ts +++ b/api/src/i18n/services/translation.service.spec.ts @@ -10,6 +10,8 @@ import { EventEmitter2 } from '@nestjs/event-emitter'; import { Test, TestingModule } from '@nestjs/testing'; import { I18nService } from '@/i18n/services/i18n.service'; +import { PluginService } from '@/plugins/plugins.service'; +import { SettingType } from '@/setting/schemas/types'; import { SettingService } from '@/setting/services/setting.service'; import { Block } from '../../chat/schemas/block.schema'; @@ -22,6 +24,7 @@ describe('TranslationService', () => { let service: TranslationService; let settingService: SettingService; let i18nService: I18nService; + let pluginService: PluginService; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -39,6 +42,12 @@ describe('TranslationService', () => { ]), }, }, + { + provide: PluginService, + useValue: { + getPlugin: jest.fn(), + }, + }, { provide: BlockService, useValue: { @@ -58,12 +67,16 @@ describe('TranslationService', () => { { provide: SettingService, useValue: { - getSettings: jest.fn().mockResolvedValue({ - chatbot_settings: { - global_fallback: true, - fallback_message: ['Global fallback message'], + getSettings: jest + .fn() + .mockResolvedValue(['Global fallback message']), + find: jest.fn().mockResolvedValue([ + { + translatable: true, + key: 'global_fallback', + value: 'Global fallback message', }, - }), + ]), }, }, { @@ -79,6 +92,7 @@ describe('TranslationService', () => { service = module.get(TranslationService); settingService = module.get(SettingService); i18nService = module.get(I18nService); + pluginService = module.get(PluginService); }); it('should call refreshDynamicTranslations with translations from findAll', async () => { @@ -98,9 +112,60 @@ describe('TranslationService', () => { expect(strings).toEqual(['Test message', 'Fallback message']); }); - it('should return an array of strings from the settings when global fallback is enabled', async () => { - const strings = await service.getSettingStrings(); - expect(strings).toEqual(['Global fallback message']); + it('should return plugin-related strings from block message with translatable args', () => { + const block: Block = { + name: 'Ollama Plugin', + patterns: [], + assign_labels: [], + trigger_channels: [], + trigger_labels: [], + nextBlocks: [], + category: '673f82f4bafd1e2a00e7e53e', + starts_conversation: false, + builtin: false, + capture_vars: [], + createdAt: new Date(), + updatedAt: new Date(), + id: '673f8724007f1087c96d30d0', + position: { x: 702, y: 321.8333282470703 }, + message: { + plugin: 'ollama-plugin', + args: { + model: 'String 1', + context: ['String 2', 'String 3'], + }, + }, + options: {}, + }; + + const mockedPlugin: any = { + name: 'ollama-plugin', + type: 'block', + settings: [ + { + label: 'model', + group: 'default', + type: SettingType.text, + value: 'llama3.2', + translatable: false, + }, + { + label: 'context', + group: 'default', + type: SettingType.multiple_text, + value: ['Answer the user QUESTION using the DOCUMENTS text above.'], + translatable: true, + }, + ], + }; + + jest + .spyOn(pluginService, 'getPlugin') + .mockImplementation(() => mockedPlugin); + + const result = service.getBlockStrings(block); + + expect(result).toEqual(['String 2', 'String 3']); }); it('should return an empty array from the settings when global fallback is disabled', async () => { diff --git a/api/src/i18n/services/translation.service.ts b/api/src/i18n/services/translation.service.ts index 73c76bf..44f22fc 100644 --- a/api/src/i18n/services/translation.service.ts +++ b/api/src/i18n/services/translation.service.ts @@ -10,6 +10,8 @@ import { Injectable } from '@nestjs/common'; import { OnEvent } from '@nestjs/event-emitter'; import { I18nService } from '@/i18n/services/i18n.service'; +import { PluginService } from '@/plugins/plugins.service'; +import { PluginType } from '@/plugins/types'; import { SettingService } from '@/setting/services/setting.service'; import { BaseService } from '@/utils/generics/base-service'; @@ -24,6 +26,7 @@ export class TranslationService extends BaseService { readonly repository: TranslationRepository, private readonly blockService: BlockService, private readonly settingService: SettingService, + private readonly pluginService: PluginService, private readonly i18n: I18nService, ) { super(repository); @@ -49,14 +52,23 @@ export class TranslationService extends BaseService { strings = strings.concat(block.message); } else if (typeof block.message === 'object') { if ('plugin' in block.message) { + const plugin = this.pluginService.getPlugin( + PluginType.block, + block.message.plugin, + ); + // plugin - Object.values(block.message.args).forEach((arg) => { - if (Array.isArray(arg)) { - // array of text - strings = strings.concat(arg); - } else if (typeof arg === 'string') { - // text - strings.push(arg); + Object.entries(block.message.args).forEach(([l, arg]) => { + const setting = plugin.settings.find(({ label }) => label === l); + + if (setting?.translatable) { + if (Array.isArray(arg)) { + // array of text + strings = strings.concat(arg); + } else if (typeof arg === 'string') { + // text + strings.push(arg); + } } }); } else if ('text' in block.message && Array.isArray(block.message.text)) { @@ -121,12 +133,18 @@ export class TranslationService extends BaseService { * @returns A promise of all strings available in a array */ async getSettingStrings(): Promise { - let strings: string[] = []; + const translatableSettings = await this.settingService.find({ + translatable: true, + }); const settings = await this.settingService.getSettings(); - if (settings.chatbot_settings.global_fallback) { - strings = strings.concat(settings.chatbot_settings.fallback_message); - } - return strings; + return Object.values(settings) + .map((group: Record) => Object.entries(group)) + .flat() + .filter(([l]) => { + return translatableSettings.find(({ label }) => label === l); + }) + .map(([, v]) => v) + .flat(); } /** diff --git a/api/src/plugins/types.ts b/api/src/plugins/types.ts index 2dfae9d..d0b6a27 100644 --- a/api/src/plugins/types.ts +++ b/api/src/plugins/types.ts @@ -24,7 +24,9 @@ export interface CustomBlocks {} type BlockAttrs = Partial & { name: string }; -export type PluginSetting = Omit; +export type PluginSetting = Omit & { + translatable?: boolean; +}; export type PluginBlockTemplate = Omit< BlockAttrs, diff --git a/api/src/setting/dto/setting.dto.ts b/api/src/setting/dto/setting.dto.ts index 6a3b025..c1a3ad9 100644 --- a/api/src/setting/dto/setting.dto.ts +++ b/api/src/setting/dto/setting.dto.ts @@ -9,6 +9,7 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; import { IsArray, + IsBoolean, IsIn, IsNotEmpty, IsOptional, @@ -65,8 +66,20 @@ export class SettingCreateDto { //TODO: adding swagger decorators config?: Record; - //TODO: adding swagger decorators + @ApiPropertyOptional({ + description: + 'Defines the display order of the setting in the user interface', + type: Number, + }) weight: number; + + @ApiPropertyOptional({ + description: 'Indicates whether this setting supports translation', + type: Boolean, + }) + @IsBoolean() + @IsOptional() + translatable?: boolean; } export class SettingUpdateDto { diff --git a/api/src/setting/schemas/setting.schema.ts b/api/src/setting/schemas/setting.schema.ts index 077a827..e80e831 100644 --- a/api/src/setting/schemas/setting.schema.ts +++ b/api/src/setting/schemas/setting.schema.ts @@ -58,6 +58,12 @@ export class Setting extends BaseSchema { default: 0, }) weight?: number; + + @Prop({ + type: Boolean, + default: false, + }) + translatable?: boolean; } export const SettingModel: ModelDefinition = LifecycleHookManager.attach({ diff --git a/api/src/setting/seeds/setting.seed-model.ts b/api/src/setting/seeds/setting.seed-model.ts index cb8b4c6..e5cfce7 100644 --- a/api/src/setting/seeds/setting.seed-model.ts +++ b/api/src/setting/seeds/setting.seed-model.ts @@ -69,6 +69,7 @@ export const DEFAULT_SETTINGS = [ ] as string[], type: SettingType.multiple_text, weight: 5, + translatable: true, }, { group: 'contact', diff --git a/api/src/utils/test/fixtures/setting.ts b/api/src/utils/test/fixtures/setting.ts index 0cff704..f1bea92 100644 --- a/api/src/utils/test/fixtures/setting.ts +++ b/api/src/utils/test/fixtures/setting.ts @@ -19,6 +19,15 @@ export const settingFixtures: SettingCreateDto[] = [ value: 'admin@example.com', type: SettingType.text, weight: 1, + translatable: false, + }, + { + group: 'contact', + label: 'greeting', + value: 'hello', + type: SettingType.text, + weight: 10, + translatable: true, }, { group: 'contact', @@ -26,6 +35,7 @@ export const settingFixtures: SettingCreateDto[] = [ value: 'Your company name', type: SettingType.text, weight: 2, + translatable: false, }, { group: 'contact', @@ -33,6 +43,7 @@ export const settingFixtures: SettingCreateDto[] = [ value: '(+999) 9999 9999 999', type: SettingType.text, weight: 3, + translatable: false, }, { group: 'contact', @@ -40,6 +51,7 @@ export const settingFixtures: SettingCreateDto[] = [ value: 'contact[at]mycompany.com', type: SettingType.text, weight: 4, + translatable: false, }, { group: 'contact', @@ -47,6 +59,7 @@ export const settingFixtures: SettingCreateDto[] = [ value: '71 Pilgrim Avenue', type: SettingType.text, weight: 5, + translatable: false, }, { group: 'contact', @@ -54,6 +67,7 @@ export const settingFixtures: SettingCreateDto[] = [ value: '', type: SettingType.text, weight: 6, + translatable: false, }, { group: 'contact', @@ -61,6 +75,7 @@ export const settingFixtures: SettingCreateDto[] = [ value: 'Chevy Chase', type: SettingType.text, weight: 7, + translatable: false, }, { group: 'contact', @@ -68,6 +83,7 @@ export const settingFixtures: SettingCreateDto[] = [ value: '85705', type: SettingType.text, weight: 8, + translatable: false, }, { group: 'contact', @@ -75,6 +91,7 @@ export const settingFixtures: SettingCreateDto[] = [ value: 'Orlando', type: SettingType.text, weight: 9, + translatable: false, }, { group: 'contact', @@ -82,6 +99,7 @@ export const settingFixtures: SettingCreateDto[] = [ value: 'US', type: SettingType.text, weight: 10, + translatable: false, }, ]; From 628d0f5819f60d52260d30f63d4a57b719a7a2e1 Mon Sep 17 00:00:00 2001 From: abdou6666 Date: Fri, 22 Nov 2024 11:51:54 +0100 Subject: [PATCH 2/4] fix: address review comments --- api/src/i18n/services/translation.service.spec.ts | 6 +++--- api/src/plugins/types.ts | 4 +--- api/src/utils/test/fixtures/setting.ts | 10 ---------- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/api/src/i18n/services/translation.service.spec.ts b/api/src/i18n/services/translation.service.spec.ts index fda9087..dfa2601 100644 --- a/api/src/i18n/services/translation.service.spec.ts +++ b/api/src/i18n/services/translation.service.spec.ts @@ -72,9 +72,9 @@ describe('TranslationService', () => { .mockResolvedValue(['Global fallback message']), find: jest.fn().mockResolvedValue([ { - translatable: true, - key: 'global_fallback', - value: 'Global fallback message', + translatable: false, + global_fallback: 'global_fallback', + fallback_message: ['Global fallback message'], }, ]), }, diff --git a/api/src/plugins/types.ts b/api/src/plugins/types.ts index d0b6a27..2dfae9d 100644 --- a/api/src/plugins/types.ts +++ b/api/src/plugins/types.ts @@ -24,9 +24,7 @@ export interface CustomBlocks {} type BlockAttrs = Partial & { name: string }; -export type PluginSetting = Omit & { - translatable?: boolean; -}; +export type PluginSetting = Omit; export type PluginBlockTemplate = Omit< BlockAttrs, diff --git a/api/src/utils/test/fixtures/setting.ts b/api/src/utils/test/fixtures/setting.ts index f1bea92..4d6355f 100644 --- a/api/src/utils/test/fixtures/setting.ts +++ b/api/src/utils/test/fixtures/setting.ts @@ -19,7 +19,6 @@ export const settingFixtures: SettingCreateDto[] = [ value: 'admin@example.com', type: SettingType.text, weight: 1, - translatable: false, }, { group: 'contact', @@ -35,7 +34,6 @@ export const settingFixtures: SettingCreateDto[] = [ value: 'Your company name', type: SettingType.text, weight: 2, - translatable: false, }, { group: 'contact', @@ -43,7 +41,6 @@ export const settingFixtures: SettingCreateDto[] = [ value: '(+999) 9999 9999 999', type: SettingType.text, weight: 3, - translatable: false, }, { group: 'contact', @@ -51,7 +48,6 @@ export const settingFixtures: SettingCreateDto[] = [ value: 'contact[at]mycompany.com', type: SettingType.text, weight: 4, - translatable: false, }, { group: 'contact', @@ -59,7 +55,6 @@ export const settingFixtures: SettingCreateDto[] = [ value: '71 Pilgrim Avenue', type: SettingType.text, weight: 5, - translatable: false, }, { group: 'contact', @@ -67,7 +62,6 @@ export const settingFixtures: SettingCreateDto[] = [ value: '', type: SettingType.text, weight: 6, - translatable: false, }, { group: 'contact', @@ -75,7 +69,6 @@ export const settingFixtures: SettingCreateDto[] = [ value: 'Chevy Chase', type: SettingType.text, weight: 7, - translatable: false, }, { group: 'contact', @@ -83,7 +76,6 @@ export const settingFixtures: SettingCreateDto[] = [ value: '85705', type: SettingType.text, weight: 8, - translatable: false, }, { group: 'contact', @@ -91,7 +83,6 @@ export const settingFixtures: SettingCreateDto[] = [ value: 'Orlando', type: SettingType.text, weight: 9, - translatable: false, }, { group: 'contact', @@ -99,7 +90,6 @@ export const settingFixtures: SettingCreateDto[] = [ value: 'US', type: SettingType.text, weight: 10, - translatable: false, }, ]; From 7d3a92e1d68882fd195a6ad1fee53d4bda291ff6 Mon Sep 17 00:00:00 2001 From: abdou6666 Date: Fri, 22 Nov 2024 14:27:24 +0100 Subject: [PATCH 3/4] fix: minor changes --- api/src/i18n/services/translation.service.spec.ts | 3 +-- api/src/i18n/services/translation.service.ts | 5 +++-- api/src/utils/test/fixtures/setting.ts | 8 -------- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/api/src/i18n/services/translation.service.spec.ts b/api/src/i18n/services/translation.service.spec.ts index dfa2601..6ad5737 100644 --- a/api/src/i18n/services/translation.service.spec.ts +++ b/api/src/i18n/services/translation.service.spec.ts @@ -73,7 +73,7 @@ describe('TranslationService', () => { find: jest.fn().mockResolvedValue([ { translatable: false, - global_fallback: 'global_fallback', + label: 'global_fallback', fallback_message: ['Global fallback message'], }, ]), @@ -175,7 +175,6 @@ describe('TranslationService', () => { fallback_message: ['Global fallback message'], }, } as Settings); - const strings = await service.getSettingStrings(); expect(strings).toEqual([]); }); diff --git a/api/src/i18n/services/translation.service.ts b/api/src/i18n/services/translation.service.ts index 44f22fc..6a4d25d 100644 --- a/api/src/i18n/services/translation.service.ts +++ b/api/src/i18n/services/translation.service.ts @@ -140,8 +140,9 @@ export class TranslationService extends BaseService { return Object.values(settings) .map((group: Record) => Object.entries(group)) .flat() - .filter(([l]) => { - return translatableSettings.find(({ label }) => label === l); + .filter(([l, value]) => { + const found = translatableSettings.find(({ label }) => label === l); + return found && !!value; }) .map(([, v]) => v) .flat(); diff --git a/api/src/utils/test/fixtures/setting.ts b/api/src/utils/test/fixtures/setting.ts index 4d6355f..0cff704 100644 --- a/api/src/utils/test/fixtures/setting.ts +++ b/api/src/utils/test/fixtures/setting.ts @@ -20,14 +20,6 @@ export const settingFixtures: SettingCreateDto[] = [ type: SettingType.text, weight: 1, }, - { - group: 'contact', - label: 'greeting', - value: 'hello', - type: SettingType.text, - weight: 10, - translatable: true, - }, { group: 'contact', label: 'company_name', From 9a885a328d1fa20eed8cdcbcdfb3a4cd41a89940 Mon Sep 17 00:00:00 2001 From: abdou6666 Date: Fri, 22 Nov 2024 16:59:49 +0100 Subject: [PATCH 4/4] test: fix unit tests --- .../i18n/services/translation.service.spec.ts | 48 ++++++++++++------- api/src/i18n/services/translation.service.ts | 6 +-- .../controllers/setting.controller.spec.ts | 23 ++++++--- .../setting/services/setting.service.spec.ts | 1 + 4 files changed, 49 insertions(+), 29 deletions(-) diff --git a/api/src/i18n/services/translation.service.spec.ts b/api/src/i18n/services/translation.service.spec.ts index 6ad5737..5f0d6bb 100644 --- a/api/src/i18n/services/translation.service.spec.ts +++ b/api/src/i18n/services/translation.service.spec.ts @@ -22,7 +22,6 @@ import { TranslationService } from '../services/translation.service'; describe('TranslationService', () => { let service: TranslationService; - let settingService: SettingService; let i18nService: I18nService; let pluginService: PluginService; @@ -67,16 +66,36 @@ describe('TranslationService', () => { { provide: SettingService, useValue: { - getSettings: jest - .fn() - .mockResolvedValue(['Global fallback message']), - find: jest.fn().mockResolvedValue([ - { - translatable: false, - label: 'global_fallback', + getSettings: jest.fn().mockResolvedValue({ + chatbot_settings: { + global_fallback: true, fallback_message: ['Global fallback message'], }, - ]), + }), + find: jest + .fn() + .mockImplementation((criteria: { translatable?: boolean }) => + [ + { + translatable: false, + group: 'default', + value: 'Global fallback', + label: 'global_fallback', + type: SettingType.checkbox, + }, + { + translatable: true, + group: 'default', + value: 'Global fallback message', + label: 'fallback_message', + type: SettingType.text, + }, + ].filter((s) => + criteria && 'translatable' in criteria + ? s.translatable === criteria.translatable + : true, + ), + ), }, }, { @@ -90,7 +109,6 @@ describe('TranslationService', () => { }).compile(); service = module.get(TranslationService); - settingService = module.get(SettingService); i18nService = module.get(I18nService); pluginService = module.get(PluginService); }); @@ -168,15 +186,9 @@ describe('TranslationService', () => { expect(result).toEqual(['String 2', 'String 3']); }); - it('should return an empty array from the settings when global fallback is disabled', async () => { - jest.spyOn(settingService, 'getSettings').mockResolvedValueOnce({ - chatbot_settings: { - global_fallback: false, - fallback_message: ['Global fallback message'], - }, - } as Settings); + it('should return the settings translation strings', async () => { const strings = await service.getSettingStrings(); - expect(strings).toEqual([]); + expect(strings).toEqual(['Global fallback message']); }); it('should return an array of strings from a block with a quick reply message', () => { diff --git a/api/src/i18n/services/translation.service.ts b/api/src/i18n/services/translation.service.ts index 6a4d25d..434d062 100644 --- a/api/src/i18n/services/translation.service.ts +++ b/api/src/i18n/services/translation.service.ts @@ -60,7 +60,6 @@ export class TranslationService extends BaseService { // plugin Object.entries(block.message.args).forEach(([l, arg]) => { const setting = plugin.settings.find(({ label }) => label === l); - if (setting?.translatable) { if (Array.isArray(arg)) { // array of text @@ -140,9 +139,8 @@ export class TranslationService extends BaseService { return Object.values(settings) .map((group: Record) => Object.entries(group)) .flat() - .filter(([l, value]) => { - const found = translatableSettings.find(({ label }) => label === l); - return found && !!value; + .filter(([l]) => { + return translatableSettings.find(({ label }) => label === l); }) .map(([, v]) => v) .flat(); diff --git a/api/src/setting/controllers/setting.controller.spec.ts b/api/src/setting/controllers/setting.controller.spec.ts index aacf019..21ad5d3 100644 --- a/api/src/setting/controllers/setting.controller.spec.ts +++ b/api/src/setting/controllers/setting.controller.spec.ts @@ -82,7 +82,13 @@ describe('SettingController', () => { ); expect(settingService.find).toHaveBeenCalled(); - expect(result).toEqualPayload(settingFixtures); + expect(result).toEqualPayload(settingFixtures, [ + 'id', + 'createdAt', + 'updatedAt', + 'subgroup', + 'translatable', + ]); }); }); @@ -97,12 +103,15 @@ describe('SettingController', () => { const result = await settingController.updateOne(id, payload); expect(settingService.updateOne).toHaveBeenCalledWith(id, payload); - expect(result).toEqualPayload({ - ...settingFixtures.find( - (settingFixture) => settingFixture.value === 'admin@example.com', - ), - value: payload.value, - }); + expect(result).toEqualPayload( + { + ...settingFixtures.find( + (settingFixture) => settingFixture.value === 'admin@example.com', + ), + value: payload.value, + }, + ['id', 'createdAt', 'updatedAt', 'subgroup', 'translatable'], + ); }); }); }); diff --git a/api/src/setting/services/setting.service.spec.ts b/api/src/setting/services/setting.service.spec.ts index fee81eb..9007082 100644 --- a/api/src/setting/services/setting.service.spec.ts +++ b/api/src/setting/services/setting.service.spec.ts @@ -84,6 +84,7 @@ describe('SettingService', () => { expect(settingRepository.findAll).toHaveBeenCalled(); expect(result).toEqualPayload( settingService.group(settingFixtures as Setting[]), + ['id', 'createdAt', 'updatedAt', 'subgroup', 'translatable'], ); }); });