[Netmanager] Fixed typescript errors and added Dockerfile #2395
Conversation
|
Warning Rate limit exceeded@Codebmk has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis pull request introduces comprehensive changes to the Changes
Sequence DiagramsequenceDiagram
participant User
participant Analytics
participant DevicesAPI
participant GridsAPI
User->>Analytics: Select Grid/Cohort
Analytics->>DevicesAPI: Fetch Device Summary
DevicesAPI-->>Analytics: Return Devices
Analytics->>GridsAPI: Fetch Grid Details
GridsAPI-->>Analytics: Return Grid Information
Analytics->>User: Display Analyzed Data
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 (
|
Codebmk
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (21)
netmanager-app/components/Analytics/AnalyticsDropdown.tsx (4)
14-19: Consider documenting the interface and making airqloudsData required.The interface is well-typed, but could benefit from JSDoc documentation for better developer experience. Also, since
airqloudsDatahas a default value in the component, consider making it required in the interface.+/** + * Props for the AnalyticsAirqloudsDropDown component + * @param isCohort - Determines if the dropdown is for cohorts or grids + * @param airqloudsData - Array of cohorts or grids to display + * @param onSelect - Callback function when an item is selected + * @param selectedId - Currently selected item ID + */ interface AnalyticsAirqloudsDropDownProps { isCohort: boolean; - airqloudsData?: Cohort[] | Grid[]; + airqloudsData: Cohort[] | Grid[]; onSelect: (id: string) => void; selectedId: string | null; }
Line range hint
21-34: Enhance type safety and maintainability of the selection handler.Consider extracting storage keys as constants and adding type guards for better type safety.
+const STORAGE_KEYS = { + COHORT: "activeCohort", + GRID: "activeGrid", +} as const; +const isCohortData = (data: Cohort | Grid): data is Cohort => { + return "devices" in data; +}; const handleAirqloudChange = (value: string) => { const selectedAirqloud = airqloudsData.find((a) => a._id === value); if (selectedAirqloud) { - const storageKey = isCohort ? "activeCohort" : "activeGrid"; + const storageKey = isCohort ? STORAGE_KEYS.COHORT : STORAGE_KEYS.GRID; localStorage.setItem(storageKey, JSON.stringify(selectedAirqloud)); onSelect(value); } };
36-43: Optimize string formatting operations.The string formatting function could be more efficient by combining operations and using a more maintainable approach.
-const formatString = (string: string) => { - return string - .replace(/_/g, " ") - .replace(/\w\S*/g, (txt) => { - return txt.charAt(0).toUpperCase() + txt.substr(1).toLowerCase(); - }) - .replace("Id", "ID"); -}; +const formatString = (string: string) => { + const SPECIAL_CASES = { + 'id': 'ID' + }; + + return string + .split('_') + .map(word => + SPECIAL_CASES[word.toLowerCase()] || + (word.charAt(0).toUpperCase() + word.slice(1).toLowerCase()) + ) + .join(' '); +};
67-93: Simplify conditional rendering logic for device/site counts.The nested ternary operations for displaying counts could be simplified using type guards and extracted into a separate function for better readability.
+const getItemCount = (item: Cohort | Grid, isCohort: boolean): string => { + if (isCohort && 'devices' in item) { + return `${item.devices?.length || 0} devices`; + } + if (!isCohort && 'sites' in item) { + return `${item.sites?.length || 0} sites`; + } + return ''; +}; {Array.isArray(airqloudsData) && airqloudsData.map((airqloud) => ( <SelectItem key={airqloud._id} value={airqloud._id} className="cursor-pointer" > <div className="grid grid-cols-12 gap-2 w-full items-center"> <div className="col-span-9"> <span className="font-medium truncate"> {formatString(airqloud.name)} </span> </div> <div className="col-span-3 text-sm text-gray-500 text-right"> <span className="text-sm text-gray-500 text-right"> - {isCohort - ? "devices" in airqloud - ? `${airqloud.devices?.length || 0} devices` - : "" - : "sites" in airqloud - ? `${airqloud.sites?.length || 0} sites` - : ""} + {getItemCount(airqloud, isCohort)} </span> </div> </div> </SelectItem> ))}netmanager-app/app/types/grids.ts (1)
21-22: Consider derivingnumberOfSitesfrom thesitesarraySince
sitesis an array ofSiteobjects, thenumberOfSitesproperty might be redundant as it can be obtained usingsites.length. To prevent data inconsistency, you could removenumberOfSitesor ensure it's always synchronized with thesitesarray length.netmanager-app/app/types/sites.ts (4)
2-95: Ensure consistent naming conventions for interface propertiesThe
Siteinterface contains a mix ofcamelCaseandsnake_caseproperty names (e.g.,nearest_tahmo_station,formatted_name,approximate_latitude). For clarity and maintainability, it's best to standardize to a single naming convention, preferablycamelCase, which is common in TypeScript.Here's a partial diff demonstrating the changes:
export interface Site { _id: string; - nearest_tahmo_station: { + nearestTahmoStation: { id: number; code: string | null; longitude: number; latitude: number; timezone: string | null; }; images: unknown[]; groups: unknown[]; site_codes: string[]; site_tags: string[]; isOnline: boolean; - formatted_name: string; + formattedName: string; - location_name: string; + locationName: string; - search_name: string; + searchName: string; parish: string; village: string; - sub_county: string; + subCounty: string; city: string; district: string; county: string; region: string; country: string; latitude: number; longitude: number; name: string; network: string; - approximate_latitude: number; + approximateLatitude: number; - approximate_longitude: number; + approximateLongitude: number; bearing_in_radians: number; approximate_distance_in_km: number; lat_long: string; generated_name: string; altitude: number; data_provider: string; description: string; weather_stations: Array<{ code: string; name: string; country: string; longitude: number; latitude: number; timezone: string; distance: number; _id: string; }>; createdAt: string; lastActive: string; grids: Array<{ _id: string; name: string; admin_level: string; visibility: boolean; }>; devices: Array<{ _id: string; visibility: boolean; mobility: boolean; status: string; isPrimaryInLocation: boolean; category: string; isActive: boolean; device_number: number; name: string; createdAt: string; device_codes: string[]; network: string; approximate_distance_in_km: number; bearing_in_radians: number; latitude: number; longitude: number; ISP: string; previous_sites: string[]; groups: string[]; host_id: string | null; cohorts: unknown[]; serial_number: string; isOnline: boolean; lastActive: string; }>; airqlouds: unknown[]; site_category?: { tags: string[]; area_name: string; category: string; highway: string; landuse: string; latitude: number; longitude: number; natural: string; search_radius: number; waterway: string; }; }
10-13: Specify concrete types instead ofunknown[]The properties
images,groups,site_codes, andsite_tagsare currently typed asunknown[]orstring[]. Providing specific types enhances type safety and code readability. Consider defining interfaces or types for these properties.
39-48: Define a type or interface forweather_stationselementsThe
weather_stationsproperty is an array of objects with a defined structure. To improve maintainability, consider extracting this structure into its own interface, such asWeatherStation, and then typingweather_stationsasWeatherStation[].Example:
export interface WeatherStation { code: string; name: string; country: string; longitude: number; latitude: number; timezone: string; distance: number; _id: string; } export interface Site { // ...other properties weather_stations: WeatherStation[]; // ...other properties }
57-82: Consider defining aDeviceinterface for thedevicesarrayThe
devicesproperty contains an array of device objects with a specific structure. Defining aDeviceinterface improves code clarity and reusability.Example:
export interface Device { _id: string; visibility: boolean; mobility: boolean; status: string; isPrimaryInLocation: boolean; category: string; isActive: boolean; device_number: number; name: string; createdAt: string; device_codes: string[]; network: string; approximate_distance_in_km: number; bearing_in_radians: number; latitude: number; longitude: number; ISP: string; previous_sites: string[]; groups: string[]; host_id: string | null; cohorts: unknown[]; serial_number: string; isOnline: boolean; lastActive: string; } export interface Site { // ...other properties devices: Device[]; // ...other properties }netmanager-app/core/apis/analytics.ts (1)
6-17: Well-structured interface with room for improvementThe
DataExportForminterface provides good type safety with union types for constrained values. However, consider adding runtime validation for date strings and documenting the expected format.Consider adding Zod schema validation:
import { z } from 'zod'; const DataExportFormSchema = z.object({ startDateTime: z.string().datetime(), endDateTime: z.string().datetime(), // ... rest of the fields });netmanager-app/core/apis/devices.ts (2)
7-7: Document the purpose of separate axios instanceConsider adding a comment explaining why a separate axios instance with token access is needed.
Add documentation:
// Separate instance for endpoints requiring token authentication const axiosInstanceWithTokenAccess = createAxiosInstance(false);
27-40: Consider refactoring error handling to reduce duplicationThe error handling logic is duplicated across methods. Consider extracting it to a shared utility function.
const handleApiError = (error: unknown, defaultMessage: string) => { const axiosError = error as AxiosError<ErrorResponse>; throw new Error( axiosError.response?.data?.message || defaultMessage ); };netmanager-app/core/hooks/useDevices.ts (2)
12-14: Consider moving ErrorResponse to shared types.The
ErrorResponseinterface could be moved to a shared types file (e.g.,types/api.ts) since it's likely to be reused across different API calls.
21-38: Remove unnecessary type assertion.The
as UseQueryOptionstype assertion can be avoided by properly typing the options object. Consider using generics instead:- } as UseQueryOptions<DevicesSummaryResponse, AxiosError<ErrorResponse>>); + }: UseQueryOptions<DevicesSummaryResponse, AxiosError<ErrorResponse>>);netmanager-app/app/types/devices.ts (2)
111-114: Consider using more specific types for no2 record.Instead of using
Record<string, unknown>forno2, consider defining a specific interface for NO2 measurements similar to pm10 and pm2_5.interface NO2Measurement { value: number; // add other relevant properties }
66-70: Add validation for health tip images.Consider adding URL validation for the
imageproperty inHealthTipinterface and document theis_reading_primaryflag's purpose.interface HealthTip { title: string; description: string; - image: string; + image: URL | string; // URL for runtime validation }Also applies to: 109-110
netmanager-app/components/Analytics/GridDashboard.tsx (2)
184-185: Good accessibility improvement with ARIA labels.Nice addition of aria-label for the chart type toggle button. Consider adding more descriptive labels for the dropdown menu items:
- <DropdownMenuItem onClick={() => setChartType("line")}>Line</DropdownMenuItem> + <DropdownMenuItem onClick={() => setChartType("line")} aria-label="Switch to line chart"> + Line + </DropdownMenuItem>Also applies to: 197-202
60-63: Optimize grid sites object creation.Consider moving the
gridSitesObjcreation to theuseMemohook to prevent unnecessary recalculations:const { activeGrid, gridSitesObj } = useMemo(() => ({ activeGrid: grids.find((grid) => grid._id === gridId), gridSitesObj: activeGrid?.sites.reduce( (acc: Record<string, Site>, curr: Site) => { acc[curr._id] = curr; return acc; }, {} ) || {} }), [grids, gridId, activeGrid?.sites]);Also applies to: 90-96
netmanager-app/components/Analytics/index.tsx (3)
32-39: Consider extracting GeoJSON types to a separate interface file.The type definition for
transformedReadingscould be moved to a dedicated types file, making it reusable across components and improving maintainability.+ // In @/app/types/geojson.ts + export interface GeoJSONFeature { + type: "Feature"; + properties: unknown; + geometry: { type: "Point"; coordinates: [number, number] }; + } + + export interface GeoJSONCollection { + type: string; + features: GeoJSONFeature[]; + } - const [transformedReadings, setTransformedReadings] = useState<{ - type: string; - features: { - type: "Feature"; - properties: unknown; - geometry: { type: "Point"; coordinates: [number, number] }; - }[]; - } | null>(null); + const [transformedReadings, setTransformedReadings] = useState<GeoJSONCollection | null>(null);
81-97: Remove or document commented code.Large blocks of commented code can make maintenance more difficult. If this code is intended for future use, consider adding a TODO comment explaining when it will be needed. Otherwise, it should be removed.
116-118: Consider making the date range configurable.The 5-day range is hardcoded. Consider making this configurable through props or user input for better flexibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
netmanager-app/Dockerfile(1 hunks)netmanager-app/app/types/devices.ts(2 hunks)netmanager-app/app/types/grids.ts(1 hunks)netmanager-app/app/types/sites.ts(1 hunks)netmanager-app/components/Analytics/AnalyticsDropdown.tsx(2 hunks)netmanager-app/components/Analytics/GridDashboard.tsx(8 hunks)netmanager-app/components/Analytics/index.tsx(3 hunks)netmanager-app/components/ui/ErrorBoundary.tsx(2 hunks)netmanager-app/core/apis/analytics.ts(1 hunks)netmanager-app/core/apis/devices.ts(2 hunks)netmanager-app/core/apis/grids.ts(1 hunks)netmanager-app/core/apis/sites.ts(0 hunks)netmanager-app/core/hooks/useDevices.ts(1 hunks)netmanager-app/core/hooks/useGrids.ts(1 hunks)netmanager-app/core/hooks/useReadingsSiteCount.ts(1 hunks)netmanager-app/core/redux/slices/gridsSlice.ts(1 hunks)netmanager-app/lib/utils.ts(2 hunks)
💤 Files with no reviewable changes (1)
- netmanager-app/core/apis/sites.ts
✅ Files skipped from review due to trivial changes (1)
- netmanager-app/components/ui/ErrorBoundary.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
netmanager-app/lib/utils.ts
[error] 31-31: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
netmanager-app/components/Analytics/index.tsx
[error] 77-77: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (11)
netmanager-app/components/Analytics/AnalyticsDropdown.tsx (2)
1-12: Clean and well-organized imports!The imports are properly structured with clear separation between React core, UI components, and type definitions. The "use client" directive correctly indicates this is a client-side component in Next.js.
Line range hint
1-101: Overall, well-implemented component with good TypeScript integration!The component is well-structured, handles both Cohort and Grid scenarios effectively, and follows React best practices. While there are some opportunities for optimization and improved type safety, the current implementation is solid and production-ready.
netmanager-app/core/hooks/useGrids.ts (3)
95-96: Ensure correct parameter usage increateGridApicallIn the
mutationFnofuseCreateGrid, the function correctly passesnewGridtocreateGridApi. Just double-check thatnewGridmatches the expected type and structure required by the API.
85-88:⚠️ Potential issueCorrect the
isLoadingproperty in the returned objectIn the returned object of
useUpdateGridDetails,isLoadingis assignedmutation.isPending, butuseMutationdoes not have anisPendingproperty. The correct property should beisLoading.Apply this change:
return { updateGridDetails: mutation.mutate, - isLoading: mutation.isPending, + isLoading: mutation.isLoading, error: mutation.error, };Likely invalid or redundant comment.
108-109:⚠️ Potential issueCorrect the
isLoadingproperty in the returned objectSimilar to the previous issue,
mutation.isPendingshould bemutation.isLoadingto accurately reflect the loading state.Apply this change:
return { createGrid: mutation.mutate, - isLoading: mutation.isPending, + isLoading: mutation.isLoading, error: mutation.error, };Likely invalid or redundant comment.
netmanager-app/core/apis/analytics.ts (1)
19-27: LGTM! Clean API implementation with proper typingThe function implementation correctly uses the new interface and maintains consistent error handling through axios.
netmanager-app/core/redux/slices/gridsSlice.ts (2)
5-10: Well-defined state interface with proper typingThe
GridsStateinterface clearly defines the shape of the state with appropriate types for each field.
18-38: Type-safe reducer implementation with proper state handlingThe reducers are well implemented with proper type safety using PayloadAction. The error and loading states are managed consistently.
netmanager-app/core/apis/grids.ts (1)
52-64: LGTM! Well-implemented create grid endpointThe
createGridApimethod has proper type safety with theCreateGridtype and consistent error handling.netmanager-app/core/hooks/useDevices.ts (1)
52-57: Clean implementation of the return object.Good use of nullish coalescing for default values and proper combination of loading/error states from both queries.
netmanager-app/core/hooks/useReadingsSiteCount.ts (1)
1-75: Uncomment and implement the hook.The entire file is commented out. If this is a work in progress, consider:
- Implementing the hook if it's needed
- Removing the file if it's no longer required
- Adding a TODO comment explaining why it's commented out
Let's check if this hook is used anywhere:
✅ Verification successful
Remove this unused file
The hook is not imported or used anywhere in the codebase. Keeping commented-out code adds unnecessary maintenance burden. If this functionality is needed in the future, it can be retrieved from version control history.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for useReadingsSiteCount usage rg "useReadingsSiteCount"Length of output: 262
| shape: { | ||
| type: "MultiPolygon" | "Polygon"; | ||
| coordinates: number[][][][]; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clarify the coordinates type in the shape property
The coordinates property is currently typed as number[][][][], which corresponds to a MultiPolygon coordinate structure. However, when the type is "Polygon", the coordinates should be number[][][]. To accurately represent both geometries, consider defining shape as a union type.
Apply this diff to adjust the shape property:
export interface CreateGrid {
name: string;
admin_level: string;
- shape: {
- type: "MultiPolygon" | "Polygon";
- coordinates: number[][][][];
- };
+ shape:
+ | {
+ type: "Polygon";
+ coordinates: number[][][];
+ }
+ | {
+ type: "MultiPolygon";
+ coordinates: number[][][][];
+ };
network: string;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| shape: { | |
| type: "MultiPolygon" | "Polygon"; | |
| coordinates: number[][][][]; | |
| }; | |
| shape: | |
| | { | |
| type: "Polygon"; | |
| coordinates: number[][][]; | |
| } | |
| | { | |
| type: "MultiPolygon"; | |
| coordinates: number[][][][]; | |
| }; |
| mutationFn: async () => await grids.updateGridDetailsApi(gridId), | ||
| onSuccess: () => { | ||
| // Invalidate and refetch the grid details | ||
| queryClient.invalidateQueries({ queryKey: ["gridDetails", gridId] }); | ||
| }, | ||
| onError: (error: AxiosError<ErrorResponse>) => { | ||
| console.error( | ||
| "Failed to update grid details:", | ||
| error.response?.data?.message | ||
| ); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Pass necessary parameters to mutationFn in useUpdateGridDetails
The mutationFn in useUpdateGridDetails calls grids.updateGridDetailsApi(gridId) without any update data. Mutations for updating data typically require both the gridId and the updated data object.
Modify the mutationFn to accept and use the update data:
export const useUpdateGridDetails = (gridId: string) => {
const queryClient = useQueryClient();
const mutation = useMutation({
- mutationFn: async () => await grids.updateGridDetailsApi(gridId),
+ mutationFn: async (updatedGridData: UpdateGridData) =>
+ await grids.updateGridDetailsApi(gridId, updatedGridData),
onSuccess: () => {
// Invalidate and refetch the grid details
queryClient.invalidateQueries({ queryKey: ["gridDetails", gridId] });
},
onError: (error: AxiosError<ErrorResponse>) => {
console.error(
"Failed to update grid details:",
error.response?.data?.message
);
},
});Ensure you define UpdateGridData with the appropriate type for the updated grid data.
Committable suggestion skipped: line range outside the PR's diff.
| export const useGridDetails = (gridId: string) => { | ||
| const mutation = useMutation({ | ||
| mutationFn: async () => await grids.getGridDetailsApi(gridId), | ||
| onSuccess: () => { | ||
| console.log("Grid details fetched successfully"); | ||
| }, | ||
| onError: (error: AxiosError<ErrorResponse>) => { | ||
| console.error( | ||
| "Failed to fetch grid details:", | ||
| error.response?.data?.message | ||
| ); | ||
| }, | ||
| }); | ||
|
|
||
| return { | ||
| getGridDetails: mutation.mutate || [], | ||
| isLoading: mutation.isPending, | ||
| error: mutation.error as Error | null, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Use useQuery instead of useMutation for fetching data in useGridDetails
The useGridDetails hook is intended to fetch data, but it uses useMutation, which is designed for create/update/delete operations. To properly fetch data and handle caching, consider using useQuery.
Apply this diff to adjust the hook:
-export const useGridDetails = (gridId: string) => {
- const mutation = useMutation({
- mutationFn: async () => await grids.getGridDetailsApi(gridId),
- onSuccess: () => {
- console.log("Grid details fetched successfully");
- },
- onError: (error: AxiosError<ErrorResponse>) => {
- console.error(
- "Failed to fetch grid details:",
- error.response?.data?.message
- );
- },
- });
-
- return {
- getGridDetails: mutation.mutate || [],
- isLoading: mutation.isPending,
- error: mutation.error as Error | null,
- };
+export const useGridDetails = (gridId: string) => {
+ const { data, isLoading, error } = useQuery({
+ queryKey: ["gridDetails", gridId],
+ queryFn: () => grids.getGridDetailsApi(gridId),
+ onSuccess: () => {
+ console.log("Grid details fetched successfully");
+ },
+ onError: (error: AxiosError<ErrorResponse>) => {
+ console.error(
+ "Failed to fetch grid details:",
+ error.response?.data?.message
+ );
+ },
+ });
+
+ return {
+ gridDetails: data,
+ isLoading,
+ error,
+ };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const useGridDetails = (gridId: string) => { | |
| const mutation = useMutation({ | |
| mutationFn: async () => await grids.getGridDetailsApi(gridId), | |
| onSuccess: () => { | |
| console.log("Grid details fetched successfully"); | |
| }, | |
| onError: (error: AxiosError<ErrorResponse>) => { | |
| console.error( | |
| "Failed to fetch grid details:", | |
| error.response?.data?.message | |
| ); | |
| }, | |
| }); | |
| return { | |
| getGridDetails: mutation.mutate || [], | |
| isLoading: mutation.isPending, | |
| error: mutation.error as Error | null, | |
| }; | |
| }; | |
| export const useGridDetails = (gridId: string) => { | |
| const { data, isLoading, error } = useQuery({ | |
| queryKey: ["gridDetails", gridId], | |
| queryFn: () => grids.getGridDetailsApi(gridId), | |
| onSuccess: () => { | |
| console.log("Grid details fetched successfully"); | |
| }, | |
| onError: (error: AxiosError<ErrorResponse>) => { | |
| console.error( | |
| "Failed to fetch grid details:", | |
| error.response?.data?.message | |
| ); | |
| }, | |
| }); | |
| return { | |
| gridDetails: data, | |
| isLoading, | |
| error, | |
| }; | |
| }; |
| await axiosInstanceWithTokenAccess.get<DevicesSummaryResponse>( | ||
| `${DEVICES_MGT_URL}/readings/map` |
There was a problem hiding this comment.
💡 Codebase verification
Type mismatch detected in map readings endpoint
The endpoint /readings/map is using DevicesSummaryResponse type but the corresponding query hook uses ReadingsApiResponse. This inconsistency should be fixed by using the correct type ReadingsApiResponse in the API call at netmanager-app/core/apis/devices.ts.
🔗 Analysis chain
Verify response type compatibility
The DevicesSummaryResponse type is being used for map readings. Verify if the response structure is identical.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the response structure matches DevicesSummaryResponse
curl -s "${DEVICES_MGT_URL}/readings/map" | jq -r 'keys'
Length of output: 36
Script:
#!/bin/bash
# Find DevicesSummaryResponse type definition
ast-grep --pattern 'type DevicesSummaryResponse = $_'
# Find interface definition if it's defined as an interface
ast-grep --pattern 'interface DevicesSummaryResponse $_'
# Search for test files or mocks containing readings/map
rg -g '*.test.*' -g '*.mock.*' -g '*.spec.*' "readings/map" -A 5 -B 5
# Find other usages of the endpoint
rg "readings/map" -A 2 -B 2
Length of output: 1979
Script:
#!/bin/bash
# Find files containing DevicesSummaryResponse
rg "DevicesSummaryResponse" -A 5 -B 5
# Find potential type files
fd "types?.ts" --exec grep -l "DevicesSummary"
# Look for response handling
rg "readings/map.*response" -A 5 -B 5
Length of output: 5676
| updateGridDetailsApi: async (gridId: string) => { | ||
| try { | ||
| const response = await axiosInstance.put( | ||
| `${DEVICES_MGT_URL}/grids/${gridId}` | ||
| ); | ||
| return response.data; | ||
| } catch (error) { | ||
| const axiosError = error as AxiosError<ErrorResponse>; | ||
| throw new Error( | ||
| axiosError.response?.data?.message || "Failed to update grid details" | ||
| ); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Add type safety to update grid details endpoint
The updateGridDetailsApi method is missing a type for the request body.
Add proper typing:
interface UpdateGridRequest {
// Add required fields here
}
updateGridDetailsApi: async (gridId: string, data: UpdateGridRequest) => {
try {
const response = await axiosInstance.put(
`${DEVICES_MGT_URL}/grids/${gridId}`,
data
);
return response.data;
} catch (error) {
const axiosError = error as AxiosError<ErrorResponse>;
throw new Error(
axiosError.response?.data?.message || "Failed to update grid details"
);
}
}| recentEventsData={activeGrid.sites.map((site) => ({ | ||
| properties: { | ||
| site_id: site._id, | ||
| pm2_5: { value: site.approximate_longitude }, | ||
| }, | ||
| }))} | ||
| grids={grids as Grid[]} |
There was a problem hiding this comment.
Fix incorrect data mapping in GridDashboard props.
The code is using site.approximate_longitude as the pm2_5 value, which appears to be incorrect. This could lead to invalid data visualization.
Please verify the correct property mapping for the PM2.5 value.
| } catch (error) { | ||
| toast({ | ||
| title: "Error", | ||
| description: "Error downloading data", | ||
| description: error, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error message handling in toast notification.
Passing the error object directly to the toast description could expose sensitive information. Consider extracting and displaying a user-friendly message instead.
- description: error,
+ description: error instanceof Error ? error.message : 'Failed to download data. Please try again.',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| description: error, | |
| description: error instanceof Error ? error.message : 'Failed to download data. Please try again.', |
netmanager-app/Dockerfile
Outdated
| # Use the official Node.js image as a base | ||
| FROM node:18 | ||
|
|
||
| # Set the working directory | ||
| WORKDIR /app | ||
|
|
||
| # Copy package.json and package-lock.json | ||
| COPY package*.json ./ | ||
|
|
||
| # Install dependencies | ||
| RUN npm install | ||
|
|
||
| # Copy the rest of the application code | ||
| COPY . . | ||
|
|
||
| # Expose the application port | ||
| EXPOSE 3000 | ||
|
|
||
| # Command to run the application | ||
| CMD ["npm", "start"] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add .dockerignore file.
Create a .dockerignore file to exclude unnecessary files from the build context:
.git
.gitignore
node_modules
.next
.env*
npm-debug.log*
README.md
.vscode
coverage
🛠️ Refactor suggestion
Enhance Dockerfile with production best practices.
Consider the following improvements:
- Add multi-stage builds to reduce final image size
- Include environment variable handling
- Add health checks
- Specify a non-root user for security
Here's an optimized version:
# Build stage
- FROM node:18
+ FROM node:18-alpine AS builder
WORKDIR /app
COPY package*.json ./
- RUN npm install
+ RUN npm ci
COPY . .
+ RUN npm run build
+ # Production stage
+ FROM node:18-alpine
+ WORKDIR /app
+ COPY --from=builder /app/.next ./.next
+ COPY --from=builder /app/node_modules ./node_modules
+ COPY --from=builder /app/package*.json ./
+ # Add healthcheck
+ HEALTHCHECK --interval=30s --timeout=3s \
+ CMD wget --no-verbose --tries=1 --spider http://localhost:3000/ || exit 1
+ # Use non-root user
+ USER node
EXPOSE 3000
- CMD ["npm", "start"]
+ CMD ["npm", "run", "start"]Committable suggestion skipped: line range outside the PR's diff.
netmanager-app/lib/utils.ts
Outdated
| coordinates: (coordinateGetter && coordinateGetter(feature)) || [ | ||
| feature[longitude], | ||
| feature[latitude], | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add safety checks for coordinate access.
Direct property access using brackets notation could fail if the coordinates are undefined. Consider adding safety checks:
- coordinates: (coordinateGetter && coordinateGetter(feature)) || [
- feature[longitude],
- feature[latitude],
- ],
+ coordinates: (coordinateGetter && coordinateGetter(feature)) || [
+ Number(feature[longitude]) || 0,
+ Number(feature[latitude]) || 0,
+ ],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| coordinates: (coordinateGetter && coordinateGetter(feature)) || [ | |
| feature[longitude], | |
| feature[latitude], | |
| ], | |
| coordinates: (coordinateGetter && coordinateGetter(feature)) || [ | |
| Number(feature[longitude]) || 0, | |
| Number(feature[latitude]) || 0, | |
| ], |
🧰 Tools
🪛 Biome (1.9.4)
[error] 31-31: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
netmanager-app/lib/utils.ts
Outdated
| data.map((feature) => { | ||
| if (filter(feature)) { | ||
| features.push({ | ||
| type: "Feature", | ||
| properties: { ...rest, ...feature }, | ||
| geometry: { | ||
| type: "Point", | ||
| coordinates: (coordinateGetter && coordinateGetter(feature)) || [ | ||
| feature[longitude], | ||
| feature[latitude], | ||
| ], | ||
| }, | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace map() with forEach() for side effects.
The map() function is being used for its side effects (pushing to array) rather than transformation. Use forEach() instead.
- data.map((feature) => {
+ data.forEach((feature) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| data.map((feature) => { | |
| if (filter(feature)) { | |
| features.push({ | |
| type: "Feature", | |
| properties: { ...rest, ...feature }, | |
| geometry: { | |
| type: "Point", | |
| coordinates: (coordinateGetter && coordinateGetter(feature)) || [ | |
| feature[longitude], | |
| feature[latitude], | |
| ], | |
| }, | |
| }); | |
| } | |
| }); | |
| data.forEach((feature) => { | |
| if (filter(feature)) { | |
| features.push({ | |
| type: "Feature", | |
| properties: { ...rest, ...feature }, | |
| geometry: { | |
| type: "Point", | |
| coordinates: (coordinateGetter && coordinateGetter(feature)) || [ | |
| feature[longitude], | |
| feature[latitude], | |
| ], | |
| }, | |
| }); | |
| } | |
| }); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 31-31: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
This pull request includes multiple changes to the
netmanager-appproject, focusing on Dockerfile setup, TypeScript type definitions, and code formatting improvements. The most important changes are summarized below:Dockerfile Setup:
Dockerfilefor thenetmanager-appto set up the application environment using Node.js, including copying necessary files, installing dependencies, and defining the command to run the application.TypeScript Type Definitions:
netmanager-app/app/types/devices.tsto include new interfaces forHealthTip,AQIRange,AQIRanges,Averages,Measurement, andReadingsApiResponse, enhancing the structure of device-related data.netmanager-app/app/types/grids.tsto importSitefromsites.tsand added a newCreateGridinterface, improving the type definitions for grid-related data. [1] [2]Siteinterface innetmanager-app/app/types/sites.tsto include additional fields such asnearest_tahmo_station,images,groups,site_codes, and more, providing a more comprehensive representation of site data.Code Formatting:
AnalyticsDropdown.tsx, including converting single quotes to double quotes, adding semicolons, and adjusting indentation for better readability. [1] [2] [3]GridDashboard.tsxby breaking down long lines, adding consistent indentation, and ensuring proper use of semicolons and brackets. [1] [2] [3] [4] [5] [6] [7]index.tsxto use consistent double quotes, added missing semicolons, and organized imports for better code clarity.