feat: point View source button to specific template folder #2579
Conversation
Console (appwrite/console)Project ID: Tip JWT tokens let functions act on behalf of users while preserving their permissions |
WalkthroughThe change modifies the template source helper to return null for unsupported hosts, extracts a folderPath from template data (either Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![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
🧹 Nitpick comments (2)
src/lib/helpers/templateSource.ts (2)
44-53: Consider URL-encoding the folder path.The
normalizedPathis directly interpolated into the URL without encoding. Folder paths containing special characters (spaces, unicode, etc.) could produce malformed URLs.Apply this approach:
if (folderPath) { const normalizedPath = folderPath.replace(/^\/+|\/+$/g, ''); if (normalizedPath) { + const encodedPath = encodeURIComponent(normalizedPath).replace(/%2F/g, '/'); const providerLower = provider.toLowerCase(); if (providerLower === 'github') { - url = `${url}/tree/main/${normalizedPath}`; + url = `${url}/tree/main/${encodedPath}`; } else if (providerLower === 'gitlab') { - url = `${url}/-/tree/main/${normalizedPath}`; + url = `${url}/-/tree/main/${encodedPath}`; } else if (providerLower === 'bitbucket') { - url = `${url}/src/main/${normalizedPath}`; + url = `${url}/src/main/${encodedPath}`; } } }Note:
encodeURIComponentencodes slashes too, so we need to restore them for path separators.
11-11: Optional: Cacheprovider.toLowerCase()result.The provider string is converted to lowercase three times (lines 22 and 46, with line 11 potentially used later). Consider caching it once:
const owner = t.providerOwner; const repo = t.providerRepositoryId; - const provider = t.vcsProvider; // e.g., "github" + const provider = t.vcsProvider?.toLowerCase(); // e.g., "github" if (!owner || !repo || !provider) return null; // Map provider → host (extend if needed) const hostMap: Record<string, string> = { github: 'github.com', gitlab: 'gitlab.com', bitbucket: 'bitbucket.org' }; - const host = hostMap[provider.toLowerCase()]; + const host = hostMap[provider]; if (!host) return null; // ... later in the code ... - const providerLower = provider.toLowerCase(); - if (providerLower === 'github') { + if (provider === 'github') {Also applies to: 22-22, 46-46
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/helpers/templateSource.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (3)
src/lib/helpers/templateSource.ts (3)
4-4: Documentation example looks good.The updated example clearly demonstrates the new URL format with the tree path.
22-23: LGTM: Explicit null return for unsupported providers.Returning null explicitly when the provider is unsupported is the correct approach, avoiding potentially incorrect URL construction.
25-39: Well-structured folderPath extraction.The type narrowing with
inoperator and explicit type checks is solid. Edge cases like empty frameworks arrays are properly handled with optional chaining.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/lib/helpers/templateSource.ts (1)
51-63: Missing URL encoding will break paths with special characters.The
normalizedPathis inserted directly into the URL without encoding. Paths containing spaces,#,?, or other special characters will break the URL.Apply this diff to encode the path:
if (folderPath) { const normalizedPath = folderPath.replace(/^\/+|\/+$/g, ''); if (normalizedPath) { + const encodedPath = encodeURIComponent(normalizedPath); const providerLower = provider.toLowerCase(); // Use 'master' as branch name - GitHub/GitLab/Bitbucket redirect it to default branch if (providerLower === 'github') { - url = `${url}/tree/master/${normalizedPath}`; + url = `${url}/tree/master/${encodedPath}`; } else if (providerLower === 'gitlab') { - url = `${url}/-/tree/master/${normalizedPath}`; + url = `${url}/-/tree/master/${encodedPath}`; } else if (providerLower === 'bitbucket') { - url = `${url}/src/master/${normalizedPath}`; + url = `${url}/src/master/${encodedPath}`; } } }Note: This also relates to the hardcoded
masterbranch issue flagged in the comment above (lines 7-10).
🧹 Nitpick comments (1)
src/lib/helpers/templateSource.ts (1)
33-47: Consider using optional chaining for cleaner code.The folder path extraction logic is correct but verbose. Optional chaining would improve readability:
let folderPath: string | undefined; -if ( - 'providerRootDirectory' in t && - t.providerRootDirectory && - typeof t.providerRootDirectory === 'string' -) { - folderPath = t.providerRootDirectory; -} else if ( - 'frameworks' in t && - t.frameworks?.length > 0 && - t.frameworks[0]?.providerRootDirectory && - typeof t.frameworks[0].providerRootDirectory === 'string' -) { - folderPath = t.frameworks[0].providerRootDirectory; -} +if ('providerRootDirectory' in t && typeof t.providerRootDirectory === 'string') { + folderPath = t.providerRootDirectory; +} else if ('frameworks' in t && typeof t.frameworks?.[0]?.providerRootDirectory === 'string') { + folderPath = t.frameworks[0].providerRootDirectory; +}The explicit truthiness checks are redundant since you validate
typeof === 'string', which already excludes empty strings from being used (they're still strings).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/helpers/templateSource.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (1)
src/lib/helpers/templateSource.ts (1)
30-31: Good explicit validation.Returning
nullfor unsupported providers is the right approach, preventing undefined behavior.
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.