feat: display activity log on device user page#45678
Open
Piccirello wants to merge 1 commit into
Open
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #45678 +/- ##
==========================================
+ Coverage 66.75% 66.78% +0.02%
==========================================
Files 2745 2745
Lines 219361 219564 +203
Branches 10839 10893 +54
==========================================
+ Hits 146435 146627 +192
Misses 59697 59697
- Partials 13229 13240 +11
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:
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an Activity panel to the My Device page and implements device-authenticated support end-to-end: frontend endpoint builders and client calls, isMyDevicePage prop threading, DeviceUserPage queries and modals, MSW handlers and tests; backend activity API and service extensions for device callers, device HTTP routes, mocks, and unit/integration tests. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/hosts/details/DeviceUserPage/DeviceUserPage.tsx`:
- Around line 989-990: The pagination handlers currently close over
activityPage; update onNextPage and onPreviousPage to use the functional form of
setActivityPage to avoid stale state and ensure pages never go negative—for
example, call setActivityPage(prev => prev + 1) for the next handler and
setActivityPage(prev => Math.max(0, prev - 1)) for the previous handler; change
the callbacks wired to onNextPage and onPreviousPage in DeviceUserPage to use
these functional updates (identify the setActivityPage state setter and the
onNextPage/onPreviousPage handlers).
In `@server/service/activities.go`:
- Around line 71-81: The device-auth path in ListHostPastActivities silently
overwrites the requested hostID with the host from context; instead, when
svc.authz.IsAuthenticatedWith(...)
(AuthnDeviceToken/AuthnDeviceCertificate/AuthnDeviceURL) and
hostctx.FromContext(ctx) returns h, compare the requested hostID variable to
h.ID and if they differ return an auth/validation error (similar to
ListHostPastActivitiesForDevice) rather than replacing hostID; update the branch
around hostctx.FromContext usage in activities.go to reject mismatches and
return an appropriate fleet.NewAuthRequiredError or authorization error.
🪄 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: a80de226-cd2a-425c-8404-b5935b3fed5e
📒 Files selected for processing (25)
changes/my-device-activity-panelfrontend/pages/hosts/details/DeviceUserPage/DeviceUserPage.tests.tsxfrontend/pages/hosts/details/DeviceUserPage/DeviceUserPage.tsxfrontend/pages/hosts/details/cards/Activity/Activity.tsxfrontend/pages/hosts/details/cards/Activity/ActivityConfig.tsxfrontend/pages/hosts/details/cards/Activity/ActivityItems/FailedEnrollmentProfileRenewalActivityItem/FailedEnrollmentProfileRenewalActivityItem.tsxfrontend/pages/hosts/details/cards/Activity/ActivityItems/RanScriptActivityItem/RanScriptActivityItem.tsxfrontend/pages/hosts/details/cards/Activity/PastActivityFeed/PastActivityFeed.tsxfrontend/pages/hosts/details/cards/Activity/UpcomingActivityFeed/UpcomingActivityFeed.tsxfrontend/services/entities/activities.tsfrontend/test/handlers/device-handler.tsfrontend/utilities/endpoints.tsserver/activity/api/list_host_past_activities.goserver/activity/api/service.goserver/activity/internal/service/handler_test.goserver/activity/internal/service/service.goserver/fleet/activities.goserver/fleet/service.goserver/mock/activity_mock.goserver/mock/service/service_mock.goserver/service/activities.goserver/service/devices.goserver/service/devices_test.goserver/service/handler.goserver/service/integration_desktop_test.go
Comment on lines
+989
to
+990
| onNextPage={() => setActivityPage(activityPage + 1)} | ||
| onPreviousPage={() => setActivityPage(activityPage - 1)} |
Contributor
There was a problem hiding this comment.
Harden activity pagination updates against stale state and negative pages.
Line 989 and Line 990 should use functional updates, and previous-page should clamp at 0 to avoid invalid page values.
Suggested fix
- onNextPage={() => setActivityPage(activityPage + 1)}
- onPreviousPage={() => setActivityPage(activityPage - 1)}
+ onNextPage={() => setActivityPage((prev) => prev + 1)}
+ onPreviousPage={() =>
+ setActivityPage((prev) => Math.max(0, prev - 1))
+ }📝 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.
Suggested change
| onNextPage={() => setActivityPage(activityPage + 1)} | |
| onPreviousPage={() => setActivityPage(activityPage - 1)} | |
| onNextPage={() => setActivityPage((prev) => prev + 1)} | |
| onPreviousPage={() => | |
| setActivityPage((prev) => Math.max(0, prev - 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 `@frontend/pages/hosts/details/DeviceUserPage/DeviceUserPage.tsx` around lines
989 - 990, The pagination handlers currently close over activityPage; update
onNextPage and onPreviousPage to use the functional form of setActivityPage to
avoid stale state and ensure pages never go negative—for example, call
setActivityPage(prev => prev + 1) for the next handler and setActivityPage(prev
=> Math.max(0, prev - 1)) for the previous handler; change the callbacks wired
to onNextPage and onPreviousPage in DeviceUserPage to use these functional
updates (identify the setActivityPage state setter and the
onNextPage/onPreviousPage handlers).
Author
There was a problem hiding this comment.
Uses same logic as existing HostDetailsPage. Also the handlers are inline arrow functions recreated on every render, so they always close over the current activityPage.
Piccirello
force-pushed
the
tom/host-activity-log
branch
3 times, most recently
from
19f10a2 to
4c12fdf
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/hosts/details/DeviceUserPage/DeviceUserPage.tsx`:
- Around line 297-307: The device-scoped react-query calls (query key
["device-past-activities", deviceAuthToken, activityPage] and the other similar
device-scoped query around lines 320-330 that call
activitiesAPI.getDeviceHostPastActivities) must not use keepPreviousData: true
because that causes the previous device's feed to flash when deviceAuthToken
changes; override DEFAULT_USE_QUERY_OPTIONS and set keepPreviousData: false for
these queries (or remove the flag) so the cache doesn't persist another device's
activities across token changes, and apply the same change to the other
device-scoped query that uses deviceAuthToken.
- Around line 334-336: refetchExtensions currently only calls
refetchDeviceCertificates, leaving the Activity card stale; update the function
so that when deviceCertificates is truthy it calls both
refetchDeviceCertificates() and requestRefetch(), and add requestRefetch to the
useCallback dependency array so the activity data is refreshed alongside
certificates after self-service actions.
🪄 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: 9e2dfaed-4caf-4dd5-903e-fa0386f8c74e
📒 Files selected for processing (25)
changes/my-device-activity-panelfrontend/pages/hosts/details/DeviceUserPage/DeviceUserPage.tests.tsxfrontend/pages/hosts/details/DeviceUserPage/DeviceUserPage.tsxfrontend/pages/hosts/details/cards/Activity/Activity.tsxfrontend/pages/hosts/details/cards/Activity/ActivityConfig.tsxfrontend/pages/hosts/details/cards/Activity/ActivityItems/FailedEnrollmentProfileRenewalActivityItem/FailedEnrollmentProfileRenewalActivityItem.tsxfrontend/pages/hosts/details/cards/Activity/ActivityItems/RanScriptActivityItem/RanScriptActivityItem.tsxfrontend/pages/hosts/details/cards/Activity/PastActivityFeed/PastActivityFeed.tsxfrontend/pages/hosts/details/cards/Activity/UpcomingActivityFeed/UpcomingActivityFeed.tsxfrontend/services/entities/activities.tsfrontend/test/handlers/device-handler.tsfrontend/utilities/endpoints.tsserver/activity/api/list_host_past_activities.goserver/activity/api/service.goserver/activity/internal/service/handler_test.goserver/activity/internal/service/service.goserver/fleet/activities.goserver/fleet/service.goserver/mock/activity_mock.goserver/mock/service/service_mock.goserver/service/activities.goserver/service/devices.goserver/service/devices_test.goserver/service/handler.goserver/service/integration_desktop_test.go
✅ Files skipped from review due to trivial changes (2)
- changes/my-device-activity-panel
- server/mock/service/service_mock.go
🚧 Files skipped from review as they are similar to previous changes (20)
- frontend/pages/hosts/details/cards/Activity/PastActivityFeed/PastActivityFeed.tsx
- server/activity/api/service.go
- frontend/utilities/endpoints.ts
- frontend/pages/hosts/details/cards/Activity/ActivityConfig.tsx
- frontend/pages/hosts/details/cards/Activity/UpcomingActivityFeed/UpcomingActivityFeed.tsx
- frontend/pages/hosts/details/cards/Activity/ActivityItems/FailedEnrollmentProfileRenewalActivityItem/FailedEnrollmentProfileRenewalActivityItem.tsx
- frontend/services/entities/activities.ts
- frontend/pages/hosts/details/cards/Activity/ActivityItems/RanScriptActivityItem/RanScriptActivityItem.tsx
- server/service/handler.go
- frontend/pages/hosts/details/cards/Activity/Activity.tsx
- server/activity/api/list_host_past_activities.go
- server/service/devices.go
- server/fleet/service.go
- server/service/integration_desktop_test.go
- frontend/test/handlers/device-handler.ts
- server/fleet/activities.go
- server/activity/internal/service/service.go
- server/service/devices_test.go
- server/activity/internal/service/handler_test.go
- server/service/activities.go
Comment on lines
+297
to
+307
| ["device-past-activities", deviceAuthToken, activityPage], | ||
| () => | ||
| activitiesAPI.getDeviceHostPastActivities( | ||
| deviceAuthToken, | ||
| activityPage, | ||
| DEVICE_ACTIVITY_PAGE_SIZE | ||
| ), | ||
| { | ||
| ...DEFAULT_USE_QUERY_OPTIONS, | ||
| enabled: !!deviceAuthToken, | ||
| keepPreviousData: true, |
Contributor
There was a problem hiding this comment.
Don't retain another device's activity feed across token changes.
These device-scoped queries use keepPreviousData: true, so if device_auth_token changes without a remount, the new page can briefly render the previous device's activities/counts until the new requests finish. On a My Device route, that's a host-isolation leak.
Possible safe fix
{
...DEFAULT_USE_QUERY_OPTIONS,
enabled: !!deviceAuthToken,
- keepPreviousData: true,
+ keepPreviousData: false,
}
);
@@
{
...DEFAULT_USE_QUERY_OPTIONS,
enabled: !!deviceAuthToken,
- keepPreviousData: true,
+ keepPreviousData: false,
}
);Also applies to: 320-330
🤖 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/hosts/details/DeviceUserPage/DeviceUserPage.tsx` around lines
297 - 307, The device-scoped react-query calls (query key
["device-past-activities", deviceAuthToken, activityPage] and the other similar
device-scoped query around lines 320-330 that call
activitiesAPI.getDeviceHostPastActivities) must not use keepPreviousData: true
because that causes the previous device's feed to flash when deviceAuthToken
changes; override DEFAULT_USE_QUERY_OPTIONS and set keepPreviousData: false for
these queries (or remove the flag) so the cache doesn't persist another device's
activities across token changes, and apply the same change to the other
device-scoped query that uses deviceAuthToken.
Author
There was a problem hiding this comment.
All other queries on this page have the same property.
Piccirello
force-pushed
the
tom/host-activity-log
branch
from
4c12fdf to
8a2c81d
Compare
The device user page now display an activity log of past and future events. It does not yet support displaying MDM commands.
Piccirello
force-pushed
the
tom/host-activity-log
branch
from
8a2c81d to
0cd815d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The device user page now display an activity log of past and future events. It does not yet support displaying MDM commands.
Related issue: Partially addresses #22491
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.
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
Added/updated automated tests
Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another)
QA'd all new/changed functionality manually
Summary by CodeRabbit
New Features
Behavior Changes
Tests