-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Netmanager] Added serial number field to create device and soft create device, fix network bug #2435
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new required field, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant D as Devices Component
participant API as Server API
U->>D: Open device registration form
D->>D: Initialize new device state (including serial_number)
U->>D: Enter device details (with serial_number)
D->>D: Validate serial_number and other data
alt Valid Form
U->>D: Submit form
D->>D: Copy and clean device data (remove empty fields)
D->>API: API call to register device
API-->>D: Return registration response
else Invalid Form
D->>U: Display error for missing serial_number
end
sequenceDiagram
participant M as Main Component
participant LS as Local Storage
participant Dispatcher as Store Actions
M->>M: Check activeNetwork value
alt activeNetwork is empty
M->>LS: Retrieve activeNetwork data
LS-->>M: Return activeNetwork
M->>Dispatcher: Dispatch addActiveNetwork with parsed data
else activeNetwork is present
M->>Dispatcher: Dispatch addCurrentUserRole
M->>LS: Update localStorage with current user role
end
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
netmanager/src/views/layouts/Main.js (1)
138-148
: Consider potential race conditions in useEffect dependencies.The useEffect hook's dependency array is missing
dispatch
, which could lead to stale closures. Additionally, the localStorage operations could be moved outside the effect to optimize performance.Consider this refactoring:
+ const activeNetworkStorage = localStorage.getItem('activeNetwork'); + const parsedActiveNetwork = activeNetworkStorage ? JSON.parse(activeNetworkStorage) : null; useEffect(() => { if (isEmpty(activeNetwork)) { - const activeNetworkStorage = localStorage.getItem('activeNetwork'); - if (activeNetworkStorage) { - dispatch(addActiveNetwork(JSON.parse(activeNetworkStorage))); + if (parsedActiveNetwork) { + dispatch(addActiveNetwork(parsedActiveNetwork)); } } else { dispatch(addCurrentUserRole(activeNetwork.role)); localStorage.setItem('currentUserRole', JSON.stringify(activeNetwork.role)); } - }, [activeNetwork]); + }, [activeNetwork, dispatch, parsedActiveNetwork]);netmanager/src/views/components/DataDisplay/Devices.js (1)
341-347
: Consider enhancing serial number validation.While the basic validation is in place, consider adding format validation for the serial number to ensure it matches your expected pattern.
const isFormValid = () => { + const isValidSerialFormat = /^[A-Za-z0-9-]+$/.test(newDevice.serial_number.trim()); return ( newDevice.long_name.trim() !== '' && newDevice.category !== '' && - newDevice.serial_number.trim() !== '' + newDevice.serial_number.trim() !== '' && + isValidSerialFormat ); };Also applies to: 616-622
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
netmanager/src/views/components/DataDisplay/Devices.js
(14 hunks)netmanager/src/views/layouts/Main.js
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: netmanager code tests
- GitHub Check: build-push-deploy-netmanager-preview
🔇 Additional comments (5)
netmanager/src/views/layouts/Main.js (2)
150-164
: LGTM! Good use of loading states.The loading state handling is well-implemented, showing a centered loader while essential data (activeNetwork, currentRole) is being initialized.
118-130
: LGTM! Comprehensive error handling.Good implementation of error handling with appropriate user feedback through MainAlert. The distinction between server errors (status >= 500) and other errors provides clear messaging.
netmanager/src/views/components/DataDisplay/Devices.js (3)
280-280
: LGTM! Network reference update is consistent.The change from
network
tonetwork.net_name
is applied consistently across both device creation components.Also applies to: 534-535, 597-598, 662-663
467-477
: LGTM! Serial number field implementation is complete.The serial number field is properly implemented with:
- Required field validation
- Error handling
- Consistent placement in both device creation forms
Also applies to: 739-749
367-375
: LGTM! Clean implementation of empty field removal.The implementation efficiently removes empty fields before API calls, reducing payload size and maintaining data cleanliness.
Also applies to: 644-649
New netmanager changes available for preview here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Codebmk !
This pull request introduces several changes to the
CreateDevice
andSoftCreateDevice
components in thenetmanager
project to include a new "serial number" field and improve the handling of device data. Additionally, it modifies theMain
layout to handle the active network more robustly. The most important changes are detailed below:Enhancements to Device Creation Forms:
serial_number
field to thenewDeviceInitState
andinitialErrors
objects in bothCreateDevice
andSoftCreateDevice
components. (netmanager/src/views/components/DataDisplay/Devices.js
) [1] [2]handleDeviceDataChange
function to include validation for theserial_number
field. (netmanager/src/views/components/DataDisplay/Devices.js
) [1] [2]serial_number
field. (netmanager/src/views/components/DataDisplay/Devices.js
) [1] [2]serial_number
. (netmanager/src/views/components/DataDisplay/Devices.js
) [1] [2]serial_number
field to the form UI in bothCreateDevice
andSoftCreateDevice
components. (netmanager/src/views/components/DataDisplay/Devices.js
) [1] [2]Improvements to Device Data Handling:
newDevice
and removed fields with empty values before sending the data to the server in bothCreateDevice
andSoftCreateDevice
components. (netmanager/src/views/components/DataDisplay/Devices.js
) [1] [2]network.net_name
instead ofnetwork
. (netmanager/src/views/components/DataDisplay/Devices.js
) [1] [2]Enhancements to Active Network Handling:
Main
layout to retrieve the active network from local storage if it is not already set, ensuring the user's role is correctly assigned. (netmanager/src/views/layouts/Main.js
)Summary by CodeRabbit
New Features
Refactor