From 04e65bb21adf068a25f8c417c8d6337fdadfd4fa Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sat, 21 Mar 2026 15:20:15 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=B9=20chore:=20Move=20direct=20model?= =?UTF-8?q?=20usage=20from=20PermissionsController=20to=20data-schemas?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../controllers/PermissionsController.js | 51 +++++------------- .../__tests__/PermissionsController.spec.js | 52 +++++-------------- packages/data-schemas/src/methods/agent.ts | 24 ++++++++- 3 files changed, 47 insertions(+), 80 deletions(-) diff --git a/api/server/controllers/PermissionsController.js b/api/server/controllers/PermissionsController.js index 59732572c0..1f200fce83 100644 --- a/api/server/controllers/PermissionsController.js +++ b/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; diff --git a/api/server/controllers/__tests__/PermissionsController.spec.js b/api/server/controllers/__tests__/PermissionsController.spec.js index 840eaf0c30..a8d9518455 100644 --- a/api/server/controllers/__tests__/PermissionsController.spec.js +++ b/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 }, diff --git a/packages/data-schemas/src/methods/agent.ts b/packages/data-schemas/src/methods/agent.ts index f4371dd15c..36d2d819cb 100644 --- a/packages/data-schemas/src/methods/agent.ts +++ b/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 { + const Agent = mongoose.models.Agent as Model; + const User = mongoose.models.User as Model; + + 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, }; }