Skip to content

Commit 677ec78

Browse files
authored
Merge pull request Expensify#83015 from Expensify/neil-fix-failure-notifier-attribution
[No QA] Fix failureNotifier creating issues for non-main branch failures
2 parents f418342 + b6f3079 commit 677ec78

4 files changed

Lines changed: 5 additions & 5 deletions

File tree

.github/actions/javascript/failureNotifier/failureNotifier.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ type PullRequest = RestEndpointMethodTypes['repos']['listPullRequestsAssociatedW
1717
* targeting the correct base branch to avoid blaming the wrong PR.
1818
*/
1919
function getMergedPR(associatedPRs: PullRequest[], targetBranch = 'main'): PullRequest | undefined {
20-
return associatedPRs.find((pr) => pr.merged_at !== null && pr.base.ref === targetBranch) ?? associatedPRs.at(0);
20+
return associatedPRs.find((pr) => pr.merged_at !== null && pr.base.ref === targetBranch);
2121
}
2222

2323
async function run() {

.github/actions/javascript/failureNotifier/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9559,7 +9559,7 @@ const github = __importStar(__nccwpck_require__(5438));
95599559
* targeting the correct base branch to avoid blaming the wrong PR.
95609560
*/
95619561
function getMergedPR(associatedPRs, targetBranch = 'main') {
9562-
return associatedPRs.find((pr) => pr.merged_at !== null && pr.base.ref === targetBranch) ?? associatedPRs.at(0);
9562+
return associatedPRs.find((pr) => pr.merged_at !== null && pr.base.ref === targetBranch);
95639563
}
95649564
async function run() {
95659565
const token = core.getInput('GITHUB_TOKEN', { required: true });

.github/workflows/failureNotifier.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ name: Notify on Workflow Failure
33
on:
44
workflow_run:
55
workflows: ["Process new code merged to main", "Remote Build iOS", "Remote Build Android"]
6+
branches: [main]
67
types:
78
- completed
89

tests/unit/FailureNotifierTest.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,12 @@ describe('getMergedPR', () => {
6565
expect(result?.number).toBe(82016);
6666
});
6767

68-
it('should fall back to first PR if no merged PR is found', () => {
68+
it('should return undefined when no merged PR is found instead of blaming an unmerged PR', () => {
6969
const associatedPRs = [openPRWithMainMerged, openPRDifferentBase];
7070

7171
const result = getMergedPR(associatedPRs);
7272

73-
// Falls back to first element when no merged PR matches
74-
expect(result?.number).toBe(80254);
73+
expect(result).toBeUndefined();
7574
});
7675

7776
it('should return undefined for empty array', () => {

0 commit comments

Comments
 (0)