From d942657abd3dedbc28d4c549fa0156aec31e8544 Mon Sep 17 00:00:00 2001 From: ribhavsharma Date: Mon, 3 Nov 2025 23:56:08 -0800 Subject: [PATCH 01/11] feat: add option to send email notificatoin when chatbot answer is changed --- packages/common/index.ts | 5 + .../components/EditChatbotQuestionModal.tsx | 65 ++++++++++- packages/frontend/app/api/index.ts | 16 +++ .../server/src/chatbot/chatbot.controller.ts | 105 +++++++++++++++++- 4 files changed, 188 insertions(+), 3 deletions(-) diff --git a/packages/common/index.ts b/packages/common/index.ts index 0285a3ba3..985c72b03 100644 --- a/packages/common/index.ts +++ b/packages/common/index.ts @@ -312,6 +312,7 @@ export enum MailServiceType { ASYNC_QUESTION_NEW_COMMENT_ON_MY_POST = 'async_question_new_comment_on_my_post', ASYNC_QUESTION_NEW_COMMENT_ON_OTHERS_POST = 'async_question_new_comment_on_others_post', COURSE_CLONE_SUMMARY = 'course_clone_summary', + CHATBOT_ANSWER_UPDATED = 'chatbot_answer_updated', } /** * Represents one of three possible user roles in a course. @@ -484,6 +485,10 @@ export interface UpdateChatbotQuestionParams { docId: string pageNumbersString: string }[] + // Frontend-only flag to request notification emails after an update. + // Server must validate the answer actually changed and will strip this before + // forwarding to the external chatbot service. + emailNotifyOnAnswerUpdate?: boolean } // this is the response from the backend when new questions are asked diff --git a/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx b/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx index e7b94bc83..7fbc5962e 100644 --- a/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx +++ b/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react' +import { useEffect, useState, memo } from 'react' import { Button, Checkbox, @@ -46,6 +46,22 @@ interface EditChatbotQuestionModalProps { cid: number deleteQuestion: (id: string) => void } +type AnswerUpdateCheckboxProps = { + form: any + originalAnswer: string + checked?: boolean + onChange?: (e: any) => void +} + +const AnswerUpdateCheckbox = memo( + ({ form, originalAnswer, checked, onChange }) => { + const currentAnswer = Form.useWatch('answer', form) + const changed = (currentAnswer ?? '') !== (originalAnswer ?? '') + return ( + + ) + }, +) const EditChatbotQuestionModal: React.FC = ({ open, @@ -158,8 +174,33 @@ const EditChatbotQuestionModal: React.FC = ({ await API.chatbot.staffOnly .updateQuestion(cid, valuesWithId) - .then(() => { + .then(async () => { message.success('Question updated successfully') + const notify = (values as any).emailNotifyOnAnswerUpdate + if (notify) { + try { + const resp = await API.chatbot.staffOnly.notifyAnswerUpdate( + cid, + editingRecord.vectorStoreId, + { + oldAnswer: editingRecord.answer, + newAnswer: values.answer, + oldQuestion: editingRecord.question, + newQuestion: values.question, + }, + ) + if (resp?.recipients != undefined) { + message.success( + `Notification email sent to ${resp.recipients} student${ + resp.recipients === 1 ? '' : 's' + }`, + ) + } + } catch (e) { + const errorMessage = getErrorMessage(e) + message.error('Failed to send notification email: ' + errorMessage) + } + } onSuccessfulUpdate() }) .catch((e) => { @@ -227,6 +268,7 @@ const EditChatbotQuestionModal: React.FC = ({ question: editingRecord.question, verified: editingRecord.verified, suggested: editingRecord.suggested, + emailNotifyOnAnswerUpdate: false, sourceDocuments: editingRecord.sourceDocuments, }} clearOnDestroy @@ -256,6 +298,25 @@ const EditChatbotQuestionModal: React.FC = ({ > + +

+ Sends an email to the student(s) who previously asked this + question in this course with a before/after of the answer. +

+ + } + > + + => this.req('DELETE', `/api/v1/chatbot/document/${courseId}/${docId}`), + notifyAnswerUpdate: async ( + courseId: number, + questionId: string, + body: { + oldAnswer: string + newAnswer: string + oldQuestion?: string + newQuestion?: string + }, + ): Promise<{ recipients: number }> => + this.req( + 'POST', + `/api/v1/chatbot/question/${courseId}/${questionId}/notify`, + undefined, + body, + ), uploadDocument: async ( courseId: number, body: FormData, diff --git a/packages/server/src/chatbot/chatbot.controller.ts b/packages/server/src/chatbot/chatbot.controller.ts index 0b727fa33..c8e403f4f 100644 --- a/packages/server/src/chatbot/chatbot.controller.ts +++ b/packages/server/src/chatbot/chatbot.controller.ts @@ -45,6 +45,7 @@ import { GetChatbotHistoryResponse, GetInteractionsAndQuestionsResponse, InteractionResponse, + MailServiceType, LLMType, OllamaLLMType, OpenAILLMType, @@ -87,6 +88,8 @@ import { IgnoreableClassSerializerInterceptor, IgnoreSerializer, } from '../interceptors/IgnoreableClassSerializerInterceptor'; +import { MailService } from '../mail/mail.service'; +import { ChatbotQuestionModel } from './question.entity'; @Controller('chatbot') @UseGuards(JwtAuthGuard, EmailVerifiedGuard) @@ -95,6 +98,7 @@ export class ChatbotController { constructor( private readonly chatbotService: ChatbotService, private readonly chatbotApiService: ChatbotApiService, + private readonly mailService: MailService, ) {} // @@ -331,8 +335,10 @@ export class ChatbotController { @User({ chat_token: true }) user: UserModel, ): Promise { handleChatbotTokenCheck(user); + // Do not forward frontend-only flags to the chatbot API + const { emailNotifyOnAnswerUpdate, ...sanitized } = questionData as any; return await this.chatbotApiService.updateQuestion( - questionData, + sanitized, courseId, user.chat_token.token, ); @@ -378,6 +384,103 @@ export class ChatbotController { // } // resets all chatbot data for the course. Unused + + // Professor-only: send notification email to all students who asked this question + // Body must include oldAnswer and newAnswer, and can optionally include question changes for context + @Post('question/:courseId/:questionId/notify') + @UseGuards(CourseRolesGuard) + @Roles(Role.PROFESSOR) + async notifyUpdatedAnswer( + @Param('courseId', ParseIntPipe) courseId: number, + @Param('questionId') questionId: string, + @Body() + body: { + oldAnswer: string; + newAnswer: string; + oldQuestion?: string; + newQuestion?: string; + }, + @User({ chat_token: true }) user: UserModel, + ): Promise<{ recipients: number }> { + handleChatbotTokenCheck(user); + + const { oldAnswer, newAnswer, oldQuestion, newQuestion } = body ?? {}; + if (oldAnswer == undefined || newAnswer == undefined) { + throw new BadRequestException( + 'oldAnswer and newAnswer must be provided to send notifications.', + ); + } + // Enforce: only allow notify when answer changed + if ((oldAnswer ?? '').trim() === (newAnswer ?? '').trim()) { + throw new BadRequestException( + 'Cannot send notification: answer was not changed.', + ); + } + + // Find all interactions in this course where this question was asked + const asked = await ChatbotQuestionModel.find({ + where: { vectorStoreId: questionId }, + relations: { interaction: { user: true, course: true } }, + }); + const recipients = Array.from( + new Set( + asked + .filter((a) => a.interaction?.course?.id === courseId) + .map((a) => a.interaction?.user?.email) + .filter((e): e is string => !!e), + ), + ); + + if (recipients.length === 0) { + return { recipients: 0 }; + } + + // Building email content + const qChanged = + (oldQuestion ?? '').trim() !== (newQuestion ?? oldQuestion ?? '').trim(); + + const questionSection = qChanged + ? `Question (before)${ + oldQuestion ?? '' + } + Question (after)${ + newQuestion ?? '' + }` + : `Question${ + oldQuestion ?? newQuestion ?? '' + }`; + + const html = ` +
+

The answer to a chatbot question you asked has been updated.

+ + ${questionSection} + + + + + + + + +
Answer (before)${oldAnswer}
Answer (after)${newAnswer}
+
+

You can revisit the course chatbot from your course page.

+
+ `; + + await this.mailService + .sendEmail({ + receiverOrReceivers: recipients, + subject: 'Chatbot answer updated', + type: MailServiceType.CHATBOT_ANSWER_UPDATED, + content: html, + track: false, + }) + .catch(() => {}); + + return { recipients: recipients.length }; + } // @Get('resetCourse/:courseId') // @UseGuards(CourseRolesGuard) // @Roles(Role.PROFESSOR, Role.TA) From 7fbf3926c8cd7247dc95f16615444cfacbe74865 Mon Sep 17 00:00:00 2001 From: ribhavsharma Date: Tue, 11 Nov 2025 15:32:15 -0800 Subject: [PATCH 02/11] add mail service to provider --- packages/server/src/chatbot/chatbot.module.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/server/src/chatbot/chatbot.module.ts b/packages/server/src/chatbot/chatbot.module.ts index ee640d5ce..3dba0cdcf 100644 --- a/packages/server/src/chatbot/chatbot.module.ts +++ b/packages/server/src/chatbot/chatbot.module.ts @@ -7,6 +7,7 @@ import { CacheModule } from '@nestjs/cache-manager'; import { ChatbotDataSourceService } from './chatbot-datasource/chatbot-datasource.service'; import { ChatbotDataSourceModule } from './chatbot-datasource/chatbot-datasource.module'; import { PostgresConnectionOptions } from 'typeorm/driver/postgres/PostgresConnectionOptions'; +import { MailService } from 'mail/mail.service'; @Module({ controllers: [ChatbotController], @@ -21,7 +22,12 @@ export class ChatbotModule { CacheModule.register(), ChatbotDataSourceModule.forRoot(connectionOptions), ], - providers: [ChatbotService, ChatbotApiService, ChatbotSettingsSubscriber], + providers: [ + ChatbotService, + ChatbotApiService, + ChatbotSettingsSubscriber, + MailService, + ], }; } } From 31447deff13c9c0b6363db346d0b5cd1ece2ee9e Mon Sep 17 00:00:00 2001 From: ribhavsharma Date: Mon, 24 Nov 2025 17:52:50 -0800 Subject: [PATCH 03/11] remove unnecessary dependency from lms test --- .../lmsIntegration/lmsIntegration.service.spec.ts | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/packages/server/src/lmsIntegration/lmsIntegration.service.spec.ts b/packages/server/src/lmsIntegration/lmsIntegration.service.spec.ts index d0bfa627a..ccc551dda 100644 --- a/packages/server/src/lmsIntegration/lmsIntegration.service.spec.ts +++ b/packages/server/src/lmsIntegration/lmsIntegration.service.spec.ts @@ -6,11 +6,7 @@ import { } from './lmsIntegration.service'; import { Test, TestingModule } from '@nestjs/testing'; import { CACHE_MANAGER } from '@nestjs/cache-manager'; -import { - TestChatbotConnectionOptions, - TestConfigModule, - TestTypeOrmModule, -} from '../../test/util/testUtils'; +import { TestConfigModule, TestTypeOrmModule } from '../../test/util/testUtils'; import { AbstractLMSAdapter, LMSIntegrationAdapter, @@ -39,7 +35,6 @@ import { LMSAnnouncementModel } from './lmsAnnouncement.entity'; import { LMSAssignmentModel } from './lmsAssignment.entity'; import { FactoryModule } from 'factory/factory.module'; import { FactoryService } from 'factory/factory.service'; -import { ChatbotModule } from '../chatbot/chatbot.module'; import { ChatbotApiService } from '../chatbot/chatbot-api.service'; const mockCacheManager = { @@ -60,12 +55,7 @@ describe('LMSIntegrationService', () => { beforeAll(async () => { const module: TestingModule = await Test.createTestingModule({ - imports: [ - TestTypeOrmModule, - TestConfigModule, - FactoryModule, - ChatbotModule.forRoot(TestChatbotConnectionOptions), - ], + imports: [TestTypeOrmModule, TestConfigModule, FactoryModule], providers: [ LMSIntegrationService, LMSIntegrationAdapter, From eca8b32a2e7f9105874185761cde82138b4026af Mon Sep 17 00:00:00 2001 From: Adam Fipke Date: Tue, 25 Nov 2025 18:08:54 -0800 Subject: [PATCH 04/11] reduce bottom margin of answer field --- .../chatbot_questions/components/EditChatbotQuestionModal.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx b/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx index 7fbc5962e..64e61c263 100644 --- a/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx +++ b/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx @@ -287,6 +287,7 @@ const EditChatbotQuestionModal: React.FC = ({ , classNames: { From 11eeef31edd5e661fabb9f3942d4e2f9e2a7286c Mon Sep 17 00:00:00 2001 From: ribhavsharma Date: Mon, 1 Dec 2025 21:45:51 -0800 Subject: [PATCH 05/11] addressing feedback --- packages/common/index.ts | 20 ++- .../components/EditChatbotQuestionModal.tsx | 44 ++++--- packages/frontend/app/api/index.ts | 4 +- .../src/chatbot/chatbot.controller.spec.ts | 122 ++++++++++++++++++ .../server/src/chatbot/chatbot.controller.ts | 77 ++++++----- .../src/chatbot/chatbot.service.spec.ts | 9 ++ packages/server/test/chatbot.integration.ts | 113 +++++++++++++++- 7 files changed, 330 insertions(+), 59 deletions(-) create mode 100644 packages/server/src/chatbot/chatbot.controller.spec.ts diff --git a/packages/common/index.ts b/packages/common/index.ts index 985c72b03..7eb9d7dbc 100644 --- a/packages/common/index.ts +++ b/packages/common/index.ts @@ -485,10 +485,22 @@ export interface UpdateChatbotQuestionParams { docId: string pageNumbersString: string }[] - // Frontend-only flag to request notification emails after an update. - // Server must validate the answer actually changed and will strip this before - // forwarding to the external chatbot service. - emailNotifyOnAnswerUpdate?: boolean +} + +export class NotifyUpdatedChatbotAnswerParams { + @IsString() + oldAnswer!: string + + @IsString() + newAnswer!: string + + @IsString() + @IsOptional() + oldQuestion?: string + + @IsString() + @IsOptional() + newQuestion?: string } // this is the response from the backend when new questions are asked diff --git a/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx b/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx index 64e61c263..d16d40c79 100644 --- a/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx +++ b/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState, memo } from 'react' +import { useEffect, useState } from 'react' import { Button, Checkbox, @@ -31,6 +31,7 @@ interface FormValues { answer: string verified: boolean suggested: boolean + emailNotifyOnAnswerUpdate?: boolean sourceDocuments: SourceDocument[] selectedDocuments: { docId: string @@ -53,15 +54,16 @@ type AnswerUpdateCheckboxProps = { onChange?: (e: any) => void } -const AnswerUpdateCheckbox = memo( - ({ form, originalAnswer, checked, onChange }) => { - const currentAnswer = Form.useWatch('answer', form) - const changed = (currentAnswer ?? '') !== (originalAnswer ?? '') - return ( - - ) - }, -) +const AnswerUpdateCheckbox: React.FC = ({ + form, + originalAnswer, + checked, + onChange, +}) => { + const currentAnswer = Form.useWatch('answer', form) + const changed = (currentAnswer ?? '') !== (originalAnswer ?? '') + return +} const EditChatbotQuestionModal: React.FC = ({ open, @@ -166,8 +168,9 @@ const EditChatbotQuestionModal: React.FC = ({ }) } + const { emailNotifyOnAnswerUpdate, ...sanitizedValues } = values const valuesWithId = { - ...values, + ...sanitizedValues, id: editingRecord.vectorStoreId, sourceDocuments: values.sourceDocuments || [], } @@ -176,8 +179,7 @@ const EditChatbotQuestionModal: React.FC = ({ .updateQuestion(cid, valuesWithId) .then(async () => { message.success('Question updated successfully') - const notify = (values as any).emailNotifyOnAnswerUpdate - if (notify) { + if (emailNotifyOnAnswerUpdate) { try { const resp = await API.chatbot.staffOnly.notifyAnswerUpdate( cid, @@ -190,11 +192,15 @@ const EditChatbotQuestionModal: React.FC = ({ }, ) if (resp?.recipients != undefined) { - message.success( - `Notification email sent to ${resp.recipients} student${ - resp.recipients === 1 ? '' : 's' - }`, - ) + if (resp.recipients === 5) { + message.success('Notification email sent to 5 users (max 5).') + } else { + message.success( + `Notification email sent to ${resp.recipients} user${ + resp.recipients === 1 ? '' : 's' + }`, + ) + } } } catch (e) { const errorMessage = getErrorMessage(e) @@ -308,7 +314,7 @@ const EditChatbotQuestionModal: React.FC = ({

Sends an email to the student(s) who previously asked this - question in this course with a before/after of the answer. + question with a before/after of the answer.

} diff --git a/packages/frontend/app/api/index.ts b/packages/frontend/app/api/index.ts index a04a9c161..95a0030b2 100644 --- a/packages/frontend/app/api/index.ts +++ b/packages/frontend/app/api/index.ts @@ -314,7 +314,7 @@ class APIClient { this.req('DELETE', `/api/v1/chatbot/document/${courseId}/${docId}`), notifyAnswerUpdate: async ( courseId: number, - questionId: string, + vectorStoreId: string, body: { oldAnswer: string newAnswer: string @@ -324,7 +324,7 @@ class APIClient { ): Promise<{ recipients: number }> => this.req( 'POST', - `/api/v1/chatbot/question/${courseId}/${questionId}/notify`, + `/api/v1/chatbot/question/${courseId}/${vectorStoreId}/notify`, undefined, body, ), diff --git a/packages/server/src/chatbot/chatbot.controller.spec.ts b/packages/server/src/chatbot/chatbot.controller.spec.ts new file mode 100644 index 000000000..1c284a623 --- /dev/null +++ b/packages/server/src/chatbot/chatbot.controller.spec.ts @@ -0,0 +1,122 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { ChatbotController } from './chatbot.controller'; +import { ChatbotService } from './chatbot.service'; +import { ChatbotApiService } from './chatbot-api.service'; +import { MailService } from 'mail/mail.service'; +import { ChatbotQuestionModel } from './question.entity'; +import { BadRequestException } from '@nestjs/common'; +import { NotifyUpdatedChatbotAnswerParams, Role } from '@koh/common'; + +describe('ChatbotController - notifyUpdatedAnswer', () => { + let controller: ChatbotController; + let mailService: { sendEmail: jest.Mock }; + + const mockChatbotService = {}; + const mockChatbotApiService = {}; + + beforeAll(async () => { + mailService = { + sendEmail: jest.fn(), + }; + + const module: TestingModule = await Test.createTestingModule({ + controllers: [ChatbotController], + providers: [ + { provide: ChatbotService, useValue: mockChatbotService }, + { provide: ChatbotApiService, useValue: mockChatbotApiService }, + { provide: MailService, useValue: mailService }, + ], + }).compile(); + + controller = module.get(ChatbotController); + }); + + afterEach(() => { + jest.restoreAllMocks(); + mailService.sendEmail.mockReset(); + }); + + const makeUser = (courseId: number, role: Role): any => ({ + id: 1, + courses: [{ courseId, role }], + }); + + const makeBody = ( + overrides: Partial = {}, + ): NotifyUpdatedChatbotAnswerParams => ({ + oldAnswer: 'old', + newAnswer: 'new', + ...overrides, + }); + + it('throws BadRequestException when answer did not change', async () => { + const body = makeBody({ oldAnswer: 'same', newAnswer: ' same ' }); + + await expect( + controller.notifyUpdatedAnswer( + 1, + 'vec-id', + body, + makeUser(1, Role.PROFESSOR), + ), + ).rejects.toBeInstanceOf(BadRequestException); + }); + + it('throws BadRequestException when there are no recipients', async () => { + jest.spyOn(ChatbotQuestionModel, 'find').mockResolvedValue([] as any); + + const body = makeBody(); + + await expect( + controller.notifyUpdatedAnswer( + 1, + 'vec-id', + body, + makeUser(1, Role.PROFESSOR), + ), + ).rejects.toBeInstanceOf(BadRequestException); + + expect(ChatbotQuestionModel.find).toHaveBeenCalledTimes(1); + expect(mailService.sendEmail).not.toHaveBeenCalled(); + }); + + it('sends individual emails and caps at 5 recipients', async () => { + const courseId = 1; + const vectorStoreId = 'vec-id'; + const emails = [ + 'u1@example.com', + 'u2@example.com', + 'u3@example.com', + 'u4@example.com', + 'u5@example.com', + 'u6@example.com', + ]; + + jest.spyOn(ChatbotQuestionModel, 'find').mockResolvedValue( + emails.map((email) => ({ + interaction: { + course: { id: courseId, name: 'Test Course' }, + user: { email }, + }, + })) as any, + ); + + const body = makeBody(); + + const result = await controller.notifyUpdatedAnswer( + courseId, + vectorStoreId, + body, + makeUser(courseId, Role.PROFESSOR), + ); + + // Should only send to first 5 emails + expect(mailService.sendEmail).toHaveBeenCalledTimes(5); + const calledRecipients = mailService.sendEmail.mock.calls.map( + (args) => args[0].receiverOrReceivers, + ); + expect(calledRecipients).toEqual(emails.slice(0, 5)); + + expect(result).toEqual({ recipients: 5 }); + }); +}); diff --git a/packages/server/src/chatbot/chatbot.controller.ts b/packages/server/src/chatbot/chatbot.controller.ts index c8e403f4f..73bb68530 100644 --- a/packages/server/src/chatbot/chatbot.controller.ts +++ b/packages/server/src/chatbot/chatbot.controller.ts @@ -52,6 +52,7 @@ import { OrganizationChatbotSettings, OrganizationChatbotSettingsDefaults, OrganizationRole, + NotifyUpdatedChatbotAnswerParams, Role, UpdateChatbotProviderBody, UpdateChatbotQuestionParams, @@ -335,10 +336,8 @@ export class ChatbotController { @User({ chat_token: true }) user: UserModel, ): Promise { handleChatbotTokenCheck(user); - // Do not forward frontend-only flags to the chatbot API - const { emailNotifyOnAnswerUpdate, ...sanitized } = questionData as any; return await this.chatbotApiService.updateQuestion( - sanitized, + questionData, courseId, user.chat_token.token, ); @@ -387,29 +386,17 @@ export class ChatbotController { // Professor-only: send notification email to all students who asked this question // Body must include oldAnswer and newAnswer, and can optionally include question changes for context - @Post('question/:courseId/:questionId/notify') + @Post('question/:courseId/:vectorStoreId/notify') @UseGuards(CourseRolesGuard) @Roles(Role.PROFESSOR) async notifyUpdatedAnswer( @Param('courseId', ParseIntPipe) courseId: number, - @Param('questionId') questionId: string, - @Body() - body: { - oldAnswer: string; - newAnswer: string; - oldQuestion?: string; - newQuestion?: string; - }, - @User({ chat_token: true }) user: UserModel, + @Param('vectorStoreId') vectorStoreId: string, + @Body() body: NotifyUpdatedChatbotAnswerParams, + @User({ courses: true }) user: UserModel, ): Promise<{ recipients: number }> { - handleChatbotTokenCheck(user); - const { oldAnswer, newAnswer, oldQuestion, newQuestion } = body ?? {}; - if (oldAnswer == undefined || newAnswer == undefined) { - throw new BadRequestException( - 'oldAnswer and newAnswer must be provided to send notifications.', - ); - } + // Enforce: only allow notify when answer changed if ((oldAnswer ?? '').trim() === (newAnswer ?? '').trim()) { throw new BadRequestException( @@ -419,7 +406,14 @@ export class ChatbotController { // Find all interactions in this course where this question was asked const asked = await ChatbotQuestionModel.find({ - where: { vectorStoreId: questionId }, + where: { + vectorStoreId: vectorStoreId, + interaction: { + course: { + id: courseId, + }, + }, + }, relations: { interaction: { user: true, course: true } }, }); const recipients = Array.from( @@ -432,10 +426,11 @@ export class ChatbotController { ); if (recipients.length === 0) { - return { recipients: 0 }; + throw new BadRequestException( + 'Cannot send notification: no recipients for this question.', + ); } - // Building email content const qChanged = (oldQuestion ?? '').trim() !== (newQuestion ?? oldQuestion ?? '').trim(); @@ -450,9 +445,21 @@ export class ChatbotController { oldQuestion ?? newQuestion ?? '' }`; + const courseName = asked[0]?.interaction?.course?.name ?? 'your course'; + + const courseRole = + user.courses?.find((uc: any) => uc.courseId === courseId)?.role ?? null; + + const staffDescriptor = + courseRole === Role.PROFESSOR + ? 'the Instructor' + : courseRole === Role.TA + ? 'a TA' + : 'a staff member'; + const html = `
-

The answer to a chatbot question you asked has been updated.

+

The answer to a chatbot question you asked in ${courseName} has been updated by ${staffDescriptor}.

${questionSection} @@ -465,21 +472,27 @@ export class ChatbotController {

-

You can revisit the course chatbot from your course page.

+

+ Want to follow up? Create an + Anytime Question + to discuss it more! +

`; - await this.mailService - .sendEmail({ - receiverOrReceivers: recipients, - subject: 'Chatbot answer updated', + const limitedRecipients = recipients.slice(0, 5); + + for (const email of limitedRecipients) { + await this.mailService.sendEmail({ + receiverOrReceivers: email, + subject: `Chatbot answer updated by staff member for ${courseName}`, type: MailServiceType.CHATBOT_ANSWER_UPDATED, content: html, track: false, - }) - .catch(() => {}); + }); + } - return { recipients: recipients.length }; + return { recipients: limitedRecipients.length }; } // @Get('resetCourse/:courseId') // @UseGuards(CourseRolesGuard) diff --git a/packages/server/src/chatbot/chatbot.service.spec.ts b/packages/server/src/chatbot/chatbot.service.spec.ts index 9140becbe..a2b9a83a8 100644 --- a/packages/server/src/chatbot/chatbot.service.spec.ts +++ b/packages/server/src/chatbot/chatbot.service.spec.ts @@ -51,6 +51,7 @@ import { chatbotTables, } from './chatbot-datasource/chatbot-datasource.service'; import { ChatbotApiService } from './chatbot-api.service'; +import { MailService } from 'mail/mail.service'; const mockCacheManager = { get: jest.fn(), @@ -59,6 +60,10 @@ const mockCacheManager = { reset: jest.fn(), }; +const mockMailService = { + sendEmail: jest.fn(), +}; + describe('ChatbotService', () => { let service: ChatbotService; let subscriber: ChatbotSettingsSubscriber; @@ -91,6 +96,10 @@ describe('ChatbotService', () => { provide: CACHE_MANAGER, useValue: mockCacheManager, }, + { + provide: MailService, + useValue: mockMailService, + }, ], }).compile(); diff --git a/packages/server/test/chatbot.integration.ts b/packages/server/test/chatbot.integration.ts index b6f89c132..30b56c520 100644 --- a/packages/server/test/chatbot.integration.ts +++ b/packages/server/test/chatbot.integration.ts @@ -7,6 +7,7 @@ import { CreateOrganizationChatbotSettingsBody, dropUndefined, ERROR_MESSAGES, + MailServiceType, OrganizationRole, Role, UpdateChatbotProviderBody, @@ -25,7 +26,12 @@ import { UserCourseFactory, UserFactory, } from './util/factories'; -import { setupIntegrationTest } from './util/testUtils'; +import { + expectEmailNotSent, + expectEmailSent, + overrideEmailService, + setupIntegrationTest, +} from './util/testUtils'; import { ChatbotQuestionModel } from 'chatbot/question.entity'; import { OrganizationModel } from '../src/organization/organization.entity'; import { CourseModel } from '../src/course/course.entity'; @@ -40,7 +46,10 @@ import { UserModel } from '../src/profile/user.entity'; import { ChatbotApiService } from '../src/chatbot/chatbot-api.service'; describe('ChatbotController Integration', () => { - const { supertest } = setupIntegrationTest(ChatbotModule); + const { supertest } = setupIntegrationTest( + ChatbotModule, + overrideEmailService, + ); describe('PATCH /chatbot/questionScore/:courseId/:questionId', () => { it('should allow a student to update the score of a question', async () => { @@ -133,6 +142,106 @@ describe('ChatbotController Integration', () => { }); }); + describe('POST /chatbot/question/:courseId/:vectorStoreId/notify', () => { + it('sends individual emails (capped at 5 recipients) to users who asked the question', async () => { + const professor = await UserFactory.create(); + const course = await CourseFactory.create(); + await UserCourseFactory.create({ + user: professor, + course, + role: Role.PROFESSOR, + }); + + const vectorStoreId = 'vec-notify-123'; + + const students = await Promise.all( + Array.from({ length: 6 }).map(() => UserFactory.create()), + ); + + for (const student of students) { + await UserCourseFactory.create({ + user: student, + course, + role: Role.STUDENT, + }); + const interaction = await InteractionFactory.create({ + user: student, + course, + }); + await ChatbotQuestionModel.create({ + vectorStoreId, + questionText: 'Original question text', + responseText: 'Old answer', + verified: true, + suggested: true, + sourceDocuments: [], + interaction, + }).save(); + } + + const body = { + oldAnswer: 'Old answer', + newAnswer: 'New updated answer', + oldQuestion: 'Old question?', + newQuestion: 'New updated question?', + }; + + const res = await supertest({ userId: professor.id }) + .post(`/chatbot/question/${course.id}/${vectorStoreId}/notify`) + .send(body) + .expect(201); + + expect(res.body).toEqual({ recipients: 5 }); + expectEmailSent( + students.slice(0, 5).map((s) => s.email), + Array(5).fill(MailServiceType.CHATBOT_ANSWER_UPDATED), + ); + }); + + it('returns 400 and does not send emails when answer did not change', async () => { + const professor = await UserFactory.create(); + const course = await CourseFactory.create(); + await UserCourseFactory.create({ + user: professor, + course, + role: Role.PROFESSOR, + }); + + const vectorStoreId = 'vec-notify-unchanged'; + const student = await UserFactory.create(); + await UserCourseFactory.create({ + user: student, + course, + role: Role.STUDENT, + }); + const interaction = await InteractionFactory.create({ + user: student, + course, + }); + await ChatbotQuestionModel.create({ + vectorStoreId, + questionText: 'Some question', + responseText: 'Same answer', + verified: true, + suggested: true, + sourceDocuments: [], + interaction, + }).save(); + + const body = { + oldAnswer: 'Same answer', + newAnswer: ' Same answer ', + }; + + await supertest({ userId: professor.id }) + .post(`/chatbot/question/${course.id}/${vectorStoreId}/notify`) + .send(body) + .expect(400); + + expectEmailNotSent(); + }); + }); + describe('GET /chatbot/history', () => { it('should return the chatbot history for a user', async () => { const user = await UserFactory.create(); From 6110ee7f255e5e7e7fa36482aaab97d3f1fa7181 Mon Sep 17 00:00:00 2001 From: ribhavsharma Date: Mon, 1 Dec 2025 22:21:48 -0800 Subject: [PATCH 06/11] trying to fix test --- packages/server/test/chatbot.integration.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/server/test/chatbot.integration.ts b/packages/server/test/chatbot.integration.ts index 30b56c520..96c22fb8c 100644 --- a/packages/server/test/chatbot.integration.ts +++ b/packages/server/test/chatbot.integration.ts @@ -172,9 +172,7 @@ describe('ChatbotController Integration', () => { vectorStoreId, questionText: 'Original question text', responseText: 'Old answer', - verified: true, suggested: true, - sourceDocuments: [], interaction, }).save(); } @@ -222,9 +220,7 @@ describe('ChatbotController Integration', () => { vectorStoreId, questionText: 'Some question', responseText: 'Same answer', - verified: true, suggested: true, - sourceDocuments: [], interaction, }).save(); From 0003ee1824271f532996f5746e7df26fb164d769 Mon Sep 17 00:00:00 2001 From: ribhavsharma Date: Mon, 1 Dec 2025 23:06:29 -0800 Subject: [PATCH 07/11] fixing... test --- packages/server/test/chatbot.integration.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/server/test/chatbot.integration.ts b/packages/server/test/chatbot.integration.ts index 96c22fb8c..275167fac 100644 --- a/packages/server/test/chatbot.integration.ts +++ b/packages/server/test/chatbot.integration.ts @@ -143,6 +143,10 @@ describe('ChatbotController Integration', () => { }); describe('POST /chatbot/question/:courseId/:vectorStoreId/notify', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('sends individual emails (capped at 5 recipients) to users who asked the question', async () => { const professor = await UserFactory.create(); const course = await CourseFactory.create(); @@ -155,7 +159,11 @@ describe('ChatbotController Integration', () => { const vectorStoreId = 'vec-notify-123'; const students = await Promise.all( - Array.from({ length: 6 }).map(() => UserFactory.create()), + Array.from({ length: 6 }).map((_, i) => + UserFactory.create({ + email: `chatbot-notify-${i}@example.com`, + }), + ), ); for (const student of students) { From 9569e980e961982f9e4bf41ea80ec2683c64367c Mon Sep 17 00:00:00 2001 From: Adam Fipke Date: Tue, 2 Dec 2025 15:03:55 -0800 Subject: [PATCH 08/11] fix notifyUpdatedAnswer only callable by profs. Removed an . Added a comment explaining why we limit the number of recpients --- packages/server/src/chatbot/chatbot.controller.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/server/src/chatbot/chatbot.controller.ts b/packages/server/src/chatbot/chatbot.controller.ts index 73bb68530..78dc9f4b9 100644 --- a/packages/server/src/chatbot/chatbot.controller.ts +++ b/packages/server/src/chatbot/chatbot.controller.ts @@ -384,11 +384,11 @@ export class ChatbotController { // resets all chatbot data for the course. Unused - // Professor-only: send notification email to all students who asked this question + // staff-only: send notification email to all students who asked this question // Body must include oldAnswer and newAnswer, and can optionally include question changes for context @Post('question/:courseId/:vectorStoreId/notify') @UseGuards(CourseRolesGuard) - @Roles(Role.PROFESSOR) + @Roles(Role.PROFESSOR, Role.TA) async notifyUpdatedAnswer( @Param('courseId', ParseIntPipe) courseId: number, @Param('vectorStoreId') vectorStoreId: string, @@ -448,7 +448,7 @@ export class ChatbotController { const courseName = asked[0]?.interaction?.course?.name ?? 'your course'; const courseRole = - user.courses?.find((uc: any) => uc.courseId === courseId)?.role ?? null; + user.courses?.find((uc) => uc.courseId === courseId)?.role ?? null; const staffDescriptor = courseRole === Role.PROFESSOR @@ -479,7 +479,9 @@ export class ChatbotController {

`; - + // TODO: Once we get our official HelpMe email, we can remove this cap + // (this was added so we don't get rate limited by gmail as easily). + // Note that we CANNOT just 1 email with all the recipients, since then students will be able to see each others emails (and that they asked a particular question) const limitedRecipients = recipients.slice(0, 5); for (const email of limitedRecipients) { From cf46dada8cd9b7ab15eff38841e328a172308e06 Mon Sep 17 00:00:00 2001 From: ribhavsharma Date: Wed, 25 Feb 2026 19:10:12 -0800 Subject: [PATCH 09/11] addressing feedback --- .../components/EditChatbotQuestionModal.tsx | 8 +- packages/frontend/app/api/index.ts | 6 +- .../src/chatbot/chatbot.controller.spec.ts | 122 ------- .../server/src/chatbot/chatbot.controller.ts | 114 +------ .../src/chatbot/chatbot.service.spec.ts | 146 +++++++++ .../server/src/chatbot/chatbot.service.ts | 137 ++++++++ packages/server/test/chatbot.integration.ts | 303 ++++++++++++++---- 7 files changed, 549 insertions(+), 287 deletions(-) delete mode 100644 packages/server/src/chatbot/chatbot.controller.spec.ts diff --git a/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx b/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx index 90dc9995a..7886457b5 100644 --- a/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx +++ b/packages/frontend/app/(dashboard)/course/[cid]/(settings)/settings/chatbot_questions/components/EditChatbotQuestionModal.tsx @@ -192,8 +192,12 @@ const EditChatbotQuestionModal: React.FC = ({ }, ) if (resp?.recipients != undefined) { - if (resp.recipients === 5) { - message.success('Notification email sent to 5 users (max 5).') + if (resp.totalRecipients === 5) { + message.success( + `Notification email sent to ${resp.recipients} user${ + resp.recipients === 1 ? '' : 's' + } (max 5).`, + ) } else { message.success( `Notification email sent to ${resp.recipients} user${ diff --git a/packages/frontend/app/api/index.ts b/packages/frontend/app/api/index.ts index 4e37f9d9e..c903a9527 100644 --- a/packages/frontend/app/api/index.ts +++ b/packages/frontend/app/api/index.ts @@ -404,7 +404,11 @@ export class APIClient { oldQuestion?: string newQuestion?: string }, - ): Promise<{ recipients: number }> => + ): Promise<{ + recipients: number + totalRecipients: number + unsubscribedRecipients: number + }> => this.req( 'POST', `/api/v1/chatbot/question/${courseId}/${vectorStoreId}/notify`, diff --git a/packages/server/src/chatbot/chatbot.controller.spec.ts b/packages/server/src/chatbot/chatbot.controller.spec.ts deleted file mode 100644 index 1c284a623..000000000 --- a/packages/server/src/chatbot/chatbot.controller.spec.ts +++ /dev/null @@ -1,122 +0,0 @@ -import { Test, TestingModule } from '@nestjs/testing'; -import { ChatbotController } from './chatbot.controller'; -import { ChatbotService } from './chatbot.service'; -import { ChatbotApiService } from './chatbot-api.service'; -import { MailService } from 'mail/mail.service'; -import { ChatbotQuestionModel } from './question.entity'; -import { BadRequestException } from '@nestjs/common'; -import { NotifyUpdatedChatbotAnswerParams, Role } from '@koh/common'; - -describe('ChatbotController - notifyUpdatedAnswer', () => { - let controller: ChatbotController; - let mailService: { sendEmail: jest.Mock }; - - const mockChatbotService = {}; - const mockChatbotApiService = {}; - - beforeAll(async () => { - mailService = { - sendEmail: jest.fn(), - }; - - const module: TestingModule = await Test.createTestingModule({ - controllers: [ChatbotController], - providers: [ - { provide: ChatbotService, useValue: mockChatbotService }, - { provide: ChatbotApiService, useValue: mockChatbotApiService }, - { provide: MailService, useValue: mailService }, - ], - }).compile(); - - controller = module.get(ChatbotController); - }); - - afterEach(() => { - jest.restoreAllMocks(); - mailService.sendEmail.mockReset(); - }); - - const makeUser = (courseId: number, role: Role): any => ({ - id: 1, - courses: [{ courseId, role }], - }); - - const makeBody = ( - overrides: Partial = {}, - ): NotifyUpdatedChatbotAnswerParams => ({ - oldAnswer: 'old', - newAnswer: 'new', - ...overrides, - }); - - it('throws BadRequestException when answer did not change', async () => { - const body = makeBody({ oldAnswer: 'same', newAnswer: ' same ' }); - - await expect( - controller.notifyUpdatedAnswer( - 1, - 'vec-id', - body, - makeUser(1, Role.PROFESSOR), - ), - ).rejects.toBeInstanceOf(BadRequestException); - }); - - it('throws BadRequestException when there are no recipients', async () => { - jest.spyOn(ChatbotQuestionModel, 'find').mockResolvedValue([] as any); - - const body = makeBody(); - - await expect( - controller.notifyUpdatedAnswer( - 1, - 'vec-id', - body, - makeUser(1, Role.PROFESSOR), - ), - ).rejects.toBeInstanceOf(BadRequestException); - - expect(ChatbotQuestionModel.find).toHaveBeenCalledTimes(1); - expect(mailService.sendEmail).not.toHaveBeenCalled(); - }); - - it('sends individual emails and caps at 5 recipients', async () => { - const courseId = 1; - const vectorStoreId = 'vec-id'; - const emails = [ - 'u1@example.com', - 'u2@example.com', - 'u3@example.com', - 'u4@example.com', - 'u5@example.com', - 'u6@example.com', - ]; - - jest.spyOn(ChatbotQuestionModel, 'find').mockResolvedValue( - emails.map((email) => ({ - interaction: { - course: { id: courseId, name: 'Test Course' }, - user: { email }, - }, - })) as any, - ); - - const body = makeBody(); - - const result = await controller.notifyUpdatedAnswer( - courseId, - vectorStoreId, - body, - makeUser(courseId, Role.PROFESSOR), - ); - - // Should only send to first 5 emails - expect(mailService.sendEmail).toHaveBeenCalledTimes(5); - const calledRecipients = mailService.sendEmail.mock.calls.map( - (args) => args[0].receiverOrReceivers, - ); - expect(calledRecipients).toEqual(emails.slice(0, 5)); - - expect(result).toEqual({ recipients: 5 }); - }); -}); diff --git a/packages/server/src/chatbot/chatbot.controller.ts b/packages/server/src/chatbot/chatbot.controller.ts index 78dc9f4b9..93031fe08 100644 --- a/packages/server/src/chatbot/chatbot.controller.ts +++ b/packages/server/src/chatbot/chatbot.controller.ts @@ -45,7 +45,6 @@ import { GetChatbotHistoryResponse, GetInteractionsAndQuestionsResponse, InteractionResponse, - MailServiceType, LLMType, OllamaLLMType, OpenAILLMType, @@ -89,8 +88,6 @@ import { IgnoreableClassSerializerInterceptor, IgnoreSerializer, } from '../interceptors/IgnoreableClassSerializerInterceptor'; -import { MailService } from '../mail/mail.service'; -import { ChatbotQuestionModel } from './question.entity'; @Controller('chatbot') @UseGuards(JwtAuthGuard, EmailVerifiedGuard) @@ -99,7 +96,6 @@ export class ChatbotController { constructor( private readonly chatbotService: ChatbotService, private readonly chatbotApiService: ChatbotApiService, - private readonly mailService: MailService, ) {} // @@ -394,107 +390,17 @@ export class ChatbotController { @Param('vectorStoreId') vectorStoreId: string, @Body() body: NotifyUpdatedChatbotAnswerParams, @User({ courses: true }) user: UserModel, - ): Promise<{ recipients: number }> { - const { oldAnswer, newAnswer, oldQuestion, newQuestion } = body ?? {}; - - // Enforce: only allow notify when answer changed - if ((oldAnswer ?? '').trim() === (newAnswer ?? '').trim()) { - throw new BadRequestException( - 'Cannot send notification: answer was not changed.', - ); - } - - // Find all interactions in this course where this question was asked - const asked = await ChatbotQuestionModel.find({ - where: { - vectorStoreId: vectorStoreId, - interaction: { - course: { - id: courseId, - }, - }, - }, - relations: { interaction: { user: true, course: true } }, - }); - const recipients = Array.from( - new Set( - asked - .filter((a) => a.interaction?.course?.id === courseId) - .map((a) => a.interaction?.user?.email) - .filter((e): e is string => !!e), - ), + ): Promise<{ + recipients: number; + totalRecipients: number; + unsubscribedRecipients: number; + }> { + return await this.chatbotService.notifyUpdatedAnswer( + courseId, + vectorStoreId, + body, + user, ); - - if (recipients.length === 0) { - throw new BadRequestException( - 'Cannot send notification: no recipients for this question.', - ); - } - - const qChanged = - (oldQuestion ?? '').trim() !== (newQuestion ?? oldQuestion ?? '').trim(); - - const questionSection = qChanged - ? `Question (before)${ - oldQuestion ?? '' - } - Question (after)${ - newQuestion ?? '' - }` - : `Question${ - oldQuestion ?? newQuestion ?? '' - }`; - - const courseName = asked[0]?.interaction?.course?.name ?? 'your course'; - - const courseRole = - user.courses?.find((uc) => uc.courseId === courseId)?.role ?? null; - - const staffDescriptor = - courseRole === Role.PROFESSOR - ? 'the Instructor' - : courseRole === Role.TA - ? 'a TA' - : 'a staff member'; - - const html = ` -
-

The answer to a chatbot question you asked in ${courseName} has been updated by ${staffDescriptor}.

- - ${questionSection} - - - - - - - - -
Answer (before)${oldAnswer}
Answer (after)${newAnswer}
-
-

- Want to follow up? Create an - Anytime Question - to discuss it more! -

-
- `; - // TODO: Once we get our official HelpMe email, we can remove this cap - // (this was added so we don't get rate limited by gmail as easily). - // Note that we CANNOT just 1 email with all the recipients, since then students will be able to see each others emails (and that they asked a particular question) - const limitedRecipients = recipients.slice(0, 5); - - for (const email of limitedRecipients) { - await this.mailService.sendEmail({ - receiverOrReceivers: email, - subject: `Chatbot answer updated by staff member for ${courseName}`, - type: MailServiceType.CHATBOT_ANSWER_UPDATED, - content: html, - track: false, - }); - } - - return { recipients: limitedRecipients.length }; } // @Get('resetCourse/:courseId') // @UseGuards(CourseRolesGuard) diff --git a/packages/server/src/chatbot/chatbot.service.spec.ts b/packages/server/src/chatbot/chatbot.service.spec.ts index a2b9a83a8..c97754422 100644 --- a/packages/server/src/chatbot/chatbot.service.spec.ts +++ b/packages/server/src/chatbot/chatbot.service.spec.ts @@ -37,7 +37,9 @@ import { CreateOrganizationChatbotSettingsBody, dropUndefined, ERROR_MESSAGES, + NotifyUpdatedChatbotAnswerParams, OrganizationChatbotSettingsDefaults, + Role, UpdateChatbotProviderBody, UpdateLLMTypeBody, UpsertCourseChatbotSettings, @@ -52,6 +54,7 @@ import { } from './chatbot-datasource/chatbot-datasource.service'; import { ChatbotApiService } from './chatbot-api.service'; import { MailService } from 'mail/mail.service'; +import { UserSubscriptionModel } from '../mail/user-subscriptions.entity'; const mockCacheManager = { get: jest.fn(), @@ -142,6 +145,7 @@ describe('ChatbotService', () => { ).then(); }); updateChatbotRepositorySpy.mockRestore(); + mockMailService.sendEmail.mockReset(); }); beforeEach(async () => { @@ -1885,4 +1889,146 @@ describe('ChatbotService', () => { }, ); }); + + describe('notifyUpdatedAnswer', () => { + const makeBody = ( + overrides: Partial = {}, + ): NotifyUpdatedChatbotAnswerParams => ({ + oldAnswer: 'old', + newAnswer: 'new', + ...overrides, + }); + + const makeUser = (courseId: number, role: Role): any => ({ + id: 1, + courses: [{ courseId, role }], + }); + + it('throws when answer did not change', async () => { + await expect( + service.notifyUpdatedAnswer( + 1, + 'vec-id', + makeBody({ oldAnswer: 'same', newAnswer: ' same ' }), + makeUser(1, Role.PROFESSOR), + ), + ).rejects.toBeInstanceOf(BadRequestException); + }); + + it('throws when there are no recipients', async () => { + const findSpy = jest + .spyOn(ChatbotQuestionModel, 'find') + .mockResolvedValue([] as any); + + await expect( + service.notifyUpdatedAnswer( + 1, + 'vec-id', + makeBody(), + makeUser(1, Role.PROFESSOR), + ), + ).rejects.toBeInstanceOf(BadRequestException); + + expect(mockMailService.sendEmail).not.toHaveBeenCalled(); + findSpy.mockRestore(); + }); + + it('sends individual emails and caps at 5 recipients', async () => { + const courseId = 1; + const vectorStoreId = 'vec-id'; + const emails = [ + 'u1@example.com', + 'u2@example.com', + 'u3@example.com', + 'u4@example.com', + 'u5@example.com', + 'u6@example.com', + ]; + + const findSpy = jest + .spyOn(ChatbotQuestionModel, 'find') + .mockResolvedValue( + emails.map((email) => ({ + interaction: { + course: { id: courseId, name: 'Test Course' }, + user: { email }, + }, + })) as any, + ); + const subscriptionSpy = jest + .spyOn(UserSubscriptionModel, 'find') + .mockResolvedValue([]); + + const result = await service.notifyUpdatedAnswer( + courseId, + vectorStoreId, + makeBody(), + makeUser(courseId, Role.PROFESSOR), + ); + + expect(mockMailService.sendEmail).toHaveBeenCalledTimes(5); + const calledRecipients = mockMailService.sendEmail.mock.calls.map( + (args) => args[0].receiverOrReceivers, + ); + expect(calledRecipients).toEqual(emails.slice(0, 5)); + expect(result).toEqual({ + recipients: 5, + totalRecipients: 5, + unsubscribedRecipients: 0, + }); + findSpy.mockRestore(); + subscriptionSpy.mockRestore(); + }); + + it('does not email unsubscribed recipients and reports counts', async () => { + const courseId = 1; + const vectorStoreId = 'vec-id'; + const emails = [ + 'u1@example.com', + 'u2@example.com', + 'u3@example.com', + 'u4@example.com', + 'u5@example.com', + ]; + const unsubscribedEmails = [emails[1], emails[3]]; + + const findSpy = jest + .spyOn(ChatbotQuestionModel, 'find') + .mockResolvedValue( + emails.map((email) => ({ + interaction: { + course: { id: courseId, name: 'Test Course' }, + user: { email }, + }, + })) as any, + ); + const subscriptionSpy = jest + .spyOn(UserSubscriptionModel, 'find') + .mockResolvedValue( + unsubscribedEmails.map((email) => ({ user: { email } })) as any, + ); + + const result = await service.notifyUpdatedAnswer( + courseId, + vectorStoreId, + makeBody(), + makeUser(courseId, Role.PROFESSOR), + ); + + expect(mockMailService.sendEmail).toHaveBeenCalledTimes(3); + const calledRecipients = mockMailService.sendEmail.mock.calls.map( + (args) => args[0].receiverOrReceivers, + ); + expect(calledRecipients).toEqual( + emails.filter((email) => !unsubscribedEmails.includes(email)), + ); + expect(result).toEqual({ + recipients: 3, + totalRecipients: 5, + unsubscribedRecipients: 2, + }); + findSpy.mockRestore(); + subscriptionSpy.mockRestore(); + }); + }); }); diff --git a/packages/server/src/chatbot/chatbot.service.ts b/packages/server/src/chatbot/chatbot.service.ts index d5a971567..1931af7b2 100644 --- a/packages/server/src/chatbot/chatbot.service.ts +++ b/packages/server/src/chatbot/chatbot.service.ts @@ -21,10 +21,13 @@ import { CreateOrganizationChatbotSettingsBody, dropUndefined, ERROR_MESSAGES, + MailServiceType, + NotifyUpdatedChatbotAnswerParams, OllamaLLMType, OllamaModelDescription, OpenAILLMType, OrganizationChatbotSettingsDefaults, + Role, UpdateChatbotProviderBody, UpdateLLMTypeBody, UpsertCourseChatbotSettings, @@ -39,6 +42,8 @@ import { CACHE_MANAGER } from '@nestjs/cache-manager'; import { Cache } from 'cache-manager'; import { ChatbotApiService } from './chatbot-api.service'; import { ChatbotDataSourceService } from './chatbot-datasource/chatbot-datasource.service'; +import { MailService } from '../mail/mail.service'; +import { UserSubscriptionModel } from '../mail/user-subscriptions.entity'; // OpenAI makes it really annoying to scrape any data related // to the model capabilities (text, image processing) (selfish, much?) @@ -137,6 +142,7 @@ export class ChatbotService { private dataSource: DataSource, private chatbotDataSource: ChatbotDataSourceService, private chatbotApiService: ChatbotApiService, + private readonly mailService: MailService, @Inject(CACHE_MANAGER) private cacheManager: Cache, ) {} @@ -316,6 +322,137 @@ export class ChatbotService { return question; } + async notifyUpdatedAnswer( + courseId: number, + vectorStoreId: string, + body: NotifyUpdatedChatbotAnswerParams, + user: UserModel, + ): Promise<{ + recipients: number; + totalRecipients: number; + unsubscribedRecipients: number; + }> { + const { oldAnswer, newAnswer, oldQuestion, newQuestion } = body ?? {}; + + if ((oldAnswer ?? '').trim() === (newAnswer ?? '').trim()) { + throw new BadRequestException( + 'Cannot send notification: answer was not changed.', + ); + } + + const asked = await ChatbotQuestionModel.find({ + where: { + vectorStoreId, + interaction: { + course: { + id: courseId, + }, + }, + }, + relations: { interaction: { user: true, course: true } }, + }); + + const recipients = Array.from( + new Set( + asked + .filter((a) => a.interaction?.course?.id === courseId) + .map((a) => a.interaction?.user?.email) + .filter((email): email is string => !!email), + ), + ); + + if (recipients.length === 0) { + throw new BadRequestException( + 'Cannot send notification: no recipients for this question.', + ); + } + + const qChanged = + (oldQuestion ?? '').trim() !== (newQuestion ?? oldQuestion ?? '').trim(); + + const questionSection = qChanged + ? `Question (before)${ + oldQuestion ?? '' + } + Question (after)${ + newQuestion ?? '' + }` + : `Question${ + oldQuestion ?? newQuestion ?? '' + }`; + + const courseName = asked[0]?.interaction?.course?.name ?? 'your course'; + + const courseRole = + user.courses?.find((uc) => uc.courseId === courseId)?.role ?? null; + + const staffDescriptor = + courseRole === Role.PROFESSOR + ? 'the Instructor' + : courseRole === Role.TA + ? 'a TA' + : 'a staff member'; + + const html = ` +
+

The answer to a chatbot question you asked in ${courseName} has been updated by ${staffDescriptor}.

+ + ${questionSection} + + + + + + + + +
Answer (before)${oldAnswer}
Answer (after)${newAnswer}
+
+

+ Want to follow up? Create an + Anytime Question + to discuss it more! +

+
+ `; + + // TODO: Once we get our official HelpMe email, we can remove this cap + // (this was added so we don't get rate limited by gmail as easily). + // Note that we CANNOT just 1 email with all the recipients, since then students + // will be able to see each others emails (and that they asked a particular question) + const limitedRecipients = recipients.slice(0, 5); + const unsubscribed = await UserSubscriptionModel.find({ + where: { + isSubscribed: false, + user: { email: In(limitedRecipients) }, + service: { serviceType: MailServiceType.CHATBOT_ANSWER_UPDATED }, + }, + relations: { user: true }, + }); + const unsubscribedEmails = new Set( + unsubscribed.map((sub) => sub.user.email), + ); + const subscribedRecipients = limitedRecipients.filter( + (email) => !unsubscribedEmails.has(email), + ); + + for (const email of subscribedRecipients) { + await this.mailService.sendEmail({ + receiverOrReceivers: email, + subject: `Chatbot answer updated by staff member for ${courseName}`, + type: MailServiceType.CHATBOT_ANSWER_UPDATED, + content: html, + track: false, + }); + } + + return { + recipients: subscribedRecipients.length, + totalRecipients: limitedRecipients.length, + unsubscribedRecipients: unsubscribedEmails.size, + }; + } + async isChatbotServiceLegacy(courseId: number): Promise { const course = await CourseModel.findOne({ where: { id: courseId }, diff --git a/packages/server/test/chatbot.integration.ts b/packages/server/test/chatbot.integration.ts index 072e2f690..a78c310a1 100644 --- a/packages/server/test/chatbot.integration.ts +++ b/packages/server/test/chatbot.integration.ts @@ -19,16 +19,19 @@ import { CourseFactory, InteractionFactory, LLMTypeFactory, + mailServiceFactory, OrganizationChatbotSettingsFactory, OrganizationCourseFactory, OrganizationFactory, OrganizationUserFactory, + userSubscriptionFactory, UserCourseFactory, UserFactory, } from './util/factories'; import { expectEmailNotSent, expectEmailSent, + mockEmailService, overrideEmailService, setupIntegrationTest, } from './util/testUtils'; @@ -147,97 +150,281 @@ describe('ChatbotController Integration', () => { jest.clearAllMocks(); }); - it('sends individual emails (capped at 5 recipients) to users who asked the question', async () => { - const professor = await UserFactory.create(); + const createStaffForCourse = async (role: Role = Role.PROFESSOR) => { + const staff = await UserFactory.create(); const course = await CourseFactory.create(); await UserCourseFactory.create({ - user: professor, + user: staff, course, - role: Role.PROFESSOR, + role, + }); + return { staff, course }; + }; + + const createAskedQuestion = async ( + course: CourseModel, + vectorStoreId: string, + email: string, + ) => { + const student = await UserFactory.create({ email }); + await UserCourseFactory.create({ + user: student, + course, + role: Role.STUDENT, + }); + const interaction = await InteractionFactory.create({ + user: student, + course, + }); + await ChatbotQuestionModel.create({ + vectorStoreId, + questionText: 'Original question text', + responseText: 'Old answer', + suggested: true, + interaction, + }).save(); + return student; + }; + + const notifyBody = (overrides: Record = {}) => ({ + oldAnswer: 'Old answer', + newAnswer: 'New updated answer', + oldQuestion: 'Old question?', + newQuestion: 'New updated question?', + ...overrides, + }); + + it('only allows professors and TAs to use notify endpoint', async () => { + const { course } = await createStaffForCourse(Role.PROFESSOR); + const vectorStoreId = 'vec-notify-access-control'; + await createAskedQuestion(course, vectorStoreId, 'asked@example.com'); + + const ta = await UserFactory.create(); + await UserCourseFactory.create({ + user: ta, + course, + role: Role.TA, + }); + + const student = await UserFactory.create(); + await UserCourseFactory.create({ + user: student, + course, + role: Role.STUDENT, }); + await supertest({ userId: ta.id }) + .post(`/chatbot/question/${course.id}/${vectorStoreId}/notify`) + .send(notifyBody()) + .expect(201); + + await supertest({ userId: student.id }) + .post(`/chatbot/question/${course.id}/${vectorStoreId}/notify`) + .send(notifyBody()) + .expect(403); + }); + + it('sends individual emails (capped at 5 recipients) to users who asked the question', async () => { + const { staff, course } = await createStaffForCourse(Role.PROFESSOR); + const vectorStoreId = 'vec-notify-123'; const students = await Promise.all( Array.from({ length: 6 }).map((_, i) => - UserFactory.create({ - email: `chatbot-notify-${i}@example.com`, - }), + createAskedQuestion( + course, + vectorStoreId, + `chatbot-notify-${i}@example.com`, + ), ), ); - for (const student of students) { - await UserCourseFactory.create({ - user: student, - course, - role: Role.STUDENT, - }); - const interaction = await InteractionFactory.create({ + const res = await supertest({ userId: staff.id }) + .post(`/chatbot/question/${course.id}/${vectorStoreId}/notify`) + .send(notifyBody()) + .expect(201); + + expect(res.body).toEqual({ + recipients: 5, + totalRecipients: 5, + unsubscribedRecipients: 0, + }); + expectEmailSent( + students.slice(0, 5).map((s) => s.email), + Array(5).fill(MailServiceType.CHATBOT_ANSWER_UPDATED), + ); + }); + + it('respects explicit unsubscribes and returns recipient counts', async () => { + const { staff, course } = await createStaffForCourse(Role.PROFESSOR); + + const vectorStoreId = 'vec-notify-unsubscribed'; + const students = await Promise.all( + Array.from({ length: 5 }).map((_, i) => + createAskedQuestion( + course, + vectorStoreId, + `chatbot-notify-unsub-${i}@example.com`, + ), + ), + ); + + const mailService = await mailServiceFactory.create({ + serviceType: MailServiceType.CHATBOT_ANSWER_UPDATED, + name: 'chatbot_answer_updated', + }); + + for (const [i, student] of students.entries()) { + await userSubscriptionFactory.create({ user: student, - course, + service: mailService, + isSubscribed: i < 3, }); - await ChatbotQuestionModel.create({ - vectorStoreId, - questionText: 'Original question text', - responseText: 'Old answer', - suggested: true, - interaction, - }).save(); } - const body = { - oldAnswer: 'Old answer', - newAnswer: 'New updated answer', - oldQuestion: 'Old question?', - newQuestion: 'New updated question?', - }; - - const res = await supertest({ userId: professor.id }) + const res = await supertest({ userId: staff.id }) .post(`/chatbot/question/${course.id}/${vectorStoreId}/notify`) - .send(body) + .send(notifyBody()) .expect(201); - expect(res.body).toEqual({ recipients: 5 }); + expect(res.body).toEqual({ + recipients: 3, + totalRecipients: 5, + unsubscribedRecipients: 2, + }); expectEmailSent( - students.slice(0, 5).map((s) => s.email), - Array(5).fill(MailServiceType.CHATBOT_ANSWER_UPDATED), + students.slice(0, 3).map((s) => s.email), + Array(3).fill(MailServiceType.CHATBOT_ANSWER_UPDATED), ); }); it('returns 400 and does not send emails when answer did not change', async () => { - const professor = await UserFactory.create(); - const course = await CourseFactory.create(); - await UserCourseFactory.create({ - user: professor, - course, - role: Role.PROFESSOR, - }); - + const { staff, course } = await createStaffForCourse(Role.PROFESSOR); const vectorStoreId = 'vec-notify-unchanged'; - const student = await UserFactory.create(); + await createAskedQuestion(course, vectorStoreId, 'unchanged@example.com'); + + await supertest({ userId: staff.id }) + .post(`/chatbot/question/${course.id}/${vectorStoreId}/notify`) + .send( + notifyBody({ oldAnswer: 'Same answer', newAnswer: ' Same answer ' }), + ) + .expect(400); + + expectEmailNotSent(); + }); + + it('only notifies students who asked the target question', async () => { + const { staff, course } = await createStaffForCourse(Role.PROFESSOR); + const vectorStoreId = 'vec-notify-only-asked'; + const askedStudents = await Promise.all( + Array.from({ length: 2 }).map((_, i) => + createAskedQuestion(course, vectorStoreId, `asked-${i}@example.com`), + ), + ); + + const otherStudent = await UserFactory.create({ + email: 'not-asked@example.com', + }); await UserCourseFactory.create({ - user: student, + user: otherStudent, course, role: Role.STUDENT, }); - const interaction = await InteractionFactory.create({ - user: student, + + await supertest({ userId: staff.id }) + .post(`/chatbot/question/${course.id}/${vectorStoreId}/notify`) + .send(notifyBody()) + .expect(201); + + expectEmailSent( + askedStudents.map((s) => s.email), + Array(askedStudents.length).fill( + MailServiceType.CHATBOT_ANSWER_UPDATED, + ), + ); + const calledRecipients = mockEmailService.sendEmail.mock.calls.map( + (call) => call[0].receiverOrReceivers, + ); + expect(calledRecipients).not.toContain(otherStudent.email); + }); + + it('succeeds when oldAnswer is empty string and newAnswer is non-empty', async () => { + const { staff, course } = await createStaffForCourse(Role.PROFESSOR); + const vectorStoreId = 'vec-notify-empty-old'; + await createAskedQuestion(course, vectorStoreId, 'empty-old@example.com'); + + const res = await supertest({ userId: staff.id }) + .post(`/chatbot/question/${course.id}/${vectorStoreId}/notify`) + .send(notifyBody({ oldAnswer: '', newAnswer: 'Updated answer' })) + .expect(201); + + expect(res.body.recipients).toBe(1); + }); + + it('succeeds when newAnswer is empty string and oldAnswer is non-empty', async () => { + const { staff, course } = await createStaffForCourse(Role.PROFESSOR); + const vectorStoreId = 'vec-notify-empty-new'; + await createAskedQuestion(course, vectorStoreId, 'empty-new@example.com'); + + const res = await supertest({ userId: staff.id }) + .post(`/chatbot/question/${course.id}/${vectorStoreId}/notify`) + .send(notifyBody({ oldAnswer: 'Old answer', newAnswer: '' })) + .expect(201); + + expect(res.body.recipients).toBe(1); + }); + + it('returns 400 if oldAnswer is missing', async () => { + const { staff, course } = await createStaffForCourse(Role.PROFESSOR); + const vectorStoreId = 'vec-notify-missing-old'; + await createAskedQuestion( course, - }); - await ChatbotQuestionModel.create({ vectorStoreId, - questionText: 'Some question', - responseText: 'Same answer', - suggested: true, - interaction, - }).save(); + 'missing-old@example.com', + ); - const body = { - oldAnswer: 'Same answer', - newAnswer: ' Same answer ', - }; + await supertest({ userId: staff.id }) + .post(`/chatbot/question/${course.id}/${vectorStoreId}/notify`) + .send({ newAnswer: 'updated' }) + .expect(400); + }); + + it('returns 400 if newAnswer is missing', async () => { + const { staff, course } = await createStaffForCourse(Role.PROFESSOR); + const vectorStoreId = 'vec-notify-missing-new'; + await createAskedQuestion( + course, + vectorStoreId, + 'missing-new@example.com', + ); + + await supertest({ userId: staff.id }) + .post(`/chatbot/question/${course.id}/${vectorStoreId}/notify`) + .send({ oldAnswer: 'old' }) + .expect(400); + }); + + it.each([ + [{ oldAnswer: 123, newAnswer: 'new' }, 'oldAnswer number'], + [{ oldAnswer: 'old', newAnswer: 456 }, 'newAnswer number'], + [ + { oldAnswer: 'old', newAnswer: 'new', oldQuestion: 999 }, + 'oldQuestion number', + ], + [ + { oldAnswer: 'old', newAnswer: 'new', newQuestion: false }, + 'newQuestion boolean', + ], + ])('returns 400 for invalid DTO type: %s', async (body, _label) => { + const { staff, course } = await createStaffForCourse(Role.PROFESSOR); + const vectorStoreId = `vec-notify-invalid-${Math.random().toString(36).slice(2)}`; + await createAskedQuestion( + course, + vectorStoreId, + 'invalid-type@example.com', + ); - await supertest({ userId: professor.id }) + await supertest({ userId: staff.id }) .post(`/chatbot/question/${course.id}/${vectorStoreId}/notify`) .send(body) .expect(400); From c41c2ef38b53d00117846ca22e11de336c503618 Mon Sep 17 00:00:00 2001 From: ribhavsharma Date: Wed, 25 Feb 2026 19:34:54 -0800 Subject: [PATCH 10/11] fix test --- packages/server/test/chatbot.integration.ts | 37 ++++++++++++++++++--- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/packages/server/test/chatbot.integration.ts b/packages/server/test/chatbot.integration.ts index a78c310a1..17f3a5a32 100644 --- a/packages/server/test/chatbot.integration.ts +++ b/packages/server/test/chatbot.integration.ts @@ -249,8 +249,20 @@ describe('ChatbotController Integration', () => { totalRecipients: 5, unsubscribedRecipients: 0, }); - expectEmailSent( - students.slice(0, 5).map((s) => s.email), + const calledEmails = mockEmailService.sendEmail.mock.calls.map( + (args) => args[0].receiverOrReceivers, + ); + const calledTypes = mockEmailService.sendEmail.mock.calls.map( + (args) => args[0].type, + ); + const allStudentEmails = new Set(students.map((s) => s.email)); + + expect(calledEmails).toHaveLength(5); + expect(new Set(calledEmails).size).toBe(5); + for (const email of calledEmails) { + expect(allStudentEmails.has(email)).toBe(true); + } + expect(calledTypes).toEqual( Array(5).fill(MailServiceType.CHATBOT_ANSWER_UPDATED), ); }); @@ -292,8 +304,25 @@ describe('ChatbotController Integration', () => { totalRecipients: 5, unsubscribedRecipients: 2, }); - expectEmailSent( - students.slice(0, 3).map((s) => s.email), + const calledEmails = mockEmailService.sendEmail.mock.calls.map( + (args) => args[0].receiverOrReceivers, + ); + const calledTypes = mockEmailService.sendEmail.mock.calls.map( + (args) => args[0].type, + ); + const subscribedEmails = new Set( + students.slice(0, 3).map((student) => student.email), + ); + const unsubscribedEmails = new Set( + students.slice(3).map((student) => student.email), + ); + + expect(calledEmails).toHaveLength(3); + for (const email of calledEmails) { + expect(subscribedEmails.has(email)).toBe(true); + expect(unsubscribedEmails.has(email)).toBe(false); + } + expect(calledTypes).toEqual( Array(3).fill(MailServiceType.CHATBOT_ANSWER_UPDATED), ); }); From 980bd5d2226533887b6b4955d766705a0339a71d Mon Sep 17 00:00:00 2001 From: ribhavsharma Date: Wed, 25 Feb 2026 21:48:11 -0800 Subject: [PATCH 11/11] fixing flaky test + rm questionable workaround --- packages/server/test/question.integration.ts | 40 ++++++++------------ 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/packages/server/test/question.integration.ts b/packages/server/test/question.integration.ts index 9c60f15ad..90aafe3b2 100644 --- a/packages/server/test/question.integration.ts +++ b/packages/server/test/question.integration.ts @@ -29,7 +29,6 @@ import { modifyMockNotifs, setupIntegrationTest, } from './util/testUtils'; -import { forEach } from 'lodash'; import { QuestionTypeModel } from 'questionType/question-type.entity'; import { StudentTaskProgressModel } from 'studentTaskProgress/studentTaskProgress.entity'; @@ -202,7 +201,7 @@ describe('Question Integration', () => { const questionTypes = []; - forEach(QuestionTypes, async (questionType) => { + for (const questionType of QuestionTypes) { await QuestionTypeFactory.create({ name: questionType.name, color: questionType.color, @@ -217,7 +216,7 @@ describe('Question Integration', () => { queueId: queue.id, }; questionTypes.push(sendQuestionTypes); - }); + } await TACourseFactory.create({ user, courseId: queue.courseId }); expect(await QuestionModel.count({ where: { queueId: 1 } })).toEqual(0); @@ -225,14 +224,7 @@ describe('Question Integration', () => { expect(response.status).toBe(403); }); it('post question fails with non-existent queue', async () => { - const course = await CourseFactory.create(); const user = await UserFactory.create(); - const ta = await TACourseFactory.create({ - course: course, - user: await UserFactory.create(), - }); - // wait 0.25s to help deadlock issue? - await new Promise((resolve) => setTimeout(resolve, 250)); await supertest({ userId: user.id }) .post('/questions/999') .send({ @@ -266,14 +258,14 @@ describe('Question Integration', () => { course: course2, }); const questionTypes = []; - forEach(QuestionTypes, async (questionType) => { + for (const questionType of QuestionTypes) { const currentQuestionType = await QuestionTypeFactory.create({ name: questionType.name, color: questionType.color, cid: course2.id, }); questionTypes.push(currentQuestionType); - }); + } const response = await postQuestion(user, queueImNotIn, questionTypes); expect(response.status).toBe(404); @@ -288,14 +280,14 @@ describe('Question Integration', () => { isDisabled: true, }); const questionTypes = []; - forEach(QuestionTypes, async (questionType) => { + for (const questionType of QuestionTypes) { const currentQuestionType = await QuestionTypeFactory.create({ name: questionType.name, color: questionType.color, cid: course.id, }); questionTypes.push(currentQuestionType); - }); + } const user = await UserFactory.create(); await StudentCourseFactory.create({ user, courseId: queue.courseId }); @@ -370,14 +362,14 @@ describe('Question Integration', () => { }); const questionTypes = []; - forEach(QuestionTypes, async (questionType) => { + for (const questionType of QuestionTypes) { const currentQuestionType = await QuestionTypeFactory.create({ name: questionType.name, color: questionType.color, cid: course.id, }); questionTypes.push(currentQuestionType); - }); + } const response = await postQuestion(user, queue2, questionTypes); @@ -710,14 +702,14 @@ describe('Question Integration', () => { status: OpenQuestionStatus.Drafting, }); const questionTypes = []; - forEach(QuestionTypes, async (questionType) => { + for (const questionType of QuestionTypes) { const currentQuestionType = await QuestionTypeFactory.create({ name: questionType.name, color: questionType.color, cid: course2.id, }); questionTypes.push(currentQuestionType); - }); + } const response = await postQuestion(user, queue2, questionTypes); expect(response.status).toBe(201); @@ -751,14 +743,14 @@ describe('Question Integration', () => { status: OpenQuestionStatus.Drafting, }); const questionTypes = []; - forEach(QuestionTypes, async (questionType) => { + for (const questionType of QuestionTypes) { const currentQuestionType = await QuestionTypeFactory.create({ name: questionType.name, color: questionType.color, cid: queue.courseId, }); questionTypes.push(currentQuestionType); - }); + } const response = await postQuestion(user, queue, questionTypes); expect(response.status).toBe(201); @@ -792,14 +784,14 @@ describe('Question Integration', () => { courseId: queue.courseId, }); const questionTypes = []; - forEach(QuestionTypes, async (questionType) => { + for (const questionType of QuestionTypes) { const currentQuestionType = await QuestionTypeFactory.create({ name: questionType.name, color: questionType.color, cid: queue.courseId, }); questionTypes.push(currentQuestionType); - }); + } const response = await postQuestion(user, queue, questionTypes); expect(response.status).toBe(201); @@ -829,14 +821,14 @@ describe('Question Integration', () => { courseId: queue.courseId, }); const questionTypes = []; - forEach(QuestionTypes, async (questionType) => { + for (const questionType of QuestionTypes) { const currentQuestionType = await QuestionTypeFactory.create({ name: questionType.name, color: questionType.color, cid: queue.courseId, }); questionTypes.push(currentQuestionType); - }); + } const response = await postQuestion(user, queue, questionTypes); expect(response.status).toBe(201);