fix code

devksingh4 · devksingh4 · commit a885ba6a8134 · 2025-07-14T20:41:19.000-04:00
diff --git a/src/api/functions/membership.ts b/src/api/functions/membership.ts
@@ -21,6 +21,8 @@ import Redis from "ioredis";
 import { getKey, setKey } from "./redisCache.js";
 import { FastifyBaseLogger } from "fastify";
 import type pino from "pino";
+import { createAuditLogEntry } from "./auditLog.js";
+import { Modules } from "common/modules.js";
 
 export const MEMBER_CACHE_SECONDS = 43200; // 12 hours
 
@@ -30,12 +32,14 @@ export async function patchExternalMemberList({
   remove: oldRemove,
   clients: { dynamoClient, redisClient },
   logger,
+  auditLogData: { actor, requestId },
 }: {
   listId: string;
   add: string[];
   remove: string[];
   clients: { dynamoClient: DynamoDBClient; redisClient: Redis.default };
   logger: pino.Logger | FastifyBaseLogger;
+  auditLogData: { actor: string; requestId: string };
 }) {
   const listId = oldListId.toLowerCase();
   const add = oldAdd.map((x) => x.toLowerCase());
@@ -104,11 +108,41 @@ export async function patchExternalMemberList({
       logger,
     }),
   );
+  const auditLogPromises = [];
+  if (add.length > 0) {
+    auditLogPromises.push(
+      createAuditLogEntry({
+        dynamoClient,
+        entry: {
+          module: Modules.EXTERNAL_MEMBERSHIP,
+          actor,
+          requestId,
+          message: `Added ${add.length} member(s) to target list.`,
+          target: listId,
+        },
+      }),
+    );
+  }
+  if (remove.length > 0) {
+    auditLogPromises.push(
+      createAuditLogEntry({
+        dynamoClient,
+        entry: {
+          module: Modules.EXTERNAL_MEMBERSHIP,
+          actor,
+          requestId,
+          message: `Removed ${remove.length} member(s) from target list.`,
+          target: listId,
+        },
+      }),
+    );
+  }
   await Promise.all([
     ...removeCacheInvalidation,
     ...addCacheInvalidation,
     ...batchPromises,
   ]);
+  await Promise.all(auditLogPromises);
 }
 export async function getExternalMemberList(
   list: string,
diff --git a/src/api/routes/membership.ts b/src/api/routes/membership.ts
@@ -307,6 +307,10 @@ const membershipPlugin: FastifyPluginAsync = async (fastify, _options) => {
           listId,
           clients: { dynamoClient, redisClient },
           logger: request.log,
+          auditLogData: {
+            actor: request.username!,
+            requestId: request.id,
+          },
         });
         return reply.status(201).send();
       },
diff --git a/src/common/modules.ts b/src/common/modules.ts
@@ -10,6 +10,7 @@ export enum Modules {
   AUDIT_LOG = "auditLog",
   API_KEY = "apiKey",
   ROOM_RESERVATIONS = "roomReservations",
+  EXTERNAL_MEMBERSHIP = "externalMembership",
 }
 
 
@@ -25,4 +26,5 @@ export const ModulesToHumanName: Record<Modules, string> = {
   [Modules.AUDIT_LOG]: "Audit Log",
   [Modules.API_KEY]: "API Keys",
   [Modules.ROOM_RESERVATIONS]: "Room Reservations",
+  [Modules.EXTERNAL_MEMBERSHIP]: "External Membership",
 }
diff --git a/src/common/types/logs.ts b/src/common/types/logs.ts
@@ -2,7 +2,7 @@ import { Modules } from "../modules.js";
 import * as z from "zod/v4";
 
 export const loggingEntry = z.object({
-  module: z.nativeEnum(Modules),
+  module: z.enum(Modules),
   actor: z.string().min(1),
   target: z.string().min(1),
   requestId: z.optional(z.string().min(1).uuid()),
diff --git a/src/ui/pages/externalMembership/ExternalMemberListManagement.tsx b/src/ui/pages/externalMembership/ExternalMemberListManagement.tsx
@@ -13,12 +13,16 @@ import {
   Select,
   Box,
   Textarea,
+  Accordion,
+  ScrollArea,
+  Stack,
 } from "@mantine/core";
 import { IconUserPlus, IconTrash } from "@tabler/icons-react";
 import { notifications } from "@mantine/notifications";
 import { illinoisNetId } from "@common/types/generic";
 import { AuthGuard } from "@ui/components/AuthGuard";
 import { AppRoles } from "@common/roles";
+import pluralize from "pluralize";
 
 interface ExternalMemberListManagementProps {
   fetchMembers: (listId: string) => Promise<string[]>;
@@ -32,6 +36,7 @@ interface ExternalMemberListManagementProps {
 }
 
 const ITEMS_PER_PAGE = 10;
+const CHANGE_DISPLAY_LIMIT = 10;
 
 const ExternalMemberListManagement: React.FC<
   ExternalMemberListManagementProps
@@ -202,25 +207,40 @@ const ExternalMemberListManagement: React.FC<
   };
 
   const handleReplaceList = () => {
-    const rawNetIds = replacementList
+    const allLines = replacementList
       .split("\n")
-      .map((id) => id.trim())
+      .map((line) => line.trim())
       .filter(Boolean);
     const validNetIds = new Set<string>();
     const invalidEntries: string[] = [];
 
-    for (const netId of rawNetIds) {
-      if (illinoisNetId.safeParse(netId).success) {
-        validNetIds.add(netId);
+    for (const line of allLines) {
+      // Rule: If it contains "@" but is not a valid "@illinois.edu" email, it's invalid.
+      if (line.includes("@") && !line.endsWith("@illinois.edu")) {
+        invalidEntries.push(line);
+        continue;
+      }
+
+      // Strip the domain to get the potential NetID for Zod validation
+      const potentialNetId = line.replace("@illinois.edu", "");
+
+      if (illinoisNetId.safeParse(potentialNetId).success) {
+        validNetIds.add(potentialNetId);
       } else {
-        invalidEntries.push(netId);
+        invalidEntries.push(line); // Add the original failing line
       }
     }
 
     if (invalidEntries.length > 0) {
+      const pluralize = (singular: string, plural: string, count: number) =>
+        count === 1 ? singular : plural;
+      const verbIs = pluralize("is", "are", invalidEntries.length);
+      const verbHas = pluralize("has", "have", invalidEntries.length);
+      const entriesStr = invalidEntries.join(", ");
+
       notifications.show({
         title: "Invalid Entries Skipped",
-        message: `The following ${invalidEntries.length} entries were invalid and have been ignored.`,
+        message: `${entriesStr} ${verbIs} invalid and ${verbHas} been ignored.`,
         color: "orange",
       });
     }
@@ -238,11 +258,19 @@ const ExternalMemberListManagement: React.FC<
 
     setReplaceModalOpened(false);
     setReplacementList("");
-    notifications.show({
-      title: "Changes Computed",
-      message: `Queued ${membersToAdd.length} additions and ${membersToRemove.length} removals. Click 'Save Changes' to apply.`,
-      color: "blue",
-    });
+    if (membersToAdd.length + membersToRemove.length > 0) {
+      notifications.show({
+        title: "Changes Computed",
+        message: `Queued ${membersToAdd.length} additions and ${membersToRemove.length} removals. Click 'Save Changes' to apply.`,
+        color: "blue",
+      });
+    } else {
+      notifications.show({
+        title: "No Changes Found",
+        message: `Both lists are the same.`,
+        color: "green",
+      });
+    }
   };
 
   const rows = paginatedData.map((member) => {
@@ -357,7 +385,7 @@ const ExternalMemberListManagement: React.FC<
       <Table verticalSpacing="sm" highlightOnHover>
         <Table.Thead>
           <Table.Tr>
-            <Table.Th>Member</Table.Th>
+            <Table.Th>Member NetID</Table.Th>
             <Table.Th>Status</Table.Th>
             <Table.Th>Actions</Table.Th>
           </Table.Tr>
@@ -398,16 +426,19 @@ const ExternalMemberListManagement: React.FC<
         </Table.Tbody>
       </Table>
 
-      {totalPages > 1 && (
-        <Group justify="center" mt="md">
+      <Stack justify="center" align="center" mt="md">
+        <Text size="sm" c="dimmed">
+          Found {members.length} {pluralize("member", members.length)}.
+        </Text>
+        {totalPages > 1 && (
           <Pagination
             total={totalPages}
             value={activePage}
             onChange={setPage}
             disabled={isLoading}
           />
-        </Group>
-      )}
+        )}
+      </Stack>
 
       <AuthGuard
         isAppShell={false}
@@ -461,25 +492,65 @@ const ExternalMemberListManagement: React.FC<
             <Text fw={500} size="sm" mb="xs">
               Members to Add:
             </Text>
-            {toAdd.map((netId) => (
-              <Text key={netId} fz="sm">
-                {" "}
-                - {netId}
-              </Text>
-            ))}
+            {toAdd.length > CHANGE_DISPLAY_LIMIT ? (
+              <Accordion variant="separated" radius="md">
+                <Accordion.Item value="add-list">
+                  <Accordion.Control>
+                    <Text fz="sm">{toAdd.length} members</Text>
+                  </Accordion.Control>
+                  <Accordion.Panel>
+                    <ScrollArea h={200}>
+                      {toAdd.map((netId) => (
+                        <Text key={netId} fz="sm" py={2}>
+                          - {netId}
+                        </Text>
+                      ))}
+                    </ScrollArea>
+                  </Accordion.Panel>
+                </Accordion.Item>
+              </Accordion>
+            ) : (
+              toAdd.map((netId) => (
+                <Text key={netId} fz="sm">
+                  {" "}
+                  - {netId}
+                </Text>
+              ))
+            )}
           </Box>
         )}
         {toRemove.length > 0 && (
           <Box>
             <Text fw={500} size="sm" mb="xs">
               Members to Remove:
             </Text>
-            {toRemove.map((netId) => (
-              <Text key={netId} fz="sm" c="red">
-                {" "}
-                - {netId}
-              </Text>
-            ))}
+            {toRemove.length > CHANGE_DISPLAY_LIMIT ? (
+              <Accordion variant="separated" radius="md">
+                <Accordion.Item value="remove-list">
+                  <Accordion.Control>
+                    <Text fz="sm" c="red">
+                      {toRemove.length} members
+                    </Text>
+                  </Accordion.Control>
+                  <Accordion.Panel>
+                    <ScrollArea h={200}>
+                      {toRemove.map((netId) => (
+                        <Text key={netId} fz="sm" c="red" py={2}>
+                          - {netId}
+                        </Text>
+                      ))}
+                    </ScrollArea>
+                  </Accordion.Panel>
+                </Accordion.Item>
+              </Accordion>
+            ) : (
+              toRemove.map((netId) => (
+                <Text key={netId} fz="sm" c="red">
+                  {" "}
+                  - {netId}
+                </Text>
+              ))
+            )}
           </Box>
         )}
         <Group justify="flex-end" mt="lg">
@@ -544,7 +615,7 @@ const ExternalMemberListManagement: React.FC<
           necessary additions and removals to match the list you provide.
         </Text>
         <Textarea
-          placeholder={"jdoe1\nasmith2\njohnson3"}
+          placeholder={"jdoe2\nasmith3@illinois.edu\njohns4"}
           value={replacementList}
           onChange={(e) => setReplacementList(e.currentTarget.value)}
           autosize

Original file line number	Diff line number	Diff line change
`@@ -10,6 +10,7 @@ export enum Modules {`
`10`	`10`	`AUDIT_LOG = "auditLog",`
`11`	`11`	`API_KEY = "apiKey",`
`12`	`12`	`ROOM_RESERVATIONS = "roomReservations",`
	`13`	`+ EXTERNAL_MEMBERSHIP = "externalMembership",`
`13`	`14`	`}`
`14`	`15`
`15`	`16`
`@@ -25,4 +26,5 @@ export const ModulesToHumanName: Record<Modules, string> = {`
`25`	`26`	`[Modules.AUDIT_LOG]: "Audit Log",`
`26`	`27`	`[Modules.API_KEY]: "API Keys",`
`27`	`28`	`[Modules.ROOM_RESERVATIONS]: "Room Reservations",`
	`29`	`+ [Modules.EXTERNAL_MEMBERSHIP]: "External Membership",`
`28`	`30`	`}`