Clear passcode frontend - #43084
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #43084 +/- ##
=======================================
Coverage 66.84% 66.84%
=======================================
Files 2581 2583 +2
Lines 207154 207060 -94
Branches 9180 9207 +27
=======================================
- Hits 138475 138414 -61
+ Misses 56086 56076 -10
+ Partials 12593 12570 -23
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:
|
There was a problem hiding this comment.
Pull request overview
Adds frontend support for the new Clear passcode MDM action and related activity entries, wiring it into the Host details UI and activity feeds.
Changes:
- Added a new host endpoint + service method to POST
/fleet/hosts/:id/clear_passcode. - Added “Clear passcode” to the Host actions dropdown (iOS/iPadOS only, non-personal enrollments) plus a new confirmation modal.
- Added support for the new
cleared_passcodeactivity type in host activity + global activity templates.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/utilities/endpoints.ts | Adds HOST_CLEAR_PASSCODE endpoint builder. |
| frontend/services/entities/hosts.ts | Adds hostAPI.clearPasscode POST call. |
| frontend/pages/hosts/details/HostDetailsPage/modals/ClearPasscodeModal/index.ts | Barrel export for the new modal. |
| frontend/pages/hosts/details/HostDetailsPage/modals/ClearPasscodeModal/ClearPasscodeModal.tsx | Implements the clear passcode confirmation modal + API call. |
| frontend/pages/hosts/details/HostDetailsPage/HostDetailsPage.tsx | Wires dropdown selection to open the new modal. |
| frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsx | Adds option + eligibility/disabled-state rules for “Clear passcode”. |
| frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/HostActionsDropdown.tests.tsx | Updates option rendering tests + adds clear passcode-specific tests. |
| frontend/pages/hosts/details/cards/Activity/ActivityItems/ClearedPasscodeActivityItem/index.ts | Barrel export for the host activity item. |
| frontend/pages/hosts/details/cards/Activity/ActivityItems/ClearedPasscodeActivityItem/ClearedPasscodeActivityItem.tsx | Renders host past-activity row for cleared passcode. |
| frontend/pages/hosts/details/cards/Activity/ActivityConfig.tsx | Registers the new host past activity component mapping. |
| frontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsx | Adds a template string for global activity feed rendering. |
| frontend/interfaces/activity.ts | Adds ActivityType.ClearedPasscode and updates host past activity union + filter label. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
15edc45 to
eed75f3
Compare
![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.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 similar comment
✅ Actions performedReview triggered.
|
WalkthroughThis PR introduces a "Clear passcode" action for iOS/iPadOS hosts in the Fleet frontend. The implementation includes: adding the 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/pages/hosts/details/HostDetailsPage/modals/ClearPasscodeModal/ClearPasscodeModal.tsx (1)
33-36: State update after component unmount.The
onExit()callback will unmount this modal, so the subsequentsetIsClearingPasscode(false)call attempts to update state on an unmounted component. While React 18 no longer warns about this, the state update is effectively dead code.♻️ Suggested fix: remove unnecessary state update
} finally { onExit(); - setIsClearingPasscode(false); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/hosts/details/HostDetailsPage/modals/ClearPasscodeModal/ClearPasscodeModal.tsx` around lines 33 - 36, The finally block calls onExit() which unmounts the modal and then calls setIsClearingPasscode(false), causing a state update after unmount; remove the setIsClearingPasscode(false) call from the finally block in the ClearPasscodeModal component (referencing the finally block that invokes onExit and setIsClearingPasscode) so only onExit() runs, or alternatively move setIsClearingPasscode(false) to run before onExit() if you must reset local state prior to unmounting.frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/HostActionsDropdown.tests.tsx (1)
1946-1986: Consider adding test coverage for"wiped"device status.The implementation disables "Clear passcode" for both
"wiped"and"wiping"statuses (seehelpers.tsxlines 612-620), but only"wiping"is tested here. Consider adding"wiped"to ensure both paths are covered.💡 Suggested test addition
it("is disabled with tooltip when pending wipe", async () => { + // Test "wiping" status const render = createCustomRenderer({ // ... existing test for "wiping" }); + // ... assertions + }); + + it("is disabled with tooltip when wiped", async () => { + const render = createCustomRenderer({ + context: { + app: { + isGlobalAdmin: true, + isPremiumTier: true, + isMacMdmEnabledAndConfigured: true, + currentUser: createMockUser(), + }, + }, + }); + + const { user } = render( + <HostActionsDropdown + hostTeamId={null} + onSelect={noop} + hostStatus="online" + hostPlatform="ios" + hostMdmEnrollmentStatus="On (company-owned)" + isConnectedToFleetMdm + hostMdmDeviceStatus="wiped" + hostScriptsEnabled + /> + ); + + await user.click(screen.getByText("Actions")); + + const option = screen.getByText("Clear passcode"); + expect(option).toBeInTheDocument(); + expect(option).toHaveAttribute("aria-disabled", "true"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/HostActionsDropdown.tests.tsx` around lines 1946 - 1986, Add a sibling test to the existing "is disabled with tooltip when pending wipe" case that covers the "wiped" device status: render HostActionsDropdown with hostMdmDeviceStatus="wiped" (keeping other props the same), open the Actions menu, and assert the "Clear passcode" option exists, has aria-disabled="true", and shows the same tooltip text; this ensures the helpers.tsx logic that disables Clear passcode for both "wiping" and "wiped" is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/HostActionsDropdown.tests.tsx`:
- Around line 1946-1986: Add a sibling test to the existing "is disabled with
tooltip when pending wipe" case that covers the "wiped" device status: render
HostActionsDropdown with hostMdmDeviceStatus="wiped" (keeping other props the
same), open the Actions menu, and assert the "Clear passcode" option exists, has
aria-disabled="true", and shows the same tooltip text; this ensures the
helpers.tsx logic that disables Clear passcode for both "wiping" and "wiped" is
covered.
In
`@frontend/pages/hosts/details/HostDetailsPage/modals/ClearPasscodeModal/ClearPasscodeModal.tsx`:
- Around line 33-36: The finally block calls onExit() which unmounts the modal
and then calls setIsClearingPasscode(false), causing a state update after
unmount; remove the setIsClearingPasscode(false) call from the finally block in
the ClearPasscodeModal component (referencing the finally block that invokes
onExit and setIsClearingPasscode) so only onExit() runs, or alternatively move
setIsClearingPasscode(false) to run before onExit() if you must reset local
state prior to unmounting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 92aae737-79be-48b5-a370-6b9242825de1
📒 Files selected for processing (12)
frontend/interfaces/activity.tsfrontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsxfrontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/HostActionsDropdown.tests.tsxfrontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsxfrontend/pages/hosts/details/HostDetailsPage/HostDetailsPage.tsxfrontend/pages/hosts/details/HostDetailsPage/modals/ClearPasscodeModal/ClearPasscodeModal.tsxfrontend/pages/hosts/details/HostDetailsPage/modals/ClearPasscodeModal/index.tsfrontend/pages/hosts/details/cards/Activity/ActivityConfig.tsxfrontend/pages/hosts/details/cards/Activity/ActivityItems/ClearedPasscodeActivityItem/ClearedPasscodeActivityItem.tsxfrontend/pages/hosts/details/cards/Activity/ActivityItems/ClearedPasscodeActivityItem/index.tsfrontend/services/entities/hosts.tsfrontend/utilities/endpoints.ts
sgress454
left a comment
sgress454
left a comment
There was a problem hiding this comment.
Tested 👍 , one totally non-blocking nit.
|
|
||
| const ClearPasscodeModal = ({ id, onExit }: IClearPasscodeModalProps) => { | ||
| const { renderFlash } = useContext(NotificationContext); | ||
| const [isClearingPasscode, setIsClearingPasscode] = React.useState(false); |
There was a problem hiding this comment.
@MagnusHJensen for future, double check we are pulling out named imports so this would be:
import React, { useContext, useState } from "react";
<3
There was a problem hiding this comment.
Right, that is a miss in my part, that is normally how I do it
Related issue: Resolves #42369
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. Done in backend task for whole story
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