Update code to support @octokit/request v9 and @octokit/graphql v8 #3704
Conversation
Fixes asyncapi#3690 Update code to support `@octokit/request` v9.2.2 and `@octokit/graphql` v8.2.1. * **netlify/functions/github_discussions.ts** - Remove unused import of `GraphQlQueryResponseData`. - Update `graphql` function call to use the new syntax. - Update authorization header to use the new token format. * **netlify/functions/save-discussion-background/helpers.ts** - Update authorization header to use the new token format.
Recxsmacx
requested review from
derberg,
akshatnema,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
WalkthroughThis pull request updates the way GraphQL requests are constructed and authenticated. In the Changes
Sequence Diagram(s)sequenceDiagram
participant Handler as Netlify Function Handler
participant GitHubAPI as GitHub GraphQL API
Handler->>GitHubAPI: Send GraphQL query (object format, Bearer token)
GitHubAPI-->>Handler: Return response
Poem
✨ Finishing Touches
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
Documentation and Community
|
|
We require all PRs to follow Conventional Commits specification. |
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Hello, @coderabbitai[bot]! 👋🏼 |
|
Tip For best results, initiate chat on the files or code changes. @asyncapi-bot Thank you for the information about the supported PR commands! I'll keep these in mind for future interactions. (｀･ω･´)ゞ ✏️ Learnings added
|
|
@derberg @devilkiller-ag @akshatnema @sambhavgupta0705 @anshgoyalevil @Mayaleeeee Please take a look at this PR. Thanks! 👋 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3704 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 20
Lines 732 732
=========================================
Hits 732 732 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
netlify/functions/github_discussions.ts (1)
27-40: Consider adding input validation for GraphQL mutation parameters.While the new object syntax for the GraphQL query is cleaner, the template literals could be vulnerable to injection attacks. Consider validating the
titleandfeedbackparameters before interpolating them into the query.Here's a suggested approach to add input validation:
// Add input validation helper function validateDiscussionInput(title: string, feedback: string) { if (!title?.trim()) { throw new Error('Title cannot be empty'); } if (!feedback?.trim()) { throw new Error('Feedback cannot be empty'); } // Add more validation rules as needed } // Use in the handler const { title, feedback } = JSON.parse(event.body || ''); validateDiscussionInput(title, feedback);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
netlify/functions/github_discussions.ts(1 hunks)netlify/functions/save-discussion-background/helpers.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 180000ms (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Lighthouse CI
🔇 Additional comments (2)
netlify/functions/save-discussion-background/helpers.ts (1)
21-21:❓ Verification inconclusive
LGTM! Authorization header updated to use Bearer token.
The change from
tokentoBeareraligns with OAuth 2.0 standards and the latest Octokit library requirements.Let's verify that all GitHub API calls consistently use the Bearer token format:
🏁 Script executed:
#!/bin/bash # Description: Check for any remaining instances of the old token format # and ensure consistent use of Bearer token format across the codebase # Search for old token format echo "Checking for old token format..." rg -i "authorization.*token.*\$\{.*TOKEN.*\}" # Search for new Bearer format to ensure consistency echo "Checking Bearer token format..." rg -i "authorization.*Bearer.*\$\{.*TOKEN.*\}"Length of output: 242
LGTM! The authorization header is now correctly using the Bearer token format in
netlify/functions/save-discussion-background/helpers.ts, which meets the Octokit and OAuth 2.0 requirements.However, the automated checks using the regex script produced no conclusive output for either the old token format or the new Bearer format across the codebase. I'd recommend manually verifying (or running a more targeted search) to ensure that no legacy token formatting is lurking elsewhere in the project. This extra step will help avoid inconsistencies in GitHub API authentication.
netlify/functions/github_discussions.ts (1)
37-37: LGTM! Authorization header updated to use Bearer token.The change from
tokentoBearermaintains consistency with the helper function and aligns with OAuth 2.0 standards.
Fixes #3690
Update code to support
@octokit/requestv9.2.2 and@octokit/graphqlv8.2.1.netlify/functions/github_discussions.ts
GraphQlQueryResponseData.graphqlfunction call to use the new syntax.netlify/functions/save-discussion-background/helpers.ts
Summary by CodeRabbit