BYOF: Frontend changes - #47523
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #47523 +/- ##
==========================================
- Coverage 67.21% 67.21% -0.01%
==========================================
Files 3631 3632 +1
Lines 229542 229558 +16
Branches 11957 11815 -142
==========================================
+ Hits 154298 154300 +2
- Misses 61378 61393 +15
+ Partials 13866 13865 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
39b543c to
8b9eeb0
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the Apple Business Manager (ABM) admin UI to support displaying and selecting a BYOD fleet alongside the existing macOS/iOS/iPadOS fleets, and improves the ABM tokens table UX (sorting + search).
Changes:
- Add BYOD fleet support in ABM table (new column + tooltip) and ABM “Edit fleets” modal (new BYOD dropdown + additional token details).
- Enable sorting for several ABM table columns (including renew date).
- Add client-side search to filter ABM tokens by organization name.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/pages/admin/IntegrationsPage/cards/MdmSettings/components/RenewDateCell/RenewDateCell.tsx | Makes statusConfig optional to allow reuse without tooltips. |
| frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/EditTeamsAbmModal/EditTeamsAbmModal.tsx | Updates modal layout and adds BYOD fleet dropdown. |
| frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/EditTeamsAbmModal/EditTeamsAbmModal.tests.ts | Updates helper test data for selected team mapping. |
| frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/AppleBusinessManagerTable/AppleBusinessManagerTableConfig.tsx | Adds BYOD column and enables sorting on multiple columns. |
| frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/AppleBusinessManagerTable/AppleBusinessManagerTable.tsx | Adds search UI wiring and client-side filtering. |
| frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/AppleBusinessManagerTable/_styles.scss | Adjusts column widths and responsive hide rules for the new column set. |
| frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/AppleBusinessManagerPage.tsx | Updates descriptive copy for BYOD wording. |
| frontend/interfaces/mdm.ts | Extends IMdmAbToken with byod_fleet and updates enrollment status display text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@coderabbitai review once |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get PR details
gh pr view 47523 --repo fleetdm/fleet --json title,body,files,additions,deletions,baseRefName,headRefName
echo "---FILES---"
gh pr diff 47523 --repo fleetdm/fleetLength of output: 23808
Review of PR
|
| Breakpoint | Column hidden |
|---|---|
$break-lg |
byod_team |
1230px |
ipados_team |
$break-md |
ios_team |
| (never) | macos_team — never hidden |
With 5 columns now (org name, renew date, macOS fleet, iOS fleet, iPadOS fleet, BYOD fleet — 6 including the actions column), the table may overflow on narrow screens since macos_team has no hide rule. This may be intentional if macOS is the most important column, but it's worth verifying on narrow viewports.
3. RenewDateCell in EditTeamsAbmModal — no tooltip on warning/error dates
RenewDateCell is now used in the modal without statusConfig:
<RenewDateCell
value={token.renew_date}
className="abm-renew-date-cell"
/>statusConfig was made optional (good), but the consequence is that when the renew date is in a warning or error state, the status indicator will show (colored dot) but there will be no tooltip text explaining what action to take. The original table usage passes a statusConfig with tooltip text. Consider either passing a statusConfig here or adding a fallback tooltip message.
4. Removed helpful tooltips from macOS/iOS/iPadOS dropdowns in EditTeamsAbmModal
The tooltips explaining how hosts are automatically added to fleets on ABM sync were removed:
// Removed from macOS, iOS, and iPadOS dropdowns:
tooltip={
<>
macOS hosts are automatically added to this fleet on initial sync
from ABM. ...
</>
}No equivalent tooltip was added for the new BYOD fleet dropdown either. While the modal is less cluttered now, admins lose context about how fleet assignment works. Consider keeping this guidance somewhere — perhaps as a single descriptive paragraph above the dropdowns rather than per-field tooltips.
5. byod_fleet required in IMdmAbToken — potential runtime risk
export interface IMdmAbToken {
macos_fleet: ITokenFleet;
ios_fleet: ITokenFleet;
ipados_fleet: ITokenFleet;
byod_fleet: ITokenFleet; // required
}If any existing ABM tokens in the backend don't yet include byod_fleet in the API response (e.g., older server versions or staged rollout), accessing originalRow.byod_fleet in the table accessor will throw. Consider making this optional (byod_fleet?: ITokenFleet) with a safe fallback in getFleetDisplayName, at least until the backend change is fully deployed.
Nitpicks / Positive notes
- ✅ Renaming "On (personal)" → "On (BYOD)" is clean and consistent across tests, UI map, and display.
- ✅ Making
statusConfigoptional inRenewDateCellwith a proper guard (&& statusConfig) is the right approach. - ✅ Client-side search filtering in
AppleBusinessManagerTableis straightforward and avoids unnecessary API calls. - ✅ Enabling sorting on the ABM table columns (
sortType: "caseInsensitive",sortType: "dateStrings") is a good UX improvement. - ✅ Moving Apple ID into the modal rather than the table keeps the table focused on actionable fleet assignments.
|
Checked coderabbit's response, and fixed the first one, the rest seems not really relevant. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR adds BYOD fleet support to the Apple Business Manager frontend. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. 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
🧹 Nitpick comments (1)
frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/AppleBusinessManagerTable/AppleBusinessManagerTable.tsx (1)
35-37: ⚡ Quick winSimplify by removing the redundant wrapper function.
The
handleSearchQueryChangefunction simply callssetSearchQuerywithout any additional logic. This creates unnecessary indirection.♻️ Proposed simplification
- const handleSearchQueryChange = (query: string) => { - setSearchQuery(query); - }; - const onSelectAction = (action: string, abmToken: IMdmAbToken) => {const onQueryChange = (queryData: ITableQueryData) => { - handleSearchQueryChange(queryData.searchQuery); + setSearchQuery(queryData.searchQuery); };Also applies to: 61-63
🤖 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 `@frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/AppleBusinessManagerTable/AppleBusinessManagerTable.tsx` around lines 35 - 37, Remove the redundant `handleSearchQueryChange` wrapper function that simply passes its argument to `setSearchQuery` without any additional logic. Instead, pass `setSearchQuery` directly to any component or callback that currently uses `handleSearchQueryChange`. Apply the same simplification to the similar wrapper function referenced at lines 61-63 that wraps another state setter in the same way, eliminating unnecessary function indirection throughout the component.
🤖 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
`@frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/EditTeamsAbmModal/EditTeamsAbmModal.tests.ts`:
- Around line 34-40: The test in EditTeamsAbmModal.tests.ts only validates the
assigned BYOD team case (byod_fleet_id: 3) but does not test the unassigned
scenario required by the API contract. Add a second test case that calls
getSelectedTeamIds with byod_team set to "Unassigned" and asserts that
byod_fleet_id in the returned object equals null, ensuring the contract for
unassigned BYOD teams sending null payloads is explicitly validated and prevents
future API contract drift.
---
Nitpick comments:
In
`@frontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/AppleBusinessManagerTable/AppleBusinessManagerTable.tsx`:
- Around line 35-37: Remove the redundant `handleSearchQueryChange` wrapper
function that simply passes its argument to `setSearchQuery` without any
additional logic. Instead, pass `setSearchQuery` directly to any component or
callback that currently uses `handleSearchQueryChange`. Apply the same
simplification to the similar wrapper function referenced at lines 61-63 that
wraps another state setter in the same way, eliminating unnecessary function
indirection throughout the component.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a981175e-5b35-4ae1-9417-9996792239a6
📒 Files selected for processing (11)
frontend/components/TableContainer/DataTable/HostMdmStatusCell/HostMdmStatusCell.tests.tsxfrontend/interfaces/mdm.tsfrontend/pages/DashboardPage/cards/MDM/MDM.tests.tsxfrontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/AppleBusinessManagerPage.tsxfrontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/AppleBusinessManagerTable/AppleBusinessManagerTable.tsxfrontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/AppleBusinessManagerTable/AppleBusinessManagerTableConfig.tsxfrontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/AppleBusinessManagerTable/_styles.scssfrontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/EditTeamsAbmModal/EditTeamsAbmModal.tests.tsfrontend/pages/admin/IntegrationsPage/cards/MdmSettings/AppleBusinessManagerPage/components/EditTeamsAbmModal/EditTeamsAbmModal.tsxfrontend/pages/admin/IntegrationsPage/cards/MdmSettings/components/RenewDateCell/RenewDateCell.tsxfrontend/services/entities/mdm_apple_bm.ts
|
@MagnusHJensen any chance you can screenrecord the responsive design? |
Related issue: Resolves #45601
Responsiveness:
Screen.Recording.2026-06-17.at.16.16.06.mov
Removed border from Action header:
Checklist for submitter
If some of the following don't apply, delete the relevant line.
Changes file added for user-visible changes in
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information. In another PR
Input data is properly validated,
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Timeouts are implemented and retries are limited to avoid infinite loops
If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes
Testing
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Tests