Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
There was a problem hiding this comment.
Pull request overview
This PR adds automated Jira workflow transitions after a Jira task is created from a GitHub PR, aiming to keep Jira status aligned with PR state (draft/open/approved/merged/merge-queue). It also tweaks the created Jira issue’s summary formatting.
Changes:
- Extend the
handleJiraPR payload shape to include merge/draft/state fields used for status mapping. - Update Jira issue summary format to
${pr.title} [PR #${pr.number}]. - Add Jira transition logic (
/transitionsGET + POST) and a PR-review-based status decision, posting a GitHub comment if the Jira transition fails.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (pr.merged) { | ||
| jiraTargetStatus = 'Done'; | ||
| } else if (pr.mergeable_state && pr.mergeable_state.toLowerCase().includes('queue')) { | ||
| jiraTargetStatus = 'QA Tested'; | ||
| } else if (pr.state === 'open' && pr.draft) { | ||
| jiraTargetStatus = 'In Progress'; | ||
| } else if (pr.state === 'open') { | ||
| // Check for approval status | ||
| const { owner, repo } = context.repo(); |
There was a problem hiding this comment.
The Jira status transition logic relies on pr.merged, pr.mergeable_state, pr.state, and pr.draft, but the current command handler builds pr from the issue payload (see src/index.ts where only number/title/body/url/labels/milestone/user are passed). As a result these fields will be undefined and jiraTargetStatus will stay null, so the Jira transition never runs. Consider fetching the full PR inside handleJira (e.g., octokit.pulls.get) or making these fields required and updating the caller accordingly.
| if (pr.merged) { | |
| jiraTargetStatus = 'Done'; | |
| } else if (pr.mergeable_state && pr.mergeable_state.toLowerCase().includes('queue')) { | |
| jiraTargetStatus = 'QA Tested'; | |
| } else if (pr.state === 'open' && pr.draft) { | |
| jiraTargetStatus = 'In Progress'; | |
| } else if (pr.state === 'open') { | |
| // Check for approval status | |
| const { owner, repo } = context.repo(); | |
| // Fetch full PR details to ensure we have accurate status fields | |
| const { owner, repo } = context.repo(); | |
| const pullRequest = await context.octokit.pulls.get({ | |
| owner, | |
| repo, | |
| pull_number: pr.number, | |
| }); | |
| if (pullRequest.data.merged) { | |
| jiraTargetStatus = 'Done'; | |
| } else if (pullRequest.data.mergeable_state && pullRequest.data.mergeable_state.toLowerCase().includes('queue')) { | |
| jiraTargetStatus = 'QA Tested'; | |
| } else if (pullRequest.data.state === 'open' && pullRequest.data.draft) { | |
| jiraTargetStatus = 'In Progress'; | |
| } else if (pullRequest.data.state === 'open') { | |
| // Check for approval status |
| const { owner, repo } = context.repo(); | ||
| const reviews = await context.octokit.pulls.listReviews({ | ||
| owner, | ||
| repo, | ||
| pull_number: pr.number, | ||
| }); | ||
| const approved = reviews.data.some((review: { state: string }) => review.state === 'APPROVED'); | ||
| if (approved) { | ||
| jiraTargetStatus = 'QA Tested'; | ||
| } else { | ||
| jiraTargetStatus = 'Waiting Review'; | ||
| } |
There was a problem hiding this comment.
reviews.data.some(review.state === 'APPROVED') can report approved even when a later review requested changes (or the approval was dismissed), because the API returns a history of reviews. This can transition Jira to "QA Tested" incorrectly. Prefer using GraphQL reviewDecision (APPROVED/CHANGES_REQUESTED/REVIEW_REQUIRED) or computing the latest review state per reviewer before deciding.
| const reviews = await context.octokit.pulls.listReviews({ | ||
| owner, | ||
| repo, | ||
| pull_number: pr.number, | ||
| }); | ||
| const approved = reviews.data.some((review: { state: string }) => review.state === 'APPROVED'); |
There was a problem hiding this comment.
pulls.listReviews is paginated (defaults to 30). On PRs with many reviews, an older approval could be outside the first page and the status decision would be wrong. Use octokit.paginate (or request per_page: 100 and paginate) to ensure you consider all reviews, or avoid pagination entirely by using GraphQL reviewDecision.
| const reviews = await context.octokit.pulls.listReviews({ | |
| owner, | |
| repo, | |
| pull_number: pr.number, | |
| }); | |
| const approved = reviews.data.some((review: { state: string }) => review.state === 'APPROVED'); | |
| const reviews = await context.octokit.paginate( | |
| context.octokit.pulls.listReviews, | |
| { | |
| owner, | |
| repo, | |
| pull_number: pr.number, | |
| per_page: 100, | |
| }, | |
| ); | |
| const approved = reviews.some((review: { state: string }) => review.state === 'APPROVED'); |
| const transitionsRes = await jiraFetch(jiraBaseUrl, jiraApiToken, `/rest/api/3/issue/${issueKey}/transitions`); | ||
| if (!transitionsRes.ok) throw new Error('Failed to fetch Jira transitions'); | ||
| const transitionsData = (await transitionsRes.json()) as { transitions: { id: string; to: { name: string } }[] }; | ||
| const transitions = transitionsData.transitions; | ||
| const transition = transitions.find((t) => t.to.name.toLowerCase() === targetStatus.toLowerCase()); | ||
| if (!transition) throw new Error(`No Jira transition found for status: ${targetStatus}`); | ||
| const transitionRes = await jiraFetch(jiraBaseUrl, jiraApiToken, `/rest/api/3/issue/${issueKey}/transitions`, { |
There was a problem hiding this comment.
The Jira transitions endpoints interpolate issueKey directly into the URL. To avoid path issues (and to be consistent with the existing projectHasVersion call), issueKey should be wrapped with encodeURIComponent when building /rest/api/3/issue/${...}/transitions.
| const transitionsRes = await jiraFetch(jiraBaseUrl, jiraApiToken, `/rest/api/3/issue/${issueKey}/transitions`); | |
| if (!transitionsRes.ok) throw new Error('Failed to fetch Jira transitions'); | |
| const transitionsData = (await transitionsRes.json()) as { transitions: { id: string; to: { name: string } }[] }; | |
| const transitions = transitionsData.transitions; | |
| const transition = transitions.find((t) => t.to.name.toLowerCase() === targetStatus.toLowerCase()); | |
| if (!transition) throw new Error(`No Jira transition found for status: ${targetStatus}`); | |
| const transitionRes = await jiraFetch(jiraBaseUrl, jiraApiToken, `/rest/api/3/issue/${issueKey}/transitions`, { | |
| const transitionsRes = await jiraFetch(jiraBaseUrl, jiraApiToken, `/rest/api/3/issue/${encodeURIComponent(issueKey)}/transitions`); | |
| if (!transitionsRes.ok) throw new Error('Failed to fetch Jira transitions'); | |
| const transitionsData = (await transitionsRes.json()) as { transitions: { id: string; to: { name: string } }[] }; | |
| const transitions = transitionsData.transitions; | |
| const transition = transitions.find((t) => t.to.name.toLowerCase() === targetStatus.toLowerCase()); | |
| if (!transition) throw new Error(`No Jira transition found for status: ${targetStatus}`); | |
| const transitionRes = await jiraFetch(jiraBaseUrl, jiraApiToken, `/rest/api/3/issue/${encodeURIComponent(issueKey)}/transitions`, { |
| if (!transitionsRes.ok) throw new Error('Failed to fetch Jira transitions'); | ||
| const transitionsData = (await transitionsRes.json()) as { transitions: { id: string; to: { name: string } }[] }; | ||
| const transitions = transitionsData.transitions; | ||
| const transition = transitions.find((t) => t.to.name.toLowerCase() === targetStatus.toLowerCase()); | ||
| if (!transition) throw new Error(`No Jira transition found for status: ${targetStatus}`); | ||
| const transitionRes = await jiraFetch(jiraBaseUrl, jiraApiToken, `/rest/api/3/issue/${issueKey}/transitions`, { | ||
| method: 'POST', | ||
| body: JSON.stringify({ transition: { id: transition.id } }), | ||
| }); | ||
| if (!transitionRes.ok) throw new Error(`Failed to transition Jira issue to ${targetStatus}`); | ||
| } |
There was a problem hiding this comment.
The errors thrown from updateJiraStatus drop useful debugging info (HTTP status and response body). Including status and await res.text() in these error messages will make failures actionable, especially since this runs in automation and only posts the error message back to GitHub.
I have no idea if this works :kek: