Conversation
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughPeer data now includes network address metadata and a helper that de-duplicates MAC addresses. The peer table adds a MAC column and search text. The peer details page conditionally displays MAC addresses. ChangesPeer MAC address display
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~15 minutes Change: Feature Suggested reviewers: Merge Risk: ⚪ Minimal · up to The MAC-address display changes have no unresolved merge-blocking risk in the available evidence. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit found MACs in the peer data, Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/`(dashboard)/peer/page.tsx:
- Around line 681-695: The MAC Address row in Card.ListItem is only copying the
first entry because it relies on value as the fallback copy source while
extraText is display-only. Update the peerMacAddresses(peer) rendering so the
clipboard content is built explicitly from the full set of MAC addresses, and
pass that separate copied value through the copy/copyText behavior in this
Card.ListItem usage. Keep the displayed value and notification text in sync with
the complete MAC address list, not just peerMacAddresses(peer)[0].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 90dd3d71-2ab1-4105-bea2-7fed6ae87144
📒 Files selected for processing (4)
src/app/(dashboard)/peer/page.tsxsrc/components/Card.tsxsrc/interfaces/Peer.tssrc/modules/peers/PeersTable.tsx
| {peerMacAddresses(peer).length > 0 && ( | ||
| <Card.ListItem | ||
| copy | ||
| copyText={"MAC Address"} | ||
| label={ | ||
| <> | ||
| <NetworkIcon size={16} className={"shrink-0"} /> | ||
| MAC Address | ||
| </> | ||
| } | ||
| className={ | ||
| peerMacAddresses(peer).length > 1 ? "items-start" : "" | ||
| } | ||
| value={peerMacAddresses(peer)[0]} | ||
| extraText={peerMacAddresses(peer).slice(1)} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Copying this row drops all but the first MAC address.
Card.ListItem only copies valueToCopy (or value as a fallback), so this new row copies peerMacAddresses(peer)[0] and ignores extraText. For peers with multiple NICs, the clipboard content won't match what the UI shows.
Proposed fix
{peerMacAddresses(peer).length > 0 && (
<Card.ListItem
copy
copyText={"MAC Address"}
+ valueToCopy={peerMacAddresses(peer).join(", ")}
label={
<>
<NetworkIcon size={16} className={"shrink-0"} />
MAC Address
</>Based on learnings, the copied content should be passed separately from the notification text, and both should match the displayed value set.
📝 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.
| {peerMacAddresses(peer).length > 0 && ( | |
| <Card.ListItem | |
| copy | |
| copyText={"MAC Address"} | |
| label={ | |
| <> | |
| <NetworkIcon size={16} className={"shrink-0"} /> | |
| MAC Address | |
| </> | |
| } | |
| className={ | |
| peerMacAddresses(peer).length > 1 ? "items-start" : "" | |
| } | |
| value={peerMacAddresses(peer)[0]} | |
| extraText={peerMacAddresses(peer).slice(1)} | |
| {peerMacAddresses(peer).length > 0 && ( | |
| <Card.ListItem | |
| copy | |
| copyText={"MAC Address"} | |
| valueToCopy={peerMacAddresses(peer).join(", ")} | |
| label={ | |
| <> | |
| <NetworkIcon size={16} className={"shrink-0"} /> | |
| MAC Address | |
| </> | |
| } | |
| className={ | |
| peerMacAddresses(peer).length > 1 ? "items-start" : "" | |
| } | |
| value={peerMacAddresses(peer)[0]} | |
| extraText={peerMacAddresses(peer).slice(1)} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`(dashboard)/peer/page.tsx around lines 681 - 695, The MAC Address
row in Card.ListItem is only copying the first entry because it relies on value
as the fallback copy source while extraText is display-only. Update the
peerMacAddresses(peer) rendering so the clipboard content is built explicitly
from the full set of MAC addresses, and pass that separate copied value through
the copy/copyText behavior in this Card.ListItem usage. Keep the displayed value
and notification text in sync with the complete MAC address list, not just
peerMacAddresses(peer)[0].
Source: Learnings
705fc20 to
af271eb
Compare
Surface the network_addresses (IP + MAC) now returned by the management API. Peers are searchable by MAC via a hidden table column wired into the global filter, and their MAC address is shown on the peer detail page.
Right-justify each value row in Card.ListItem so the copy icons line up on the column edge even when values render at slightly different widths (e.g. the per-interface MAC address list on the peer detail page).
af271eb to
dd13613
Compare
2 participants
The management API now returns each peer's per-interface MAC addresses (in a new
network_addressesfield). This PR uses that data in the dashboard so you can search the peer list by MAC address and see a peer's MAC on its detail page.Describe your changes
Added a hidden
maccolumn feeding the existing client-side global filter, and updated the search placeholder to"Search by name, IP, MAC, owner or group…".
NetworkAddress+network_addressesto thePeerinterface and a smallpeerMacAddresses()helper.Card.ListItemso copy icons line up in multi-line lists (the MAC list, and the existing Domain field).Depends on the management API change that adds the
network_addressesfield to the peers response: netbirdio/netbird#6553 (netbirdio/netbird#6553). Without it the field is absent and the search/detail simply show nothing (no regression to existing behavior).Issue ticket number and link
Discussion thread: https://netbirdio.slack.com/archives/C02KHAE8VLZ/p1782489164474579
Documentation
Select exactly one:
This is a self-explanatory UI affordance (an extra searchable value and a field on the peer detail page); it doesn't introduce new concepts or workflows that require documentation.
Docs PR URL (required if "docs added" is checked)
N/A
E2E tests
Optional: override the image tags used by the Playwright e2e workflow.
Defaults to
mainwhen omitted.management-cloud-tag: main
reverse-proxy-tag: main
Summary by CodeRabbit
New Features
Enhancements