Skip to content

[APPS-2777] Fix: use app_builder_url from API response (fixes custom domain orgs) #449

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gh-worker-dd-mergequeue-cf854d merged 5 commits into from
Jul 9, 2026
Merged

[APPS-2777] Fix: use app_builder_url from API response (fixes custom domain orgs) #449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5b37ce7
fix(apps): use app_builder_url from API response instead of construct…
tyffical Jul 6, 2026
acbbc63
fix(apps): surface missing app_builder_url as a warning, not silence
tyffical Jul 7, 2026
16354b3
fix(apps): name the app in the missing-URL warning
tyffical Jul 7, 2026
9ea97b4
fix(apps): include app_builder_id in the missing-URL warning
tyffical Jul 7, 2026
a9294fb
fix(apps): type the upload/release API responses instead of any
tyffical Jul 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 166 additions & 5 deletions packages/plugins/apps/src/upload.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ describe('Apps Plugin - upload', () => {
version_id: 'v123',
application_id: 'app123',
app_builder_id: 'builder123',
app_builder_url:
'https://app.datadoghq.com/app-builder/apps/edit/builder123?viewMode=preview',
} as any);

const { errors, warnings } = await uploadArchive(archive, context, logger);
Expand All @@ -248,16 +250,97 @@ describe('Apps Plugin - upload', () => {
onRetry: expect.any(Function),
});
expect(mockLogFn).toHaveBeenCalledWith(
expect.stringContaining('Your application is available at'),
expect.stringContaining(
'https://app.datadoghq.com/app-builder/apps/edit/builder123?viewMode=preview',
),
'info',
);
});

test('Should use app_builder_url from upload response', async () => {
doAuthenticatedRequestMock.mockResolvedValueOnce({
version_id: 'v123',
application_id: 'app123',
app_builder_id: 'builder123',
app_builder_url:
'https://dd.datad0g.com/app-builder/apps/edit/builder123?viewMode=preview',
} as any);

await uploadArchive(archive, context, logger);

// Uses the exact URL from the response — proves it's not reconstructed from
// context.site (datadoghq.com), which would produce a different domain.
expect(mockLogFn).toHaveBeenCalledWith(
expect.stringContaining(
'https://dd.datad0g.com/app-builder/apps/edit/builder123?viewMode=preview',
),
'info',
);
});

test('Should warn (not error) when app_builder_url is absent from the upload response', async () => {
// Skip the release call entirely — this test only cares about the upload log.
getDDEnvValueMock.mockImplementation((key) =>
key === 'APPS_PUBLISH' ? 'false' : undefined,
);
doAuthenticatedRequestMock.mockResolvedValueOnce({
version_id: 'v123',
application_id: 'app123',
app_builder_id: 'builder123',
} as any);

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

// The upload itself succeeded — a missing display URL must never fail the build.
expect(errors).toHaveLength(0);
const uploadLog = mockLogFn.mock.calls.find(([message]) =>
message.startsWith('Your application is available at'),
);
expect(uploadLog).toBeUndefined();
// But it also must not go silent — surfaced as a warning so it's visible in CI
// output (see handle-upload.ts, which logs and aggregates `warnings` without
// failing the build, unlike `errors`). Names the app by its display name (not
// context.identifier, which is an opaque hash — see identifier.ts) and includes
// the app_builder_id, so there's a concrete, unambiguous next step (find it in
// the apps list, disambiguated by ID if names collide), not just a generic warning.
expect(warnings).toHaveLength(1);
expect(warnings[0]).toContain('Could not resolve the App Builder URL');
expect(warnings[0]).toContain(context.name);
expect(warnings[0]).toContain('builder123');

// Reset — other tests in this file rely on getDDEnvValueMock's default
// (undefined) behavior and beforeEach doesn't reset this particular mock.
getDDEnvValueMock.mockReset();
});

test('Should omit the app ID suffix when app_builder_id is also absent', async () => {
getDDEnvValueMock.mockImplementation((key) =>
key === 'APPS_PUBLISH' ? 'false' : undefined,
);
// Matches the backend's own no-op-path edge case (app-builder-code's
// TestAppBuilderURL_EmptyAppBuilderID) — both app_builder_id and
// app_builder_url absent, not just the URL.
doAuthenticatedRequestMock.mockResolvedValueOnce({
version_id: 'v123',
application_id: 'app123',
} as any);

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

expect(warnings).toHaveLength(1);
expect(warnings[0]).toContain(context.name);
expect(warnings[0]).not.toContain('app ID');

getDDEnvValueMock.mockReset();
});

test('Should upload archive using the supplied request function', async () => {
const doUploadAuthenticatedRequestMock = jest.fn().mockResolvedValue({
version_id: 'v123',
application_id: 'app123',
app_builder_id: 'builder123',
app_builder_url:
'https://app.datadoghq.com/app-builder/apps/edit/builder123?viewMode=preview',
} as any);

const { errors, warnings } = await uploadArchive(
Expand Down Expand Up @@ -286,8 +369,12 @@ describe('Apps Plugin - upload', () => {
version_id: 'v123',
application_id: 'app123',
app_builder_id: 'builder123',
app_builder_url:
'https://app.datadoghq.com/app-builder/apps/edit/builder123?viewMode=preview',
})
.mockResolvedValueOnce({});
.mockResolvedValueOnce({
app_builder_url: 'https://app.datadoghq.com/app-builder/apps/builder123',
});

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

Expand All @@ -301,10 +388,80 @@ describe('Apps Plugin - upload', () => {
getData: expect.any(Function),
onRetry: expect.any(Function),
});
expect(mockLogFn).toHaveBeenCalledWith(
expect.stringContaining('Your application is available at'),
'info',
// Pin down which log is which by its distinguishing message prefix — proves not
// just that a matching call exists somewhere, but that the upload log specifically
// carries ?viewMode=preview and the release log specifically doesn't.
const uploadLog = mockLogFn.mock.calls.find(([message]) =>
message.startsWith('Your application is available at'),
);
const releaseLog = mockLogFn.mock.calls.find(([message]) =>
message.startsWith('Published uploaded version'),
);
expect(uploadLog?.[0]).toContain(
'https://app.datadoghq.com/app-builder/apps/edit/builder123?viewMode=preview',
);
expect(releaseLog?.[0]).toContain(
'https://app.datadoghq.com/app-builder/apps/builder123',
);
expect(releaseLog?.[0]).not.toContain('?viewMode');
});

test('Should use app_builder_url from release response', async () => {
doAuthenticatedRequestMock
.mockResolvedValueOnce({
version_id: 'v123',
application_id: 'app123',
app_builder_id: 'builder123',
app_builder_url:
'https://dd.datad0g.com/app-builder/apps/edit/builder123?viewMode=preview',
})
.mockResolvedValueOnce({
app_builder_url: 'https://dd.datad0g.com/app-builder/apps/builder123',
});

await uploadArchive(archive, context, logger);

// Match the release log by its distinguishing message prefix — proves the
// published URL reflects the custom domain from the release response (not
// context.site) and carries no ?viewMode (unlike the upload log).
const releaseLog = mockLogFn.mock.calls.find(([message]) =>
message.startsWith('Published uploaded version'),
);
expect(releaseLog?.[0]).toContain('https://dd.datad0g.com/app-builder/apps/builder123');
expect(releaseLog?.[0]).not.toContain('?viewMode');
});

test('Should warn (not error) when app_builder_url is absent from the release response', async () => {
doAuthenticatedRequestMock
.mockResolvedValueOnce({
version_id: 'v123',
application_id: 'app123',
app_builder_id: 'builder123',
app_builder_url:
'https://app.datadoghq.com/app-builder/apps/edit/builder123?viewMode=preview',
})
// The backend couldn't resolve the org's app URL for this release — a real,
// designed-for degradation path (e.g. a transient org-lookup failure), not a
// hypothetical. The release itself still succeeded. app_builder_id is still
// present, though — it's set from the DB independently of the URL lookup.
.mockResolvedValueOnce({ app_builder_id: 'builder123' });

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

// The release itself succeeded — a missing display URL must never fail the build.
expect(errors).toHaveLength(0);
const releaseLog = mockLogFn.mock.calls.find(([message]) =>
message.startsWith('Published uploaded version'),
);
// Still confirms the release happened, just without a trailing URL line.
expect(releaseLog?.[0]).toContain('to live.');
expect(releaseLog?.[0]).not.toContain('\n');
// Surfaced as a warning rather than a blank/malformed log line — names the app
// by its display name and includes the app_builder_id for unambiguous lookup.
expect(warnings).toHaveLength(1);
expect(warnings[0]).toContain('Could not resolve the App Builder URL');
expect(warnings[0]).toContain(context.name);
expect(warnings[0]).toContain('builder123');
});

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

const { errors } = await uploadArchive(archive, context, logger);
Expand All @@ -342,6 +501,8 @@ describe('Apps Plugin - upload', () => {
version_id: 'v123',
application_id: 'app123',
app_builder_id: 'builder123',
app_builder_url:
'https://app.datadoghq.com/app-builder/apps/edit/builder123?viewMode=preview',
});

const { errors, warnings } = await uploadArchive(archive, context, logger);
Expand Down
65 changes: 58 additions & 7 deletions packages/plugins/apps/src/upload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,19 @@ import { APPS_API_PATH, ARCHIVE_FILENAME } from './constants';

type DataResponse = Awaited<ReturnType<typeof createRequestData>>;

// Only the fields this file actually reads. app_builder_url is deliberately optional on both:
// see buildMissingAppUrlWarning's callers for what happens when the backend can't resolve it.
type UploadApiResponse = {
app_builder_id?: string;
app_builder_url?: string;
version_id?: string;
};

type ReleaseApiResponse = {
app_builder_id?: string;
app_builder_url?: string;
};

export type UploadContext = {
bundlerName: string;
doAuthenticatedRequest: DoAuthenticatedRequest;
Expand All @@ -41,6 +54,19 @@ export const getReleaseUrl = (site: string, appId: string) => {
return `https://api.${site}/${APPS_API_PATH}/${appId}/release/live`;
};

// Builds the warning shown when the backend couldn't resolve an org's App Builder URL.
// Names the app (context.name — always human-readable, unlike context.identifier's opaque
// hash) and its app_builder_id when present, which disambiguates same-named apps and lets
// anyone who knows their org's domain construct the link directly.
const buildMissingAppUrlWarning = (
action: 'upload' | 'release',
name: string,
appBuilderId?: string,
) => {
const idSuffix = appBuilderId ? ` (app ID: ${appBuilderId})` : '';
return `Could not resolve the App Builder URL for this ${action} — find "${name}"${idSuffix} in your App Builder apps list to view it.`;
};

export const getData =
(archivePath: string, defaultHeaders: Record<string, string> = {}, name: string) =>
async (): Promise<DataResponse> => {
Expand Down Expand Up @@ -105,7 +131,7 @@ Would have uploaded ${summary}`,
}

try {
const response: any = await doAuthenticatedRequest({
const response = await doAuthenticatedRequest<UploadApiResponse>({
url: intakeUrl,
method: 'POST',
type: 'json',
Expand All @@ -121,17 +147,28 @@ Would have uploaded ${summary}`,

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

if (response.app_builder_id) {
const appBuilderUrl = `https://app.${context.site}/app-builder/apps/${response.app_builder_id}`;

log.info(`Your application is available at:\n ${cyan(appBuilderUrl)}`);
if (response.app_builder_url) {
log.info(`Your application is available at:\n ${cyan(response.app_builder_url)}`);
} else {
// The backend couldn't resolve this org's App Builder URL (e.g. a transient
// lookup failure) — the upload itself still succeeded, so this doesn't fail the
// build. Uses context.name, not context.identifier, since the latter is an
// opaque hash in the default case (see identifier.ts's buildIdentifier) — not
// something anyone would recognize or search for.
const message = buildMissingAppUrlWarning(
'upload',
context.name,
response.app_builder_id,
);
warnings.push(message);
log.warn(message);
}

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

if (response.version_id && shouldPublish) {
const releaseUrl = getReleaseUrl(context.site, context.identifier);
await doAuthenticatedRequest({
const releaseResponse = await doAuthenticatedRequest<ReleaseApiResponse>({
url: releaseUrl,
method: 'PUT',
type: 'json',
Expand All @@ -150,7 +187,21 @@ Would have uploaded ${summary}`,
log.warn(message);
},
});
log.info(`Published uploaded version ${bold(response.version_id)} to live.`);

if (releaseResponse.app_builder_url) {
log.info(
`Published uploaded version ${bold(response.version_id)} to live.\n ${cyan(releaseResponse.app_builder_url)}`,
);
} else {
log.info(`Published uploaded version ${bold(response.version_id)} to live.`);
const message = buildMissingAppUrlWarning(
'release',
context.name,
releaseResponse.app_builder_id,
);
warnings.push(message);
log.warn(message);
}
} else if (response.version_id && !shouldPublish) {
log.info(`Uploaded version ${bold(response.version_id)} as a draft (publish skipped).`);
}
Expand Down
Loading