Updates api routes for camera plugin integrations #9209
Conversation
WalkthroughThe changes in this pull request involve updates to several components and routing definitions related to camera presets. The Changes
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 (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ 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: 4
🧹 Outside diff range and nitpick comments (1)
src/components/CameraFeed/routes.ts (1)
57-82: Consider API versioning strategyThe removal of the
/api/v1/prefix from all routes deviates from API versioning best practices. This could make future API version migrations more challenging.Consider:
- Maintaining version prefix for better API lifecycle management
- Documenting the versioning strategy if it's intentionally removed
- Using API gateway or proxy layer to handle version routing if needed
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/components/CameraFeed/CameraFeedWithBedPresets.tsx(1 hunks)src/components/CameraFeed/ConfigureCamera.tsx(4 hunks)src/components/CameraFeed/routes.ts(1 hunks)src/components/Facility/ConsultationDetails/ConsultationFeedTab.tsx(2 hunks)
🔇 Additional comments (6)
src/components/CameraFeed/CameraFeedWithBedPresets.tsx (1)
19-20: Verify asset.id contains the external ID
The code is passing props.asset.id as asset_external_id, but we need to verify if asset.id actually contains the external ID value expected by the backend.
Let's verify the AssetData type definition and usage:
Additionally, consider adding type safety:
const { data, loading } = useQuery(FeedRoutes.listCameraPresets, {
- query: { asset_external_id: props.asset.id, limit: 100 },
+ query: {
+ asset_external_id: props.asset.external_id ?? props.asset.id,
+ limit: 100
+ },
});src/components/CameraFeed/routes.ts (1)
57-65: Clarify the coexistence of similar routes
There appear to be two similar routes for listing camera presets:
listAssetBedPresetsat/api/camera/assetbed/position_presets/listCameraPresetsat/api/camera/position_presets/
This could lead to confusion about which endpoint to use in different scenarios.
Let's check for usage patterns:
✅ Verification successful
Both routes serve different use cases and are correctly utilized
The coexistence of these routes is intentional and properly implemented:
listAssetBedPresetsis used inConfigureCamera.tsxwithassetbed_external_idparameterlistCameraPresetsis used in:ConsultationFeedTab.tsxwithbed_external_idparameterCameraFeedWithBedPresets.tsxwithasset_external_idparameter
Each route serves a specific context and accepts different query parameters, suggesting they are designed to handle distinct use cases rather than being duplicates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of both routes to understand the transition
echo "Searching for listAssetBedPresets usage:"
rg "listAssetBedPresets" -A 2
echo -e "\nSearching for listCameraPresets usage:"
rg "listCameraPresets" -A 2
Length of output: 1576
src/components/Facility/ConsultationDetails/ConsultationFeedTab.tsx (1)
119-121: LGTM! Verify the updatePreset endpoint compatibility.
The addition of assetbed_external_id in the query parameters is consistent with the API route updates.
Let's verify the endpoint compatibility:
✅ Verification successful
Endpoint compatibility verified for assetbed_external_id parameter
Both usages of FeedRoutes.updatePreset in the codebase consistently include the assetbed_external_id query parameter:
ConsultationFeedTab.tsxusespreset.asset_bed.idConfigureCamera.tsxusesselectedAssetBed.id
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the updatePreset endpoint usage and parameters
ast-grep --pattern 'FeedRoutes.updatePreset'
Length of output: 296
Script:
#!/bin/bash
# Let's check the implementation and usage of updatePreset route
ast-grep --pattern 'updatePreset = $$$'
# Also check the usage context in both files
rg -B 5 -A 5 "FeedRoutes.updatePreset" src/components/CameraFeed/ConfigureCamera.tsx src/components/Facility/ConsultationDetails/ConsultationFeedTab.tsx
Length of output: 2341
Script:
#!/bin/bash
# Let's check the ConfigureCamera.tsx implementation to see if it also uses assetbed_external_id
rg -A 10 "FeedRoutes.updatePreset" src/components/CameraFeed/ConfigureCamera.tsx
# Also check for any route definitions
ast-grep --pattern 'updatePreset: $$$'
Length of output: 706
src/components/CameraFeed/ConfigureCamera.tsx (3)
422-422: LGTM! Good error handling and user feedback
The API call is properly structured with appropriate error handling and success notification.
530-532: LGTM! Proper error handling and user feedback
The delete preset implementation includes appropriate error handling and success notification.
104-108: Verify if the position parameter is always required
The position parameter is hardcoded to true. Consider if this parameter is necessary since it seems to be always set to true.
✅ Verification successful
Let me gather more information about the API route and its usage.
Let me try a different approach to find the API route definition and type information.
Let me check the routes definition to understand the API parameters better.
Based on the routes file content, I can now generate the final response:
The position parameter is correctly set to true
The position parameter is required as it's a crucial part of the CameraPreset type definition, which includes a position: PTZPayload field. This field stores the pan-tilt-zoom coordinates for each preset, so requesting it in the list query is necessary for proper functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the position parameter is used differently in other files
rg -A 5 "FeedRoutes.listAssetBedPresets"
Length of output: 503
Script:
#!/bin/bash
# Check the API route definition and other usages
rg -g "*.ts*" -A 5 "listAssetBedPresets.*="
# Check for any documentation or type definitions
ast-grep --pattern 'interface $_ {
$$$
position: $_
$$$
}'
Length of output: 112
Script:
#!/bin/bash
# Search for route definitions
rg -g "*.ts*" "listAssetBedPresets"
# Look for any types or interfaces related to camera presets
rg -g "*.ts*" -B 2 -A 5 "type.*Preset|interface.*Preset"
Length of output: 3134
Script:
#!/bin/bash
# Check the routes file content
cat src/components/CameraFeed/routes.ts
Length of output: 2151
src/components/Facility/ConsultationDetails/ConsultationFeedTab.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/components/CameraFeed/routes.ts (2)
Line range hint
44-54: Align route prefix with new patternThe
operateAssetroute still uses the/api/v1/prefix while new routes use/api/camera/. This inconsistency should be addressed.Consider updating the path to match the new pattern:
- path: "/api/v1/asset/{id}/operate_assets/", + path: "/api/camera/assets/{id}/operate/",
56-78: Consider implementing a migration strategyThese route changes constitute breaking changes, including:
- New route structure (
/api/camera/position-presets/)- Parameter changes (
asset_external_id,assetbed_external_id)Consider:
- Implementing version compatibility in the backend
- Adding deprecation notices for old routes
- Documenting migration steps for other teams
- Coordinating the rollout with dependent services
Would you like help creating a migration guide or deprecation timeline?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/components/CameraFeed/CameraFeedWithBedPresets.tsx(1 hunks)src/components/CameraFeed/ConfigureCamera.tsx(4 hunks)src/components/CameraFeed/routes.ts(1 hunks)src/components/Facility/ConsultationDetails/ConsultationFeedTab.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/CameraFeed/CameraFeedWithBedPresets.tsx
- src/components/CameraFeed/ConfigureCamera.tsx
🔇 Additional comments (2)
src/components/CameraFeed/routes.ts (1)
56-78: Verify backend compatibility with new route structure
The new RESTful route structure is well-organized and follows best practices. However, we need to ensure the backend PR (#2610) supports these changes.
Let's verify the backend compatibility:
✅ Verification successful
Route changes are already in use and functioning
The new route structure /api/camera/position-presets/ is already being actively used across multiple frontend components:
- ConsultationFeedTab.tsx
- ConfigureCamera.tsx
- CameraFeedWithBedPresets.tsx
These components are successfully making API calls using the new route structure for all CRUD operations (list, create, update, delete). Since these components are functioning in the codebase and no old route patterns (/api/v1/camera_presets) were found, this indicates that the backend already supports these routes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the backend PR implements the new routes
# Fetch the backend PR details
echo "Checking backend PR #2610 for route implementations:"
gh api repos/ohcnetwork/care/pulls/2610 --jq '.body' | grep -i "position-presets"
# Search for any remaining old route patterns in the frontend
echo -e "\nChecking for remaining old route patterns:"
rg -t ts -t tsx "/api/v1/.*camera_presets"
Length of output: 368
Script:
#!/bin/bash
# Let's try a different approach to verify the backend compatibility
# Search for any route definitions related to camera presets in the backend
echo "Searching for camera preset route definitions:"
rg -t py "position.?presets|camera.?presets" --context 2
# Check for any existing API endpoint tests
echo -e "\nSearching for related API tests:"
fd -e py -e yaml test.*camera.*preset
# Look for any migration files that might indicate schema changes
echo -e "\nChecking for related migrations:"
fd -e py "migration.*camera"
# Check for any remaining old route patterns in TypeScript files (fixing the file type)
echo -e "\nChecking for old route patterns in frontend:"
rg -t typescript "/api/v1/.*camera_presets"
Length of output: 731
Script:
#!/bin/bash
# Let's try one final approach focusing on the frontend changes
# Search for any references to the old or new camera preset routes
echo "Searching for camera preset route patterns:"
rg "api/.*camera.*preset" -l
# Look for any route definition files
echo -e "\nSearching for route definition files:"
fd "routes.ts"
# Check for any imports or usages of these routes
echo -e "\nChecking for route imports and usages:"
rg "positionPresets" -A 2 -B 2
Length of output: 4814
src/components/Facility/ConsultationDetails/ConsultationFeedTab.tsx (1)
61-62: Verify bed.id compatibility with external_id format
The query parameter uses bed?.id directly as the bed_external_id. Please ensure that the bed.id format matches the expected external ID format used by the API.
✅ Verification successful
Let me gather more information about the bed object structure and the API endpoint to verify the ID format compatibility.
Based on the search results, I can now generate the final response:
The bed.id usage is correct and compatible with the API's external ID format
The codebase shows consistent usage of bed.id as the external identifier:
- The bed object follows the BaseModel interface which defines
idas a string type - Other components like ConfigureCamera also use similar pattern with
bed_external_idandassetbed_external_idaccepting the bed.id directly - The API routes and query parameters are aligned with this usage pattern
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the bed object structure and ID format
# Look for bed object type definitions and usage patterns
# Check for bed type definitions
ast-grep --pattern 'type $_ = {
$$$
bed_object: {
$$$
}
$$$
}'
# Check for bed.id usage patterns
rg "bed\?\.id|bed\.id" -A 2 -B 2
Length of output: 5803
Script:
#!/bin/bash
# Check bed type definitions and interfaces
ast-grep --pattern 'interface $_ {
$$$
id: $_
$$$
}'
# Check the API route definition
rg "positionPresets|bed_external_id" -A 2 -B 2
# Check for any type transformations or validations related to bed IDs
rg "bed.*id.*external" -A 2 -B 2
Length of output: 41224
|
@DraKen0009 can you check the coderabbit suggestion CC: @rithviknishad |
|
@nihal467 not in scope of this PR; also |
|
|
|
Fixed in ohcnetwork/care_camera_asset@ffe864b |
|
@rithviknishad @DraKen0009 the camera preset is still not working, do give it a check before requesting next review |
Proposed Changes
Updated routes for the camera plugin integration in backend.
backend pr : ohcnetwork/care#2610
@ohcnetwork/care-fe-code-reviewers
Summary by CodeRabbit
New Features
ConsultationFeedTabcomponent.Bug Fixes
Chores