Skip to content

Commit feeb2fc

Browse files
committed
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
1 parent 859700b commit feeb2fc

File tree

4 files changed

+175
-5
lines changed

4 files changed

+175
-5
lines changed
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
const { IntegrationBase } = require('../../integration-base');
2+
3+
class ConfigCapturingModule {
4+
static definition = {
5+
getName: () => 'config-capturing-module'
6+
};
7+
}
8+
9+
class ConfigCapturingIntegration extends IntegrationBase {
10+
static Definition = {
11+
name: 'config-capturing',
12+
version: '1.0.0',
13+
modules: {
14+
primary: ConfigCapturingModule
15+
},
16+
display: {
17+
label: 'Config Capturing Integration',
18+
description: 'Test double for capturing config state during updates',
19+
detailsUrl: 'https://example.com',
20+
icon: 'test-icon'
21+
}
22+
};
23+
24+
static _capturedOnUpdateState = null;
25+
26+
static resetCaptures() {
27+
this._capturedOnUpdateState = null;
28+
}
29+
30+
static getCapturedOnUpdateState() {
31+
return this._capturedOnUpdateState;
32+
}
33+
34+
constructor(params) {
35+
super(params);
36+
this.integrationRepository = {
37+
updateIntegrationById: jest.fn().mockResolvedValue({}),
38+
findIntegrationById: jest.fn().mockResolvedValue({}),
39+
};
40+
this.updateIntegrationStatus = {
41+
execute: jest.fn().mockResolvedValue({})
42+
};
43+
this.updateIntegrationMessages = {
44+
execute: jest.fn().mockResolvedValue({})
45+
};
46+
}
47+
48+
async initialize() {
49+
this.registerEventHandlers();
50+
}
51+
52+
async onUpdate(params) {
53+
ConfigCapturingIntegration._capturedOnUpdateState = {
54+
thisConfig: JSON.parse(JSON.stringify(this.config)),
55+
paramsConfig: params.config
56+
};
57+
58+
this.config = this._deepMerge(this.config, params.config);
59+
}
60+
61+
_deepMerge(target, source) {
62+
const result = { ...target };
63+
for (const key of Object.keys(source)) {
64+
if (
65+
source[key] !== null &&
66+
typeof source[key] === 'object' &&
67+
!Array.isArray(source[key]) &&
68+
target[key] !== null &&
69+
typeof target[key] === 'object' &&
70+
!Array.isArray(target[key])
71+
) {
72+
result[key] = this._deepMerge(target[key], source[key]);
73+
} else {
74+
result[key] = source[key];
75+
}
76+
}
77+
return result;
78+
}
79+
}
80+
81+
module.exports = { ConfigCapturingIntegration };

packages/core/integrations/tests/doubles/dummy-integration-class.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ class DummyIntegration extends IntegrationBase {
5656
async send(event, data) {
5757
this.sendSpy(event, data);
5858
this.eventCallHistory.push({ event, data, timestamp: Date.now() });
59+
if (event === 'ON_UPDATE') {
60+
await this.onUpdate(data);
61+
}
5962
return { event, data };
6063
}
6164

@@ -68,7 +71,26 @@ class DummyIntegration extends IntegrationBase {
6871
}
6972

7073
async onUpdate(params) {
71-
return;
74+
this.config = this._deepMerge(this.config, params.config);
75+
}
76+
77+
_deepMerge(target, source) {
78+
const result = { ...target };
79+
for (const key of Object.keys(source)) {
80+
if (
81+
source[key] !== null &&
82+
typeof source[key] === 'object' &&
83+
!Array.isArray(source[key]) &&
84+
target[key] !== null &&
85+
typeof target[key] === 'object' &&
86+
!Array.isArray(target[key])
87+
) {
88+
result[key] = this._deepMerge(target[key], source[key]);
89+
} else {
90+
result[key] = source[key];
91+
}
92+
}
93+
return result;
7294
}
7395

7496
async onDelete(params) {

packages/core/integrations/tests/use-cases/update-integration.test.js

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const { UpdateIntegration } = require('../../use-cases/update-integration');
99
const { TestIntegrationRepository } = require('../doubles/test-integration-repository');
1010
const { TestModuleFactory } = require('../../../modules/tests/doubles/test-module-factory');
1111
const { DummyIntegration } = require('../doubles/dummy-integration-class');
12+
const { ConfigCapturingIntegration } = require('../doubles/config-capturing-integration');
1213

1314
describe('UpdateIntegration Use-Case', () => {
1415
let integrationRepository;
@@ -121,7 +122,7 @@ describe('UpdateIntegration Use-Case', () => {
121122
expect(dto.config.bar).toBeUndefined();
122123
});
123124

124-
it('handles deeply nested config updates', async () => {
125+
it('handles deeply nested config updates with merge semantics', async () => {
125126
const record = await integrationRepository.createIntegration(['e1'], 'user-1', { type: 'dummy', nested: { old: 'value' } });
126127

127128
const newConfig = {
@@ -135,7 +136,73 @@ describe('UpdateIntegration Use-Case', () => {
135136

136137
expect(dto.config.nested.new).toBe('value');
137138
expect(dto.config.nested.deep.level).toBe('test');
138-
expect(dto.config.nested.old).toBeUndefined();
139+
expect(dto.config.nested.old).toBe('value');
140+
});
141+
});
142+
143+
describe('partial config update semantics (issue #514)', () => {
144+
let configCapturingUseCase;
145+
146+
beforeEach(() => {
147+
ConfigCapturingIntegration.resetCaptures();
148+
configCapturingUseCase = new UpdateIntegration({
149+
integrationRepository,
150+
integrationClasses: [ConfigCapturingIntegration],
151+
moduleFactory,
152+
});
153+
});
154+
155+
it('passes existing database config to integration constructor', async () => {
156+
const existingConfig = { type: 'config-capturing', a: 1, b: 2, c: 3 };
157+
const record = await integrationRepository.createIntegration(
158+
['e1'],
159+
'user-1',
160+
existingConfig
161+
);
162+
163+
const partialUpdateConfig = { type: 'config-capturing', a: 10 };
164+
await configCapturingUseCase.execute(record.id, 'user-1', partialUpdateConfig);
165+
166+
const captured = ConfigCapturingIntegration.getCapturedOnUpdateState();
167+
expect(captured.thisConfig).toEqual(existingConfig);
168+
expect(captured.paramsConfig).toEqual(partialUpdateConfig);
169+
});
170+
171+
it('allows onUpdate to merge partial config with existing config', async () => {
172+
const existingConfig = { type: 'config-capturing', a: 1, b: 2, c: 3 };
173+
const record = await integrationRepository.createIntegration(
174+
['e1'],
175+
'user-1',
176+
existingConfig
177+
);
178+
179+
const partialUpdateConfig = { type: 'config-capturing', a: 10 };
180+
const dto = await configCapturingUseCase.execute(record.id, 'user-1', partialUpdateConfig);
181+
182+
expect(dto.config).toEqual({ type: 'config-capturing', a: 10, b: 2, c: 3 });
183+
});
184+
185+
it('preserves nested existing values during partial update', async () => {
186+
const existingConfig = {
187+
type: 'config-capturing',
188+
settings: { theme: 'dark', notifications: true },
189+
credentials: { apiKey: 'secret123' }
190+
};
191+
const record = await integrationRepository.createIntegration(
192+
['e1'],
193+
'user-1',
194+
existingConfig
195+
);
196+
197+
const partialUpdateConfig = {
198+
type: 'config-capturing',
199+
settings: { theme: 'light' }
200+
};
201+
const dto = await configCapturingUseCase.execute(record.id, 'user-1', partialUpdateConfig);
202+
203+
expect(dto.config.settings.theme).toBe('light');
204+
expect(dto.config.settings.notifications).toBe(true);
205+
expect(dto.config.credentials.apiKey).toBe('secret123');
139206
});
140207
});
141208
});

packages/core/integrations/use-cases/update-integration.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,12 @@ class UpdateIntegration {
7070
modules.push(moduleInstance);
7171
}
7272

73-
// 4. Create the Integration domain entity with modules and updated config
73+
// 4. Create the Integration domain entity with modules and existing config
7474
const integrationInstance = new integrationClass({
7575
id: integrationRecord.id,
7676
userId: integrationRecord.userId,
7777
entities: integrationRecord.entitiesIds,
78-
config: config,
78+
config: integrationRecord.config,
7979
status: integrationRecord.status,
8080
version: integrationRecord.version,
8181
messages: integrationRecord.messages,

0 commit comments

Comments
 (0)