fix: correct item.State to item.state for PR action label detection#536
Open
PhilixTheExplorer wants to merge 1 commit intofossasia:mainfrom
Open
fix: correct item.State to item.state for PR action label detection#536PhilixTheExplorer wants to merge 1 commit intofossasia:mainfrom
PhilixTheExplorer wants to merge 1 commit intofossasia:mainfrom
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts PR/MR action label logic in scrumHelper.js so that 'Made PR' / 'Made Merge Request' are correctly detected based on the actual API response fields and values from GitHub and GitLab. Flow diagram for PR/MR action label determination logicflowchart TD
A[Start label determination] --> B{Platform}
B -->|github| C[Set prAction to
isNewPR ? Made PR : Updated PR]
B -->|gitlab| D[Set prAction to
isNew Merge Request : Updated Merge Request]
C --> E{isCreatedToday
and item.state == open?}
E -->|yes| F[Set prAction to
Made PR]
E -->|no| G[Set prAction to
Updated PR]
D --> H{isCreatedToday
and item.state == opened?}
H -->|yes| I[Set prAction to
Made Merge Request]
H -->|no| J[Set prAction to
Updated Merge Request]
F --> K[Return prAction]
G --> K
I --> K
J --> K
K[End]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
bot
added
javascript
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
isCreatedToday+ state check is duplicated across the GitHub and GitLab branches; consider extracting a small helper (e.g.isNewlyOpened(item, platform)) or normalizing state earlier so the PR/MR action label logic is shared. - To avoid future state-name mismatches (
openvsopened), it may be safer to normalize the raw API state into a common internal enum/string in the existing mapping functions and compare only against that here.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `isCreatedToday` + state check is duplicated across the GitHub and GitLab branches; consider extracting a small helper (e.g. `isNewlyOpened(item, platform)`) or normalizing state earlier so the PR/MR action label logic is shared.
- To avoid future state-name mismatches (`open` vs `opened`), it may be safer to normalize the raw API state into a common internal enum/string in the existing mapping functions and compare only against that here.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes PR/MR action label detection in the scrum report by correcting state property casing and aligning GitLab’s open state value with the API response, so items created today can be labeled as “Made PR” / “Made Merge Request” correctly.
Changes:
- Corrected
item.State→item.statefor GitHub PR action detection. - Updated GitLab MR open-state comparison from
'open'→'opened'.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
1765
to
1769
| prAction = isNewPR ? 'Made PR' : 'Updated PR'; | ||
| log(`[PR DEBUG] Including PR #${number} as ${prAction}`); | ||
|
|
||
| if (isCreatedToday && item.State === 'open') { | ||
| if (isCreatedToday && item.state === 'open') { | ||
| prAction = 'Made PR'; |
There was a problem hiding this comment.
The debug log prints prAction before it gets recalculated by the isCreatedToday branch, so the log message can disagree with the final label rendered (e.g., isNewPR true but isCreatedToday false). Consider computing prAction once (or moving the log after the final assignment) to keep logs and output consistent.
Comment on lines
1773
to
1776
| } else if (platform === 'gitlab') { | ||
| prAction = isNewPR ? 'Made Merge Request' : 'Updated Merge Request'; | ||
| if (isCreatedToday && item.State === 'open') { | ||
| if (isCreatedToday && item.state === 'opened') { | ||
| prAction = 'Made Merge Request'; |
There was a problem hiding this comment.
GitLab MR state handling is now split between 'open' and 'opened' checks in this function, while mapGitLabItem normalizes 'opened' -> 'open' only for issues. To reduce scattered platform-specific string checks, consider normalizing MR state in mapGitLabItem as well so the rest of the code can consistently compare against 'open'.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Fixes
Fixes #535
📝 Summary of Changes
item.State(capital S) →item.state(lowercase s) on line 1768 (GitHub) and line 1775 (GitLab) inscrumHelper.js'open'→'opened'on line 1775, since the GitLab Merge Requests API returnsopened,closed,locked, ormerged— notopenopenorclosed, which was already correct for the GitHub path but never matched due to the capital S typoRoot cause: The condition
item.State === 'open'never evaluated totruebecause:state, notStateopenednotopenfor merge requestsThis caused PRs/MRs created today to always show "Updated PR" / "Updated Merge Request" instead of "Made PR" / "Made Merge Request".
📸 Screenshots / Demo (if UI-related)
Before
After
✅ Checklist
👀 Reviewer Notes
The
mapGitLabItem()function (lines 272, 342) already normalizes GitLab'sopened→openfor issues, but passes through the raw state for merge requests. That's why the GitLab path on line 1775 must compare against'opened'rather than'open'.Summary by Sourcery
Correct PR/MR action labeling based on creation date and platform-specific state values.
Bug Fixes: