From feeb2fcd19ca35117830fae71bb485f865a93106 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 7 Dec 2025 05:30:07 +0000 Subject: [PATCH] fix: pass existing config to integration constructor in UpdateIntegration Fixes #514 - UpdateIntegration was passing the new config instead of the existing database config to the integration constructor. This broke partial update semantics where onUpdate methods expect to merge changes with the current state via patterns like this._deepMerge(this.config, params.config). Changes: - Pass integrationRecord.config (existing) instead of config (new) to constructor - Add TDD tests verifying partial config update preserves unspecified fields - Update DummyIntegration test double to implement proper onUpdate merge behavior - Add ConfigCapturingIntegration test double for verifying config state --- .../doubles/config-capturing-integration.js | 81 +++++++++++++++++++ .../tests/doubles/dummy-integration-class.js | 24 +++++- .../use-cases/update-integration.test.js | 71 +++++++++++++++- .../use-cases/update-integration.js | 4 +- 4 files changed, 175 insertions(+), 5 deletions(-) create mode 100644 packages/core/integrations/tests/doubles/config-capturing-integration.js diff --git a/packages/core/integrations/tests/doubles/config-capturing-integration.js b/packages/core/integrations/tests/doubles/config-capturing-integration.js new file mode 100644 index 000000000..814882d6f --- /dev/null +++ b/packages/core/integrations/tests/doubles/config-capturing-integration.js @@ -0,0 +1,81 @@ +const { IntegrationBase } = require('../../integration-base'); + +class ConfigCapturingModule { + static definition = { + getName: () => 'config-capturing-module' + }; +} + +class ConfigCapturingIntegration extends IntegrationBase { + static Definition = { + name: 'config-capturing', + version: '1.0.0', + modules: { + primary: ConfigCapturingModule + }, + display: { + label: 'Config Capturing Integration', + description: 'Test double for capturing config state during updates', + detailsUrl: 'https://example.com', + icon: 'test-icon' + } + }; + + static _capturedOnUpdateState = null; + + static resetCaptures() { + this._capturedOnUpdateState = null; + } + + static getCapturedOnUpdateState() { + return this._capturedOnUpdateState; + } + + constructor(params) { + super(params); + this.integrationRepository = { + updateIntegrationById: jest.fn().mockResolvedValue({}), + findIntegrationById: jest.fn().mockResolvedValue({}), + }; + this.updateIntegrationStatus = { + execute: jest.fn().mockResolvedValue({}) + }; + this.updateIntegrationMessages = { + execute: jest.fn().mockResolvedValue({}) + }; + } + + async initialize() { + this.registerEventHandlers(); + } + + async onUpdate(params) { + ConfigCapturingIntegration._capturedOnUpdateState = { + thisConfig: JSON.parse(JSON.stringify(this.config)), + paramsConfig: params.config + }; + + this.config = this._deepMerge(this.config, params.config); + } + + _deepMerge(target, source) { + const result = { ...target }; + for (const key of Object.keys(source)) { + if ( + source[key] !== null && + typeof source[key] === 'object' && + !Array.isArray(source[key]) && + target[key] !== null && + typeof target[key] === 'object' && + !Array.isArray(target[key]) + ) { + result[key] = this._deepMerge(target[key], source[key]); + } else { + result[key] = source[key]; + } + } + return result; + } +} + +module.exports = { ConfigCapturingIntegration }; diff --git a/packages/core/integrations/tests/doubles/dummy-integration-class.js b/packages/core/integrations/tests/doubles/dummy-integration-class.js index 51c0d584e..c860c7744 100644 --- a/packages/core/integrations/tests/doubles/dummy-integration-class.js +++ b/packages/core/integrations/tests/doubles/dummy-integration-class.js @@ -56,6 +56,9 @@ class DummyIntegration extends IntegrationBase { async send(event, data) { this.sendSpy(event, data); this.eventCallHistory.push({ event, data, timestamp: Date.now() }); + if (event === 'ON_UPDATE') { + await this.onUpdate(data); + } return { event, data }; } @@ -68,7 +71,26 @@ class DummyIntegration extends IntegrationBase { } async onUpdate(params) { - return; + this.config = this._deepMerge(this.config, params.config); + } + + _deepMerge(target, source) { + const result = { ...target }; + for (const key of Object.keys(source)) { + if ( + source[key] !== null && + typeof source[key] === 'object' && + !Array.isArray(source[key]) && + target[key] !== null && + typeof target[key] === 'object' && + !Array.isArray(target[key]) + ) { + result[key] = this._deepMerge(target[key], source[key]); + } else { + result[key] = source[key]; + } + } + return result; } async onDelete(params) { diff --git a/packages/core/integrations/tests/use-cases/update-integration.test.js b/packages/core/integrations/tests/use-cases/update-integration.test.js index cbe052949..c88c73f78 100644 --- a/packages/core/integrations/tests/use-cases/update-integration.test.js +++ b/packages/core/integrations/tests/use-cases/update-integration.test.js @@ -9,6 +9,7 @@ const { UpdateIntegration } = require('../../use-cases/update-integration'); const { TestIntegrationRepository } = require('../doubles/test-integration-repository'); const { TestModuleFactory } = require('../../../modules/tests/doubles/test-module-factory'); const { DummyIntegration } = require('../doubles/dummy-integration-class'); +const { ConfigCapturingIntegration } = require('../doubles/config-capturing-integration'); describe('UpdateIntegration Use-Case', () => { let integrationRepository; @@ -121,7 +122,7 @@ describe('UpdateIntegration Use-Case', () => { expect(dto.config.bar).toBeUndefined(); }); - it('handles deeply nested config updates', async () => { + it('handles deeply nested config updates with merge semantics', async () => { const record = await integrationRepository.createIntegration(['e1'], 'user-1', { type: 'dummy', nested: { old: 'value' } }); const newConfig = { @@ -135,7 +136,73 @@ describe('UpdateIntegration Use-Case', () => { expect(dto.config.nested.new).toBe('value'); expect(dto.config.nested.deep.level).toBe('test'); - expect(dto.config.nested.old).toBeUndefined(); + expect(dto.config.nested.old).toBe('value'); + }); + }); + + describe('partial config update semantics (issue #514)', () => { + let configCapturingUseCase; + + beforeEach(() => { + ConfigCapturingIntegration.resetCaptures(); + configCapturingUseCase = new UpdateIntegration({ + integrationRepository, + integrationClasses: [ConfigCapturingIntegration], + moduleFactory, + }); + }); + + it('passes existing database config to integration constructor', async () => { + const existingConfig = { type: 'config-capturing', a: 1, b: 2, c: 3 }; + const record = await integrationRepository.createIntegration( + ['e1'], + 'user-1', + existingConfig + ); + + const partialUpdateConfig = { type: 'config-capturing', a: 10 }; + await configCapturingUseCase.execute(record.id, 'user-1', partialUpdateConfig); + + const captured = ConfigCapturingIntegration.getCapturedOnUpdateState(); + expect(captured.thisConfig).toEqual(existingConfig); + expect(captured.paramsConfig).toEqual(partialUpdateConfig); + }); + + it('allows onUpdate to merge partial config with existing config', async () => { + const existingConfig = { type: 'config-capturing', a: 1, b: 2, c: 3 }; + const record = await integrationRepository.createIntegration( + ['e1'], + 'user-1', + existingConfig + ); + + const partialUpdateConfig = { type: 'config-capturing', a: 10 }; + const dto = await configCapturingUseCase.execute(record.id, 'user-1', partialUpdateConfig); + + expect(dto.config).toEqual({ type: 'config-capturing', a: 10, b: 2, c: 3 }); + }); + + it('preserves nested existing values during partial update', async () => { + const existingConfig = { + type: 'config-capturing', + settings: { theme: 'dark', notifications: true }, + credentials: { apiKey: 'secret123' } + }; + const record = await integrationRepository.createIntegration( + ['e1'], + 'user-1', + existingConfig + ); + + const partialUpdateConfig = { + type: 'config-capturing', + settings: { theme: 'light' } + }; + const dto = await configCapturingUseCase.execute(record.id, 'user-1', partialUpdateConfig); + + expect(dto.config.settings.theme).toBe('light'); + expect(dto.config.settings.notifications).toBe(true); + expect(dto.config.credentials.apiKey).toBe('secret123'); }); }); }); \ No newline at end of file diff --git a/packages/core/integrations/use-cases/update-integration.js b/packages/core/integrations/use-cases/update-integration.js index 6080e5ba5..a5187cffd 100644 --- a/packages/core/integrations/use-cases/update-integration.js +++ b/packages/core/integrations/use-cases/update-integration.js @@ -70,12 +70,12 @@ class UpdateIntegration { modules.push(moduleInstance); } - // 4. Create the Integration domain entity with modules and updated config + // 4. Create the Integration domain entity with modules and existing config const integrationInstance = new integrationClass({ id: integrationRecord.id, userId: integrationRecord.userId, entities: integrationRecord.entitiesIds, - config: config, + config: integrationRecord.config, status: integrationRecord.status, version: integrationRecord.version, messages: integrationRecord.messages,