Fixes PR thank you msg. from potentially tagging the PR author twice #9228
Conversation
WalkthroughThe pull request modifies the workflow file Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.github/workflows/thank-you.yml (1)
40-43: Remove trailing spacesThere are trailing spaces at the end of these lines that should be removed.
Apply this diff to fix the formatting:
- await github.rest.issues.createComment({ - ...options, - body: `${taggedUsers} ${thankyouNote}` - }); + await github.rest.issues.createComment({ + ...options, + body: `${taggedUsers} ${thankyouNote}` + });🧰 Tools
🪛 yamllint (1.35.1)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 42-42: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/thank-you.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/thank-you.yml
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 42-42: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/thank-you.yml (1)
36-43: Verify thank-you message handling across the codebase
Let's ensure there are no other locations handling thank-you messages that might need similar updates.
✅ Verification successful
Changes to thank-you message handling are isolated
The changes to user mention handling in the thank-you workflow are isolated to this single file. The workflow is self-contained and there are no other instances of similar thank-you message handling or user mention patterns that would need to be updated.
- The only implementation of thank-you message handling is in
.github/workflows/thank-you.yml - The user mention logic (
@login) is specific to this workflow and doesn't affect other parts of the codebase - No test files need to be updated as this is a standalone GitHub workflow
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other files that might handle thank-you messages or user mentions
rg -l "thank.*you|createComment|@.*login" --type yaml
Length of output: 55
Script:
#!/bin/bash
# Let's try a broader search across all file types for thank-you message handling
rg -i "thank.*you|createcomment|@.*login" .
# Also search for workflow files specifically
fd -e yml -e yaml . .github/workflows/
Length of output: 985
Script:
#!/bin/bash
# Let's check the content of other workflow files for similar patterns
cat .github/workflows/thank-you.yml
# Also search for any other files that might be creating comments or handling user mentions
rg "github.*createComment|@.*login|taggedUsers" -A 3 -B 3
Length of output: 1486
🧰 Tools
🪛 yamllint (1.35.1)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 42-42: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
.github/workflows/thank-you.yml (3)
21-21: Consider externalizing the thank you messageThe thank you message is currently hardcoded and contains domain-specific content. Consider moving it to a configuration file for better maintainability and reusability.
36-38: Add validation for user login valuesConsider adding validation to ensure user login values are valid before creating mentions.
const taggedUsers = Array.from(uniqueUsers) + .filter(login => login && login.trim()) .map(login => '@' + login) .join(' ');
40-43: Remove trailing spacesThere are trailing spaces in the comment creation block that should be removed.
- await github.rest.issues.createComment({ - ...options, - body: `${taggedUsers} ${thankyouNote}` + await github.rest.issues.createComment({ + ...options, + body: `${taggedUsers} ${thankyouNote}` });🧰 Tools
🪛 yamllint (1.35.1)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 42-42: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/thank-you.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/thank-you.yml
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 42-42: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/thank-you.yml (1)
30-34: LGTM! Effective solution for preventing duplicate mentions
The use of Set for deduplication is an elegant solution that directly addresses the issue of duplicate pings mentioned in #9224.
rithviknishad
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.github/workflows/thank-you.yml (1)
30-32: LGTM! Effective deduplication implementationThe use of Set effectively prevents duplicate mentions, addressing the core issue. However, there are some trailing whitespace issues to clean up.
- const taggedUsers = [...new Set( - assignees.map(u => "@"+u.login).concat("@"+user.login) - )].join(" ") + const taggedUsers = [...new Set( + assignees.map(u => "@"+u.login).concat("@"+user.login) + )].join(" ");🧰 Tools
🪛 yamllint (1.35.1)
[error] 31-31: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/thank-you.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/thank-you.yml
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
.github/workflows/thank-you.yml (3)
21-21: LGTM! Well-formatted thank you message
The message content is appropriate and properly formatted.
30-32: Verify the fix prevents duplicate pings
Let's confirm the deduplication works as expected.
✅ Verification successful
Deduplication implementation verified successfully
The implementation effectively prevents duplicate pings by:
- Using
Setto ensure unique user mentions by combining assignees and PR author - Having a single workflow file (.github/workflows/thank-you.yml) that handles thank you comments
- Implementing a clean approach to user mentions that won't cause duplicate notifications
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the deduplication logic by checking for potential duplicate scenarios
# Test: Check if there are any other thank you workflows that might cause duplicate pings
echo "Checking for other thank you workflows..."
fd -e yml -e yaml . .github/workflows/ --exec grep -l "thank.*you" {}
# Test: Look for other places where we might be creating comments that could cause duplicate pings
echo "Checking for other comment creation patterns..."
rg -l "createComment.*thank.*you" .
Length of output: 341
Script:
#!/bin/bash
# Let's examine the thank-you workflow implementation more closely
# Check the complete thank-you workflow file
echo "Content of thank-you workflow:"
cat .github/workflows/thank-you.yml
# Look for any other potential user mention patterns
echo -e "\nChecking for other user mention patterns:"
rg "@\{.*login" .
Length of output: 1442
🧰 Tools
🪛 yamllint (1.35.1)
[error] 31-31: trailing spaces
(trailing-spaces)
28-28: 🛠️ Refactor suggestion
Add error handling for the API call
The API call could fail silently. Consider adding error handling to provide better debugging information.
- const { data : { assignees, user } } = await github.rest.issues.get(options);
+ let assignees, user;
+ try {
+ const { data: { assignees: a, user: u } } = await github.rest.issues.get(options);
+ assignees = a;
+ user = u;
+ } catch (error) {
+ core.setFailed(`Failed to fetch issue details: ${error.message}`);
+ return;
+ }Likely invalid or redundant comment.
| await github.rest.issues.createComment({ | ||
| ...options, | ||
| body: `${taggedUsers} ${thankyouNote}` | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and fix formatting
The comment creation logic looks good, but needs error handling and formatting fixes.
- await github.rest.issues.createComment({
- ...options,
- body: `${taggedUsers} ${thankyouNote}`
- });
+ try {
+ await github.rest.issues.createComment({
+ ...options,
+ body: `${taggedUsers} ${thankyouNote}`
+ });
+ } catch (error) {
+ core.setFailed(`Failed to create thank you comment: ${error.message}`);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await github.rest.issues.createComment({ | |
| ...options, | |
| body: `${taggedUsers} ${thankyouNote}` | |
| }); | |
| try { | |
| await github.rest.issues.createComment({ | |
| ...options, | |
| body: `${taggedUsers} ${thankyouNote}` | |
| }); | |
| } catch (error) { | |
| core.setFailed(`Failed to create thank you comment: ${error.message}`); | |
| } |
🧰 Tools
🪛 yamllint (1.35.1)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/thank-you.yml (2)
28-32: LGTM! Consider a more concise implementation.The use of Set for deduplication effectively solves the issue of duplicate pings. However, the implementation could be more concise.
Consider this more concise version:
- const taggedUsers = [...new Set( - assignees.map(u => "@"+u.login).concat("@"+user.login) - )].join(" ") + const taggedUsers = [...new Set([ + ...assignees.map(u => `@${u.login}`), + `@${user.login}` + ])].join(' ');Also, please fix the trailing spaces at line 31.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 31-31: trailing spaces
(trailing-spaces)
34-37: LGTM! Fix formatting issues.The comment creation logic is clean and correctly implemented. However, please fix the trailing spaces at lines 34, 35, and 36.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/thank-you.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/thank-you.yml
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/thank-you.yml (2)
21-26: LGTM!
The thank you note and options setup are well-structured and appropriate for the context.
30-32: Verify the deduplication works as expected.
Let's confirm the solution prevents duplicate pings in various scenarios.
✅ Verification successful
Deduplication implementation is correctly implemented
The implementation uses JavaScript's Set data structure which guarantees unique values, and the logic is sound:
assignees.map()creates an array of "@username" strings for all assignees.concat("@"+user.login)adds the PR author's usernamenew Set()removes any duplicates if the PR author was also an assigneejoin(" ")creates a space-separated string of unique @mentions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Test the deduplication logic by checking if there are any remaining instances
# of duplicate user mentions in thank you comments
# Search for thank you comments with duplicate mentions
rg -l '@\w+.*@\w+.*thank.*you' .
Length of output: 34
Script:
#!/bin/bash
# Let's analyze the implementation more thoroughly
# First, check the thank-you workflow implementation
cat .github/workflows/thank-you.yml
# Then search for any test files related to this functionality
fd -t f "thank.*test" -E "node_modules"
Length of output: 1292
🧰 Tools
🪛 yamllint (1.35.1)
[error] 31-31: trailing spaces
(trailing-spaces)
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit