Skip to content

Commit 35c8631

Browse files
committed
fix(nx-fly-deployment-action): deprecated apps filter returned too much
closed COD-228
1 parent adc348d commit 35c8631

File tree

2 files changed

+71
-18
lines changed

2 files changed

+71
-18
lines changed

packages/nx-fly-deployment-action/src/lib/fly-deployment.spec.ts

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as coreAction from '@codeware/core/actions';
66
import { type DeployAppOptions, Fly } from '@codeware/fly-node';
77
import * as devkit from '@nx/devkit';
88
import { ProjectConfiguration } from '@nx/devkit';
9-
import { PullRequestEvent } from '@octokit/webhooks-types';
9+
import type { PullRequestEvent } from '@octokit/webhooks-types';
1010
import { vol } from 'memfs';
1111

1212
import { flyDeployment } from './fly-deployment';
@@ -63,6 +63,7 @@ describe('flyDeployment', () => {
6363
const mockCoreInfo = vi.mocked(core.info);
6464
const mockCoreStartGroup = vi.mocked(core.startGroup);
6565
const mockCoreWarning = vi.mocked(core.warning);
66+
const mockCoreActionGetPullRequest = vi.mocked(coreAction.getPullRequest);
6667
const mockExecGetExecOutput = vi.mocked(exec.getExecOutput);
6768
const mockGithubContext = vi.mocked(github.context);
6869
const mockFly = vi.mocked(Fly);
@@ -646,7 +647,7 @@ describe('flyDeployment', () => {
646647
);
647648
});
648649

649-
it('should destroy preview apps', async () => {
650+
it('should destroy preview apps when pull request is closed', async () => {
650651
setContext('pr-closed');
651652
setupMocks({
652653
flyApps: [
@@ -655,6 +656,9 @@ describe('flyDeployment', () => {
655656
] as Awaited<ReturnType<typeof Fly.prototype.apps.list>>
656657
});
657658
const config = setupTest();
659+
mockCoreActionGetPullRequest.mockResolvedValue({
660+
state: 'closed'
661+
} as any);
658662
const result = await flyDeployment(config, true);
659663

660664
expect(getMockFly().apps.destroy).toHaveBeenCalledWith('app-one-pr-1');
@@ -674,6 +678,54 @@ describe('flyDeployment', () => {
674678
]
675679
} satisfies ActionOutputs);
676680
});
681+
682+
it('should destroy preview apps with pull request number', async () => {
683+
setContext('pr-closed');
684+
setupMocks({
685+
flyApps: [{ name: 'app-one-1' }, { name: 'app-two-pr-1' }] as Awaited<
686+
ReturnType<typeof Fly.prototype.apps.list>
687+
>
688+
});
689+
const config = setupTest();
690+
mockCoreActionGetPullRequest.mockResolvedValue({
691+
state: 'closed'
692+
} as any);
693+
const result = await flyDeployment(config, true);
694+
695+
expect(getMockFly().apps.destroy).toHaveBeenCalledWith('app-two-pr-1');
696+
697+
expect(result).toEqual({
698+
environment: 'preview',
699+
projects: [
700+
{
701+
action: 'destroy',
702+
app: 'app-two-pr-1'
703+
}
704+
]
705+
} satisfies ActionOutputs);
706+
});
707+
708+
it('should not destroy preview apps when pull request is open', async () => {
709+
setContext('pr-closed');
710+
setupMocks({
711+
flyApps: [
712+
{ name: 'app-one-pr-1' },
713+
{ name: 'app-two-pr-1' }
714+
] as Awaited<ReturnType<typeof Fly.prototype.apps.list>>
715+
});
716+
const config = setupTest();
717+
mockCoreActionGetPullRequest.mockResolvedValue({
718+
state: 'open'
719+
} as any);
720+
const result = await flyDeployment(config, true);
721+
722+
expect(getMockFly().apps.destroy).not.toHaveBeenCalled();
723+
724+
expect(result).toEqual({
725+
environment: 'preview',
726+
projects: []
727+
} satisfies ActionOutputs);
728+
});
677729
});
678730

679731
describe('deploy to production', () => {

packages/nx-fly-deployment-action/src/lib/utils/run-destroy-apps.ts

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,24 +25,25 @@ export const runDestroyApps = async (
2525
const apps = await fly.apps.list();
2626

2727
// Filter deprecated apps to destroy
28-
const appsToDestroy = await Promise.all(
29-
apps.filter(async (app) => {
30-
// Extract pull request number from app name
31-
const prNumber = app.name.match(/-pr-(\d+)/)?.[1];
32-
if (!prNumber) {
33-
return false;
34-
}
28+
const appPromises = apps.map(async (app) => {
29+
// Extract pull request number from app name
30+
const prNumber = app.name.match(/-pr-(\d+)/)?.[1];
31+
if (!prNumber) {
32+
return null;
33+
}
34+
35+
const pullRequest = await getPullRequest(config.token, Number(prNumber));
36+
if (!pullRequest) {
37+
core.warning(`Pull request #${prNumber} not found, skip destroy`);
38+
return null;
39+
}
3540

36-
const pullRequest = await getPullRequest(config.token, Number(prNumber));
37-
if (!pullRequest) {
38-
core.warning(`Pull request #${prNumber} not found, skip destroy`);
39-
return false;
40-
}
41+
// App should be destroyed when PR is closed
42+
return pullRequest.state === 'closed' ? app : null;
43+
});
4144

42-
// App should be destroyed when PR is closed
43-
return pullRequest.state === 'closed';
44-
})
45-
);
45+
const result = await Promise.all(appPromises);
46+
const appsToDestroy = result.filter((app) => app !== null);
4647

4748
core.info(`Found ${appsToDestroy.length} apps to destroy`);
4849

0 commit comments

Comments
 (0)