Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 13 additions & 39 deletions apps/admin-dashboard/app/routes/_dashboard.students.remove.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as Sentry from '@sentry/react-router';
import {
type ActionFunctionArgs,
data,
Expand All @@ -10,7 +9,6 @@ import {
import z from 'zod';

import { job } from '@oyster/core/bull';
import { db } from '@oyster/db';
import {
Button,
ErrorMessage,
Expand Down Expand Up @@ -54,10 +52,18 @@ export async function action({ request }: ActionFunctionArgs) {
return data(result, { status: 400 });
}

const count = await removeMembers(result.data.memberIds);
const ids = result.data.memberIds;

const batches = splitArray(ids, 10);

for (const batch of batches) {
job('student.batch_remove', {
memberIds: batch,
});
}

toast(session, {
message: `Removed ${count} members.`,
message: `Removing ${ids.length} members asynchronously.`,
});

return redirect(Route['/students'], {
Expand All @@ -67,38 +73,6 @@ export async function action({ request }: ActionFunctionArgs) {
});
}

async function removeMembers(ids: string[]): Promise<number> {
const batches = splitArray(ids, 10);

let count = 0;

for (const batch of batches) {
try {
const students = await db
.deleteFrom('students')
.where('id', 'in', batch)
.returning(['airtableId', 'email', 'firstName', 'slackId'])
.execute();

for (const student of students) {
job('student.removed', {
airtableId: student.airtableId as string,
email: student.email,
firstName: student.firstName,
sendViolationEmail: false,
slackId: student.slackId,
});
}

count += students.length;
} catch (e) {
Sentry.captureException(e);
}
}

return count;
}

export default function RemoveMembersPage() {
const { error, errors } = getErrors(useActionData<typeof action>());

Expand All @@ -115,9 +89,9 @@ export default function RemoveMembersPage() {
</Modal.Description>

<Callout color="blue">
Note: These members will immediately be removed from our database, but
it may take some time for them to be removed from Slack, Mailchimp and
Airtable.
Note: This process will run asynchronously and if there are a lot of
members to remove, it may take several hours to fully remove them from
Slack, Mailchimp and Airtable.
</Callout>

<Form className="form" method="post">
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/infrastructure/bull.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,12 @@ export const StudentBullJob = z.discriminatedUnion('name', [
name: z.literal('student.anniversary.email'),
data: z.object({}),
}),
z.object({
name: z.literal('student.batch_remove'),
data: z.object({
memberIds: z.array(Student.shape.id),
}),
}),
z.object({
name: z.literal('student.birthdate.daily'),
data: z.object({}),
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/modules/members/members.worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
} from '@/modules/airtable';
import { sendCompanyReviewNotifications } from '@/modules/employment/use-cases/send-company-review-notifications';
import { syncLinkedInProfiles } from '@/modules/linkedin';
import { batchRemoveMembers } from '@/modules/members/use-cases/batch-remove-members';
import { sendAnniversaryEmail } from '@/modules/members/use-cases/send-anniversary-email';
import { sendGraduationEmail } from '@/modules/members/use-cases/send-graduation-email';
import { success } from '@/shared/utils/core';
Expand All @@ -32,6 +33,9 @@ export const memberWorker = registerWorker(
.with({ name: 'student.anniversary.email' }, ({ data }) => {
return sendAnniversaryEmail(data);
})
.with({ name: 'student.batch_remove' }, ({ data }) => {
return batchRemoveMembers(data);
})
.with({ name: 'student.birthdate.daily' }, ({ data }) => {
return sendBirthdayNotification(data);
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { db } from '@oyster/db';

import { job } from '@/infrastructure/bull';
import { type GetBullJobData } from '@/infrastructure/bull.types';

export async function batchRemoveMembers({
memberIds,
}: GetBullJobData<'student.batch_remove'>) {
const students = await db
.deleteFrom('students')
.where('id', 'in', memberIds)
.returning(['airtableId', 'email', 'firstName', 'slackId'])
.execute();

for (const student of students) {
job('student.removed', {
airtableId: student.airtableId as string,
email: student.email,
firstName: student.firstName,
sendViolationEmail: false,
slackId: student.slackId,
});
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Missing Error Handling in Batch Member Removal

The batchRemoveMembers function is missing the try-catch error handling and Sentry reporting from the original removeMembers function. Without it, database operations and subsequent job queueing failures won't be caught or reported, leading to unhandled exceptions and reduced observability of member removal issues.

Fix in Cursor Fix in Web