🧹 chore: Move direct model usage from PermissionsController to data-schemas

api/server/controllers/PermissionsController.js

@@ -15,19 +15,11 @@ const {
   ensurePrincipalExists,
   getAvailableRoles,
 } = require('~/server/services/PermissionService');
-const {
-  searchPrincipals: searchLocalPrincipals,
-  sortPrincipalsByRelevance,
-  calculateRelevanceScore,
-  findRoleByIdentifier,
-  aggregateAclEntries,
-  bulkWriteAclEntries,
-} = require('~/models');
 const {
   entraIdPrincipalFeatureEnabled,
   searchEntraIdPrincipals,
 } = require('~/server/services/GraphApiService');
-const { Agent, AclEntry, AccessRole, User } = require('~/db/models');
+const db = require('~/models');
 
 /**
  * Generic controller for resource permission endpoints
@@ -46,28 +38,6 @@ const validateResourceType = (resourceType) => {
   }
 };
 
-/**
- * Removes an agent from the favorites of specified users (fire-and-forget).
- * Both AGENT and REMOTE_AGENT resource types share the Agent collection.
- * @param {string} resourceId - The agent's MongoDB ObjectId hex string
- * @param {string[]} userIds - User ObjectId strings whose favorites should be cleaned
- */
-const removeRevokedAgentFromFavorites = (resourceId, userIds) =>
-  Agent.findOne({ _id: resourceId }, { id: 1 })
-    .lean()
-    .then((agent) => {
-      if (!agent) {
-        return;
-      }
-      return User.updateMany(
-        { _id: { $in: userIds }, 'favorites.agentId': agent.id },
-        { $pull: { favorites: { agentId: agent.id } } },
-      );
-    })
-    .catch((err) => {
-      logger.error('[removeRevokedAgentFromFavorites] Error cleaning up favorites', err);
-    });
-
 /**
  * Bulk update permissions for a resource (grant, update, remove)
  * @route PUT /api/{resourceType}/{resourceId}/permissions
@@ -187,7 +157,9 @@ const updateResourcePermissions = async (req, res) => {
       .map((p) => p.id);
 
     if (isAgentResource && revokedUserIds.length > 0) {
-      removeRevokedAgentFromFavorites(resourceId, revokedUserIds);
+      db.removeAgentFromUserFavorites(resourceId, revokedUserIds).catch((err) => {
+        logger.error('[removeRevokedAgentFromFavorites] Error cleaning up favorites', err);
+      });
     }
 
     /** @type {TUpdateResourcePermissionsResponse} */
@@ -220,7 +192,7 @@ const getResourcePermissions = async (req, res) => {
     const { resourceType, resourceId } = req.params;
     validateResourceType(resourceType);
 
-    const results = await aggregateAclEntries([
+    const results = await db.aggregateAclEntries([
       // Match ACL entries for this resource
       {
         $match: {
@@ -317,9 +289,9 @@ const getResourcePermissions = async (req, res) => {
 
     if (resourceType === ResourceType.REMOTE_AGENT) {
       const enricherDeps = {
-        aggregateAclEntries,
-        bulkWriteAclEntries,
-        findRoleByIdentifier,
+        aggregateAclEntries: db.aggregateAclEntries,
+        bulkWriteAclEntries: db.bulkWriteAclEntries,
+        findRoleByIdentifier: db.findRoleByIdentifier,
         logger,
       };
       const enrichResult = await enrichRemoteAgentPrincipals(enricherDeps, resourceId, principals);
@@ -438,7 +410,7 @@ const searchPrincipals = async (req, res) => {
       typeFilters = validTypes.length > 0 ? validTypes : null;
     }
 
-    const localResults = await searchLocalPrincipals(query.trim(), searchLimit, typeFilters);
+    const localResults = await db.searchPrincipals(query.trim(), searchLimit, typeFilters);
     let allPrincipals = [...localResults];
 
     const useEntraId = entraIdPrincipalFeatureEnabled(req.user);
@@ -494,10 +466,11 @@ const searchPrincipals = async (req, res) => {
     }
     const scoredResults = allPrincipals.map((item) => ({
       ...item,
-      _searchScore: calculateRelevanceScore(item, query.trim()),
+      _searchScore: db.calculateRelevanceScore(item, query.trim()),
     }));
 
-    const finalResults = sortPrincipalsByRelevance(scoredResults)
+    const finalResults = db
+      .sortPrincipalsByRelevance(scoredResults)
       .slice(0, searchLimit)
       .map((result) => {
         const { _searchScore, ...resultWithoutScore } = result;

api/server/controllers/__tests__/PermissionsController.spec.js

@@ -29,10 +29,13 @@ jest.mock('~/server/services/PermissionService', () => ({
   getResourcePermissionsMap: jest.fn(),
 }));
 
+const mockRemoveAgentFromUserFavorites = jest.fn();
+
 jest.mock('~/models', () => ({
   searchPrincipals: jest.fn(),
   sortPrincipalsByRelevance: jest.fn(),
   calculateRelevanceScore: jest.fn(),
+  removeAgentFromUserFavorites: (...args) => mockRemoveAgentFromUserFavorites(...args),
 }));
 
 jest.mock('~/server/services/GraphApiService', () => ({
@@ -40,20 +43,6 @@ jest.mock('~/server/services/GraphApiService', () => ({
   searchEntraIdPrincipals: jest.fn(),
 }));
 
-const mockAgentFindOne = jest.fn();
-const mockUserUpdateMany = jest.fn();
-
-jest.mock('~/db/models', () => ({
-  Agent: {
-    findOne: (...args) => mockAgentFindOne(...args),
-  },
-  AclEntry: {},
-  AccessRole: {},
-  User: {
-    updateMany: (...args) => mockUserUpdateMany(...args),
-  },
-}));
-
 const { updateResourcePermissions } = require('../PermissionsController');
 
 const createMockReq = (overrides = {}) => ({
@@ -90,10 +79,7 @@ describe('PermissionsController', () => {
         errors: [],
       });
 
-      mockAgentFindOne.mockReturnValue({
-        lean: () => Promise.resolve({ _id: agentObjectId, id: 'agent_abc123' }),
-      });
-      mockUserUpdateMany.mockResolvedValue({ modifiedCount: 1 });
+      mockRemoveAgentFromUserFavorites.mockResolvedValue(undefined);
     });
 
     it('removes agent from revoked users favorites on AGENT resource type', async () => {
@@ -111,11 +97,7 @@ describe('PermissionsController', () => {
       await flushPromises();
 
       expect(res.status).toHaveBeenCalledWith(200);
-      expect(mockAgentFindOne).toHaveBeenCalledWith({ _id: agentObjectId }, { id: 1 });
-      expect(mockUserUpdateMany).toHaveBeenCalledWith(
-        { _id: { $in: [revokedUserId] }, 'favorites.agentId': 'agent_abc123' },
-        { $pull: { favorites: { agentId: 'agent_abc123' } } },
-      );
+      expect(mockRemoveAgentFromUserFavorites).toHaveBeenCalledWith(agentObjectId, [revokedUserId]);
     });
 
     it('removes agent from revoked users favorites on REMOTE_AGENT resource type', async () => {
@@ -132,8 +114,7 @@ describe('PermissionsController', () => {
       await updateResourcePermissions(req, res);
       await flushPromises();
 
-      expect(mockAgentFindOne).toHaveBeenCalledWith({ _id: agentObjectId }, { id: 1 });
-      expect(mockUserUpdateMany).toHaveBeenCalled();
+      expect(mockRemoveAgentFromUserFavorites).toHaveBeenCalledWith(agentObjectId, [revokedUserId]);
     });
 
     it('uses results.revoked (validated) not raw request payload', async () => {
@@ -163,10 +144,7 @@ describe('PermissionsController', () => {
       await updateResourcePermissions(req, res);
       await flushPromises();
 
-      expect(mockUserUpdateMany).toHaveBeenCalledWith(
-        expect.objectContaining({ _id: { $in: [validId] } }),
-        expect.any(Object),
-      );
+      expect(mockRemoveAgentFromUserFavorites).toHaveBeenCalledWith(agentObjectId, [validId]);
     });
 
     it('skips cleanup when no USER principals are revoked', async () => {
@@ -190,8 +168,7 @@ describe('PermissionsController', () => {
       await updateResourcePermissions(req, res);
       await flushPromises();
 
-      expect(mockAgentFindOne).not.toHaveBeenCalled();
-      expect(mockUserUpdateMany).not.toHaveBeenCalled();
+      expect(mockRemoveAgentFromUserFavorites).not.toHaveBeenCalled();
     });
 
     it('skips cleanup for non-agent resource types', async () => {
@@ -216,13 +193,11 @@ describe('PermissionsController', () => {
       await flushPromises();
 
       expect(res.status).toHaveBeenCalledWith(200);
-      expect(mockAgentFindOne).not.toHaveBeenCalled();
+      expect(mockRemoveAgentFromUserFavorites).not.toHaveBeenCalled();
     });
 
     it('handles agent not found gracefully', async () => {
-      mockAgentFindOne.mockReturnValue({
-        lean: () => Promise.resolve(null),
-      });
+      mockRemoveAgentFromUserFavorites.mockResolvedValue(undefined);
 
       const req = createMockReq({
         params: { resourceType: ResourceType.AGENT, resourceId: agentObjectId },
@@ -237,13 +212,12 @@ describe('PermissionsController', () => {
       await updateResourcePermissions(req, res);
       await flushPromises();
 
-      expect(mockAgentFindOne).toHaveBeenCalled();
-      expect(mockUserUpdateMany).not.toHaveBeenCalled();
+      expect(mockRemoveAgentFromUserFavorites).toHaveBeenCalled();
       expect(res.status).toHaveBeenCalledWith(200);
     });
 
-    it('logs error when User.updateMany fails without blocking response', async () => {
-      mockUserUpdateMany.mockRejectedValue(new Error('DB connection lost'));
+    it('logs error when removeAgentFromUserFavorites fails without blocking response', async () => {
+      mockRemoveAgentFromUserFavorites.mockRejectedValue(new Error('DB connection lost'));
 
       const req = createMockReq({
         params: { resourceType: ResourceType.AGENT, resourceId: agentObjectId },

packages/data-schemas/src/methods/agent.ts

@@ -741,19 +741,39 @@ export function createAgentMethods(mongoose: typeof import('mongoose'), deps: Ag
     return await Agent.countDocuments({ is_promoted: true });
   }
 
+  /** Removes an agent from the favorites of specified users. */
+  async function removeAgentFromUserFavorites(
+    resourceId: string,
+    userIds: string[],
+  ): Promise<void> {
+    const Agent = mongoose.models.Agent as Model<IAgent>;
+    const User = mongoose.models.User as Model<unknown>;
+
+    const agent = await Agent.findOne({ _id: resourceId }, { id: 1 }).lean();
+    if (!agent) {
+      return;
+    }
+
+    await User.updateMany(
+      { _id: { $in: userIds }, 'favorites.agentId': agent.id },
+      { $pull: { favorites: { agentId: agent.id } } },
+    );
+  }
+
   return {
-    createAgent,
     getAgent,
     getAgents,
+    createAgent,
     updateAgent,
     deleteAgent,
     deleteUserAgents,
     revertAgentVersion,
     countPromotedAgents,
     addAgentResourceFile,
-    removeAgentResourceFiles,
     getListAgentsByAccess,
+    removeAgentResourceFiles,
     generateActionMetadataHash,
+    removeAgentFromUserFavorites,
   };
 }