Skip to content

Commit c8d0664

Browse files
committed
fix: error handling + tests
1 parent d0fe4cf commit c8d0664

File tree

4 files changed

+196
-39
lines changed

4 files changed

+196
-39
lines changed
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { expect } from 'chai';
2+
import { PlainClientAPI } from 'contentful-management';
3+
import sinon from 'sinon';
4+
import { getExistingAction } from './client';
5+
6+
describe('wrapped CMA client', () => {
7+
describe('getExistingAction', () => {
8+
it('should return null if the action does not exist', async () => {
9+
const error = new Error('Not Found');
10+
Object.assign(error, { name: 'NotFound' });
11+
const client = { appAction: { get: sinon.stub().rejects(error) } } as unknown as PlainClientAPI;
12+
13+
const result = await getExistingAction(client, 'appDefId', 'actionId');
14+
15+
expect(result).to.eq(null);
16+
});
17+
18+
it('should throw an error if an unexpected error occurs', async () => {
19+
const error = new Error('Something else went wrong');
20+
const client = { appAction: { get: sinon.stub().rejects(error) } } as unknown as PlainClientAPI;
21+
22+
try {
23+
await getExistingAction(client, 'appDefId', 'actionId');
24+
} catch (e) {
25+
expect(e).to.eq(error);
26+
}
27+
});
28+
});
29+
});

packages/contentful--app-scripts/src/upsert-actions/client.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { cyan } from 'chalk';
2-
import { CreateAppActionProps, PlainClientAPI } from 'contentful-management';
2+
import { AppActionProps, CreateAppActionProps, PlainClientAPI } from 'contentful-management';
33

44
export async function createAction(
55
client: PlainClientAPI,
@@ -13,7 +13,7 @@ export async function createAction(
1313
payload
1414
);
1515
console.log(`
16-
${cyan('Success!')} Created Action ${cyan(action.name)} with ID ${cyan(
16+
${cyan('Success!')} Created Action ${cyan(action.name)} with ID ${cyan(
1717
action.sys.id
1818
)} for App ${cyan(appDefinitionId)}.`);
1919
return action;
@@ -42,7 +42,7 @@ export async function updateAction(
4242
appDefinitionId: string,
4343
appActionId: string,
4444
payload: CreateAppActionProps
45-
) {
45+
): Promise<AppActionProps> {
4646
const action = await client.appAction.update(
4747
{
4848
appDefinitionId,
@@ -51,7 +51,7 @@ export async function updateAction(
5151
payload
5252
);
5353
console.log(`
54-
${cyan('Success!')} Updated Action ${cyan(action.name)} with ID ${cyan(appActionId)} for App ${cyan(
54+
${cyan('Success!')} Updated Action ${cyan(action.name)} with ID ${cyan(appActionId)} for App ${cyan(
5555
appDefinitionId
5656
)}.`);
5757
return action;

packages/contentful--app-scripts/src/upsert-actions/upsert-action.test.ts

Lines changed: 124 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@ import { expect } from 'chai';
22
import { PlainClientAPI } from 'contentful-management';
33
import sinon from 'sinon';
44
import * as clientWrapper from './client';
5-
import { CreateAppActionPayload } from './types';
6-
import { doUpsert } from './upsert-actions';
5+
import { AppActionManifest, CreateAppActionPayload } from './types';
6+
import { doUpsert, processActionManifests, syncUpsertToManifest } from './upsert-actions';
7+
import fs from 'node:fs';
78

89
describe('doUpsert', () => {
910
let createStub: sinon.SinonStub;
@@ -97,3 +98,124 @@ describe('doUpsert', () => {
9798
expect(updateStub.called).to.be.false;
9899
});
99100
});
101+
102+
describe('syncUpsertToManifest', () => {
103+
let writeFileSyncStub: sinon.SinonStub;
104+
let consoleLogStub: sinon.SinonStub;
105+
106+
beforeEach(() => {
107+
writeFileSyncStub = sinon.stub(fs, 'writeFileSync');
108+
consoleLogStub = sinon.stub(console, 'log');
109+
});
110+
111+
afterEach(() => {
112+
writeFileSyncStub.restore();
113+
consoleLogStub.restore();
114+
});
115+
116+
it('should sync actions and write updated manifest to file in order', () => {
117+
const manifestActions = [
118+
{ id: '1', name: 'action1', type: 'endpoint', url: 'https://example.com/1' },
119+
{ id: '2', name: 'action2', type: 'endpoint', url: 'https://example.com/2' },
120+
{ id: '3', name: 'action3', type: 'endpoint', url: 'https://example.com/3' }
121+
];
122+
123+
const actionsToSync = {
124+
1: { id: '2', name: 'updated-action2', type: 'endpoint', url: 'https://example.com/1' }
125+
};
126+
127+
const manifest = {
128+
functions: [{ id: '1', name: 'function1' }],
129+
actions: manifestActions
130+
};
131+
132+
const manifestFile = 'manifest.json';
133+
134+
syncUpsertToManifest(manifestActions as any as AppActionManifest[], actionsToSync as any as { [i: number]: AppActionManifest }, manifest, manifestFile);
135+
136+
expect(writeFileSyncStub.calledOnce).to.be.true;
137+
expect(writeFileSyncStub.firstCall.args[0]).to.equal(manifestFile);
138+
expect(JSON.parse(writeFileSyncStub.firstCall.args[1])).to.deep.equal({
139+
// preserve other properties of the manifest
140+
functions: [{ id: '1', name: 'function1' }],
141+
actions: [
142+
{ id: '1', name: 'action1', type: 'endpoint', url: 'https://example.com/1' },
143+
{ id: '2', name: 'updated-action2', type: 'endpoint', url: 'https://example.com/1' },
144+
{ id: '3', name: 'action3', type: 'endpoint', url: 'https://example.com/3' }
145+
]
146+
});
147+
});
148+
});
149+
150+
describe('processActionManifests', () => {
151+
it('should process actions and return synced actions when successful', async () => {
152+
const actions = [
153+
{ name: 'action1', type: 'endpoint' },
154+
{ name: 'action2', type: 'endpoint' }
155+
];
156+
157+
const doUpsertStub = () => Promise.resolve({
158+
sys: {
159+
id: 'test-id'
160+
},
161+
name: 'action1',
162+
type: 'endpoint'
163+
});
164+
165+
const { actionsToSync, errors } = await processActionManifests(actions as AppActionManifest[], doUpsertStub as any);
166+
167+
expect(Object.keys(actionsToSync).length).to.equal(2);
168+
expect(actionsToSync[0]).to.deep.include({
169+
name: 'action1',
170+
type: 'endpoint',
171+
id: 'test-id'
172+
});
173+
expect(errors).to.be.empty;
174+
});
175+
176+
it('should collect errors when action processing fails', async () => {
177+
const actions = [
178+
{ name: 'action1', type: 'endpoint' },
179+
{ name: 'action2', type: 'endpoint' }
180+
];
181+
182+
const error = new Error('Processing failed')
183+
const doUpsertStub = () => Promise.reject(error);
184+
185+
const { actionsToSync, errors } = await processActionManifests(actions as AppActionManifest[], doUpsertStub);
186+
187+
expect(Object.keys(actionsToSync)).to.be.empty;
188+
expect(errors).to.have.length(2);
189+
expect(errors[0].details).to.equal(error);
190+
expect(errors[0].path).to.deep.equal(['actions', '0']);
191+
expect(errors[1].details).to.equal(error);
192+
expect(errors[1].path).to.deep.equal(['actions', '1']);
193+
});
194+
195+
it('should handle mixed success and failure cases', async () => {
196+
const actions = [
197+
{ name: 'action1', type: 'endpoint' },
198+
{ name: 'action2', type: 'endpoint' }
199+
];
200+
201+
const successResponse = { sys: { id: 'success-id' } };
202+
const error = new Error('Processing failed');
203+
204+
const doUpsertStubMixed = sinon.stub();
205+
doUpsertStubMixed.onFirstCall().resolves(successResponse);
206+
doUpsertStubMixed.onSecondCall().rejects(error);
207+
208+
const { actionsToSync, errors } = await processActionManifests(actions as AppActionManifest[], doUpsertStubMixed);
209+
210+
expect(Object.keys(actionsToSync)).to.have.length(1);
211+
expect(actionsToSync[0]).to.deep.include({
212+
name: 'action1',
213+
type: 'endpoint',
214+
id: 'success-id'
215+
});
216+
expect(errors).to.have.length(1);
217+
expect(errors[0].details).to.equal(error);
218+
expect(errors[0].path).to.deep.equal(['actions', '1']);
219+
});
220+
});
221+

packages/contentful--app-scripts/src/upsert-actions/upsert-actions.ts

Lines changed: 39 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { yellow } from 'chalk';
2-
import { CreateAppActionProps, createClient, PlainClientAPI } from 'contentful-management';
2+
import { AppActionProps, CreateAppActionProps, createClient, PlainClientAPI } from 'contentful-management';
33
import fs from 'node:fs';
44
import ora from 'ora';
55
import { resolveManifestFile, throwError } from '../utils';
@@ -12,42 +12,65 @@ export async function doUpsert(
1212
client: PlainClientAPI,
1313
appDefinitionId: string,
1414
payload: CreateAppActionPayload
15-
) {
15+
): Promise<AppActionProps> {
1616
if (payload.id) {
1717
const existingAction = await getExistingAction(client, appDefinitionId, payload.id);
1818
if (existingAction) {
19-
return await updateAction(
19+
const { id, ...update } = payload;
20+
return updateAction(
2021
client,
2122
appDefinitionId,
22-
payload.id,
23-
payload as CreateAppActionProps
23+
id,
24+
update as CreateAppActionProps
2425
);
2526
} else if (!existingAction && payload.type === 'endpoint') {
2627
throw new Error(
2728
`Action with id ${payload.id} not found. Endpoint actions may not set a custom ID.`
2829
);
2930
}
3031
}
31-
return await createAction(client, appDefinitionId, payload as CreateAppActionProps);
32+
return createAction(client, appDefinitionId, payload as CreateAppActionProps);
3233
}
3334

34-
function syncRemoteDataToManifest(
35+
export function syncUpsertToManifest(
3536
manifestActions: AppActionManifest[],
36-
actionsToSync: AppActionManifest[],
37+
actionsToSync: { [i: number]: AppActionManifest },
3738
manifest: Record<string, any>,
3839
manifestFile: string
3940
) {
40-
const dedupedActionsToSync = manifestActions
41-
.filter((action) => !actionsToSync.find((a) => a.name === action.name))
42-
.concat(actionsToSync);
41+
const actions = manifestActions.map((action, i) => {
42+
const syncedAction = actionsToSync[i];
43+
return syncedAction || action;
44+
});
4345

4446
fs.writeFileSync(
4547
manifestFile,
46-
JSON.stringify({ ...manifest, actions: dedupedActionsToSync }, null, 2)
48+
JSON.stringify({ ...manifest, actions }, null, 2)
4749
);
4850
console.log(`Remote updates synced to your manifest file at ${yellow(manifestFile)}.`);
4951
}
5052

53+
export async function processActionManifests(actions: AppActionManifest[], doUpsert: (payload: CreateAppActionPayload) => Promise<AppActionProps>) {
54+
const actionsToSync: { [i: number]: AppActionManifest } = {}
55+
const errors: { details: any; path: (string | number)[] }[] = [];
56+
for (const i in actions) {
57+
const action = actions[i];
58+
const payload = makeAppActionCMAPayload(action);
59+
60+
try {
61+
const appAction = await doUpsert(payload);
62+
actionsToSync[i] = {
63+
...action,
64+
id: appAction.sys.id,
65+
};
66+
} catch (err: any) {
67+
errors.push({ details: err, path: ['actions', i] });
68+
}
69+
}
70+
71+
return { actionsToSync, errors };
72+
}
73+
5174
export async function upsertAppActions(settings: Required<CreateAppActionSettings>) {
5275
const { accessToken, appDefinitionId, host, organizationId, manifestFile } = settings;
5376
const manifest = await resolveManifestFile({ manifestFile });
@@ -67,32 +90,15 @@ export async function upsertAppActions(settings: Required<CreateAppActionSetting
6790
}
6891
);
6992

70-
const actionsToSync: AppActionManifest[] = [];
71-
const errors: { details: any; path: (string | number)[] }[] = [];
72-
for (const i in actions) {
73-
const action = actions[i];
74-
const payload = makeAppActionCMAPayload(action);
75-
76-
try {
77-
const appAction = await doUpsert(client, appDefinitionId, payload);
78-
actionsToSync.push({
79-
...action,
80-
id: appAction.sys.id,
81-
});
82-
} catch (err: any) {
83-
errors.push({ details: err, path: ['actions', i] });
84-
}
85-
}
93+
const { actionsToSync, errors } = await processActionManifests(actions, async (payload) => doUpsert(client, appDefinitionId, payload));
8694

87-
syncRemoteDataToManifest(actions, actionsToSync, manifest, manifestFile);
95+
syncUpsertToManifest(actions, actionsToSync, manifest, manifestFile);
8896

8997
if (errors.length) {
9098
const error = new Error(
91-
JSON.stringify({
92-
message: 'Something went wrong while syncing your actions manifest to Contentful',
93-
details: errors,
94-
})
99+
`Failed to upsert actions`
95100
);
101+
Object.assign(error, { details: errors.map(({ details }) => details) });
96102
throwError(error, 'Failed to upsert actions');
97103
}
98104

0 commit comments

Comments
 (0)