Activity bounded context: /api/latest/fleet/activities (2 of 2)

[getvictor]

Related issue: Resolves #37806

Removed ds.ListActivities from the legacy datastore and updated code/tests to use the new activity bounded context instead.

The changes to cron.go and most changes to mysql/activities_test.go will eventually be migrated to the activity bounded context. The current changes are an intermediate step.

The issues tracked by #38234 will be addressed in additional/parallel PRs shortly.

Checklist for submitter

• Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
  • Done in the previous PR

Testing

• Added/updated automated tests
• QA'd all new/changed functionality manually

Summary by CodeRabbit

• Refactor

  • Migrated activity retrieval from direct datastore calls to a service-based architecture for improved maintainability and consistency.
  • Enhanced system context handling for background automation tasks to ensure proper authorization during scheduled operations.
  • Streamlined activity recording for automated processes with dedicated system identity tracking.

• Tests

  • Updated test infrastructure with new helpers for activity service integration across test suites.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[codecov]

Codecov Report

❌ Patch coverage is 83.67347% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.05%. Comparing base (6bdebf5) to head (23c86f5).
⚠️ Report is 56 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/fleet/serve.go	0.00%	5 Missing ⚠️
cmd/fleet/cron.go	87.50%	1 Missing ⚠️
server/service/activities.go	0.00%	1 Missing ⚠️
server/service/modules/activities/activities.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #38478      +/-   ##
==========================================
- Coverage   66.08%   66.05%   -0.03%     
==========================================
  Files        2413     2409       -4     
  Lines      192673   192475     -198     
  Branches     8526     8459      -67     
==========================================
- Hits       127329   127143     -186     
+ Misses      53779    53770       -9     
+ Partials    11565    11562       -3     

Flag	Coverage Δ
backend	67.90% <83.67%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[getvictor]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Walkthrough

Replaces legacy datastore activity listing with the new activity bounded-context service across server and cron code, adds a system-level context for cron automation, removes legacy ListActivities API/types, and updates tests and test helpers to use the activity_api ListActivities service and types.

Changes

Cohort / File(s)	Summary
Cron & Serve wiring cmd/fleet/cron.go, cmd/fleet/serve.go, cmd/fleet/serve_test.go	Injects activity_api.ListActivitiesService (activitySvc) into cron scheduling and serve bootstrap; updates signatures to pass activitySvc into activity streaming scheduler and tests.
Activity bounded-context bootstrap cmd/fleet/serve.go	createActivityBoundedContext now returns (activity_api.Service, endpointer.HandlerRoutesFunc) so callers can wire the activity service into other subsystems.
Legacy datastore API removal server/fleet/app.go, server/fleet/datastore.go, server/datastore/mysql/activities.go, server/mock/datastore_mock.go	Removes legacy ListActivitiesOptions type and Datastore.ListActivities method and its MySQL/mock implementations; internal automation author string handling consolidated.
System context & viewer server/contexts/viewer/viewer.go, server/contexts/viewer/viewer_test.go	Adds systemUser and NewSystemContext(ctx) to provide an admin/system viewer for internal/cron operations; includes unit test.
Activity automation constant & usage server/fleet/activities.go, server/service/activities.go, server/service/modules/activities/activities.go	Adds ActivityAutomationAuthor constant ("Fleet") and updates code to construct automation actor names via ptr.String(...) instead of local variables.
Test helpers & migration server/datastore/mysql/testing_utils.go, server/datastore/mysql/activities_test.go, server/service/hosts_test.go, server/service/testing_client.go, server/service/integration_*_test.go	Adds NewTestActivityService and ListActivitiesAPI test helpers; refactors tests to use activity_api.ListOptions, activity_api.Activity, and the activity service instead of direct datastore calls; introduces listActivities() convenience helper in test client.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Cron
participant Scheduler
participant ActivitySvc as ActivityService
participant Datastore
participant Audit as AuditLogger

Cron->>Scheduler: trigger activities streaming job
Scheduler->>ActivitySvc: ListActivities(ctx=NewSystemContext, opts)
ActivitySvc-->>Datastore: query activities (pagination)
Datastore-->>ActivitySvc: return activities
ActivitySvc-->>Scheduler: return activity list
Scheduler->>Audit: log streamed activities
Audit-->>Cron: ack

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Activity bounded context: /api/latest/fleet/activities (1 of 2) #38115 — Introduces the activity bounded context and activity_api.Service that this PR wires into serve.go and cron jobs.
• Created activity bounded context with 1 API endpoint. #37807 — Issue/POC describing the overall migration of the activities HTTP API into the activity bounded context; strongly related to the goals implemented here.

Suggested reviewers

• cdcme
• JordanMontgomery

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 17.86% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The PR description is incomplete. It lacks several critical sections from the template including input validation, SQL injection prevention, database migrations, and most importantly, it is missing all checked items' details and implementation descriptions.	Complete the PR description by adding details for all checked items in the template, particularly clarifying the testing approach, validation methodology, and any database-related changes made.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: removing the legacy ds.ListActivities and migrating to the new activity bounded context API, which is part 2 of the implementation.
Linked Issues check	✅ Passed	The PR successfully implements the Phase 13-15 objectives from issue #37806: removes legacy ListActivities from datastore/service, updates cron jobs to use the activity API service, and provides test helpers for legacy tests.
Out of Scope Changes check	✅ Passed	All changes are directly related to the PR objectives. The modifications remove legacy activity listing, wire the new activity bounded context, update cron jobs, and adapt tests - all within scope of migrating to the new activity API.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch victor/37806-fleet-activities-2

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[getvictor]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@server/service/integration_core_test.go`:
- Around line 3313-3318: The test relies on s.listActivities() returning a
complete, deterministically ordered set but that helper may be paginated or
unordered; update the test or the helper so the count-based assertions are
stable: either (a) modify listActivities() to explicitly fetch all activities
(no pagination) and return them sorted by timestamp/ID (e.g., newest-first) so
len(s.listActivities()) is deterministic, or (b) change the test to query the
datastore directly for the total activity count or to call the datastore method
used by NewActivity (e.g., s.ds.ListActivities or similar) with explicit
per_page and sort parameters to ensure the newly created activity is included;
reference s.listActivities, s.ds.NewActivity and any
ListActivities/ListActivitiesOptions functions to locate where to add explicit
pagination/sorting or to change the test to a direct datastore query.

♻️ Duplicate comments (2)

server/service/integration_core_test.go (2)

8031-8053: Same listActivities() pagination/ordering sensitivity as above.

---

8184-8212: Same listActivities() pagination/ordering sensitivity as above.

[iansltx]

Seems like there's still a fair amount of activities tendrils in the main modules here, particularly for testing, but this is at least a step in a decoupled direction. Feedback here is minor.

[JordanMontgomery]

PR Looks good to me. The only questions I had were around the activities cron which seemingly makes use of the new service + old DS methods but it looks like the work to finish that is captured in your tickets so I am not concerned

[getvictor]

PR Looks good to me. The only questions I had were around the activities cron which seemingly makes use of the new service + old DS methods but it looks like the work to finish that is captured in your tickets so I am not concerned

Yes, moving the cron is the next story: #38534

[getvictor]

@lucasmrod does not have time to review this right now due to critical speccing work, but he agreed to review this when he has time. He's OK merging this now, so as not to block follow-up work.