[Platform] Enhancements on JSON and Map errors #2548
Conversation
📝 WalkthroughWalkthroughThis pull request refactors several components and utilities to improve the robustness of localStorage data retrieval and JSON parsing. The changes introduce try-catch blocks across multiple modules to handle parsing errors, log issues, and clean up invalid data. Additionally, a new custom hook centralizes user data retrieval from localStorage, while key components, such as the dropdowns, modals, sidebars, and settings pages, now conditionally execute further logic only if the required data has been successfully retrieved and validated. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Component
participant LS as LocalStorage
participant T as Try-Catch Block
participant L as Logger
C->>LS: Retrieve stored value (JSON string)
LS-->>C: Return value (valid/invalid)
C->>T: Attempt JSON.parse(value)
alt Parsing Successful
T-->>C: Return parsed data
C->>C: Continue normal execution
else Parsing Fails
T->>L: Log parsing error
C->>LS: Remove invalid value
C-->>C: Exit or return default value
end
Possibly related PRs
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🪧 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 (
|
OchiengPaul442
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: 1
🔭 Outside diff range comments (1)
src/platform/src/pages/settings/Tabs/Profile.jsx (1)
492-494:⚠️ Potential issuePotential safety issue with avatar display.
If the user's first or last name is undefined or an empty string, attempting to access the first character could cause runtime errors. Consider adding a safety check:
- {userData.firstName[0] + userData.lastName[0]} + {(userData.firstName?.[0] || '') + (userData.lastName?.[0] || '')}
🧹 Nitpick comments (4)
src/platform/src/common/components/SideBar/AuthenticatedSidebar.jsx (1)
75-95: Robust localStorage error handling - excellent addition.The added try-catch blocks for parsing localStorage items prevent potential runtime errors from invalid JSON data. This approach is consistent with modern error handling best practices and improves application stability.
Consider extending this implementation to remove invalid localStorage items when parsing fails. This would prevent repeated parsing attempts of corrupted data:
} catch (error) { console.error( 'Error parsing "collocationOpen" from localStorage:', error, ); + localStorage.removeItem('collocationOpen'); }src/platform/src/common/components/SideBar/SideBarDrawer.jsx (1)
28-42: Improved state initialization with robust error handling.The enhanced state initialization function correctly handles potential errors when retrieving data from localStorage. Using a functional state initializer with try-catch is the right approach here.
For consistency with the error handling pattern established in other components, consider removing the invalid localStorage item:
console.error( 'Error parsing "collocationOpen" from localStorage:', error, ); + localStorage.removeItem('collocationOpen'); return false;src/platform/src/common/components/Dropdowns/OrganizationDropdown.jsx (1)
41-65: Strong error handling and data validation.This is a comprehensive enhancement to the activeGroup handling. The implementation now:
- Safely parses the JSON with try-catch
- Validates that the parsed object has the expected structure
- Provides meaningful error logging
- Cleans up invalid data from localStorage
A couple of minor improvements:
Consider using optional chaining for the property checks:
- if (defaultGroup && defaultGroup.grp_title) { + if (defaultGroup?.grp_title) {- if (defaultGroup && defaultGroup.grp_title) { + if (defaultGroup?.grp_title) {🧰 Tools
🪛 Biome (1.9.4)
[error] 51-51: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/platform/src/common/components/Settings/Teams/InviteForm.jsx (1)
80-107: Excellent error handling improvements for localStorage data retrieval.The addition of comprehensive error handling for retrieving and parsing the active group from localStorage significantly improves the component's resilience. The structured approach with existence checks before parsing, try-catch blocks for JSON parsing, and detailed error messages aligns well with the PR objective.
Consider uncommenting line 104 to remove invalid items from localStorage, which would prevent repeated parsing errors on subsequent renders:
- // localStorage.removeItem('activeGroup'); + localStorage.removeItem('activeGroup');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/platform/src/common/components/Dropdowns/OrganizationDropdown.jsx(1 hunks)src/platform/src/common/components/Modal/dataDownload/modules/AddLocations.jsx(1 hunks)src/platform/src/common/components/Modal/dataDownload/modules/SelectMore.jsx(1 hunks)src/platform/src/common/components/Settings/Teams/InviteForm.jsx(2 hunks)src/platform/src/common/components/SideBar/AuthenticatedSidebar.jsx(1 hunks)src/platform/src/common/components/SideBar/SideBarDrawer.jsx(1 hunks)src/platform/src/core/utils/protectedRoute.js(2 hunks)src/platform/src/pages/Home/index.jsx(2 hunks)src/platform/src/pages/settings/Tabs/OrganizationProfile.jsx(5 hunks)src/platform/src/pages/settings/Tabs/Profile.jsx(7 hunks)src/platform/src/pages/settings/index.jsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/platform/src/common/components/Dropdowns/OrganizationDropdown.jsx
[error] 51-51: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 69-69: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-push-deploy-analytics-platform-preview
🔇 Additional comments (23)
src/platform/src/pages/Home/index.jsx (2)
61-81: Well-structured custom hook for user data retrieval.Creating the
useUserDatahook is an excellent refactoring that centralizes localStorage access logic with proper error handling. The implementation correctly:
- Checks for browser environment
- Validates storage item existence
- Handles JSON parsing errors
- Uses memoization for performance optimization
97-97: Clean integration of the new custom hook.The implementation correctly replaces direct localStorage access with the new hook, improving code readability and maintainability.
src/platform/src/common/components/Modal/dataDownload/modules/AddLocations.jsx (2)
62-75: Enhanced user ID retrieval with proper validation and error handling.The improved implementation correctly:
- Checks for the existence of the localStorage item before parsing
- Uses try-catch for safe JSON parsing
- Provides detailed error logs
- Uses optional chaining to safely access nested properties
This implementation aligns well with the other localStorage access improvements throughout the application, creating a consistent approach to error handling.
149-158: The userID check works well with the improved user data retrieval.Now that the
userIDvalue could be null when retrieval fails, this conditional check provides an appropriate error message to the user.src/platform/src/common/components/Modal/dataDownload/modules/SelectMore.jsx (1)
62-75: Great error handling improvement!The addition of proper error handling when retrieving and parsing user data from localStorage is an excellent enhancement. This change helps prevent potential runtime errors if the stored JSON is malformed or invalid.
The implementation now:
- Checks if the data exists before attempting to parse
- Uses try-catch to safely handle parsing errors
- Provides meaningful error logging
- Returns a consistent null value when parsing fails
src/platform/src/core/utils/protectedRoute.js (2)
38-49: Good enhancement to error handling in withPermission.The improved error handling for JSON parsing is a valuable addition. Initializing
parsedUserGroupas an empty object first and then carefully parsing the storedUserGroup with proper error handling prevents potential crashes from malformed JSON data.
69-78: Robust error handling for checkAccess.This is a good improvement to the checkAccess function. Initializing
currentRoleas null and safely parsing the stored group object with proper error handling ensures the function remains resilient against localStorage parsing errors.src/platform/src/common/components/Dropdowns/OrganizationDropdown.jsx (1)
73-73: Good dependency array update.Adding
isFetchingActiveGroupto the dependency array is important to ensure the effect runs when the fetching state changes.src/platform/src/pages/settings/index.jsx (3)
37-50: Improved error handling for inactive groups.Good enhancement with early returns and proper error handling when parsing the activeGroup. The code now:
- Returns early if no stored group exists
- Safely parses JSON with proper error logging
- Sets loading state appropriately in all failure cases
52-64: Better state management with parsed data.Using the already parsed activeGroup object directly in the state and for extracting properties is more efficient than parsing multiple times. The fallback to an empty array for role permissions is also a good defensive programming practice.
66-76: More readable promise chain.Refactoring the API call to use promise chaining (.then/.catch/.finally) makes the code more readable than the nested try-catch pattern. Also, adding
dispatchto the dependency array ensures the effect runs correctly when the dispatch function changes.src/platform/src/common/components/Settings/Teams/InviteForm.jsx (4)
64-65: Improved validation check for empty email input.The change from explicit empty string check to a falsy check is more robust, as it will catch not only empty strings but also
nullandundefinedvalues.
109-117: Added validation for the parsed activeGroup object.The additional check for
activeGroup._idensures that the component only proceeds when the group data is not only parseable but also contains the required properties.
119-144: Structured error handling for the invitation process.The invitation API call is now properly placed within the validation flow, ensuring that it only executes when valid data is available. The error handling in the catch block provides clear feedback to users.
145-153: Clearer error messaging for invalid emails.The error messaging for invalid emails has been simplified and made more explicit, improving the user experience.
src/platform/src/pages/settings/Tabs/OrganizationProfile.jsx (3)
133-153: Robust error handling in handleSubmit.The structured approach to safely retrieving and validating the active group ID before proceeding with the update is well-implemented. This prevents potential runtime errors from invalid localStorage data.
286-361: Comprehensive refactoring of handleProfileImageUpdate.The function has been significantly improved with:
- A clear sequential flow of operations
- Proper error handling at each step
- Safe retrieval and validation of the active group ID
- Functional updates for state management
This structured approach makes the code more robust and maintainable.
365-389: Enhanced error handling in deleteProfileImage.The function now safely retrieves and validates the group ID before attempting to delete the profile image, preventing potential runtime errors from invalid data.
src/platform/src/pages/settings/Tabs/Profile.jsx (5)
90-128: Improved client-side checking and user data retrieval.The addition of client-side checking prevents server-side rendering issues, and the safer approach to retrieving and parsing user data from localStorage significantly improves robustness. The structured error handling with try-catch blocks and conditional execution is well-implemented.
138-157: Enhanced error handling in handleSubmit.The safe retrieval and validation of the logged user before proceeding with the submit operation prevents potential runtime errors from invalid localStorage data.
209-248: Improved user data handling in handleCancel.The function now safely retrieves and parses user data, providing a fallback to empty fields if no valid user is found. This prevents potential issues when resetting the form.
321-391: Well-structured handleProfileImageUpdate function.The comprehensive refactoring with clear sequential steps, proper error handling, and functional updates for state management significantly improves the robustness of the profile image update process.
394-442: Enhanced error handling in deleteProfileImage.The function now safely retrieves and validates the user ID before attempting to delete the profile image, preventing potential runtime errors.
| let activeGroupId = null; | ||
| const storedActiveGroup = localStorage.getItem('activeGroup'); | ||
| const storedActiveGroupID = | ||
| storedActiveGroup && JSON.parse(storedActiveGroup)._id; | ||
|
|
||
| // get group information | ||
| try { | ||
| dispatch(fetchGroupInfo(storedActiveGroupID)); | ||
| } catch (error) { | ||
| console.error(`Error fetching group info: ${error}`); | ||
| } finally { | ||
| setLoading(false); | ||
| if (storedActiveGroup) { | ||
| try { | ||
| const parsedActiveGroup = JSON.parse(storedActiveGroup); | ||
| activeGroupId = parsedActiveGroup?._id || null; | ||
| } catch (error) { | ||
| console.error('Error parsing "activeGroup" from localStorage:', error); | ||
| } | ||
| } | ||
| }, []); | ||
|
|
||
| // If we have a valid ID, fetch group info | ||
| if (activeGroupId) { | ||
| try { | ||
| dispatch(fetchGroupInfo(activeGroupId)); | ||
| } catch (error) { | ||
| console.error(`Error fetching group info: ${error}`); | ||
| } | ||
| } | ||
|
|
||
| setLoading(false); | ||
| }, [dispatch]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improved initialization and error handling in useEffect.
The safer approach to retrieving and parsing the active group ID is a significant improvement. The code now initializes variables properly, checks for existence, safely parses JSON with error handling, and conditionally executes further logic.
However, there's a potential issue with the loading state:
- setLoading(false);
+ // Only set loading to false here if we're not fetching group info
+ if (!activeGroupId) {
+ setLoading(false);
+ }Since fetchGroupInfo is asynchronous, setting setLoading(false) unconditionally might prematurely indicate that loading is complete.
📝 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.
| let activeGroupId = null; | |
| const storedActiveGroup = localStorage.getItem('activeGroup'); | |
| const storedActiveGroupID = | |
| storedActiveGroup && JSON.parse(storedActiveGroup)._id; | |
| // get group information | |
| try { | |
| dispatch(fetchGroupInfo(storedActiveGroupID)); | |
| } catch (error) { | |
| console.error(`Error fetching group info: ${error}`); | |
| } finally { | |
| setLoading(false); | |
| if (storedActiveGroup) { | |
| try { | |
| const parsedActiveGroup = JSON.parse(storedActiveGroup); | |
| activeGroupId = parsedActiveGroup?._id || null; | |
| } catch (error) { | |
| console.error('Error parsing "activeGroup" from localStorage:', error); | |
| } | |
| } | |
| }, []); | |
| // If we have a valid ID, fetch group info | |
| if (activeGroupId) { | |
| try { | |
| dispatch(fetchGroupInfo(activeGroupId)); | |
| } catch (error) { | |
| console.error(`Error fetching group info: ${error}`); | |
| } | |
| } | |
| setLoading(false); | |
| }, [dispatch]); | |
| let activeGroupId = null; | |
| const storedActiveGroup = localStorage.getItem('activeGroup'); | |
| if (storedActiveGroup) { | |
| try { | |
| const parsedActiveGroup = JSON.parse(storedActiveGroup); | |
| activeGroupId = parsedActiveGroup?._id || null; | |
| } catch (error) { | |
| console.error('Error parsing "activeGroup" from localStorage:', error); | |
| } | |
| } | |
| // If we have a valid ID, fetch group info | |
| if (activeGroupId) { | |
| try { | |
| dispatch(fetchGroupInfo(activeGroupId)); | |
| } catch (error) { | |
| console.error(`Error fetching group info: ${error}`); | |
| } | |
| } | |
| // Only set loading to false here if we're not fetching group info | |
| if (!activeGroupId) { | |
| setLoading(false); | |
| } | |
| }, [dispatch]); |
|
New next-platform changes available for preview here |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/platform/src/common/components/Modal/dataDownload/components/CustomFields.jsx (2)
28-28: Consider adding the same safety check to the organization store method.For consistency with the other methods, consider adding the same safety check to the
storemethod of theorganizationfield.- store: (value) => value, + store: (value) => { + // Apply the same safety pattern for consistency + const safeValue = typeof value === 'string' ? value : ''; + return safeValue; + },
10-14: Note the existing validation in formatName function.The
formatNamefunction already has input validation that returns the original value if it's not a string. Your new safety checks add an extra layer of protection by ensuring a string is always passed toformatName. This belt-and-suspenders approach is good for critical code paths that must be resilient against unpredictable input.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/platform/src/common/components/Modal/dataDownload/components/CustomFields.jsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-analytics-platform-changes
- GitHub Check: build-push-deploy-analytics-platform-preview
🔇 Additional comments (2)
src/platform/src/common/components/Modal/dataDownload/components/CustomFields.jsx (2)
23-27: Good addition of safety checks for null/undefined values.Adding type checking before working with strings is a robust practice that prevents runtime errors when dealing with potentially undefined data from localStorage or API responses. This defensive coding approach aligns well with the PR objectives for improving JSON error handling.
31-40: Consistent error handling implemented across default formatters.The consistent application of the same safety pattern to both display and store methods in the default formatter shows a thorough approach to defensive programming. The comments also clearly explain the purpose of these checks, which improves code maintainability.
|
New next-platform changes available for preview here |
OchiengPaul442
changed the title
OchiengPaul442
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/platform/src/common/components/Map/AirQoMap.jsx (1)
250-257: Safer window resize handling.The try-catch block around resize operations prevents errors during window resize events from affecting the application.
Consider using optional chaining as suggested by static analysis:
- try { - if (mapRef.current && mapRef.current.isStyleLoaded()) { - mapRef.current.resize(); - } - } catch (error) { + try { + if (mapRef.current?.isStyleLoaded()) { + mapRef.current.resize(); + } + } catch (error) {🧰 Tools
🪛 Biome (1.9.4)
[error] 252-252: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/platform/src/common/components/Map/components/MapNodes.js (3)
142-161: Switch statement refactoring improves readability.Replacing nested conditionals with a switch statement makes the code more maintainable and easier to follow. The default case returns an empty string, but consider returning a more descriptive message instead.
default: - return ''; + return 'Air quality information unavailable';
296-300: Consider adding try-catch for toFixed() call.While you're validating the data type above, there's still a direct call to
feature.properties.pm2_5.toFixed(2)on line 323 that could potentially throw if the value changes between validation and usage.- <div class="text-[#3C4555] font-semibold text-sm leading-4">${feature.properties.pm2_5.toFixed(2)} µg/m³</div> + <div class="text-[#3C4555] font-semibold text-sm leading-4">${ + typeof feature.properties.pm2_5 === 'number' + ? feature.properties.pm2_5.toFixed(2) + : 'N/A' + } µg/m³</div>
248-249: Consider using optional chaining for nested properties.While using the OR operator works for providing fallbacks, optional chaining would be safer when accessing potentially undefined nested properties.
- const firstAQIValue = firstAQI.pm2_5 || firstAQI.no2 || firstAQI.pm10; - const secondAQIValue = secondAQI.pm2_5 || secondAQI.no2 || secondAQI.pm10; + const firstAQIValue = firstAQI?.pm2_5 || firstAQI?.no2 || firstAQI?.pm10; + const secondAQIValue = secondAQI?.pm2_5 || secondAQI?.no2 || secondAQI?.pm10;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/platform/src/common/components/Map/AirQoMap.jsx(9 hunks)src/platform/src/common/components/Map/components/LayerModal.js(5 hunks)src/platform/src/common/components/Map/components/MapNodes.js(6 hunks)src/platform/src/common/components/Map/components/sidebar/components/CountryList.jsx(4 hunks)src/platform/src/common/components/Map/components/sidebar/components/LocationAlertCard.jsx(2 hunks)src/platform/src/common/components/Map/components/sidebar/components/LocationCards.jsx(1 hunks)src/platform/src/common/components/Map/components/sidebar/components/PollutantCard.jsx(1 hunks)src/platform/src/common/components/Map/components/sidebar/components/Predictions.jsx(1 hunks)src/platform/src/common/components/Map/components/sidebar/components/SearchResultsSkeleton.jsx(1 hunks)src/platform/src/common/components/Map/components/sidebar/components/Sections.jsx(0 hunks)src/platform/src/common/components/Map/components/sidebar/components/SidebarHeader.jsx(0 hunks)src/platform/src/common/components/Map/components/sidebar/components/TabSelector.jsx(0 hunks)src/platform/src/common/components/Map/components/sidebar/index.jsx(11 hunks)src/platform/src/common/components/Map/functions/useLocationBoundaries.jsx(3 hunks)src/platform/src/common/components/Map/functions/useMapControls.jsx(10 hunks)src/platform/src/common/components/Map/functions/useMapData.jsx(12 hunks)
💤 Files with no reviewable changes (3)
- src/platform/src/common/components/Map/components/sidebar/components/TabSelector.jsx
- src/platform/src/common/components/Map/components/sidebar/components/Sections.jsx
- src/platform/src/common/components/Map/components/sidebar/components/SidebarHeader.jsx
✅ Files skipped from review due to trivial changes (3)
- src/platform/src/common/components/Map/components/sidebar/index.jsx
- src/platform/src/common/components/Map/components/sidebar/components/LocationAlertCard.jsx
- src/platform/src/common/components/Map/components/sidebar/components/Predictions.jsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/platform/src/common/components/Map/AirQoMap.jsx
[error] 252-252: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-push-deploy-analytics-platform-preview
🔇 Additional comments (54)
src/platform/src/common/components/Map/components/sidebar/components/SearchResultsSkeleton.jsx (1)
7-12: Nice code improvement.The change from
Array(numElements).fill().map(...)toArray.from({ length: numElements }).map(...)is a good refinement. This approach eliminates the need for the intermediate.fill()call while achieving the same result of creating an array with a specific length for mapping. Both methods generate identical output, but the updated implementation is more direct and slightly more efficient.While this change doesn't directly relate to the PR's focus on JSON error handling, it's a worthwhile code quality improvement that makes the component slightly more elegant.
src/platform/src/common/components/Map/components/sidebar/components/CountryList.jsx (6)
3-3: Good addition of PropTypes!Adding PropTypes for type checking improves the code's robustness and documentation.
18-20: Improved condition for empty data handling.The simplified check for array data is more explicit and direct compared to the previous implementation.
27-27: Clean defensive check.This guard clause correctly prevents processing when a country is undefined.
41-41: Direct length check is cleaner.Using
sortedData.length === 0for the skeleton loading condition is a cleaner approach.
82-84: Error handling for image loading is concise.The error handling for flag images is implemented correctly, setting a default image if loading fails.
96-101: Well-structured PropTypes definition.The PropTypes are defined correctly with appropriate requirements for each prop.
src/platform/src/common/components/Map/components/sidebar/components/PollutantCard.jsx (3)
18-26: Improved date handling with defensive checks.Breaking down the date comparison logic into separate variables improves readability and adds robustness with null checks for prediction time.
31-31: Added important optional chaining.The optional chaining for
pm2_5?.value?.toFixed(2)prevents potential runtime errors when these values might be undefined.
35-35: Consistent optional chaining for PM2.5 values.The same safety improvement has been correctly applied to this instance as well.
src/platform/src/common/components/Map/functions/useLocationBoundaries.jsx (9)
8-8: Added request cancellation support.Adding axios CancelToken enables clean cancellation of in-flight requests when the component unmounts, preventing memory leaks.
10-12: Good defensive check for map reference.Early return if the map reference isn't available prevents potential errors.
14-28: Enhanced error handling for map operations.Wrapping the removeLayer and removeSource operations in try-catch blocks prevents unhandled exceptions that could break the component when manipulating map layers.
42-42: Added cancelToken to request.Attaching the cancelToken to the axios request allows for proper cleanup.
66-66: Simplified conditional zoom logic.The simplified condition for zoom level is more readable and removes redundant checks.
69-84: Better event handling with proper cleanup.Extracting the zoomHandler function and adding a cleanup function ensures the event listener is properly managed, preventing potential memory leaks.
87-91: Comprehensive error handling for request cancellation.Differentiating between cancellation errors and other errors improves logging clarity.
97-102: Proper cleanup implementation.The cleanup mechanism correctly handles both the axios request cancellation and any cleanup function returned from the fetchLocationBoundaries call.
103-103: Updated effect dependencies.Adding mapRef and setLoading to the dependency array ensures the effect runs correctly when these dependencies change.
src/platform/src/common/components/Map/AirQoMap.jsx (3)
51-65: Added error handling for URL parameter parsing.Wrapping the URL parameter parsing in a try-catch block prevents potential runtime errors from breaking the application if malformed parameters are provided.
104-145: Robust error handling for map initialization.The added try-catch blocks during map initialization and within the load event handler provide better error isolation and logging, preventing unhandled exceptions from crashing the application.
164-175: Protected map style updates with try-catch.Error handling around map style updates prevents potential problems when changing the map style or flying to a new location.
src/platform/src/common/components/Map/components/sidebar/components/LocationCards.jsx (3)
59-60: Potential null reference risks.The optional chaining operator (
?.) has been removed fromgrid.descriptionandgrid.region. While the component does filter out null IDs and has a check for array validity, there's no guarantee these specific properties exist on each grid object.Consider retaining optional chaining here to prevent potential runtime errors:
- capitalizeAllText(formatDescription(grid.description)) + capitalizeAllText(formatDescription(grid?.description)) - capitalizeAllText(formatRegion(grid.region)) + capitalizeAllText(formatRegion(grid?.region))
62-64: Potential null reference risks.The optional chaining operator (
?.) has been removed fromgrid.place_idandgrid.search_name. While the code later uses these values conditionally, removing the optional chaining could lead to runtime errors if any of these properties are undefined.Consider retaining optional chaining for safer property access:
- grid.place_id + grid?.place_id - grid.search_name?.split(',')[0] + grid?.search_name?.split(',')[0]
69-69: Key handling looks good.The key construction uses fallback with logical OR, ensuring a unique key even if one property is missing.
src/platform/src/common/components/Map/components/LayerModal.js (8)
1-9: Good performance optimization with React.memo.Wrapping the Option component with React.memo will prevent unnecessary re-renders when props haven't changed. This aligns with good React practices for optimization.
10-18: Well-implemented error handling.The addition of a try-catch block in the click handler provides robust error handling, ensuring that UI interactions remain smooth even if the callback encounters an issue. Good practice to log the error for debugging.
20-29: Clean template literal formatting for class names.Moving from concatenated strings to template literals improves readability and maintainability. The multi-line formatting makes the conditional classes much easier to understand.
38-47: Good optimization with Image priority.Adding the priority attribute for selected images ensures they load with higher priority, improving the perceived performance when users make selections.
54-54: Helpful displayName addition.Setting the displayName property explicitly helps with debugging, especially in React DevTools and error messages.
89-97: Comprehensive error handling in handleApply.The try-catch block around the modal's apply functionality properly handles potential errors during state changes and callback execution. The error logging is clear and would help with debugging issues.
84-87: Simplified useEffect logic.The useEffect implementation is now more concise and readable with direct conditional assignments rather than unnecessarily nested blocks.
172-172: Helpful displayName addition for LayerModal.As with the Option component, explicitly setting displayName helps with debugging and component identification.
src/platform/src/common/components/Map/functions/useMapData.jsx (5)
29-29: Simplified function signature.Removing the unused
mapStyleparameter from the hook's signature makes the code cleaner and more maintainable. Good practice to eliminate parameters that aren't being utilized.
39-39: Improved comments for better code documentation.The updated comments are more precise and descriptive, clearly explaining the purpose of each function. This enhances code readability and maintainability.
Also applies to: 60-60, 75-75, 180-180, 210-210, 334-334
123-126: Added missing dependency in useCallback.Adding
getAQICategoryto the dependency array ensures that the callback will be properly updated if this function changes, following React's hooks rules.
231-232: Simplified DOM element creation.Removing unnecessary checks before creating DOM elements streamlines the code. Since this function is client-side only and only runs in browser environments, these checks were redundant.
275-275: Direct style property assignment.Setting
el.style.zIndexdirectly is clearer and more consistent with the code style used elsewhere in the file.src/platform/src/common/components/Map/functions/useMapControls.jsx (8)
20-24: Robust error handling for zoom operations.Adding try-catch blocks to the zoom operations ensures that the UI remains responsive even if an error occurs during map manipulation. Good practice to log the error for debugging.
29-33: Consistent error handling for map operations.Similar to the zoom-in operation, adding try-catch for zoom-out ensures consistent error handling throughout the component. This systematic approach to error management aligns with the PR objectives.
37-37: Improved comment clarity.The updated comment more precisely describes what's happening in the code, making it easier for other developers to understand the component's structure.
88-103: Enhanced URL updating with robust error handling.The method signature change and addition of try-catch improves the error handling for URL state updates. The code now safely handles edge cases and potential errors in the navigation API.
123-124: Defensive programming for callback handling.The addition of a type check for
setToastMessageprevents potential errors if the function is not provided or is incorrectly typed. This is a good defensive programming practice.
204-257: Comprehensive error handling for geolocation operations.Wrapping the map update operations within a try-catch block ensures that geolocation functionality remains robust, even when encountering errors with map manipulation or layer creation. The error message is clear and helpful.
303-308: Improved user feedback for missing map reference.Adding a specific error message when the map reference is unavailable provides better feedback to users and makes debugging easier.
376-376: More descriptive error logging.The updated error message provides more context about the specific operation that failed, making it easier to identify and troubleshoot issues.
src/platform/src/common/components/Map/components/MapNodes.js (8)
13-20: Improved icon handling with fallback for undefined categories!Adding an explicit fallback rendering for undefined categories is an excellent defensive programming approach. This will prevent potential runtime errors when dealing with unexpected data.
108-108: Good defensive coding practice with color fallback.Adding a fallback color ensures the UI doesn't break even when encountering unexpected category values.
113-118: Great addition of a fallback case.This ensures the function always returns a valid object even when no category matches the input value, preventing potential undefined returns and subsequent errors.
172-175: Proper validation with error logging is a great addition.This prevents the application from crashing when encountering malformed data and provides useful debugging information.
187-187: Good fallback handling for PM2.5 values.Using the || operator with 'N/A' ensures users always see meaningful content even when data is missing.
219-238: Excellent defensive programming with detailed validation.The thorough validation and detailed error logging will make debugging much easier. This prevents silent failures and provides clear information about what went wrong and where.
242-255: Comprehensive fallback strategy for cluster node data.The code now gracefully handles missing or malformed data at multiple levels, with appropriate type checking and formatting. This is a significant improvement in robustness.
279-279: Clean conditional rendering for count display.Using inline style with display property is a clean approach to conditionally show/hide elements without requiring additional wrapper logic.
|
New next-platform changes available for preview here |
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
Screenshots (optional)
Summary by CodeRabbit
Bug Fixes
Refactor