UDAE: fetch and allow download default setup assistant profile - #44253
Conversation
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.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #44253 +/- ##
==========================================
- Coverage 66.77% 66.77% -0.01%
==========================================
Files 2630 2630
Lines 211248 211251 +3
Branches 9428 9547 +119
==========================================
Hits 141071 141071
- Misses 57354 57357 +3
Partials 12823 12823
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
WalkthroughThe PR adds UI and service support for Fleet’s default Apple automatic enrollment profile. The SetupAssistant component detects a missing team profile (404) and conditionally fetches the default profile; when present it renders a default-profile variant before the uploader and updates the section description with a “Learn more” link. SetupAssistantProfileCard gained a Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/components/SetupAssistantProfileCard/SetupAssistantProfileCard.tsx (2)
12-22: Tighten the prop typing fordefaultProfilemode.
profileis required and typed asIAppleSetupEnrollmentProfileResponse, but indefaultProfilemode the caller inSetupAssistant.tsxcasts anIDefaultAppleSetupEnrollmentProfileResponse(which has noname/uploaded_at) — and may even passundefinedwhile the default-profile query is in flight or has errored. The component currently dodges the missing fields via thedefaultProfilebranch in JSX, butonDownloadstill doesprofile.enrollment_profile, which will throw ifprofileisundefined.A discriminated union avoids the unsafe cast at the call site and makes the contract explicit:
♻️ Suggested typing
-interface ISetupAssistantProfileCardProps { - profile: IAppleSetupEnrollmentProfileResponse; - onDelete?: () => void; - defaultProfile?: boolean; -} +type ISetupAssistantProfileCardProps = + | { + defaultProfile: true; + profile: IDefaultAppleSetupEnrollmentProfileResponse; + onDelete?: never; + } + | { + defaultProfile?: false; + profile: IAppleSetupEnrollmentProfileResponse; + onDelete: () => void; + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/components/SetupAssistantProfileCard/SetupAssistantProfileCard.tsx` around lines 12 - 22, The component's prop typing allows unsafe casts: when defaultProfile mode is used the caller may pass an IDefaultAppleSetupEnrollmentProfileResponse or undefined, but SetupAssistantProfileCard still types profile as IAppleSetupEnrollmentProfileResponse and onDownload accesses profile.enrollment_profile which can throw; change ISetupAssistantProfileCardProps into a discriminated union (e.g. { defaultProfile: true; profile?: IDefaultAppleSetupEnrollmentProfileResponse } | { defaultProfile?: false; profile: IAppleSetupEnrollmentProfileResponse }) and update the SetupAssistantProfileCard signature accordingly, then guard all uses (especially onDownload and any profile.name/ uploaded_at accesses) for the defaultProfile branch or undefined profile, and update the call sites in SetupAssistant.tsx to match the new union.
23-37: LGTM, with one optional polish.Pretty-printing the JSON and the conditional filename look good. Optional nit: the default-profile download produces a filename like
2026-04-27_default-automatic-enrollment.json(the date is prefixed before what is already a file name). If the intent (per Figma) is the user getsdefault-automatic-enrollment.jsonexactly, drop the date prefix in the default branch.♻️ Optional refactor
const onDownload = () => { - const date = new Date(); - const filename = `${date.toISOString().split("T")[0]}_${ - defaultProfile ? "default-automatic-enrollment.json" : profile.name - }`; + const filename = defaultProfile + ? "default-automatic-enrollment.json" + : `${new Date().toISOString().split("T")[0]}_${profile.name}`; const file = new global.window.File( [JSON.stringify(profile.enrollment_profile, null, 2)], filename );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/components/SetupAssistantProfileCard/SetupAssistantProfileCard.tsx` around lines 23 - 37, The download filename currently always prefixes the date but the review requests that when defaultProfile is true the file be named exactly "default-automatic-enrollment.json" (no date); update the onDownload function to build filename conditionally: if defaultProfile then set filename to "default-automatic-enrollment.json" else keep the existing date-prefixed `${dateISOString}_${profile.name}` logic; modify the reference in SetupAssistantProfileCard's onDownload (where defaultProfile, profile.name and FileSaver.saveAs are used) so the FileSaver.saveAs call receives the corrected filename.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/SetupAssistant.tsx`:
- Around line 126-138: The default-profile branch can render with
defaultEnrollmentProfileData === undefined and crash; update the render logic in
SetupAssistant (the conditional that currently checks enrollmentProfileNotFound
|| !enrollmentProfileData) to only render SetupAssistantProfileCard when
defaultEnrollmentProfileData is actually present (e.g., require
defaultEnrollmentProfileData truthy before casting) and keep
SetupAssistantProfileUploader separate; additionally, detect and surface non-404
errors from the team profile query (do not let non-404 errors fall through to
the default branch) so that enrollmentProfileData/error state renders an
explicit error UI instead of the default card, and remove the unsafe cast to
IAppleSetupEnrollmentProfileResponse so the component only receives a valid
profile object for download actions (ensure download handler also guards
profile.enrollment_profile).
- Around line 69-82: The current logic only enables the default-profile fetch
when enrollmentProfileNotFound (404), causing other errors to skip fetching and
fall through to the default-profile UI; update the useQuery enabled predicate to
trigger when either a 404 or any enrollmentProfileError exists (e.g., enabled:
enrollmentProfileNotFound || !!enrollmentProfileError) and/or adjust the render
branch that checks enrollmentProfileNotFound || !enrollmentProfileData to
instead branch explicitly on enrollmentProfileError vs missing data so non-404
errors preserve the intended empty/disabled states; locate and change the
symbols enrollmentProfileNotFound, enrollmentProfileError, the useQuery call for
defaultEnrollmentProfileData, and the render condition that references
enrollmentProfileNotFound || !enrollmentProfileData.
In `@frontend/services/entities/mdm.ts`:
- Around line 71-75: The interface IDefaultAppleSetupEnrollmentProfileResponse
currently declares an unused/incorrect optional field updated_at which
mismatches the component expectation (SetupAssistantProfileCard expects
IAppleSetupEnrollmentProfileResponse with uploaded_at); remove updated_at from
IDefaultAppleSetupEnrollmentProfileResponse to avoid casting and dead fields, or
if the backend actually provides a timestamp named uploaded_at, replace
updated_at with uploaded_at and align the type with
IAppleSetupEnrollmentProfileResponse so the component can consume the field
without casting — update any call sites that cast between these interfaces
accordingly.
In `@frontend/utilities/endpoints.ts`:
- Line 188: The frontend adds MDM_APPLE_DEFAULT_SETUP_ENROLLMENT_PROFILE and
SetupAssistant.tsx calls getDefaultSetupEnrollmentProfile(), but the backend
currently lacks a GET handler for the /enrollment_profiles/automatic/default
route; add a GET route handler that responds to GET
/enrollment_profiles/automatic/default (alongside the existing base
/enrollment_profiles/automatic handlers) and ensure the response JSON matches
IDefaultAppleSetupEnrollmentProfileResponse with keys updated_at (optional) and
enrollment_profile so the frontend receives the expected shape.
---
Nitpick comments:
In
`@frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/components/SetupAssistantProfileCard/SetupAssistantProfileCard.tsx`:
- Around line 12-22: The component's prop typing allows unsafe casts: when
defaultProfile mode is used the caller may pass an
IDefaultAppleSetupEnrollmentProfileResponse or undefined, but
SetupAssistantProfileCard still types profile as
IAppleSetupEnrollmentProfileResponse and onDownload accesses
profile.enrollment_profile which can throw; change
ISetupAssistantProfileCardProps into a discriminated union (e.g. {
defaultProfile: true; profile?: IDefaultAppleSetupEnrollmentProfileResponse } |
{ defaultProfile?: false; profile: IAppleSetupEnrollmentProfileResponse }) and
update the SetupAssistantProfileCard signature accordingly, then guard all uses
(especially onDownload and any profile.name/ uploaded_at accesses) for the
defaultProfile branch or undefined profile, and update the call sites in
SetupAssistant.tsx to match the new union.
- Around line 23-37: The download filename currently always prefixes the date
but the review requests that when defaultProfile is true the file be named
exactly "default-automatic-enrollment.json" (no date); update the onDownload
function to build filename conditionally: if defaultProfile then set filename to
"default-automatic-enrollment.json" else keep the existing date-prefixed
`${dateISOString}_${profile.name}` logic; modify the reference in
SetupAssistantProfileCard's onDownload (where defaultProfile, profile.name and
FileSaver.saveAs are used) so the FileSaver.saveAs call receives the corrected
filename.
🪄 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: e012a8eb-3021-43e1-b2aa-19f0382bd3b7
📒 Files selected for processing (5)
frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/SetupAssistant.tsxfrontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/components/SetupAssistantProfileCard/SetupAssistantProfileCard.tsxfrontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/components/SetupAssistantProfileCard/_styles.scssfrontend/services/entities/mdm.tsfrontend/utilities/endpoints.ts
There was a problem hiding this comment.
Pull request overview
Updates the Setup Assistant (automatic enrollment) UI to show a Fleet-provided default enrollment profile (with Download action) when no custom profile exists, and wires the download behavior to a new API endpoint.
Changes:
- Updated MDM automatic enrollment profile API endpoints and added a new “default profile” endpoint constant.
- Added an MDM service method to fetch the default Setup Assistant enrollment profile.
- Updated the Setup Assistant UI/card to render a default-profile variant (download-only) and support downloading formatted JSON.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/utilities/endpoints.ts | Switches automatic enrollment profile endpoint to the non-deprecated route and adds a constant for the default-profile download endpoint. |
| frontend/services/entities/mdm.ts | Adds the default-profile response type and a getDefaultSetupEnrollmentProfile API method. |
| frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/components/SetupAssistantProfileCard/_styles.scss | Extends card styling to support a “default profile” visual variant and description text. |
| frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/components/SetupAssistantProfileCard/SetupAssistantProfileCard.tsx | Adds default-profile rendering (no delete action) and downloads prettified JSON with improved filename formatting. |
| frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/SetupAssistant.tsx | Fetches and displays the default profile card when a custom profile is absent, and adds the “Learn more” link per design. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/SetupAssistant.tsx (1)
126-140:⚠️ Potential issue | 🟠 MajorNon-404 enrollment-profile failures still fall through to the default/uploader UI.
At Line 126,
enrollmentProfileNotFound || !enrollmentProfileDatatreats any failed fetch with empty data (e.g. 403/500/network) as “no profile,” so this branch renders instead of an explicit error/empty-state path. That can hide the intended gated states and make failures look like normal upload state. Please branch non-404 errors explicitly before the default-profile path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/SetupAssistant.tsx` around lines 126 - 140, The current conditional (enrollmentProfileNotFound || !enrollmentProfileData) treats any missing data as "not found" and lets non-404 failures fall through to the default profile/uploader UI; update the branch to detect non-404 errors explicitly (e.g., an enrollmentProfileError or status !== 404) and render an explicit error/empty-state path before falling back to the defaultProfile/uploader; specifically, modify the logic surrounding enrollmentProfileNotFound and enrollmentProfileData in SetupAssistant.tsx so that if there is an error that is not a 404 you render an error UI (or early return) instead of rendering SetupAssistantProfileCard/SetupAssistantProfileUploader with defaultEnrollmentProfileData.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/SetupAssistant.tsx`:
- Around line 126-140: The current conditional (enrollmentProfileNotFound ||
!enrollmentProfileData) treats any missing data as "not found" and lets non-404
failures fall through to the default profile/uploader UI; update the branch to
detect non-404 errors explicitly (e.g., an enrollmentProfileError or status !==
404) and render an explicit error/empty-state path before falling back to the
defaultProfile/uploader; specifically, modify the logic surrounding
enrollmentProfileNotFound and enrollmentProfileData in SetupAssistant.tsx so
that if there is an error that is not a 404 you render an error UI (or early
return) instead of rendering
SetupAssistantProfileCard/SetupAssistantProfileUploader with
defaultEnrollmentProfileData.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b54f8490-af56-4d31-9bb3-16c7216e3556
📒 Files selected for processing (2)
frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/SetupAssistant.tsxfrontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/components/SetupAssistantProfileCard/SetupAssistantProfileCard.tsx
✅ Files skipped from review due to trivial changes (1)
- frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/components/SetupAssistantProfileCard/SetupAssistantProfileCard.tsx
a1a35a5 to
a646b20
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/SetupAssistant.tsx`:
- Line 70: The detection of a 404 uses enrollmentProfileError?.status which can
be undefined for network errors; update the check where
enrollmentProfileNotFound is defined to use
enrollmentProfileError?.response?.status === 404 (referencing the
enrollmentProfileError variable and enrollmentProfileNotFound constant in
SetupAssistant.tsx) so the code correctly detects HTTP 404 responses only when a
response exists and doesn't mis-handle network errors.
🪄 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: f07c25bf-3112-4a67-ac05-a524108bf35c
📒 Files selected for processing (5)
frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/SetupAssistant.tsxfrontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/components/SetupAssistantProfileCard/SetupAssistantProfileCard.tsxfrontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/components/SetupAssistantProfileCard/_styles.scssfrontend/services/entities/mdm.tsfrontend/utilities/endpoints.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/services/entities/mdm.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/utilities/endpoints.ts
- frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/components/SetupAssistantProfileCard/_styles.scss
- frontend/pages/ManageControlsPage/SetupExperience/cards/SetupAssistant/components/SetupAssistantProfileCard/SetupAssistantProfileCard.tsx
nulmete
left a comment
There was a problem hiding this comment.
LGTM. Saw it's been merged before I was able to submit my comments but I think they're worth tackling as a follow-up 😄
| const baseClass = `setup-assistant-profile-card${ | ||
| props.defaultProfile ? "-default-profile" : "" | ||
| }`; |
There was a problem hiding this comment.
since we're following the BEM convention, I think we should always have setup-assistant-profile-card as the base class, and append setup-assistant-profile-card--default-profile if it's a default profile (--default-profile would act as a modifier IMO)
TL;DR:
- if default profile -> class="setup-assistant-profile-card setup-assistant-profile-card--default-profile"
- otherwise -> class="setup-assistant-profile-card"
There was a problem hiding this comment.
Got it, will followup wasn't aware of the BEM convention
| @@ -1,4 +1,4 @@ | |||
| .setup-assistant-profile-card { | |||
| .setup-assistant-profile-card, .setup-assistant-profile-card-default-profile { | |||
There was a problem hiding this comment.
following my comment related to the BEM convention, I think this could be nested like:
.setup-assistant-profile-card {
&--default-profile { ... }
}| .setup-assistant-profile-card-default-profile { | ||
| background-color: $ui-off-white; | ||
| } No newline at end of file |
There was a problem hiding this comment.
this could also be part of the same block above
Follow up from comments on: #44253 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Improved component styling architecture to follow modern CSS naming conventions, enhancing code maintainability and consistency across the setup assistant profile card. No changes to user-facing functionality or appearance. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Needs #44236
Related issue: Resolves #43790
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. Added in backend 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
New Features
User-facing behavior
Documentation
Style