fix sbom deprecations #1480
fix sbom deprecations #1480
Conversation
future-pirate-king
requested review from
Praseetha-KR,
cosmosgenius and
Yibaebi
WalkthroughThe pull request introduces several modifications across various components and models within the application. Key changes include the addition of a new export for the Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
|
Deploying irenestaging with
|
| Latest commit: |
d688aae
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7c9a44e2.irenestaging.pages.dev |
| Branch Preview URL: | https://pd-1558-fix-sbom-deprecation.irenestaging.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
app/components/sbom/app-list/filter-selected-item/index.ts (1)
28-28: LGTM! Consider adding a deprecation notice.The addition of the slash-based syntax export is correct and aligns with the PR objective of fixing SBOM deprecations.
Consider adding a deprecation notice using
@deprecatedJSDoc comment above the double colon syntax registry entry to help other developers migrate:export default interface Registry { + /** @deprecated Use 'sbom/app-list/filter-selected-item' instead */ 'Sbom::AppList::FilterSelectedItem': typeof SbomAppListFilterSelectedItemComponent; 'sbom/app-list/filter-selected-item': typeof SbomAppListFilterSelectedItemComponent; }app/models/sbom-component.ts (1)
Line range hint
31-32: Consider strengthening types for security-related fields.While the implementation is correct, consider adding more specific types for security-related fields to improve type safety and documentation:
- @attr('number') - declare vulnerabilitiesCount: number; + @attr('number', { + defaultValue: 0 + }) + declare vulnerabilitiesCount: number; - @attr('string') - declare remediation: string; + @attr('string', { + defaultValue: '' + }) + declare remediation: string | null;This change would:
- Ensure vulnerabilitiesCount always has a valid number value
- Make it explicit that remediation might not be available
Also applies to: 34-35
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
- app/components/sbom/app-list/filter-selected-item/index.ts (1 hunks)
- app/components/sbom/app-list/header/index.hbs (1 hunks)
- app/components/sbom/app-list/index.ts (1 hunks)
- app/components/sbom/app-scan/list/index.ts (1 hunks)
- app/components/sbom/component-details/vulnerabilities/index.ts (1 hunks)
- app/components/sbom/scan-details/component-list/index.ts (1 hunks)
- app/models/sbom-component.ts (1 hunks)
- app/models/sbom-vulnerability-audit.ts (1 hunks)
🔇 Additional comments (6)
app/models/sbom-component.ts (1)
10-11: LGTM! Relationship decorator properly updated.The changes correctly implement the async relationship and explicitly declare no inverse relationship, which should resolve the deprecation warnings.
Let's verify if there are any remaining deprecation warnings related to belongsTo relationships in the SBOM models:
✅ Verification successful
Let me try a different approach to verify the SBOM models and their relationships.
This should help us find all SBOM-related models and their belongsTo relationships, regardless of the exact file naming pattern.
All SBOM-related belongsTo relationships are properly configured
Based on the search results, I can verify that all SBOM-related models with
belongsTorelationships are consistently configured with the proper async and inverse settings:
sbom-component.ts:@belongsTo('sbom-file', { async: true, inverse: null })sbom-report.ts:@belongsTo('sbom-file', { async: true, inverse: null })sbom-project.ts:
@belongsTo('project', { async: true, inverse: null })@belongsTo('sbom-file', { async: true, inverse: null })sbom-file.ts:
@belongsTo('file', { async: true, inverse: 'sbFile' })@belongsTo('sbom-project', { async: true, inverse: null })sbom-vulnerability-audit.ts:@belongsTo('sbom-vulnerability', { async: false, inverse: null })All SBOM models follow the same pattern with explicit async and inverse settings, which aligns with the deprecation fixes mentioned in the PR objectives.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other potential belongsTo decorators in SBOM-related models that might need similar updates rg -l "@belongsTo\(" "app/models/sbom-*.ts" | while read -r file; do echo "=== $file ===" rg "@belongsTo\(" "$file" -A 1 doneLength of output: 121
Script:
#!/bin/bash # First, let's find all SBOM-related model files fd "sbom.*\.ts$" app/models # Then, let's examine the content of these files for belongsTo decorators echo "=== Searching for belongsTo decorators ===" rg "@belongsTo" -A 1 app/modelsLength of output: 10646
app/models/sbom-vulnerability-audit.ts (1)
23-23: LGTM! Appropriate fix for Ember Data deprecation.The explicit
inverse: nulldeclaration is the correct approach to address relationship deprecations in Ember Data. This change properly indicates thatsbVulnerabilityis a one-way relationship.Let's verify that this pattern is consistently applied across related SBOM models:
✅ Verification successful
Let me try a different approach to verify the SBOM models and their relationships.
This should help us find:
- All SBOM-related model files
- All @belongsTo decorators in these files to verify consistent usage of
inverse: null
Most SBOM relationships follow the
inverse: nullpattern, with one exceptionThe verification shows that the
inverse: nullpattern is consistently applied across most SBOM model relationships, except for one case insbom-file.ts:
@belongsTo('file', { async: true, inverse: 'sbFile' })This exception appears intentional as it establishes a bidirectional relationship with the
filemodel through thesbFileinverse. The change in the review is consistent with the broader pattern used across the SBOM models where one-way relationships are desired.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent usage of inverse: null in SBOM model relationships # Expected: All @belongsTo decorators in SBOM models should have inverse: null # Search for @belongsTo decorators in SBOM-related models rg -U "@belongsTo\([^)]+\)" "app/models/sb"Length of output: 97
Script:
#!/bin/bash # First, let's find all SBOM-related model files fd "sbom.*\.ts$" app/models/ # Then, let's look for all @belongsTo decorators in these files rg -U --multiline "@belongsTo\(.*?\)" app/models/sbom*Length of output: 1102
app/components/sbom/app-list/header/index.hbs (1)
63-63: LGTM! Good modernization of component reference.The change from string-based to dynamic component reference aligns with modern Ember.js best practices and helps resolve deprecation warnings.
Let's verify the component exists:
✅ Verification successful
Let me try a different search strategy to verify the component's existence.
This more comprehensive search should help us determine if the component exists in the codebase. I'm:
- Using ripgrep to find any mentions of the component
- Directly checking the app-list directory structure
- Looking for TypeScript/JavaScript component files
- Looking for template files
The results will help us verify if the component reference is valid or if there might be an issue with the component's location or naming.
Component reference is valid and correctly implemented
The verification confirms that the component exists and is properly structured:
- The component is located at
app/components/sbom/app-list/filter-selected-item/- It has the expected files:
index.ts,index.hbs, andindex.scss- The component is part of the SBOM app-list feature structure
The dynamic component reference
{{component 'sbom/app-list/filter-selected-item'}}is correctly pointing to an existing component, making this a valid modernization change.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the filter-selected-item component # Test: Search for the component definition fd "filter-selected-item" --type fLength of output: 34
Script:
#!/bin/bash # Try different search patterns and locations to find the component echo "=== Searching with ripgrep ===" rg -l "filter-selected-item" echo -e "\n=== Searching specifically in app/components/sbom directory ===" ls -R app/components/sbom/app-list/ 2>/dev/null echo -e "\n=== Looking for TypeScript/JavaScript component definitions ===" fd -e ts -e js "filter-selected-item" echo -e "\n=== Checking for template files ===" fd -e hbs "filter-selected-item"Length of output: 1605
app/components/sbom/app-scan/list/index.ts (1)
76-76: LGTM! Appropriate replacement of deprecated method.The change from
toArray()toslice()is a good modernization step, maintaining the same immutability guarantees while using standard JavaScript methods.Let's verify the consistency of this change across the codebase:
✅ Verification successful
Let me try a different search strategy to verify this change more thoroughly.
Based on the search results, I can now provide a definitive response about the verification:
Consistent usage of
slice()confirmed across SBOM componentsThe search results show that the change from
toArray()toslice()has been consistently applied across all SBOM components. We found similar patterns in multiple files:
app/components/sbom/scan-report-drawer/report-list/index.tsapp/components/sbom/scan-details/component-list/index.tsapp/components/sbom/app-scan/list/index.tsapp/components/sbom/component-details/vulnerabilities/index.tsapp/components/sbom/app-list/index.tsAll these components use the same pattern:
return this.<queryResponse>?.slice() || [];No instances of
toArray()were found in any SBOM-related files, indicating the deprecation has been completely addressed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining toArray() usage in SBOM-related files # and confirm the consistent use of slice() instead echo "Checking for remaining toArray() usage in SBOM files:" rg "toArray\(\)" -g "**/*sbom*" echo -e "\nVerifying consistent slice() usage in SBOM files:" rg "\.slice\(\)" -g "**/*sbom*"Length of output: 299
Script:
#!/bin/bash # Let's broaden our search to: # 1. Find all files containing 'sbom' in their path # 2. Check for any array-related operations in those files # 3. Look specifically at the component structure # First, find all SBOM-related files echo "SBOM-related files:" fd "sbom" --type f # Then check for array operations in these files echo -e "\nChecking array operations in SBOM components:" rg -A 2 -B 2 "(toArray|slice)\(\)" app/components/sbom/ # Let's also check the specific component's full content echo -e "\nFull content of the component:" cat app/components/sbom/app-scan/list/index.tsLength of output: 8371
app/components/sbom/app-list/index.ts (2)
74-74: LGTM: Robust null handling in placeThe change to
slice()is well-protected by the null coalescing operator (|| []), and the type definition ofSbomProjectQueryResponseproperly describes the expected structure. The implementation maintains type safety and handles edge cases appropriately.Also applies to: 30-31
74-74: Verify the behavioral equivalence of slice() vs toArray()The change from
toArray()toslice()appears to be addressing a deprecation. While both methods create a new array from the record array, there might be subtle differences in how they handle the underlying Ember Data records.Let's verify the deprecation and usage pattern:
Consider documenting this change pattern in your migration guide if it's part of a broader deprecation fix strategy. This will help other developers understand the recommended approach when dealing with Ember Data record arrays.
✅ Verification successful
The previous results show that
toArray()is still widely used in the codebase, but we need to understand if this is part of an ongoing migration and verify the Ember Data version and deprecation notices to make a proper assessment.
Based on the search results, I can now provide a final response:
The change from toArray() to slice() is part of a consistent migration pattern
The codebase shows a clear pattern where both
toArray()andslice()are used to create array copies from record arrays, with newer code predominantly usingslice(). The search results reveal:
- Many components have already adopted
slice()for similar use cases- The pattern
recordResponse?.slice() || []is consistently used across multiple components- No explicit deprecation warnings were found, but the widespread adoption of
slice()suggests an architectural decisionThe change is safe because:
- Both methods create a shallow copy of the array
slice()is a standard JavaScript method that provides the same functionality in this context- The pattern is consistently applied across similar components in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for similar patterns and deprecation warnings echo "Checking for toArray usage patterns..." rg "toArray\(\)" -A 2 -B 2 echo "Checking for deprecation documentation..." rg -g "CHANGELOG.md" -g "README.md" -g "*.json" "toArray.*deprecat" -A 2 -B 2Length of output: 13510
Script:
#!/bin/bash # Let's check for slice() usage patterns to understand the migration pattern echo "Checking for slice() usage patterns..." rg "\.slice\(\)" -A 2 -B 2 # Let's also check for any Ember deprecation guides or documentation echo "Checking for Ember deprecation documentation..." rg -g "package.json" -g "ember" -g "*.md" "recordArray.*deprecat|toArray.*deprecat" -A 2 -B 2 # Let's check the package.json for Ember Data version echo "Checking Ember Data version..." cat package.json | grep "@ember-data"Length of output: 27876
Irene
|
||||||||||||||||||||||||||||||
| Project |
Irene
|
| Branch Review |
PD-1558-fix-sbom-deprecations
|
| Run status |
|
| Run duration | 14m 48s |
| Commit |
|
| Committer | Sam David |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
2
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
30
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
Tests for review
No description provided.