Update license Component for Github SBOM #9755
Conversation
WalkthroughThe pull request introduces a comprehensive overhaul of the Software Bill of Materials (SBOM) handling in the application. It replaces the manual SBOM generation with a new script that fetches SBOM data directly from GitHub repositories. The changes include removing the existing Changes
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (4)
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: 0
🧹 Nitpick comments (7)
src/components/Licenses/SBOMViewer.tsx (7)
52-66: Consider more robust error handling.
Currently, any fetch failure logs an error to the console and returnsnull. Think about handling errors more gracefully in the UI, for instance by showing a user-facing alert or fallback message.
86-86: Tab naming could be more descriptive.
Using "bom" vs "beBom" might be confusing. Renaming them to “frontendBom” / “backendBom” can improve clarity.
118-118: Consider displaying additional SBOM metadata.
AlongsidespdxVersion, you might also display thedataLicenseor other relevant metadata for a more comprehensive SBOM overview.
131-131: Use stable keys when rendering lists.
Consider using a unique identifier from each package (e.g.,pkg.SPDXIDorpkg.name) instead of the arrayindexfor improved React performance.
137-137: Commented-out externalRefs link.
This might be leftover debug code or a temporary workaround. Consider removing or adding a rationale comment if intentionally disabled.
150-150: Fallback anchor for unrecognized license IDs.
Navigating to "#" might not provide user feedback. Consider a placeholder page or tooltip to clarify no recognized license URL is available.
172-172: Consider stable keys for external references.
Usingidxas the key can lead to React diffing issues if references change order. Prefer a unique property from each reference, if available.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Licenses/SBOMViewer.tsx(3 hunks)
🔇 Additional comments (14)
src/components/Licenses/SBOMViewer.tsx (14)
2-2: Imports look good.
The newly added import for React hooks is appropriate and aligns with best practices in functional components.
15-27: Validate optional fields for GitHubPackage.
All fields in this interface appear to match the GitHub SBOM format. Consider verifying whether any additional fields from GitHub’s SBOM API may need to be included or marked optional for better type safety.
30-41: Interface structure aligns with GitHub SBOM data.
It’s good that each property is optional, preventing runtime errors when the SBOM might be missing certain fields.
49-51: Separate state variables for frontend and backend SBOM.
Assigning each SBOM to its own state variable is clear and logical.
68-85: Concise async loading for both FE/BE.
Fetching FE and BE SBOM data in parallel is efficient. If partial fetch success is acceptable (only one resource might load), confirm thatnullstates are handled correctly in the UI.
93-93: Array fallback for safe rendering.
The usage of|| []ensures no errors whenbomDataorsbomisnull. Nice defensive coding.
122-123: Date formatting is handled gracefully.
Falling back to "N/A" if a date is missing ensures the UI won’t break.
129-129: Package header is clear.
Labeling this section is helpful in guiding the user to the package list.
143-143: Package name and version.
Displaying both name and version in the same heading is user-friendly and concise.
146-146: Conditional rendering of the license.
Filtering out packages without alicenseConcludedfield reduces UI clutter. Implementation is clear.
155-155: License fallback "N/A."
Good to maintain a sensible default text if the license field is somehow empty.
175-175: External reference link fallback.
IfreferenceLocatoris missing, a fallback of "#" prevents errors. This maintains a functioning UI.
178-178: Optional reference locator text.
Gracefully handles missing locator content by providing "N/A."
180-182: Category display is optional.
The checks forref.referenceCategoryandref.referenceLocatoralign with the optional nature of external references.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
scripts/fetchSbomData.js (1)
49-49: Add proper error handling for unhandled rejections.The script should handle unhandled promise rejections properly.
Add this error handling:
+process.on('unhandledRejection', (error) => { + console.error('Unhandled promise rejection:', error); + process.exit(1); +}); + fetchData();src/components/Licenses/SBOMViewer.tsx (1)
Line range hint
134-157: Add error handling for clipboard operations.The clipboard functionality should handle potential errors.
- const handleCopy = () => { + const handleCopy = (text: string, result: boolean) => { + if (!result) { + console.error('Failed to copy to clipboard'); + return; + } setCopyStatus(true); setTimeout(() => setCopyStatus(false), 2000); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitignore(1 hunks)package.json(1 hunks)scripts/fetchSbomData.js(1 hunks)src/components/Licenses/SBOMViewer.tsx(4 hunks)
🧰 Additional context used
🪛 GitHub Actions: Lint Code Base
package.json
[warning] Node.js version incompatibility - Required: >=22.8.0, Current: v20.18.1
[warning] Multiple deprecated package dependencies: sourcemap-codec, rimraf, npmlog, inflight, gauge, are-we-there-yet, boolean, @humanwhocodes/config-array, @humanwhocodes/object-schema, glob, eslint
scripts/fetchSbomData.js
[error] 1-1: Cannot use import statement outside a module. Need to set "type": "module" in package.json or use .mjs extension
🔇 Additional comments (5)
.gitignore (1)
68-70: LGTM! Appropriate entries added to .gitignore.The new entries correctly exclude the generated SBOM data files from version control.
src/components/Licenses/SBOMViewer.tsx (2)
1-13: LGTM! Good implementation of i18n and SBOM data imports.The changes properly implement internationalization and correctly update the SBOM data imports to use the new GitHub SBOM format.
Line range hint
34-133: LGTM! Well-structured UI with proper i18n implementation.The UI components are well-organized with proper internationalization of all strings and good TypeScript/React patterns.
package.json (2)
38-38: LGTM! Appropriate addition to postinstall script.The script is correctly added to run after platform dependencies are installed.
🧰 Tools
🪛 GitHub Actions: Lint Code Base
[warning] Node.js version incompatibility - Required: >=22.8.0, Current: v20.18.1
[warning] Multiple deprecated package dependencies: sourcemap-codec, rimraf, npmlog, inflight, gauge, are-we-there-yet, boolean, @humanwhocodes/config-array, @humanwhocodes/object-schema, glob, eslint
Line range hint
1-1: Update Node.js version and deprecated dependencies.The pipeline shows several issues that need attention:
- Node.js version incompatibility (required >=22.8.0, current v20.18.1)
- Multiple deprecated package dependencies
Please update the Node.js version in your CI environment and update the following deprecated packages:
- sourcemap-codec
- rimraf
- npmlog
- inflight
- gauge
- are-we-there-yet
- boolean
- @humanwhocodes/config-array
- @humanwhocodes/object-schema
- glob
- eslint
Run this script to check for the latest versions:
🧰 Tools
🪛 GitHub Actions: Lint Code Base
[warning] Node.js version incompatibility - Required: >=22.8.0, Current: v20.18.1
[warning] Multiple deprecated package dependencies: sourcemap-codec, rimraf, npmlog, inflight, gauge, are-we-there-yet, boolean, @humanwhocodes/config-array, @humanwhocodes/object-schema, glob, eslint
| @@ -187,12 +144,12 @@ const BOMDisplay: React.FC = () => { | |||
| onCopy={handleCopy} | |||
| > | |||
| <button className="text-md hover:bg-primary-dark w-full rounded-md bg-primary px-4 py-2 text-white transition-all duration-300 focus:outline-none md:w-auto"> | |||
| Copy BOM JSON | |||
| {t("Copy BOM JSON")} | |||
There was a problem hiding this comment.
this is not the way to add translations. see how other translations are done
.gitignore
Outdated
| src/components/Licenses/feBomData.json |
There was a problem hiding this comment.
let's keep this inside the public dir. instead.
(we'd also be able to access it by care.ohc.network/licenses/backend|frontend.json)
…o issues/9534/update-license-sbom
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/components/Licenses/SBOMViewer.tsx (1)
Line range hint
102-153: Add TypeScript interfaces for the GitHub SBOM format.Define proper TypeScript interfaces to improve type safety and code maintainability.
interface ExternalRef { referenceLocator?: string; referenceCategory?: string; } interface Package { name?: string; versionInfo?: string; licenseConcluded?: string; externalRefs?: ExternalRef[]; } interface GitHubSBOM { sbom: { spdxVersion: string; creationInfo: { created: string; }; packages: Package[]; }; }Extract package card into a separate component.
Consider extracting the package display logic into a reusable component to improve code organization and maintainability.
interface PackageCardProps { pkg: Package; index: number; showExternalRefs: number | null; onToggleRefs: (index: number) => void; } const PackageCard: React.FC<PackageCardProps> = ({ pkg, index, showExternalRefs, onToggleRefs, }) => { const { t } = useTranslation(); return ( <div className="block rounded-md border p-2 transition-all duration-300 hover:shadow-lg"> {/* ... existing package card JSX ... */} </div> ); };
🧹 Nitpick comments (6)
src/components/Licenses/SBOMViewer.tsx (3)
17-21: Enhance error handling in fetchJsonData function.The error handling could be more informative by including the HTTP status code and response details.
const fetchJsonData = async (url: string) => { const response = await fetch(url); - if (!response.ok) throw new Error("Failed to fetch data"); + if (!response.ok) { + throw new Error( + `Failed to fetch data: ${response.status} ${response.statusText}` + ); + } return response.json(); };
29-45: Add retry and caching configuration to useQuery hooks.Consider adding retry and caching configurations to improve the data fetching reliability and performance.
const { data: feBomData, isLoading: feBomLoading, error: feBomError, } = useQuery({ queryKey: ["feBomData"], queryFn: () => fetchJsonData("/licenses/feBomData.json"), + retry: 2, + staleTime: 5 * 60 * 1000, // 5 minutes + cacheTime: 30 * 60 * 1000, // 30 minutes });
58-60: Enhance error state display.Consider providing more detailed error information to help users understand and report issues.
- return <div>{t("error_404")}</div>; + return ( + <div className="text-red-600"> + <div>{t("error_404")}</div> + <div className="text-sm mt-2"> + {feBomError?.message || beBomError?.message} + </div> + </div> + );scripts/fetchSbomData.ts (3)
1-2: Consider using native fetch API instead of node-fetchThe native fetch API is now available in Node.js versions >= 18.0.0. Consider using it instead of the node-fetch package to reduce dependencies.
-import fetch from "node-fetch";
4-7: Strengthen the SBOMData interface typingThe current interface is too permissive with
Record<string, unknown>. Consider defining more specific types based on the actual SBOM data structure from GitHub's API.interface SBOMData { sbom: { metadata: { version: string; tools: Array<{ name: string; version: string; }>; }; components: Array<{ name: string; version: string; licenses?: Array<{ license: { id: string; }; }>; }>; }; }
62-62: Improve script execution with proper error handlingAdd proper process handling and success/failure indication.
-fetchData(); +fetchData() + .then(() => { + console.log('Successfully fetched and saved SBOM data'); + process.exit(0); + }) + .catch((error) => { + console.error('Failed to fetch SBOM data:', error); + process.exit(1); + }); + +// Handle unexpected errors +process.on('unhandledRejection', (error) => { + console.error('Unhandled rejection:', error); + process.exit(1); +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.gitignore(1 hunks)package.json(1 hunks)public/locale/en.json(6 hunks)scripts/fetchSbomData.ts(1 hunks)src/components/Licenses/SBOMViewer.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- package.json
🔇 Additional comments (2)
public/locale/en.json (1)
473-474: LGTM! All required translations are present.The translations for the SBOM viewer component have been properly added and are consistent with the component's usage.
Also applies to: 1090-1090, 1185-1185, 1297-1297, 1662-1662
scripts/fetchSbomData.ts (1)
13-28:⚠️ Potential issueAdd GitHub authentication and improve error handling
- GitHub API has rate limits for unauthenticated requests. Add authentication using a GitHub token.
- Implement retry logic for transient failures.
- Add more specific error handling based on HTTP status codes.
Apply these improvements:
const fetchSBOMData = async (url: string): Promise<SBOMData> => { + const MAX_RETRIES = 3; + const GITHUB_TOKEN = process.env.GITHUB_TOKEN; + + if (!GITHUB_TOKEN) { + throw new Error('GITHUB_TOKEN environment variable is required'); + } + + for (let attempt = 1; attempt <= MAX_RETRIES; attempt++) { const response = await fetch(url, { headers: { Accept: "application/vnd.github+json", "X-GitHub-Api-Version": "2022-11-28", + Authorization: `Bearer ${GITHUB_TOKEN}`, }, }); if (!response.ok) { + if (response.status === 429) { + const retryAfter = response.headers.get('Retry-After'); + await new Promise(resolve => + setTimeout(resolve, (retryAfter ? parseInt(retryAfter) : 60) * 1000) + ); + continue; + } + + if (attempt === MAX_RETRIES) { const error: FetchError = new Error(`Error fetching SBOM data from ${url}`); error.response = response; throw error; + } + + await new Promise(resolve => setTimeout(resolve, 1000 * attempt)); + continue; } return (await response.json()) as SBOMData; + } + + throw new Error(`Failed to fetch SBOM data after ${MAX_RETRIES} attempts`); };
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
scripts/fetchSbomData.ts (1)
60-75:⚠️ Potential issueAddress pipeline failures by ensuring directory exists.
The pipeline is failing because the target directory doesn't exist. This was addressed in a previous review but the changes weren't applied.
const fetchData = async (): Promise<void> => { + const outputDir = "./public/licenses"; + + // Ensure directory exists + if (!fs.existsSync(outputDir)) { + fs.mkdirSync(outputDir, { recursive: true }); + } + const [frontendData, backendData] = await Promise.all([ - fetchSBOMData(FE_SBOM_URL), - fetchSBOMData(BE_SBOM_URL), + fetchSbomData(FE_SBOM_URL), + fetchSbomData(BE_SBOM_URL), ]); fs.writeFileSync( - "./public/licenses/feBomData.json", + `${outputDir}/feBomData.json`, JSON.stringify(frontendData, null, 2), ); fs.writeFileSync( - "./public/licenses/beBomData.json", + `${outputDir}/beBomData.json`, JSON.stringify(backendData, null, 2), ); };🧰 Tools
🪛 GitHub Actions: Lint Code Base
[error] 66-66: Failed to write to feBomData.json during postinstall script execution
🪛 GitHub Actions: Cypress Tests
[error] 66-66: Failed to write to './public/licenses/feBomData.json' - directory does not exist (ENOENT error)
🪛 GitHub Actions: Deploy Care Fe
[error] 66-66: Failed to write to './public/licenses/feBomData.json': directory does not exist
🧹 Nitpick comments (2)
public/locale/en.json (1)
651-653: Consider using sentence case for consistency.While the changes are functionally correct, consider using sentence case for "BOM JSON" to match the style of other UI elements (e.g., "Bom Json").
- "copy_bom_json": "Copy BOM JSON", + "copy_bom_json": "Copy Bom Json",scripts/fetchSbomData.ts (1)
46-47: Consider making API version configurable.The GitHub API version is hardcoded. Consider moving it to a configuration file or environment variable for easier updates.
+const GITHUB_API_VERSION = "2022-11-28"; + const fetchSbomData = async (url: string): Promise<GitHubSbomApiResponse> => { const response = await fetch(url, { headers: { Accept: "application/vnd.github+json", - "X-GitHub-Api-Version": "2022-11-28", + "X-GitHub-Api-Version": GITHUB_API_VERSION, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(6 hunks)scripts/fetchSbomData.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Lint Code Base
scripts/fetchSbomData.ts
[error] 66-66: Failed to write to feBomData.json during postinstall script execution
🪛 GitHub Actions: Cypress Tests
scripts/fetchSbomData.ts
[error] 66-66: Failed to write to './public/licenses/feBomData.json' - directory does not exist (ENOENT error)
🪛 GitHub Actions: Deploy Care Fe
scripts/fetchSbomData.ts
[error] 66-66: Failed to write to './public/licenses/feBomData.json': directory does not exist
🔇 Additional comments (6)
public/locale/en.json (2)
507-508: LGTM! Clear and consistent naming for backend/frontend components.The added translations maintain consistency with the application's naming conventions.
1180-1180: LGTM! Appropriate translations for SBOM-related terms.The added translations for license, N/A, packages, and SPDX SBOM version are clear and appropriate for the context.
Also applies to: 1282-1282, 1415-1415, 1835-1835
scripts/fetchSbomData.ts (4)
1-7: LGTM! Well-structured imports and constants.Good job moving the URLs to top-level constants as suggested in the previous review.
9-41: LGTM! Well-defined type interface.The
GitHubSbomApiResponseinterface accurately reflects the GitHub SBOM API structure as discussed in the previous review.
43-58: Follow the agreed naming convention for abbreviations.As discussed in the previous review, we should use "Sbom" instead of "SBOM" in the function name.
-const fetchSBOMData = async (url: string): Promise<GitHubSbomApiResponse> => { +const fetchSbomData = async (url: string): Promise<GitHubSbomApiResponse> => {
1-77: Verify the implementation in the CI environment.Let's ensure the script works correctly in the CI environment by checking:
- Directory creation
- File permissions
- API rate limits
✅ Verification successful
Implementation verified successfully
The script should work correctly in the CI environment:
- Directory permissions and structure are properly set up
- GitHub API endpoints are accessible
- Rate limits (60 requests/hour) are sufficient for the required API calls (2 requests)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation in CI environment # Check if the script has write permissions in the target directory echo "Checking directory permissions..." mkdir -p ./public/licenses if [ ! -w "./public/licenses" ]; then echo "Error: No write permissions in ./public/licenses" exit 1 fi # Check GitHub API rate limits echo "Checking GitHub API rate limits..." curl -s -I -H "Accept: application/vnd.github+json" \ -H "X-GitHub-Api-Version: 2022-11-28" \ "https://api.github.com/rate_limit"Length of output: 465
Script:
#!/bin/bash # Description: Verify GitHub API access with correct methods echo "Checking GitHub API rate limits..." curl -s \ -H "Accept: application/vnd.github+json" \ -H "X-GitHub-Api-Version: 2022-11-28" \ "https://api.github.com/rate_limit" echo -e "\nChecking SBOM API endpoints..." curl -s -o /dev/null -w "FE SBOM Status: %{http_code}\n" \ -H "Accept: application/vnd.github+json" \ -H "X-GitHub-Api-Version: 2022-11-28" \ "https://api.github.com/repos/ohcnetwork/care_fe/dependency-graph/sbom" curl -s -o /dev/null -w "BE SBOM Status: %{http_code}\n" \ -H "Accept: application/vnd.github+json" \ -H "X-GitHub-Api-Version: 2022-11-28" \ "https://api.github.com/repos/ohcnetwork/care/dependency-graph/sbom"Length of output: 1445
🧰 Tools
🪛 GitHub Actions: Lint Code Base
[error] 66-66: Failed to write to feBomData.json during postinstall script execution
🪛 GitHub Actions: Cypress Tests
[error] 66-66: Failed to write to './public/licenses/feBomData.json' - directory does not exist (ENOENT error)
🪛 GitHub Actions: Deploy Care Fe
[error] 66-66: Failed to write to './public/licenses/feBomData.json': directory does not exist
…o issues/9534/update-license-sbom
public/licenses/.gitkeep
Outdated
There was a problem hiding this comment.
instead of this, we can do in the fetch sbom data script.
await mkdir(path, { recursive: true })| </h3> | ||
| {bomData.components?.map((component, index) => ( | ||
| {packages.map((pkg: any, index: number) => ( |
There was a problem hiding this comment.
now that we have strong types defined, let's not use any.
| {packages.map((pkg: any, index: number) => ( | |
| {packages.map((package, index) => ( |
… for SBOM packages
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/Licenses/SBOMViewer.tsx (1)
30-46: 🛠️ Refactor suggestionMove SBOM data fetching to build/postinstall step.
The current implementation fetches SBOM data on every render, which is inefficient. As suggested in previous reviews, this data should be fetched during build or postinstall step.
- Add a build script to
package.json:{ "scripts": { "fetch-sbom": "ts-node scripts/fetchSbomData.ts", "postinstall": "npm run fetch-sbom" } }
- Then modify this component to import the JSON files directly:
import feBomData from '@/licenses/feBomData.json'; import beBomData from '@/licenses/beBomData.json'; const BOMDisplay: React.FC = () => { const bomData = activeTab === "bom" ? feBomData : beBomData; // ... rest of the component };
🧹 Nitpick comments (3)
src/components/Licenses/SBOMViewer.tsx (3)
18-22: Enhance error handling in fetchJsonData.The current error handling is basic and could be improved for better error reporting and recovery.
Consider this enhanced implementation:
const fetchJsonData = async (url: string) => { - const response = await fetch(url); - if (!response.ok) throw new Error("Failed to fetch data"); - return response.json(); + try { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 5000); + + const response = await fetch(url, { + signal: controller.signal, + headers: { 'Accept': 'application/json' } + }); + + clearTimeout(timeoutId); + + if (!response.ok) { + throw new Error(`HTTP error! status: ${response.status} - ${response.statusText}`); + } + + return response.json(); + } catch (error) { + if (error instanceof Error) { + throw new Error(`Failed to fetch SBOM data: ${error.message}`); + } + throw error; + } };
Line range hint
140-154: Improve accessibility and UX of external references section.The external references section needs accessibility improvements and better URL handling.
- <h4 - className="block cursor-pointer font-semibold text-primary" - onClick={() => - setShowExternalRefs( - showExternalRefs === index ? null : index, - ) - } - > - <CareIcon icon="l-info-circle" /> - </h4> + <button + className="block font-semibold text-primary" + onClick={() => setShowExternalRefs(showExternalRefs === index ? null : index)} + aria-expanded={showExternalRefs === index} + aria-controls={`external-refs-${index}`} + > + <CareIcon icon="l-info-circle" aria-hidden="true" /> + <span className="sr-only">{t("toggle_external_references")}</span> + </button> {showExternalRefs === index && ( - <ul className="list-inside list-disc pl-4 text-xs"> + <ul + id={`external-refs-${index}`} + className="list-inside list-disc pl-4 text-xs" + > {pkg.externalRefs?.map((ref, idx) => ( <li key={idx}> <a href={ref.referenceLocator || "#"} + aria-label={t("external_reference_link", { category: ref.referenceCategory })} className="hover:text-primary-dark block break-words text-primary" + onClick={(e) => { + if (ref.referenceLocator === "#") { + e.preventDefault(); + } + }} > - {ref.referenceLocator || t("n_a")} + {new URL(ref.referenceLocator || "#").hostname || t("n_a")} </a>
Line range hint
165-176: Improve copy button accessibility and feedback.The copy button and feedback message need accessibility improvements.
<CopyToClipboard text={JSON.stringify(bomData, null, 2)} onCopy={handleCopy} > <button className="text-md hover:bg-primary-dark w-full rounded-md bg-primary px-4 py-2 text-white transition-all duration-300 focus:outline-none md:w-auto" + aria-label={t("copy_bom_json_to_clipboard")} > {t("copy_bom_json")} </button> </CopyToClipboard> {copyStatus && ( - <span className="mt-2 block text-sm text-gray-600"> + <span + className="mt-2 block text-sm text-gray-600" + role="status" + aria-live="polite" + > {t("copied_to_clipboard")} </span> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.json(2 hunks)scripts/fetchSbomData.ts(1 hunks)src/components/Licenses/SBOMViewer.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- scripts/fetchSbomData.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: OSSAR-Scan
package.json
Outdated
| @@ -105,6 +106,7 @@ | |||
| "lucide-react": "^0.469.0", | |||
| "markdown-it": "^14.1.0", | |||
| "next-themes": "^0.4.3", | |||
| "node-fetch": "^3.3.2", | |||
There was a problem hiding this comment.
this should be in dev-dependencies and not in dependencies
The old SBOM files in components/licenses were removed . I also saw some in the public dir ; pretty sure i didnt add that.Also removed them too lmk if that was necessary in some case. |
|
PR was expected to remove that too 😄 |
Yeah then, the changes up to this point have been done. 👍 |
package.json
Outdated
| @@ -165,6 +166,7 @@ | |||
| "lint-staged": "^15.2.10", | |||
| "local-cypress": "^1.2.6", | |||
| "marked": "^14.1.4", | |||
| "node-fetch": "^3.3.2", | |||
There was a problem hiding this comment.
| "node-fetch": "^3.3.2", |
we have fetch built-in in node 22
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/types/license.ts (1)
1-33: LGTM! Consider adding JSDoc comments.The interface accurately represents GitHub's SBOM schema with proper type definitions.
Consider adding JSDoc comments to document the purpose of the interface and its major sections (sbom, packages, relationships) for better maintainability:
+/** + * Interface representing GitHub's Software Bill of Materials (SBOM) format + * following SPDX specification. + */ export interface LicensesSbom { sbom: {scripts/generate-sbom-data.ts (2)
19-22: Enhance error message with response details.The error message could be more helpful by including the response status code and potentially the response body.
if (!response.ok) { + const errorBody = await response.text(); throw new Error( - `Error fetching SBOM data from ${url}: ${response.statusText}`, + `Error fetching SBOM data from ${url}: ${response.status} ${response.statusText}\n${errorBody}`, ); }
5-7: Consider making repository paths configurable.The SBOM folder path and repository names are hardcoded. Consider making them configurable through environment variables.
const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); -const sbomFolderPath = path.join(__dirname, "..", "public", "sbom"); +const sbomFolderPath = process.env.SBOM_FOLDER_PATH || path.join(__dirname, "..", "public", "sbom");src/pages/Licenses/Licenses.tsx (1)
84-94: Add loading state for copy operation.The copy to clipboard operation should show a loading state to prevent multiple clicks.
+ const [isCopying, setIsCopying] = useState(false); <Button variant="outline" className="w-full" + disabled={isCopying} onClick={async () => { + setIsCopying(true); await navigator.clipboard.writeText(JSON.stringify(sbom, null, 2)); toast.info(t("copied_to_clipboard")); + setIsCopying(false); }} > - <CopyIcon className="mr-2" /> + {isCopying ? <Loading className="mr-2" /> : <CopyIcon className="mr-2" />} {t("copy_bom_json")} </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
.gitignore(1 hunks)package.json(1 hunks)public/locale/en.json(5 hunks)scripts/generate-sbom-data.ts(1 hunks)src/Routers/PublicRouter.tsx(1 hunks)src/components/Licenses/LicensesPage.tsx(0 hunks)src/components/Licenses/SBOMViewer.tsx(0 hunks)src/pages/Licenses/Licenses.tsx(1 hunks)src/types/license.ts(1 hunks)
💤 Files with no reviewable changes (2)
- src/components/Licenses/LicensesPage.tsx
- src/components/Licenses/SBOMViewer.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- .gitignore
- package.json
- public/locale/en.json
🧰 Additional context used
📓 Learnings (1)
src/Routers/PublicRouter.tsx (1)
Learnt from: sainak
PR: ohcnetwork/care_fe#8979
File: src/Routers/SessionRouter.tsx:1-8
Timestamp: 2024-11-12T10:23:10.322Z
Learning: In `src/Routers/SessionRouter.tsx`, the `LicensesPage` component opens in a new page and is correctly handled by the top-level `Suspense`, so wrapping it in a `Suspense` component within `SessionRouter.tsx` is unnecessary.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test
- GitHub Check: CodeQL-Build
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (1)
src/Routers/PublicRouter.tsx (1)
11-11: LGTM! Direct import aligns with top-level Suspense handling.The change from lazy loading to direct import is appropriate as the component is handled by the top-level Suspense component.
Also applies to: 11-11
| <a | ||
| href={licenseUrls[pkg.licenseConcluded as keyof typeof licenseUrls]} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="hover:text-primary-dark text-primary" | ||
| > | ||
| {pkg.licenseConcluded} | ||
| </a> |
There was a problem hiding this comment.
Add security attributes for external license links.
External links to licenses should include security-related attributes to protect against potential vulnerabilities.
<a
href={licenseUrls[pkg.licenseConcluded as keyof typeof licenseUrls]}
target="_blank"
rel="noopener noreferrer"
+ referrerPolicy="no-referrer"
+ rel="noopener noreferrer nofollow"
className="hover:text-primary-dark text-primary"
>Committable suggestion skipped: line range outside the PR's diff.
|
@DonXavierdev Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
Changes
Removed
Performance