Generate link to the package manager in SBOM page using PURL #10625
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughA new function Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SbomPackage
participant getPackageUrl
participant AnchorElement
Client->>SbomPackage: Provide package data
SbomPackage->>getPackageUrl: getPackageUrl(pkgName, purl)
getPackageUrl-->>SbomPackage: Return generated URL
SbomPackage->>AnchorElement: Render anchor with URL
AnchorElement-->>Client: Display anchor link
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
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: 0
🧹 Nitpick comments (2)
src/pages/Licenses/Licenses.tsx (2)
22-38: Improve thegetPackageUrlfunction implementation.The function can be enhanced for better maintainability and type safety:
Consider this alternative implementation:
-function getPackageUrl(pkgName: string, purl: string) { +type PackageType = 'pypi' | 'npm' | 'github' | 'githubactions'; + +function getPackageUrl(pkgName: string, purl: string | undefined): string { + if (!purl || !pkgName) return ''; + + const urlMap: Record<PackageType, string> = { + pypi: `https://pypi.org/project/${pkgName}`, + npm: `https://www.npmjs.com/package/${pkgName}`, + github: `https://github.com/${pkgName}`, + githubactions: `https://github.com/actions/${pkgName}`, + }; + + const type = Object.keys(urlMap).find(key => purl.startsWith(`pkg:${key}/`)); + return type ? urlMap[type as PackageType] : purl; +} - if (purl) { - switch (true) { - case purl.startsWith("pkg:pypi/"): - return `https://pypi.org/project/${pkgName}`; - case purl.startsWith("pkg:npm/"): - return `https://www.npmjs.com/package/${pkgName}`; - case purl.startsWith("pkg:github/"): - return `https://github.com/${pkgName}`; - case purl.startsWith("pkg:githubactions/"): - return `https://github.com/actions/${pkgName}`; - default: - return purl; - } - } - return purl; -}Benefits:
- Type safety with TypeScript types
- More maintainable with a lookup object
- Input validation for both parameters
- Eliminates the switch-true anti-pattern
- Removes redundant return statement
128-128: Simplify the href attribute and improve error handling.The current implementation can be enhanced:
-href={`${getPackageUrl(pkg.name, pkg.externalRefs?.[0].referenceLocator)}`} +href={getPackageUrl(pkg.name, pkg.externalRefs?.[0]?.referenceLocator)}Consider adding error handling for missing package data:
if (!pkg.name || !pkg.versionInfo) { return null; // or a fallback UI }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/Licenses/Licenses.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
rajku-dev
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/pages/Licenses/Licenses.tsx (1)
22-37: Enhance type safety and URL handling in getPackageUrl.The function works but could be improved in several ways:
- Add explicit parameter types
- Handle version information from purl
- Use constants for URL prefixes
- Add validation for external reference type
Consider this improved implementation:
-function getPackageUrl(pkgName: string, purl: string): string { +const PACKAGE_URL_PREFIXES = { + pypi: "https://pypi.org/project/", + npm: "https://www.npmjs.com/package/", + github: "https://github.com/", + githubactions: "https://github.com/actions/", +} as const; + +function getPackageUrl(pkgName: string, purl: string | undefined): string { if (!purl || !pkgName) return ""; - const urlMap: Record<PackageType, string> = { - pypi: `https://pypi.org/project/${pkgName}`, - npm: `https://www.npmjs.com/package/${pkgName}`, - github: `https://github.com/${pkgName}`, - githubactions: `https://github.com/actions/${pkgName}`, - }; + const urlMap: Record<PackageType, string> = Object.fromEntries( + Object.entries(PACKAGE_URL_PREFIXES).map(([key, prefix]) => [ + key, + `${prefix}${pkgName}`, + ]) + ) as Record<PackageType, string>; const pkgtype = Object.keys(urlMap).find((key) => purl.startsWith(`pkg:${key}/`), ); + // Extract version from purl if available + const version = purl.match(/[@]([^?]+)/)?.[1]; + const baseUrl = pkgtype ? urlMap[pkgtype as PackageType] : purl; + + return version ? `${baseUrl}/v${version}` : baseUrl; - return pkgtype ? urlMap[pkgtype as PackageType] : purl; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/Licenses/Licenses.tsx(2 hunks)src/types/license.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
🔇 Additional comments (2)
src/types/license.ts (1)
35-35: LGTM! Well-defined type for package managers.The
PackageTypetype is well-defined and includes all necessary package types for the SBOM page.src/pages/Licenses/Licenses.tsx (1)
15-15: LGTM! Clean import statement.The
PackageTypeimport is correctly added and used in the code.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/pages/Licenses/Licenses.tsx (1)
127-127:⚠️ Potential issueAdd validation for external references.
The current implementation assumes the first external reference is the correct package URL, which might not always be true. Consider adding validation for the reference type.
Apply this improvement:
-href={`${getPackageUrl(pkg.name, pkg.externalRefs[0].referenceLocator)}`} +href={`${getPackageUrl( + pkg.name, + pkg.externalRefs?.find(ref => + ref.referenceType === "purl" || + ref.referenceType === "package-url" + )?.referenceLocator +)}`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/Licenses/Licenses.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (1)
src/pages/Licenses/Licenses.tsx (1)
15-15: LGTM!The import of PackageType is correctly added and aligns with the new functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/pages/Licenses/utils.ts (1)
3-21: 🛠️ Refactor suggestionAdd input validation and JSDoc documentation.
The function needs better error handling and documentation.
Apply these improvements as suggested in the past review:
+/** + * Generates a package manager URL from a package name and PURL. + * @param pkgName - The name of the package + * @param purl - The Package URL (PURL) string + * @returns The package manager URL or empty string if inputs are invalid + */ function getPackageUrl(pkgName: string, purl: string): string { if (!purl || !pkgName) return ""; + // Validate PURL format + const purlRegex = /^pkg:[a-zA-Z]+\/.+/; + if (!purlRegex.test(purl)) return ""; + + // Extract version from PURL if present + const versionMatch = purl.match(/@([^/]+)/); + const version = versionMatch?.[1]; + const urlMap: Record<PackageType, string> = { - pypi: `https://pypi.org/project/${pkgName}`, - npm: `https://www.npmjs.com/package/${pkgName}`, + pypi: `https://pypi.org/project/${pkgName}${version ? `/${version}` : ''}`, + npm: `https://www.npmjs.com/package/${pkgName}${version ? `/v/${version}` : ''}`, github: `https://github.com/${pkgName}`, githubactions: `https://github.com/actions/${pkgName}`, };src/pages/Licenses/Licenses.tsx (1)
111-111: 🛠️ Refactor suggestionAdd validation for external references.
The current implementation assumes the first external reference is the correct package URL, which might not always be true.
Apply this improvement as suggested in the past review:
-href={`${getPackageUrl(pkg.name, pkg.externalRefs?.[0].referenceLocator)}`} +href={`${getPackageUrl( + pkg.name, + pkg.externalRefs?.find(ref => + ref.referenceType === "purl" || + ref.referenceType === "package-url" + )?.referenceLocator +)}`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/Licenses/Licenses.tsx(2 hunks)src/pages/Licenses/utils.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/pages/Licenses/utils.ts (2)
7-12: Consider moving URLs to configuration.The hardcoded URLs would be better maintained in a configuration file, allowing for easier updates and environment-specific changes.
+import { config } from '@/config'; + export const getPackageUrl = ( pkgName: LicensesSbom["sbom"]["packages"][number]["name"], purl: LicensesSbom["sbom"]["packages"][number]["externalRefs"][number]["referenceLocator"], ): URL => { - const urlMap: Record<PackageType, string> = { - pypi: `https://pypi.org/project/${pkgName}`, - npm: `https://www.npmjs.com/package/${pkgName}`, - github: `https://github.com/${pkgName}`, - githubactions: `https://github.com/actions/${pkgName}`, - }; + const urlMap: Record<PackageType, string> = config.packageUrls;
3-19: Add tests and documentation for the utility function.To ensure reliability and maintainability:
- Add unit tests to verify URL generation for different package types
- Add JSDoc documentation explaining the function's purpose, parameters, and return value
Would you like me to help generate:
- Unit tests for various package types and edge cases?
- JSDoc documentation for the utility function?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/Licenses/utils.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (1)
src/pages/Licenses/utils.ts (1)
1-1: LGTM!Types are properly imported using the project's path alias convention.
|
can we show the count of packages, as done for Users/Patients etc |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/pages/Licenses/Licenses.tsx (1)
111-111:⚠️ Potential issueAdd validation for external references and error handling.
The current implementation assumes the first external reference exists and is the correct package URL, which might not always be true.
- href={`${getPackageUrl(pkg.name, pkg.versionInfo, pkg.externalRefs[0].referenceLocator)}`} + href={pkg.externalRefs?.length > 0 + ? `${getPackageUrl( + pkg.name, + pkg.versionInfo, + pkg.externalRefs.find(ref => + ref.referenceType === "purl" || + ref.referenceType === "package-url" + )?.referenceLocator ?? '' + )}` + : "#" + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/Licenses/Licenses.tsx(2 hunks)src/pages/Licenses/utils.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (3)
src/pages/Licenses/utils.ts (2)
1-7: Add input validation for parameters.The function should validate input parameters to handle null/undefined values gracefully.
export const getPackageUrl = ( pkgName: LicensesSbom["sbom"]["packages"][number]["name"], version: LicensesSbom["sbom"]["packages"][number]["versionInfo"], purl: LicensesSbom["sbom"]["packages"][number]["externalRefs"][number]["referenceLocator"], ): URL => { + if (!pkgName || !version || !purl) { + throw new Error('Package name, version, and PURL are required'); + }
15-19: Add PURL validation and error handling.The URL construction should validate PURL format and handle invalid URLs gracefully.
+ const isPurlValid = (purl: string): boolean => { + return /^pkg:[a-zA-Z]+\/.+/.test(purl); + }; + + if (!isPurlValid(purl)) { + throw new Error('Invalid PURL format'); + } + const pkgType = Object.keys(urlMap).find((key) => purl.startsWith(`pkg:${key}/`), ); - return pkgType ? new URL(urlMap[pkgType as PackageType]) : new URL(purl); + try { + return pkgType ? new URL(urlMap[pkgType as PackageType]) : new URL(purl); + } catch (error) { + throw new Error(`Invalid URL: ${error.message}`); + }src/pages/Licenses/Licenses.tsx (1)
15-15: LGTM!The import statement is correctly placed and properly imports the utility function.
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Refactor
New Types