Enhance GitHub Token Configuration Modal #667
Conversation
* Added support for selecting between classic and fine-grained GitHub tokens. * Implemented validation logic for token format and type. * Updated API calls to handle different token types and their required scopes. * Improved UI with toggle buttons for token type selection and dynamic labels. * Enhanced error handling and user feedback for token validation.
WalkthroughThe changes introduce support for both "classic" and "fine-grained" GitHub tokens in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ConfigureGithubModalBody
participant AuthUtils
User->>ConfigureGithubModalBody: Select token type (classic/fine-grained)
User->>ConfigureGithubModalBody: Enter token and domain
ConfigureGithubModalBody->>AuthUtils: getTokenType(token)
ConfigureGithubModalBody->>AuthUtils: checkGitHubValidity(token, domain, tokenType)
alt classic token
ConfigureGithubModalBody->>AuthUtils: getMissingPATScopes(token, domain)
else fine-grained token
ConfigureGithubModalBody->>AuthUtils: getMissingFineGrainedScopes(token, domain)
end
ConfigureGithubModalBody->>User: Show validation result, errors, or proceed with linking
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![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 comments (1)
web-server/src/content/Dashboards/ConfigureGithubModalBody.tsx (1)
97-100:⚠️ Potential issueMissing tokenType parameter in checkGitHubValidity call.
The function call doesn't pass the
tokenTypeparameter, causing it to always use the default 'classic' type. This will fail for fine-grained tokens.Apply this fix:
const isValid = await checkGitHubValidity( token.value, - customDomain.valueRef.current + customDomain.valueRef.current, + tokenType.value );
🧹 Nitpick comments (1)
web-server/src/content/Dashboards/ConfigureGithubModalBody.tsx (1)
287-375: Unused tokenType prop in TokenPermissions component.The component receives
tokenTypebut doesn't use it. Is this intentional or should the component display different permission requirements for each token type?If different permissions should be shown for each token type, consider updating the component to display token-specific information. Otherwise, you can remove the unused prop to keep the code clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web-server/src/content/Dashboards/ConfigureGithubModalBody.tsx(13 hunks)web-server/src/utils/auth.ts(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
web-server/src/utils/auth.ts (2)
web-server/src/constants/urls.ts (1)
DEFAULT_GH_URL(1-1)web-server/http-server.js (1)
axios(5-5)
web-server/src/content/Dashboards/ConfigureGithubModalBody.tsx (1)
web-server/src/utils/auth.ts (4)
getTokenType(135-139)getMissingPATScopes(56-76)getMissingFineGrainedScopes(78-103)linkProvider(13-27)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: All file linting
🔇 Additional comments (6)
web-server/src/utils/auth.ts (3)
31-51: LGTM! Proper token type handling implemented.The function correctly handles both classic and fine-grained tokens with appropriate authorization headers. The default parameter ensures backward compatibility.
54-54: Appropriate scope selection for fine-grained tokens.The defined scopes provide necessary read-only access for repository operations.
135-139: Correct token type detection implementation.The function properly identifies token types based on GitHub's standard prefixes.
web-server/src/content/Dashboards/ConfigureGithubModalBody.tsx (3)
1-35: Good setup for dual token type support.The new state variables and imports are well-structured. Default token type of 'classic' ensures backward compatibility.
54-68: Excellent token validation logic.The implementation provides clear user feedback when token format doesn't match the selected type, improving the user experience.
160-210: Well-designed UI for token type selection.The toggle button group provides clear selection between token types. Dynamic labels and help links enhance the user experience. Resetting the token on type change prevents confusion.
* Improved the logic for checking missing fine-grained token scopes. * Utilized Promise.all for concurrent API calls to enhance performance. * Added error handling for specific permission checks and improved response management. * Cleaned up code formatting for better readability.
* Introduced a new enum for GitHub token types to improve type safety. * Updated the token type state management to utilize the new enum. * Enhanced the UI to reflect the enum values in toggle buttons and labels. * Improved conditional rendering based on the selected token type for better clarity and user experience.
|
The permissions image looks stretched |
* Modified the width and height of the token permissions display based on the selected GitHub token type for improved responsiveness. * Updated fine-grained styles with new dimensions to enhance layout consistency. * Replaced static text with a consistent message for better user guidance.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
web-server/src/content/Dashboards/ConfigureGithubModalBody.tsx (1)
330-331: RGB component out of range (256) – browsers clamp unpredictably
alpha('rgb(256,0,0)', 0.4)uses256, which exceeds the 0-255 range and may be clamped differently across renderers. Use255instead.- border: `2px solid ${alpha('rgb(256,0,0)', 0.4)}`, + border: `2px solid ${alpha('rgb(255,0,0)', 0.4)}`,
🧹 Nitpick comments (3)
web-server/src/constants/style.ts (1)
1-15: Define a sharedStyletype to prevent typo-prone string literalsBoth
ClassicStylesandFineGrainedStylesare arrays of the same shape, yet the object structure is implicit. Declaring a small reusable type (orReadonlyArray) conveys intent, gives autocompletion, and fails compilation on invalid keys.+export interface PermissionBoxStyle { + height: string; // e.g. '42px' + top: string; // e.g. '230px' +} -export const ClassicStyles = [ +export const ClassicStyles: Readonly<PermissionBoxStyle[]> = [ { height: '42px', top: '230px' }, ... ]; -export const FineGrainedStyles = [ +export const FineGrainedStyles: Readonly<PermissionBoxStyle[]> = [ ... ];web-server/src/content/Dashboards/ConfigureGithubModalBody.tsx (2)
65-78: ReturnGithubTokenTypefromgetTokenTypefor stronger type-safety
detectedTypeis compared against the enum but is currently typed as string ('classic' | 'fine-grained' | 'unknown'). Returning the enum itself avoids stringly-typed bugs and removes the need for implicit comparisons.-const detectedType = getTokenType(e); // returns string +const detectedType = getTokenType(e) as GithubTokenType | 'unknown';Even better, change the util signature:
export function getTokenType(token: string): GithubTokenType | 'unknown' { ... }
367-378: Asset / alt-text naming inconsistencyUI copy refers to "Fine Grained Tokens (FGT)" while the alt text uses
FST_permissions.png.
Aligning nomenclature improves accessibility & maintainability.- : '/assets/FST_permissions.png' + : '/assets/FGT_permissions.png' - : 'FST_permissions' + : 'FGT_permissions'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
web-server/public/assets/FST_permissions.pngis excluded by!**/*.png
📒 Files selected for processing (2)
web-server/src/constants/style.ts(1 hunks)web-server/src/content/Dashboards/ConfigureGithubModalBody.tsx(14 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: All file linting
🔇 Additional comments (1)
web-server/src/content/Dashboards/ConfigureGithubModalBody.tsx (1)
306-310: Consider also disabling “Confirm” when domain input is invalid
disabled={!isTokenValid.value}blocks submission on token problems, but still allows a click whenshowDomainErroris populated.
Either include that state in the predicate or rely solely on validation insidehandleSubmission(); mixing both might confuse users.-disabled={!isTokenValid.value} +disabled={!isTokenValid.value || !!showDomainError.value}
* Simplified the focus logic for the custom domain input by removing unnecessary submission call. * Improved code clarity by streamlining the useCallback dependencies.
|
@jayantbh have fixed add new permission image with all required scope Screen.Recording.2025-06-17.at.12.16.12.AM.mov |
|
I don't think it needs the Read and WRITE permissions. Isn't just read enough? |
Linked Issue(s)
#643
Acceptance Criteria fulfillment
Proposed changes (including videos or screenshots)
Screen.Recording.2025-06-02.at.3.41.18.PM.mov
Screen.Recording.2025-06-10.at.12.40.06.AM.mov
Further comments
As per the issue #643, add GitHub fine grained tokens auth
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes