Merge pull request #1564 from Northeastern-Electric-Racing/#1425-allow-leads-edit-vendors

#1425 - Allow Finance Leads to Edit Vendors on the Admin Page

Author: RChandler234

.github/workflows/run-tests.yml

@@ -20,6 +20,8 @@ jobs:
     steps:
       - name: Checkout repository
         uses: actions/checkout@v3
+      - name: Set env variables
+        run: touch .env && echo "FINANCE_TEAM_ID=0" >> .env
       - name: Set up Node.js
         uses: actions/setup-node@v3
         with:

src/backend/src/prisma/seed.ts

@@ -22,7 +22,7 @@ import ChangeRequestsService from '../services/change-requests.services';
 import projectQueryArgs from '../prisma-query-args/projects.query-args';
 import TeamsService from '../services/teams.services';
 import WorkPackagesService from '../services/work-packages.services';
-import { ChangeRequest, ClubAccount, StandardChangeRequest, validateWBS, WbsElementStatus, WorkPackageStage } from 'shared';
+import { ClubAccount, StandardChangeRequest, validateWBS, WbsElementStatus, WorkPackageStage } from 'shared';
 import TasksService from '../services/tasks.services';
 import DescriptionBulletsService from '../services/description-bullets.services';
 import { seedProject } from './seed-data/projects.seed';
@@ -208,15 +208,12 @@ const performSeed: () => Promise<void> = async () => {
     batman,
     justiceLeague.teamId,
     [
-      wonderwoman,
       flash,
       aquaman,
       superman,
       hawkMan,
       hawkWoman,
-      cyborg,
       greenLantern,
-      martianManhunter,
       lexLuther,
       hawkgirl,
       elongatedMan,
@@ -227,6 +224,11 @@ const performSeed: () => Promise<void> = async () => {
       hankHeywood
     ].map((user) => user.userId)
   );
+  await TeamsService.setTeamLeads(
+    batman,
+    justiceLeague.teamId,
+    [wonderwoman, cyborg, martianManhunter].map((user) => user.userId)
+  );
   await TeamsService.setTeamMembers(
     aang,
     avatarBenders.teamId,

src/backend/src/routes/reimbursement-requests.routes.ts

@@ -86,8 +86,6 @@ reimbursementRequestsRouter.post(
 reimbursementRequestsRouter.post(
   '/vendors/create',
   nonEmptyString(body('name')),
-  body('allowedRefundSources').isArray(),
-  isAccount(body('allowedRefundSources.*')),
   validateInputs,
   ReimbursementRequestController.createVendor
 );

src/backend/src/services/reimbursement-requests.services.ts

@@ -21,6 +21,7 @@ import {
 } from 'shared';
 import prisma from '../prisma/prisma';
 import {
+  isUserLeadOrHeadOfFinanceTeam,
   removeDeletedReceiptPictures,
   updateReimbursementProducts,
   validateReimbursementProducts,
@@ -451,7 +452,12 @@ export default class ReimbursementRequestService {
    * @returns the created vendor
    */
   static async createVendor(submitter: User, name: string) {
-    if (!isAdmin(submitter.role)) throw new AccessDeniedAdminOnlyException('create vendors');
+    const failedAuthorizationException = new AccessDeniedException(
+      'Only admins, finance leads, and finance heads can create vendors.'
+    );
+
+    const isAuthorized = isAdmin(submitter.role) || (await isUserLeadOrHeadOfFinanceTeam(submitter));
+    if (!isAuthorized) throw failedAuthorizationException;
 
     const vendor = await prisma.vendor.create({
       data: {

src/backend/src/transformers/auth-user.transformer.ts

@@ -1,7 +1,11 @@
 import { Prisma } from '@prisma/client';
 import { AuthenticatedUser } from 'shared';
 import authUserQueryArgs from '../prisma-query-args/auth-user.query-args';
-import { isAuthUserHeadOfFinance, isAuthUserOnFinance } from '../utils/reimbursement-requests.utils';
+import {
+  isAuthUserHeadOfFinance,
+  isAuthUserAtLeastLeadForFinance,
+  isAuthUserOnFinance
+} from '../utils/reimbursement-requests.utils';
 
 const authenticatedUserTransformer = (user: Prisma.UserGetPayload<typeof authUserQueryArgs>): AuthenticatedUser => {
   return {
@@ -16,6 +20,7 @@ const authenticatedUserTransformer = (user: Prisma.UserGetPayload<typeof authUse
     favoritedProjectsId: user.favoriteProjects.map((project) => project.projectId),
     isFinance: isAuthUserOnFinance(user),
     isHeadOfFinance: isAuthUserHeadOfFinance(user),
+    isAtLeastFinanceLead: isAuthUserAtLeastLeadForFinance(user),
     changeRequestsToReviewId: user.changeRequestsToReview.map((changeRequest) => changeRequest.crId)
   };
 };

src/backend/src/utils/reimbursement-requests.utils.ts

@@ -202,17 +202,70 @@ const createNewProducts = async (products: ReimbursementProductCreateArgs[], rei
   }
 };
 
+/**
+ * Validates that the given user is on the finance team.
+ *
+ * @param user The user to validate.
+ * @throws {AccessDeniedException} Fails validation when user is not on the
+ * finance team.
+ */
 export const validateUserIsPartOfFinanceTeam = async (user: User) => {
+  const isUserAuthorized = await isUserOnFinanceTeam(user);
+
+  if (!isUserAuthorized) {
+    throw new AccessDeniedException(`You are not a member of the finance team!`);
+  }
+};
+
+/**
+ * Determines if a user is part of the finance team.
+ *
+ * To be used for Prisma input validation of a plain User, as opposed to
+ * <code>isAuthUserOnFinance</code>, which uses the additional fields
+ * produced by authUserQueryArgs that are not in the User type by default.
+ *
+ * @param user the user to authenticate
+ * @returns whether the user is on the finance team
+ * @throws {HttpException} if finance team not found in database
+ */
+export const isUserOnFinanceTeam = async (user: User): Promise<boolean> => {
+  if (!process.env.FINANCE_TEAM_ID) {
+    throw new Error('FINANCE_TEAM_ID not in env');
+  }
+
   const financeTeam = await prisma.team.findUnique({
     where: { teamId: process.env.FINANCE_TEAM_ID },
     include: { head: true, leads: true, members: true }
   });
 
   if (!financeTeam) throw new HttpException(500, 'Finance team does not exist!');
+  return isUserOnTeam(financeTeam, user);
+};
 
-  if (!isUserOnTeam(financeTeam, user)) {
-    throw new AccessDeniedException(`You are not a member of the finance team!`);
+/**
+ * Determines if a user is lead or head of the finance team.
+ *
+ * To be used for Prisma input validation of a plain User, as opposed to
+ * <code>isAuthUserAtLeastLeadForFinance</code>, which uses the additional fields
+ * produced by authUserQueryArgs that are not in the User type by default.
+ *
+ * @param user the user to authenticate
+ * @returns whether the user is lead or head of the finance team
+ * @throws {HttpException} if finance team not found in database
+ */
+export const isUserLeadOrHeadOfFinanceTeam = async (user: User): Promise<boolean> => {
+  if (!process.env.FINANCE_TEAM_ID) {
+    throw new Error('FINANCE_TEAM_ID not in env');
   }
+
+  const financeTeam = await prisma.team.findUnique({
+    where: { teamId: process.env.FINANCE_TEAM_ID },
+    include: { head: true, leads: true }
+  });
+
+  if (!financeTeam) throw new HttpException(500, 'Finance team does not exist!');
+
+  return user.userId === financeTeam.headId || financeTeam.leads.map((u) => u.userId).includes(user.userId);
 };
 
 export const isAuthUserOnFinance = (user: Prisma.UserGetPayload<typeof authUserQueryArgs>) => {
@@ -226,6 +279,18 @@ export const isAuthUserOnFinance = (user: Prisma.UserGetPayload<typeof authUserQ
   );
 };
 
+/**
+ * Determines if the user is a finance lead or head.
+ * @param user the user to check
+ * @returns Whether they are a finance lead.
+ */
+export const isAuthUserAtLeastLeadForFinance = (user: Prisma.UserGetPayload<typeof authUserQueryArgs>) => {
+  if (!process.env.FINANCE_TEAM_ID) return false;
+  const financeTeamId = process.env.FINANCE_TEAM_ID;
+  const { teamAsHead, teamsAsLead } = user;
+  return teamAsHead?.teamId === financeTeamId || isTeamIdInList(financeTeamId, teamsAsLead);
+};
+
 export const isAuthUserHeadOfFinance = (user: Prisma.UserGetPayload<typeof authUserQueryArgs>) => {
   return user.teamAsHead?.teamId === process.env.FINANCE_TEAM_ID;
 };

src/backend/tests/reimbursement-requests.test.ts

@@ -24,15 +24,24 @@ import {
   prismaReimbursementStatus,
   sharedGiveMeMyMoney
 } from './test-data/reimbursement-requests.test-data';
-import { alfred, batman, flash, sharedBatman, superman, wonderwoman, theVisitor } from './test-data/users.test-data';
+import {
+  alfred,
+  batman,
+  flash,
+  sharedBatman,
+  superman,
+  wonderwoman,
+  theVisitor,
+  aquaman,
+  greenlantern
+} from './test-data/users.test-data';
 import reimbursementRequestQueryArgs from '../src/prisma-query-args/reimbursement-requests.query-args';
 import { Prisma, Reimbursement_Status_Type } from '@prisma/client';
 import {
   reimbursementRequestTransformer,
   reimbursementTransformer
 } from '../src/transformers/reimbursement-requests.transformer';
-import { prismaTeam1 } from './test-data/teams.test-data';
-import { justiceLeague, primsaTeam2 } from './test-data/teams.test-data';
+import { justiceLeague, prismaTeam1, primsaTeam2 } from './test-data/teams.test-data';
 
 describe('Reimbursement Requests', () => {
   beforeEach(() => {});
@@ -42,6 +51,12 @@ describe('Reimbursement Requests', () => {
   });
 
   describe('Vendor Tests', () => {
+    beforeAll(() => {
+      // Circular Dependency Check
+      expect(prismaTeam1.head).toBeDefined();
+      expect(primsaTeam2.head).toBeDefined();
+    });
+
     test('Get all vendors works', async () => {
       vi.spyOn(prisma.vendor, 'findMany').mockResolvedValue([]);
 
@@ -51,13 +66,39 @@ describe('Reimbursement Requests', () => {
       expect(res).toStrictEqual([]);
     });
 
-    test('Create Vendor throws error if user is not admin', async () => {
-      await expect(ReimbursementRequestService.createVendor(wonderwoman, 'HOLA BUDDY')).rejects.toThrow(
-        new AccessDeniedAdminOnlyException('create vendors')
+    test('Create Vendor throws error if user is not admin or finance lead', async () => {
+      vi.spyOn(prisma.team, 'findUnique').mockResolvedValue(prismaTeam1);
+      await expect(ReimbursementRequestService.createVendor(aquaman, 'HOLA BUDDY')).rejects.toThrow(
+        new AccessDeniedException('Only admins, finance leads, and finance heads can create vendors.')
+      );
+    });
+
+    test('Create Vendor works for finance leads', async () => {
+      vi.spyOn(prisma.team, 'findUnique').mockResolvedValue(prismaTeam1);
+      vi.spyOn(prisma.vendor, 'create').mockResolvedValue(PopEyes);
+      await expect(ReimbursementRequestService.createVendor(wonderwoman, 'HOLA BUDDY')).resolves.not.toThrow(
+        new AccessDeniedException('Only admins, finance leads, and finance heads can create vendors.')
+      );
+    });
+
+    test('Create Vendor works for finance head', async () => {
+      vi.spyOn(prisma.team, 'findUnique').mockResolvedValue({ ...primsaTeam2, headId: 5 });
+      vi.spyOn(prisma.vendor, 'create').mockResolvedValue(PopEyes);
+      await expect(ReimbursementRequestService.createVendor(greenlantern, 'HOLA BUDDY')).resolves.not.toThrow(
+        new AccessDeniedException('Only admins, finance leads, and finance heads can create vendors.')
+      );
+    });
+
+    test('Create Vendor works for admin', async () => {
+      vi.spyOn(prisma.team, 'findUnique').mockResolvedValue(primsaTeam2);
+      vi.spyOn(prisma.vendor, 'create').mockResolvedValue(PopEyes);
+      await expect(ReimbursementRequestService.createVendor(flash, 'HOLA BUDDY')).resolves.not.toThrow(
+        new AccessDeniedException('Only admins, finance leads, and finance heads can create vendors.')
       );
     });
 
     test('Create Vendor Successfully returns vendor Id', async () => {
+      vi.spyOn(prisma.team, 'findUnique').mockResolvedValueOnce(prismaTeam1);
       vi.spyOn(prisma.vendor, 'create').mockResolvedValue(PopEyes);
 
       const vendor = await ReimbursementRequestService.createVendor(batman, 'HOLA BUDDY');

src/backend/tests/test-data/users.test-data.ts

@@ -1,6 +1,5 @@
 import { Role as PrismaRole, Theme, User as PrismaUser, User_Settings, User_Secure_Settings, Team } from '@prisma/client';
 import { User as SharedUser } from 'shared';
-import { prismaTeam1 } from './teams.test-data';
 
 export const batman: PrismaUser = {
   userId: 1,
@@ -114,7 +113,9 @@ export const alfred: PrismaUser & { teamsAsMember: Team[]; teamsAsLead: Team[] }
   emailId: 'butler',
   role: PrismaRole.APP_ADMIN,
   googleAuthId: 'u',
-  teamsAsMember: [prismaTeam1],
+  // Do NOT put a team here! This will create a circular dependency that breaks tests.
+  // Do this instead: { ...alfred, teamsAsMember: [<your team>]}
+  teamsAsMember: [],
   teamsAsLead: []
 };
 

src/frontend/src/layouts/NavTopBar/NavUserMenu.tsx

@@ -17,7 +17,7 @@ import ListItemText from '@mui/material/ListItemText';
 import ListItemIcon from '@mui/material/ListItemIcon';
 import SettingsIcon from '@mui/icons-material/Settings';
 import LogoutIcon from '@mui/icons-material/Logout';
-import { isHead } from 'shared';
+import { canAccessAdminTools } from '../../utils/users';
 
 const NavUserMenu: React.FC = () => {
   const [anchorEl, setAnchorEl] = useState<null | HTMLElement>(null);
@@ -115,7 +115,7 @@ const NavUserMenu: React.FC = () => {
           </ListItemIcon>
           <ListItemText>Settings</ListItemText>
         </MenuItem>
-        {isHead(auth.user?.role) && <AdminTools />}
+        {canAccessAdminTools(auth.user) && <AdminTools />}
         {import.meta.env.MODE === 'development' ? <DevLogout /> : <ProdLogout />}
       </Menu>
     </>

src/frontend/src/pages/AdminToolsPage/AdminTools.tsx

@@ -1,16 +1,16 @@
 import { Redirect, Route, Switch } from 'react-router-dom';
-import { isHead } from 'shared';
 import LoadingIndicator from '../../components/LoadingIndicator';
 import { useAuth } from '../../hooks/auth.hooks';
 import { routes } from '../../utils/routes';
 import AdminToolsPage from './AdminToolsPage';
+import { canAccessAdminTools } from '../../utils/users';
 
 const AdminTools: React.FC = () => {
   const auth = useAuth();
 
   if (!auth.user) return <LoadingIndicator />;
 
-  if (!isHead(auth.user.role)) {
+  if (!canAccessAdminTools(auth.user)) {
     return (
       <Redirect
         to={{