Converted old activity module into function. Cleaned up activity types. by getvictor · Pull Request #40752 · fleetdm/fleet

getvictor · 2026-02-28T01:13:23Z

Related issue: Resolves #38536

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/ or ee/fleetd-chrome/changes.
- Changes file present in previous PR.

Testing

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

Summary by CodeRabbit

Refactor
- Reorganized internal activity tracking infrastructure across services to improve code maintainability and reduce complexity.

getvictor · 2026-02-28T01:13:47Z

@coderabbitai full review

coderabbitai · 2026-02-28T01:13:57Z

✅ Actions performed

Full review triggered.

coderabbitai · 2026-02-28T01:18:17Z

Walkthrough

The PR completes the activity bounded context migration by removing the legacy ActivityModule facade and replacing it with direct fleet.NewActivityFunc function-based delegation. It eliminates server/service/modules/activities/activities.go, converts Activity and ActivityDetails to type aliases of their bounded-context equivalents, and updates all service and handler constructors to inject the new function type instead of the module. Changes span cmd/fleet/serve.go, SCIM handlers, Android MDM services, and test utilities. The NewActivity method signature is updated to accept the bounded-context ActivityDetails type. This removes an intermediate translation layer and ensures activity operations flow directly through the bounded context.

Possibly related PRs

Refactors activity wiring in cmd/fleet/serve.go and switches handlers to use NewActivity function instead of ActivityModule
Updates activity wiring across services and removes the legacy activities module facade in similar fashion
Migrates activity creation from injected ActivityModule to NewActivityFunc in service constructors and call sites

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	PR description is incomplete. It includes a partial checklist but is missing critical sections from the template such as input validation confirmation, database migration details, and testing explanation.	Complete the PR description by filling out all relevant checklist items, explaining what was tested and why, and addressing database migration concerns if applicable.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title accurately summarizes the main changes: converting activity module to function-based approach and cleaning up activity types.
Linked Issues check	✅ Passed	The PR implements most objectives from `#38536`: NewActivity moved to bounded context, activity cleanup split, legacy code removed from activities module and datastore, and types cleaned in fleet/activities.go.
Out of Scope Changes check	✅ Passed	Changes are focused on activity module refactoring, type cleanup, and wiring updates across affected services; all align with `#38536` objectives of completing the activity bounded context migration.

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

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch victor/38536-activity-cleanup2

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

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

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

server/mdm/android/service/enterprises_test.go (1)
31-32: Optional cleanup: inline noopNewActivity at callsites.

newActivity := noopNewActivity is repeated and can be passed directly to NewServiceWithClient for less test noise.

Also applies to: 129-132, 249-250, 269-270, 315-316, 364-365, 405-406, 446-447, 513-514, 543-544
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/mdm/android/service/enterprises_test.go` around lines 31 - 32, The
test repeats the trivial assignment newActivity := noopNewActivity before
calling NewServiceWithClient, creating noise; remove the local newActivity
variable and pass noopNewActivity directly as the argument to
NewServiceWithClient (e.g., replace newActivity usage with noopNewActivity in
each NewServiceWithClient call), and delete the now-unused newActivity
declarations so noopNewActivity is inlined at all call sites (references:
noopNewActivity, NewServiceWithClient).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/service/testing_utils.go`:
- Line 438: TestServerOpts.NewActivity (type NewActivityFunc) was added but
never used when constructing the test service, so setting it has no effect;
update the test service creation to pass TestServerOpts.NewActivity into the
service/activity initialization path — locate where TestServerOpts is consumed
(e.g., in NewTestServer or the test service builder), and if present use the
NewActivityFunc to replace the default activity factory by wiring it into the
code that constructs fleet.NewActivity (or the field NewActivity) so tests that
set TestServerOpts.NewActivity actually get the provided function.

---

Nitpick comments:
In `@server/mdm/android/service/enterprises_test.go`:
- Around line 31-32: The test repeats the trivial assignment newActivity :=
noopNewActivity before calling NewServiceWithClient, creating noise; remove the
local newActivity variable and pass noopNewActivity directly as the argument to
NewServiceWithClient (e.g., replace newActivity usage with noopNewActivity in
each NewServiceWithClient call), and delete the now-unused newActivity
declarations so noopNewActivity is inlined at all call sites (references:
noopNewActivity, NewServiceWithClient).

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9fa8952 and 8e94e14.

📒 Files selected for processing (17)

cmd/fleet/serve.go
ee/server/scim/scim.go
ee/server/scim/users.go
ee/server/scim/users_test.go
server/fleet/activities.go
server/mdm/android/arch_test.go
server/mdm/android/service/enterprises_test.go
server/mdm/android/service/pubsub.go
server/mdm/android/service/pubsub_test.go
server/mdm/android/service/service.go
server/mdm/android/tests/testing_utils.go
server/service/activities.go
server/service/integration_mdm_test.go
server/service/integrationtest/android/suite.go
server/service/modules/activities/activities.go
server/service/testing_client.go
server/service/testing_utils.go

💤 Files with no reviewable changes (2)

server/mdm/android/arch_test.go
server/service/modules/activities/activities.go

server/service/testing_utils.go

Copilot

Pull request overview

This PR continues the activity bounded-context migration by removing the legacy ActivityModule facade and standardizing activity creation around a fleet.NewActivityFunc, while also consolidating activity types by aliasing fleet.Activity/fleet.ActivityDetails to the canonical server/activity/api types.

Changes:

Remove the server/service/modules/activities module and replace usage with fleet.NewActivityFunc injections.
Alias legacy activity domain types (fleet.Activity, fleet.ActivityDetails) to server/activity/api to reduce duplication and clarify ownership.
Update Android MDM and SCIM codepaths/tests to pass and use the new activity creation function.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
server/service/testing_utils.go	Removes `ActivityModule` wiring from test server options and related setup; adds `NewActivity` opt (currently unused).
server/service/testing_client.go	Restores `listActivitiesResponse` type in the test client helpers after removal from service code.
server/service/modules/activities/activities.go	Deletes the legacy activity module facade.
server/service/integrationtest/android/suite.go	Updates Android integration suite to pass `fleetSvc.NewActivity` directly instead of an activity module.
server/service/integration_mdm_test.go	Updates Android service setup in integration tests to use a `NewActivity` function instead of a module.
server/service/activities.go	Switches `Service.NewActivity` to accept the canonical `activity_api.ActivityDetails` and removes now-unneeded response type definition.
server/mdm/android/tests/testing_utils.go	Replaces test `ActivityModule` no-op with a `noopNewActivity` function.
server/mdm/android/service/service.go	Updates Android service constructors and internals to store/call `fleet.NewActivityFunc` instead of `ActivityModule`.
server/mdm/android/service/pubsub_test.go	Updates Android pubsub tests to pass `noopNewActivity`.
server/mdm/android/service/pubsub.go	Replaces activity module calls with the injected `newActivity` function.
server/mdm/android/service/enterprises_test.go	Updates enterprise tests to pass `noopNewActivity` and removes the module type.
server/mdm/android/arch_test.go	Removes now-deleted `server/service/modules/activities` from allowed dependencies.
server/fleet/activities.go	Aliases `fleet.Activity` and `fleet.ActivityDetails` to `server/activity/api` canonical types.
ee/server/scim/users_test.go	Updates SCIM handler tests to provide `svc.NewActivity` function.
ee/server/scim/users.go	Updates SCIM user handler to depend on `fleet.NewActivityFunc` instead of `ActivityModule`.
ee/server/scim/scim.go	Updates SCIM registration to pass `svc.NewActivity` into the handler.
cmd/fleet/serve.go	Removes activity module bootstrap and passes an activity function into Android service initialization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

server/service/testing_utils.go

codecov · 2026-02-28T01:42:26Z

Codecov Report

❌ Patch coverage is 57.14286% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.30%. Comparing base (861d0ee) to head (2b9eaf8).
⚠️ Report is 11 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/fleet/serve.go	0.00%	5 Missing ⚠️
server/mdm/android/service/pubsub.go	20.00%	2 Missing and 2 partials ⚠️
server/mdm/android/service/service.go	25.00%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #40752      +/-   ##
==========================================
- Coverage   66.30%   66.30%   -0.01%     
==========================================
  Files        2470     2469       -1     
  Lines      197754   197714      -40     
  Branches     8668     8661       -7     
==========================================
- Hits       131125   131090      -35     
+ Misses      54766    54760       -6     
- Partials    11863    11864       +1

Flag	Coverage Δ
backend	`68.09% <57.14%> (-0.01%)`	⬇️

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.

…-cleanup2 # Conflicts: # server/mdm/android/service/enterprises_test.go # server/mdm/android/tests/testing_utils.go # server/service/integration_mdm_test.go # server/service/integrationtest/android/suite.go

Converted old activity module into function. Cleaned up activity types.

8e94e14

getvictor temporarily deployed to Docker Hub February 28, 2026 01:13 — with GitHub Actions Inactive

getvictor requested a review from Copilot February 28, 2026 01:13

Copilot started reviewing on behalf of getvictor February 28, 2026 01:14 View session

coderabbitai bot reviewed Feb 28, 2026

View reviewed changes

server/service/testing_utils.go Outdated Show resolved Hide resolved

Copilot AI reviewed Feb 28, 2026

View reviewed changes

server/service/testing_utils.go Outdated Show resolved Hide resolved

Removed dead code.

a794799

getvictor temporarily deployed to Docker Hub February 28, 2026 01:28 — with GitHub Actions Inactive

getvictor marked this pull request as ready for review February 28, 2026 01:42

getvictor requested a review from a team as a code owner February 28, 2026 01:42

getvictor assigned JordanMontgomery Feb 28, 2026

cdcme previously approved these changes Mar 2, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into victor/38536-activity…

10a5e48

…-cleanup2 # Conflicts: # server/mdm/android/service/enterprises_test.go # server/mdm/android/tests/testing_utils.go # server/service/integration_mdm_test.go # server/service/integrationtest/android/suite.go

getvictor dismissed cdcme’s stale review via 10a5e48 March 2, 2026 20:55

getvictor temporarily deployed to Docker Hub March 2, 2026 20:56 — with GitHub Actions Inactive

Fixed test.

2b9eaf8

getvictor temporarily deployed to Docker Hub March 2, 2026 21:03 — with GitHub Actions Inactive

getvictor requested a review from cdcme March 2, 2026 21:34

cdcme approved these changes Mar 2, 2026

View reviewed changes

getvictor merged commit a0581a3 into main Mar 3, 2026
51 checks passed

getvictor deleted the victor/38536-activity-cleanup2 branch March 3, 2026 07:01

coderabbitai bot mentioned this pull request Mar 7, 2026

Renamed activity tables and moved host activities cleanup to activity bounded context. #41194

Merged

5 tasks

coderabbitai bot mentioned this pull request Mar 17, 2026

Recovery lock password activities #41529

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Converted old activity module into function. Cleaned up activity types.#40752

Converted old activity module into function. Cleaned up activity types.#40752
getvictor merged 4 commits intomainfrom
victor/38536-activity-cleanup2

getvictor commented Feb 28, 2026 •

edited

Loading

Uh oh!

getvictor commented Feb 28, 2026

Uh oh!

coderabbitai bot commented Feb 28, 2026

Uh oh!

coderabbitai bot commented Feb 28, 2026 •

edited

Loading

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

codecov bot commented Feb 28, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

getvictor commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist for submitter

Testing

Summary by CodeRabbit

Uh oh!

getvictor commented Feb 28, 2026

Uh oh!

coderabbitai bot commented Feb 28, 2026

Uh oh!

coderabbitai bot commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Possibly related PRs

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

codecov bot commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

getvictor commented Feb 28, 2026 •

edited

Loading

coderabbitai bot commented Feb 28, 2026 •

edited

Loading

codecov bot commented Feb 28, 2026 •

edited

Loading