Skip to content

Commit e71bfca

Browse files
joshjohanningjoshjohanning
committed
fix: address PR review findings for branch extraction
- Fix GHAS incomplete string escaping: escape backslashes in branchName - Allow add-work-item-from-branch as standalone (update guard condition) - Cap branch ID extraction at 5 to limit API calls - Fix misleading azure-devops-token/organization descriptions in README - Add test for standalone branch extraction usage
1 parent 02e5f7d commit e71bfca

4 files changed

Lines changed: 52 additions & 8 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ jobs:
8080
| `validate-work-item-exists` | Validate that the work item(s) referenced in commits and PR exist in Azure DevOps (requires `azure-devops-token` and `azure-devops-organization`) | `false` | `true` |
8181
| `add-work-item-table` | Add a "Linked Work Items" table to the PR body showing titles for `AB#xxx` references (original references are preserved). Requires `azure-devops-token` and `azure-devops-organization` | `false` | `false` |
8282
| `add-work-item-from-branch` | Automatically extract work item ID(s) from the head branch name and add `AB#xxx` to the PR body if not already present. Each ID is always validated against Azure DevOps before being added (regardless of the `validate-work-item-exists` setting). Requires `azure-devops-token` and `azure-devops-organization` | `false` | `false` |
83-
| `azure-devops-organization` | Only if `check-commits=true`, link the work items found in commits to the pull request | `false` | `''` |
84-
| `azure-devops-token` | Only required if `link-commits-to-pull-request=true`, Azure DevOps PAT used to link work item to PR (needs to be a `full` PAT) | `false` | `''` |
83+
| `azure-devops-organization` | The name of the Azure DevOps organization. Required when any of these are enabled: `link-commits-to-pull-request`, `validate-work-item-exists`, `add-work-item-table`, or `add-work-item-from-branch` | `false` | `''` |
84+
| `azure-devops-token` | Azure DevOps PAT (needs to be a `full` PAT). Required when any of these are enabled: `link-commits-to-pull-request`, `validate-work-item-exists`, `add-work-item-table`, or `add-work-item-from-branch` | `false` | `''` |
8585
| `github-token` | The GitHub token that has contents-read and pull_request-write access | `true` | `${{ github.token }}` |
8686
| `comment-on-failure` | Comment on the pull request if the action fails | `true` | `true` |
8787

‎__tests__/index.test.js‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ describe('Azure DevOps Commit Validator', () => {
151151
mockContext.payload.pull_request = originalPR;
152152
});
153153

154-
it('should fail if both check-commits and check-pull-request are false', async () => {
154+
it('should fail if both check-commits and check-pull-request are false and add-work-item-from-branch is false', async () => {
155155
mockGetInput.mockImplementation(name => {
156156
if (name === 'check-commits') return 'false';
157157
if (name === 'check-pull-request') return 'false';
@@ -162,7 +162,7 @@ describe('Azure DevOps Commit Validator', () => {
162162
await run();
163163

164164
expect(mockSetFailed).toHaveBeenCalledWith(
165-
"At least one of 'check-commits' or 'check-pull-request' must be set to true. Both are currently set to false."
165+
"At least one of 'check-commits', 'check-pull-request', or 'add-work-item-from-branch' must be set to true."
166166
);
167167
});
168168

@@ -2601,5 +2601,40 @@ describe('Azure DevOps Commit Validator', () => {
26012601
expect(mockOctokit.rest.pulls.update).not.toHaveBeenCalled();
26022602
expect(mockWarning).toHaveBeenCalledWith(expect.stringContaining('99999'));
26032603
});
2604+
2605+
it('should work standalone when check-commits and check-pull-request are both false', async () => {
2606+
mockContext.payload.pull_request = { number: 42, head: { ref: 'task/12345/fix' } };
2607+
2608+
mockGetInput.mockImplementation(name => {
2609+
const inputs = {
2610+
'check-commits': 'false',
2611+
'check-pull-request': 'false',
2612+
'fail-if-missing-workitem-commit-link': 'false',
2613+
'link-commits-to-pull-request': 'false',
2614+
'comment-on-failure': 'false',
2615+
'validate-work-item-exists': 'false',
2616+
'add-work-item-from-branch': 'true',
2617+
'github-token': 'github-token',
2618+
'azure-devops-token': 'fake-token',
2619+
'azure-devops-organization': 'my-org'
2620+
};
2621+
return inputs[name] || '';
2622+
});
2623+
2624+
mockOctokit.rest.pulls.get.mockResolvedValue({
2625+
data: { title: 'My PR', body: 'Description' }
2626+
});
2627+
2628+
mockValidateWorkItemExists.mockResolvedValueOnce(true);
2629+
2630+
await run();
2631+
2632+
expect(mockSetFailed).not.toHaveBeenCalled();
2633+
expect(mockOctokit.rest.pulls.update).toHaveBeenCalledWith(
2634+
expect.objectContaining({
2635+
body: expect.stringContaining('AB#12345')
2636+
})
2637+
);
2638+
});
26042639
});
26052640
});

‎badges/coverage.svg‎

Lines changed: 1 addition & 1 deletion
Loading

‎src/index.js‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,9 @@ export async function run() {
5757
}
5858

5959
// Validate that at least one check is enabled
60-
if (!checkPullRequest && !checkCommits) {
60+
if (!checkPullRequest && !checkCommits && !addWorkItemFromBranch) {
6161
core.setFailed(
62-
`At least one of 'check-commits' or 'check-pull-request' must be set to true. Both are currently set to false.`
62+
`At least one of 'check-commits', 'check-pull-request', or 'add-work-item-from-branch' must be set to true.`
6363
);
6464
return;
6565
}
@@ -756,6 +756,15 @@ async function addWorkItemsToPRBody(octokit, context, pullNumber, azureDevopsOrg
756756
return;
757757
}
758758

759+
// Cap the number of IDs to validate to avoid excessive API calls
760+
const MAX_BRANCH_IDS = 5;
761+
if (workItemIds.length > MAX_BRANCH_IDS) {
762+
core.warning(
763+
`Found ${workItemIds.length} potential work item IDs in branch name, only processing the first ${MAX_BRANCH_IDS}`
764+
);
765+
workItemIds.length = MAX_BRANCH_IDS;
766+
}
767+
759768
core.info(`Found work item ID(s) in branch: ${workItemIds.join(', ')}`);
760769

761770
// Get current PR body
@@ -809,7 +818,7 @@ async function addWorkItemsToPRBody(octokit, context, pullNumber, azureDevopsOrg
809818
body: updatedBody
810819
});
811820
core.info('PR body updated with work item tag(s) from branch name');
812-
const sanitizedBranchName = branchName.replace(/`/g, '\\`');
821+
const sanitizedBranchName = branchName.replace(/\\/g, '\\\\').replace(/`/g, '\\`');
813822
core.summary.addRaw(`- :link: **Added from branch:** ${abTags} extracted from branch \`${sanitizedBranchName}\`\n`);
814823
}
815824

0 commit comments

Comments
 (0)