Skip to content

Commit 5b37ce7

Browse files
committed
fix(apps): use app_builder_url from API response instead of constructing it
The backend now returns app_builder_url in upload and release responses, which reflects the correct host for orgs with custom domains (e.g. dd.datad0g.com). The previous hardcoded https://app.${site}/... produced wrong URLs for those orgs. The upload response's URL includes ?viewMode=preview (edit view); the release response's URL does not (published view) — this matches the backend behavior in ddoghq/dd-source#1038. Follows the "omit when absent" test discipline from #446: adds a test confirming no available-at message is logged when app_builder_url is absent from the upload response, matching the existing guard. Related: ddoghq/dd-source#1038
1 parent a2ed1e9 commit 5b37ce7

2 files changed

Lines changed: 108 additions & 11 deletions

File tree

packages/plugins/apps/src/upload.test.ts

Lines changed: 101 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,8 @@ describe('Apps Plugin - upload', () => {
229229
version_id: 'v123',
230230
application_id: 'app123',
231231
app_builder_id: 'builder123',
232+
app_builder_url:
233+
'https://app.datadoghq.com/app-builder/apps/edit/builder123?viewMode=preview',
232234
} as any);
233235

234236
const { errors, warnings } = await uploadArchive(archive, context, logger);
@@ -248,16 +250,65 @@ describe('Apps Plugin - upload', () => {
248250
onRetry: expect.any(Function),
249251
});
250252
expect(mockLogFn).toHaveBeenCalledWith(
251-
expect.stringContaining('Your application is available at'),
253+
expect.stringContaining(
254+
'https://app.datadoghq.com/app-builder/apps/edit/builder123?viewMode=preview',
255+
),
252256
'info',
253257
);
254258
});
255259

260+
test('Should use app_builder_url from upload response', async () => {
261+
doAuthenticatedRequestMock.mockResolvedValueOnce({
262+
version_id: 'v123',
263+
application_id: 'app123',
264+
app_builder_id: 'builder123',
265+
app_builder_url:
266+
'https://dd.datad0g.com/app-builder/apps/edit/builder123?viewMode=preview',
267+
} as any);
268+
269+
await uploadArchive(archive, context, logger);
270+
271+
// Uses the exact URL from the response — proves it's not reconstructed from
272+
// context.site (datadoghq.com), which would produce a different domain.
273+
expect(mockLogFn).toHaveBeenCalledWith(
274+
expect.stringContaining(
275+
'https://dd.datad0g.com/app-builder/apps/edit/builder123?viewMode=preview',
276+
),
277+
'info',
278+
);
279+
});
280+
281+
test('Should not log an available-at message when app_builder_url is absent', async () => {
282+
// Skip the release call entirely — this test only cares about the upload log.
283+
getDDEnvValueMock.mockImplementation((key) =>
284+
key === 'APPS_PUBLISH' ? 'false' : undefined,
285+
);
286+
doAuthenticatedRequestMock.mockResolvedValueOnce({
287+
version_id: 'v123',
288+
application_id: 'app123',
289+
app_builder_id: 'builder123',
290+
} as any);
291+
292+
const { errors } = await uploadArchive(archive, context, logger);
293+
294+
expect(errors).toHaveLength(0);
295+
const uploadLog = mockLogFn.mock.calls.find(([message]) =>
296+
message.startsWith('Your application is available at'),
297+
);
298+
expect(uploadLog).toBeUndefined();
299+
300+
// Reset — other tests in this file rely on getDDEnvValueMock's default
301+
// (undefined) behavior and beforeEach doesn't reset this particular mock.
302+
getDDEnvValueMock.mockReset();
303+
});
304+
256305
test('Should upload archive using the supplied request function', async () => {
257306
const doUploadAuthenticatedRequestMock = jest.fn().mockResolvedValue({
258307
version_id: 'v123',
259308
application_id: 'app123',
260309
app_builder_id: 'builder123',
310+
app_builder_url:
311+
'https://app.datadoghq.com/app-builder/apps/edit/builder123?viewMode=preview',
261312
} as any);
262313

263314
const { errors, warnings } = await uploadArchive(
@@ -286,8 +337,12 @@ describe('Apps Plugin - upload', () => {
286337
version_id: 'v123',
287338
application_id: 'app123',
288339
app_builder_id: 'builder123',
340+
app_builder_url:
341+
'https://app.datadoghq.com/app-builder/apps/edit/builder123?viewMode=preview',
289342
})
290-
.mockResolvedValueOnce({});
343+
.mockResolvedValueOnce({
344+
app_builder_url: 'https://app.datadoghq.com/app-builder/apps/builder123',
345+
});
291346

292347
const { errors, warnings } = await uploadArchive(archive, context, logger);
293348

@@ -301,10 +356,47 @@ describe('Apps Plugin - upload', () => {
301356
getData: expect.any(Function),
302357
onRetry: expect.any(Function),
303358
});
304-
expect(mockLogFn).toHaveBeenCalledWith(
305-
expect.stringContaining('Your application is available at'),
306-
'info',
359+
// Pin down which log is which by its distinguishing message prefix — proves not
360+
// just that a matching call exists somewhere, but that the upload log specifically
361+
// carries ?viewMode=preview and the release log specifically doesn't.
362+
const uploadLog = mockLogFn.mock.calls.find(([message]) =>
363+
message.startsWith('Your application is available at'),
364+
);
365+
const releaseLog = mockLogFn.mock.calls.find(([message]) =>
366+
message.startsWith('Published uploaded version'),
367+
);
368+
expect(uploadLog?.[0]).toContain(
369+
'https://app.datadoghq.com/app-builder/apps/edit/builder123?viewMode=preview',
370+
);
371+
expect(releaseLog?.[0]).toContain(
372+
'https://app.datadoghq.com/app-builder/apps/builder123',
373+
);
374+
expect(releaseLog?.[0]).not.toContain('?viewMode');
375+
});
376+
377+
test('Should use app_builder_url from release response', async () => {
378+
doAuthenticatedRequestMock
379+
.mockResolvedValueOnce({
380+
version_id: 'v123',
381+
application_id: 'app123',
382+
app_builder_id: 'builder123',
383+
app_builder_url:
384+
'https://dd.datad0g.com/app-builder/apps/edit/builder123?viewMode=preview',
385+
})
386+
.mockResolvedValueOnce({
387+
app_builder_url: 'https://dd.datad0g.com/app-builder/apps/builder123',
388+
});
389+
390+
await uploadArchive(archive, context, logger);
391+
392+
// Match the release log by its distinguishing message prefix — proves the
393+
// published URL reflects the custom domain from the release response (not
394+
// context.site) and carries no ?viewMode (unlike the upload log).
395+
const releaseLog = mockLogFn.mock.calls.find(([message]) =>
396+
message.startsWith('Published uploaded version'),
307397
);
398+
expect(releaseLog?.[0]).toContain('https://dd.datad0g.com/app-builder/apps/builder123');
399+
expect(releaseLog?.[0]).not.toContain('?viewMode');
308400
});
309401

310402
test.each(['false', '0', 'False', 'FALSE', 'off', 'no'])(
@@ -317,6 +409,8 @@ describe('Apps Plugin - upload', () => {
317409
version_id: 'v123',
318410
application_id: 'app123',
319411
app_builder_id: 'builder123',
412+
app_builder_url:
413+
'https://app.datadoghq.com/app-builder/apps/edit/builder123?viewMode=preview',
320414
});
321415

322416
const { errors } = await uploadArchive(archive, context, logger);
@@ -342,6 +436,8 @@ describe('Apps Plugin - upload', () => {
342436
version_id: 'v123',
343437
application_id: 'app123',
344438
app_builder_id: 'builder123',
439+
app_builder_url:
440+
'https://app.datadoghq.com/app-builder/apps/edit/builder123?viewMode=preview',
345441
});
346442

347443
const { errors, warnings } = await uploadArchive(archive, context, logger);

packages/plugins/apps/src/upload.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,17 +121,15 @@ Would have uploaded ${summary}`,
121121

122122
log.debug(`Uploaded ${summary}\n`);
123123

124-
if (response.app_builder_id) {
125-
const appBuilderUrl = `https://app.${context.site}/app-builder/apps/${response.app_builder_id}`;
126-
127-
log.info(`Your application is available at:\n ${cyan(appBuilderUrl)}`);
124+
if (response.app_builder_url) {
125+
log.info(`Your application is available at:\n ${cyan(response.app_builder_url)}`);
128126
}
129127

130128
const shouldPublish = parseBoolEnv(getDDEnvValue('APPS_PUBLISH'), true);
131129

132130
if (response.version_id && shouldPublish) {
133131
const releaseUrl = getReleaseUrl(context.site, context.identifier);
134-
await doAuthenticatedRequest({
132+
const releaseResponse: any = await doAuthenticatedRequest({
135133
url: releaseUrl,
136134
method: 'PUT',
137135
type: 'json',
@@ -150,7 +148,10 @@ Would have uploaded ${summary}`,
150148
log.warn(message);
151149
},
152150
});
153-
log.info(`Published uploaded version ${bold(response.version_id)} to live.`);
151+
152+
log.info(
153+
`Published uploaded version ${bold(response.version_id)} to live.\n ${cyan(releaseResponse.app_builder_url)}`,
154+
);
154155
} else if (response.version_id && !shouldPublish) {
155156
log.info(`Uploaded version ${bold(response.version_id)} as a draft (publish skipped).`);
156157
}

0 commit comments

Comments
 (0)