diff --git a/packages/common/index.ts b/packages/common/index.ts index 6a93e2c44..4d5017abb 100644 --- a/packages/common/index.ts +++ b/packages/common/index.ts @@ -430,6 +430,7 @@ export interface SourceDocument { courseId?: string fromLMS?: boolean apiDocId?: number + asyncQuestionId?: number // inserted async questions only } type?: string // TODO: is it content or pageContent? since this file uses both. EDIT: It seems to be both/either. Gross. @@ -490,6 +491,7 @@ export interface AddDocumentChunkParams { name: string type: string source?: string + asyncQuestionId?: number loc?: Loc id?: string courseId?: number @@ -1397,6 +1399,14 @@ export class AsyncQuestionParams { @IsOptional() @IsInt() votesSum?: number + + @IsOptional() + @IsBoolean() + saveToChatbot?: boolean + + @IsOptional() + @IsBoolean() + refreshAIAnswer?: boolean } export class AsyncQuestionVotes { @IsOptional() @@ -3964,6 +3974,8 @@ export interface ToolUsageExportData { export const ERROR_MESSAGES = { common: { pageOutOfBounds: "Can't retrieve out of bounds page.", + noDiskSpace: + 'There is not enough disk space left to store an image (<1GB). Please immediately contact your course staff and let them know. They will contact the HelpMe team as soon as possible.', }, questionService: { getDBClient: 'Error getting DB client', @@ -4243,8 +4255,6 @@ export const ERROR_MESSAGES = { noProfilePicture: "User doesn't have a profile picture", noCoursesToDelete: "User doesn't have any courses to delete", emailInUse: 'Email is already in use', - noDiskSpace: - 'There is no disk space left to store an image. Please immediately contact your course staff and let them know. They will contact the HelpMe team as soon as possible.', }, alertController: { duplicateAlert: 'This alert has already been sent', diff --git a/packages/frontend/app/(dashboard)/components/CourseCloneForm.tsx b/packages/frontend/app/(dashboard)/components/CourseCloneForm.tsx index ae98bbe70..b6302094d 100644 --- a/packages/frontend/app/(dashboard)/components/CourseCloneForm.tsx +++ b/packages/frontend/app/(dashboard)/components/CourseCloneForm.tsx @@ -269,7 +269,7 @@ const CourseCloneForm: React.FC = ({ valuePropName="checked" label="Documents" layout="horizontal" - tooltip="Clone the documents you uploaded to the chatbot. Note that after you clone these, you may want to review them and remove any that contain out-of-date information" + tooltip="Clone the documents you uploaded to the chatbot knowledge base. Note that after you clone these, you may want to review them and remove any that contain out-of-date information" className={`${formItemClassNames}`} > @@ -289,7 +289,7 @@ const CourseCloneForm: React.FC = ({ valuePropName="checked" label="Inserted Questions" layout="horizontal" - tooltip="Clone over any chatbot questions that were inserted as a source into the chatbot." + tooltip="Clone over any Chatbot Questions and Anytime Questions that were inserted into the chatbot knowledge base." className={`${formItemClassNames}`} > @@ -299,7 +299,7 @@ const CourseCloneForm: React.FC = ({ valuePropName="checked" label="Inserted LMS Data" layout="horizontal" - tooltip="Clone over any LMS data (e.g. assignment descriptions, announcements) that was inserted as a source into the chatbot. Defaulted to false since announcements usually have outdated information." + tooltip="Clone over any LMS data (e.g. assignment descriptions, announcements) that was inserted into the chatbot knowledge base. Defaulted to false since announcements usually have outdated information." className={`${formItemClassNames}`} > diff --git a/packages/frontend/app/(dashboard)/course/[cid]/async_centre/components/modals/PostResponseModal.tsx b/packages/frontend/app/(dashboard)/course/[cid]/async_centre/components/modals/PostResponseModal.tsx index 0ffc24f15..46a09d00b 100644 --- a/packages/frontend/app/(dashboard)/course/[cid]/async_centre/components/modals/PostResponseModal.tsx +++ b/packages/frontend/app/(dashboard)/course/[cid]/async_centre/components/modals/PostResponseModal.tsx @@ -57,6 +57,7 @@ const PostResponseModal: React.FC = ({ useState(false) const courseFeatures = useCourseFeatures(courseId) const authorCanSetVisible = courseFeatures?.asyncCentreAuthorPublic ?? false + const [saveToChatbot, setSaveToChatbot] = useState(true) const [hasCheckedPopconfirm, setHasCheckedPopconfirm] = useState(!authorCanSetVisible) @@ -88,6 +89,7 @@ const PostResponseModal: React.FC = ({ staffSetVisible: staffSetVisible, status: newStatus, verified: values.verified, + saveToChatbot: saveToChatbot, }) .then(() => { message.success('Response Successfully Posted/Edited') @@ -108,6 +110,17 @@ const PostResponseModal: React.FC = ({ title="Post/Edit response to Student question" okText="Finish" cancelText="Cancel" + cancelButtonProps={{ + className: 'md:w-24', + }} + width={{ + xs: '100%', + sm: '100%', + md: '100%', + lg: '60%', + xl: '50%', + xxl: '35%', + }} okButtonProps={{ autoFocus: true, htmlType: 'submit', @@ -129,11 +142,10 @@ const PostResponseModal: React.FC = ({ onCancel={onCancel} // display delete button for mobile in footer footer={(_, { OkBtn, CancelBtn }) => ( -
- {question.creator.id == userInfo.id && ( -
- Actions -
+
+
+ {question.creator.id === userInfo.id ? ( // special case where a TA created their own question + <> = ({ onClick={() => setCreateAsyncQuestionModalOpen(true)} > {' '} - Edit + Edit Question )} -
- - Post Response - -
- )} -
- {question.creator.id != userInfo.id ? ( -
- -
+ ) : ( -
+ // Standard case + )} -
- - - +
+
+ + { + onFinish().then() + setConfirmPopoverOpen(false) + }} + onCancel={() => setConfirmPopoverOpen(false)} + /> + setSaveToChatbot(e.target.checked)} + // Antd checkboxes will automatically put its children into a span with some padding, so this targets it to get rid of the padding + className="[&>span]:!px-0 [&>span]:text-center" + > + +

+ Keeping this enabled will insert this Q&A into the + chatbot's knowledge base, allowing the chatbot to + reference it in future answers. +

+

+ Please consider disabling this if the question contains + private information. +

+
} - open={confirmPopoverOpen} - arrow={false} - okText="Yes" - cancelText="No" - onConfirm={() => { - onFinish().then() - setConfirmPopoverOpen(false) - }} - onCancel={() => setConfirmPopoverOpen(false)} - > -
+ > + + Save to Chatbot + + + +
)} @@ -311,7 +342,7 @@ const DeleteButton: React.FC = ({ return ( trigger.parentNode as HTMLElement} diff --git a/packages/server/src/asyncQuestion/asyncQuestion.controller.ts b/packages/server/src/asyncQuestion/asyncQuestion.controller.ts index 203eab57f..e74da7b7e 100644 --- a/packages/server/src/asyncQuestion/asyncQuestion.controller.ts +++ b/packages/server/src/asyncQuestion/asyncQuestion.controller.ts @@ -40,7 +40,7 @@ import { CourseRolesGuard } from 'guards/course-roles.guard'; import { AsyncQuestionRolesGuard } from 'guards/async-question-roles.guard'; import { pick } from 'lodash'; import { UserModel } from 'profile/user.entity'; -import { Not } from 'typeorm'; +import { In, Not } from 'typeorm'; import { ApplicationConfigService } from '../config/application_config.service'; import { AsyncQuestionService } from './asyncQuestion.service'; import { UnreadAsyncQuestionModel } from './unread-async-question.entity'; @@ -323,10 +323,10 @@ export class asyncQuestionController { @Patch('faculty/:questionId') @UseGuards(AsyncQuestionRolesGuard) @Roles(Role.TA, Role.PROFESSOR) - async updateTAQuestion( + async updateQuestionStaff( @Param('questionId', ParseIntPipe) questionId: number, @Body() body: UpdateAsyncQuestions, - @UserId() userId: number, + @User({ chat_token: true }) user: UserModel, ): Promise { const question = await AsyncQuestionModel.findOne({ where: { id: questionId }, @@ -354,7 +354,7 @@ export class asyncQuestionController { // Verify if user is TA/PROF of the course const requester = await UserCourseModel.findOne({ where: { - userId: userId, + userId: user.id, courseId: courseId, }, }); @@ -373,7 +373,7 @@ export class asyncQuestionController { if (body.status === asyncQuestionStatus.HumanAnswered) { question.closedAt = new Date(); - question.taHelpedId = userId; + question.taHelpedId = user.id; await this.asyncQuestionService.sendQuestionAnsweredEmails(question); } else if ( body.status !== asyncQuestionStatus.TADeleted && @@ -390,6 +390,14 @@ export class asyncQuestionController { const updatedQuestion = await question.save(); + if (body.saveToChatbot) { + await this.asyncQuestionService.upsertQAToChatbotChunk( + updatedQuestion, + courseId, + user.chat_token.token, + ); + } + // Mark as new unread for all students if the question is marked as visible const courseSettings = await CourseSettingsModel.findOne({ where: { courseId: courseId }, @@ -406,7 +414,7 @@ export class asyncQuestionController { await this.asyncQuestionService.markUnreadForRoles( updatedQuestion, [Role.STUDENT], - userId, + user.id, ); } // When the question creator gets their question human verified, notify them @@ -760,11 +768,19 @@ export class asyncQuestionController { let all: AsyncQuestionModel[]; if (!asyncQuestionKeys || Object.keys(asyncQuestionKeys).length === 0) { - console.log('Fetching from Database'); + console.log( + `Fetching async questions from Database for courseId ${courseId}`, + ); all = await AsyncQuestionModel.find({ where: { courseId, - status: Not(asyncQuestionStatus.StudentDeleted), + // don't include studentDeleted or TADeleted questions + status: Not( + In([ + asyncQuestionStatus.StudentDeleted, + asyncQuestionStatus.TADeleted, + ]), + ), }, relations: [ 'creator', @@ -783,7 +799,9 @@ export class asyncQuestionController { if (all) await this.redisQueueService.setAsyncQuestions(`c:${courseId}:aq`, all); } else { - console.log('Fetching from Redis'); + console.log( + `Fetching async questions from Redis for courseId ${courseId}`, + ); all = Object.values(asyncQuestionKeys).map( (question) => question as AsyncQuestionModel, ); diff --git a/packages/server/src/asyncQuestion/asyncQuestion.module.ts b/packages/server/src/asyncQuestion/asyncQuestion.module.ts index 788b6904b..72cbd5c47 100644 --- a/packages/server/src/asyncQuestion/asyncQuestion.module.ts +++ b/packages/server/src/asyncQuestion/asyncQuestion.module.ts @@ -6,6 +6,8 @@ import { MailModule, MailTestingModule } from 'mail/mail.module'; import { RedisQueueService } from '../redisQueue/redis-queue.service'; import { ApplicationConfigService } from '../config/application_config.service'; import { RedisQueueModule } from '../redisQueue/redis-queue.module'; +import { ChatbotModule } from 'chatbot/chatbot.module'; +import { ChatbotApiService } from 'chatbot/chatbot-api.service'; @Module({ controllers: [asyncQuestionController], @@ -13,16 +15,21 @@ import { RedisQueueModule } from '../redisQueue/redis-queue.module'; AsyncQuestionService, RedisQueueService, ApplicationConfigService, + ChatbotApiService, ], - imports: [NotificationModule, MailModule, RedisQueueModule], + imports: [NotificationModule, MailModule, RedisQueueModule, ChatbotModule], exports: [AsyncQuestionService], }) export class asyncQuestionModule {} @Module({ controllers: [asyncQuestionController], - providers: [AsyncQuestionService, ApplicationConfigService], - imports: [NotificationModule, MailTestingModule], + providers: [ + AsyncQuestionService, + ChatbotApiService, + ApplicationConfigService, + ], + imports: [NotificationModule, MailTestingModule, ChatbotModule], exports: [AsyncQuestionService], }) export class asyncQuestionTestingModule {} diff --git a/packages/server/src/asyncQuestion/asyncQuestion.service.ts b/packages/server/src/asyncQuestion/asyncQuestion.service.ts index 119a15143..7cccdacb7 100644 --- a/packages/server/src/asyncQuestion/asyncQuestion.service.ts +++ b/packages/server/src/asyncQuestion/asyncQuestion.service.ts @@ -1,4 +1,9 @@ -import { MailServiceType, parseThinkBlock, Role } from '@koh/common'; +import { + AddDocumentChunkParams, + MailServiceType, + parseThinkBlock, + Role, +} from '@koh/common'; import { Injectable } from '@nestjs/common'; import { MailService } from 'mail/mail.service'; import { UserSubscriptionModel } from 'mail/user-subscriptions.entity'; @@ -10,10 +15,14 @@ import * as Sentry from '@sentry/nestjs'; import { UnreadAsyncQuestionModel } from './unread-async-question.entity'; import { CourseSettingsModel } from '../course/course_settings.entity'; import { SentEmailModel } from '../mail/sent-email.entity'; +import { ChatbotApiService } from '../chatbot/chatbot-api.service'; @Injectable() export class AsyncQuestionService { - constructor(private mailService: MailService) {} + constructor( + private readonly mailService: MailService, + private readonly chatbotApiService: ChatbotApiService, + ) {} async sendNewCommentOnMyQuestionEmail( commenter: UserModel, @@ -387,6 +396,50 @@ export class AsyncQuestionService { .execute(); } + /* + */ + async upsertQAToChatbotChunk( + question: AsyncQuestionModel, + courseId: number, + userToken: string, + ) { + // Since the name can take up quite a bit of space, no more than 40 characters (show ... if longer) + const chunkName = `Previously Asked Anytime Question: ${(question.questionAbstract ?? question.questionText).slice(0, 40)}${(question.questionAbstract ?? question.questionText).length > 40 ? '...' : ''}`; + const chunkParams: AddDocumentChunkParams = { + documentText: `${this.formatQuestionTextForChatbot(question)}\n\nAnswer: ${question.answerText}`, + metadata: { + name: chunkName, + type: 'inserted_question', + asyncQuestionId: question.id, + source: `/course/${courseId}/async_centre`, + courseId: courseId, + }, + }; + // Note that because the chunk splitter will split big chunks into multiple, + // we must first delete any existing chunks with the async question ID and then re-add them. + await this.chatbotApiService.deleteDocumentChunksByAsyncQuestionId( + question.id, + courseId, + userToken, + ); + await this.chatbotApiService.addDocumentChunk( + chunkParams, + courseId, + userToken, + ); + } + + /* Just for formatting the details of the question for sending to the chatbot (for getting image summaries) or for a chunk. + Does stuff like if there's only an abstract, the abstract will just be called "Question" instead of having "Question Abstract" and "Question Text" + */ + formatQuestionTextForChatbot(question: AsyncQuestionModel) { + return `${question.questionText ? `Question Abstract: ${question.questionAbstract}` : `Question: ${question.questionAbstract}`} + ${question.questionText ? `Question Text: ${question.questionText}` : ''} + ${question.questionTypes && question.questionTypes.length > 0 ? `Question Types: ${question.questionTypes.map((questionType) => questionType.name).join(', ')}` : ''} + `; + // TODO: once images are added, add this: ${`Question Image Descriptions: ${question.images.map((image, idx) => `Image ${idx + 1}: ${image.aiSummary}`).join('\n')}`} + } + /** * Takes in a userId and async questionId and hashes them to return a random index from ANONYMOUS_ANIMAL_AVATAR.ANIMAL_NAMES * Note that 70 is the length of ANONYMOUS_ANIMAL_AVATAR.ANIMAL_NAMES diff --git a/packages/server/src/chatbot/chatbot-api.service.ts b/packages/server/src/chatbot/chatbot-api.service.ts index 6f979e866..fc352fc6c 100644 --- a/packages/server/src/chatbot/chatbot-api.service.ts +++ b/packages/server/src/chatbot/chatbot-api.service.ts @@ -254,6 +254,18 @@ export class ChatbotApiService { return this.request('DELETE', `document/${courseId}/${docId}`, userToken); } + async deleteDocumentChunksByAsyncQuestionId( + asyncQuestionId: number, + courseId: number, + userToken: string, + ): Promise { + return this.request( + 'DELETE', + `document/${courseId}/asyncQuestion/${asyncQuestionId}`, + userToken, + ); + } + async deleteDocument(docId: string, courseId: number, userToken: string) { return this.request( 'DELETE', diff --git a/packages/server/src/profile/profile.service.ts b/packages/server/src/profile/profile.service.ts index 13996496f..8b393dd8f 100644 --- a/packages/server/src/profile/profile.service.ts +++ b/packages/server/src/profile/profile.service.ts @@ -127,7 +127,7 @@ export class ProfileService { const spaceLeft = await checkDiskSpace(path.parse(process.cwd()).root); if (spaceLeft.free < 1_000_000_000) { throw new ServiceUnavailableException( - ERROR_MESSAGES.profileController.noDiskSpace, + ERROR_MESSAGES.common.noDiskSpace, ); } diff --git a/packages/server/test/asyncQuestion.integration.ts b/packages/server/test/asyncQuestion.integration.ts index 49c5ffeb8..3998eb7a8 100644 --- a/packages/server/test/asyncQuestion.integration.ts +++ b/packages/server/test/asyncQuestion.integration.ts @@ -4,6 +4,7 @@ import { UserModel } from 'profile/user.entity'; import { AsyncQuestionCommentFactory, AsyncQuestionFactory, + ChatTokenFactory, CourseFactory, CourseSettingsFactory, QuestionTypeFactory, @@ -11,7 +12,12 @@ import { UserFactory, VotesFactory, } from './util/factories'; -import { overrideRedisQueue, setupIntegrationTest } from './util/testUtils'; +import { + mockChatbotService, + overrideChatbotService, + overrideRedisQueue, + setupIntegrationTest, +} from './util/testUtils'; import { asyncQuestionModule } from 'asyncQuestion/asyncQuestion.module'; import { AsyncQuestion, asyncQuestionStatus, Role } from '@koh/common'; import { AsyncQuestionVotesModel } from 'asyncQuestion/asyncQuestionVotes.entity'; @@ -19,10 +25,10 @@ import { UnreadAsyncQuestionModel } from 'asyncQuestion/unread-async-question.en import { AsyncQuestionCommentModel } from '../src/asyncQuestion/asyncQuestionComment.entity'; describe('AsyncQuestion Integration', () => { - const { supertest } = setupIntegrationTest( - asyncQuestionModule, + const { supertest } = setupIntegrationTest(asyncQuestionModule, [ overrideRedisQueue, - ); + overrideChatbotService, + ]); let course: CourseModel; let TAuser: UserModel; @@ -60,6 +66,9 @@ describe('AsyncQuestion Integration', () => { course, role: Role.TA, }); + await ChatTokenFactory.create({ + user: TAuser, + }); await UserCourseFactory.create({ user: studentUser, course, @@ -227,6 +236,64 @@ describe('AsyncQuestion Integration', () => { expect(response.body.status).toBe(asyncQuestionStatus.HumanAnswered); }); }); + it('will insert the Q&A as new chatbot chunks if saveToChatbot is true (just checks if it calls the right chatbot endpoints)', async () => { + await supertest({ userId: TAuser.id }) + .patch(`/asyncQuestions/faculty/${asyncQuestion.id}`) + .send({ + questionAbstract: 'new abstract', + questionText: 'new text', + answerText: 'new answer', + saveToChatbot: true, + }) + .expect(200) + .then((response) => { + expect(response.body).toHaveProperty( + 'questionAbstract', + 'new abstract', + ); + expect(response.body).toHaveProperty('questionText', 'new text'); + expect(response.body).toHaveProperty('answerText', 'new answer'); + }); + expect( + mockChatbotService.deleteDocumentChunksByAsyncQuestionId, + ).toHaveBeenCalledWith( + asyncQuestion.id, + course.id, + expect.any(String), // userToken + ); + expect(mockChatbotService.addDocumentChunk).toHaveBeenCalledWith( + expect.objectContaining({ + documentText: expect.stringMatching( + /(?=.*new abstract)(?=.*new text)(?=.*new answer)/s, + ), // all three must appear in documentText + }), + course.id, + expect.any(String), // userToken + ); + }); + it('will NOT call chatbot methods if saveToChatbot is false (or not provided)', async () => { + await supertest({ userId: TAuser.id }) + .patch(`/asyncQuestions/faculty/${asyncQuestion.id}`) + .send({ + questionAbstract: 'new abstract', + questionText: 'new text', + answerText: 'new answer', + // saveToChatbot is not set (defaults to false/undefined) + }) + .expect(200) + .then((response) => { + expect(response.body).toHaveProperty( + 'questionAbstract', + 'new abstract', + ); + expect(response.body).toHaveProperty('questionText', 'new text'); + expect(response.body).toHaveProperty('answerText', 'new answer'); + }); + expect(mockChatbotService.addDocumentChunk).not.toHaveBeenCalled(); + expect( + mockChatbotService.deleteDocumentChunksByAsyncQuestionId, + ).not.toHaveBeenCalled(); + }); }); describe('PATCH /asyncQuestions/student/:questionId', () => { diff --git a/packages/server/test/asyncQuestionEmails.integration.ts b/packages/server/test/asyncQuestionEmails.integration.ts index 8110303a5..e67b6b1cb 100644 --- a/packages/server/test/asyncQuestionEmails.integration.ts +++ b/packages/server/test/asyncQuestionEmails.integration.ts @@ -26,10 +26,9 @@ import { MailServiceModel } from 'mail/mail-services.entity'; The reason why these aren't in asyncQuestion.integration.ts is these have different setups and mocks and separating them helps keep it organised. */ describe('AsyncQuestion Integration - Email Tests', () => { - const { supertest } = setupIntegrationTest( - asyncQuestionModule, + const { supertest } = setupIntegrationTest(asyncQuestionModule, [ overrideEmailService, - ); + ]); let course: CourseModel; let questionOwner: UserModel; diff --git a/packages/server/test/course.integration.ts b/packages/server/test/course.integration.ts index 8171dddb2..9e39d6e5c 100644 --- a/packages/server/test/course.integration.ts +++ b/packages/server/test/course.integration.ts @@ -2346,7 +2346,7 @@ describe('Course Integration', () => { expect(updatedTa.TANotes).not.toEqual('This is a test note'); }); }); -describe('GET /courses/:id/export-tool-usage', () => { + describe('GET /courses/:id/export-tool-usage', () => { it('should return 400 when no students are enrolled in the course', async () => { const course = await CourseFactory.create(); const professor = await UserFactory.create(); @@ -2710,243 +2710,245 @@ describe('GET /courses/:id/export-tool-usage', () => { expect(Number(anytimeData[0]?.count)).toBe(1); // Should only count the non-deleted one }); - describe('POST /courses/:courseId/clone_course', () => { - const modifyModule = (builder) => { - return builder.overrideProvider(CourseService).useValue({ - cloneCourse: jest - .fn() - .mockImplementation((courseId, userId, body, token) => { - return Promise.resolve({ - course: { - id: courseId, - name: 'Test Sample Course', - semesterId: 1, - enabled: true, - sectionGroupName: '001', - }, - role: Role.PROFESSOR, - favourited: true, - } as UserCourse); - }), - }); - }; - - const { supertest, getTestModule } = setupIntegrationTest( - CourseModule, - modifyModule, - [MailModule], - ); - - it('should return 401 if user is not authenticated', async () => { - await supertest().post('/courses/1/clone_course').expect(401); - }); - - it('should return 401 if user is professor and professors disallowed from creating courses', async () => { - const professor = await UserFactory.create({ chat_token: null }); - const course = await CourseFactory.create(); - const organization = await OrganizationFactory.create(); - await OrganizationSettingsFactory.create({ - organizationId: organization.id, - organization, - allowProfCourseCreate: false, - }); - - await OrganizationUserFactory.create({ - organizationUser: professor, - organization: organization, - role: OrganizationRole.PROFESSOR, - }); + describe('POST /courses/:courseId/clone_course', () => { + const modifyModule = (builder) => { + return builder.overrideProvider(CourseService).useValue({ + cloneCourse: jest + .fn() + .mockImplementation((courseId, userId, body, token) => { + return Promise.resolve({ + course: { + id: courseId, + name: 'Test Sample Course', + semesterId: 1, + enabled: true, + sectionGroupName: '001', + }, + role: Role.PROFESSOR, + favourited: true, + } as UserCourse); + }), + }); + }; - await OrganizationCourseFactory.create({ - course: course, - organization: organization, - }); + const { supertest, getTestModule } = setupIntegrationTest( + CourseModule, + [modifyModule], + [MailModule], + ); - await UserCourseFactory.create({ - user: professor, - role: Role.PROFESSOR, - course, + it('should return 401 if user is not authenticated', async () => { + await supertest().post('/courses/1/clone_course').expect(401); }); - await supertest({ userId: professor.id }) - .post(`/courses/${course.id}/clone_course`) - .send({ - name: 'Cloned Course', - semesterId: 1, - }) - .expect(401); - }); + it('should return 401 if user is professor and professors disallowed from creating courses', async () => { + const professor = await UserFactory.create({ chat_token: null }); + const course = await CourseFactory.create(); + const organization = await OrganizationFactory.create(); + await OrganizationSettingsFactory.create({ + organizationId: organization.id, + organization, + allowProfCourseCreate: false, + }); - it('should return 404 if user has no chat token', async () => { - const professor = await UserFactory.create({ chat_token: null }); - const course = await CourseFactory.create(); - const organization = await OrganizationFactory.create(); + await OrganizationUserFactory.create({ + organizationUser: professor, + organization: organization, + role: OrganizationRole.PROFESSOR, + }); - await OrganizationUserFactory.create({ - organizationUser: professor, - organization: organization, - }); + await OrganizationCourseFactory.create({ + course: course, + organization: organization, + }); - await OrganizationCourseFactory.create({ - course: course, - organization: organization, - }); + await UserCourseFactory.create({ + user: professor, + role: Role.PROFESSOR, + course, + }); - await UserCourseFactory.create({ - user: professor, - role: Role.PROFESSOR, - course, + await supertest({ userId: professor.id }) + .post(`/courses/${course.id}/clone_course`) + .send({ + name: 'Cloned Course', + semesterId: 1, + }) + .expect(401); }); - // capture console.error - const consoleError = jest.spyOn(console, 'error').mockImplementation(); + it('should return 404 if user has no chat token', async () => { + const professor = await UserFactory.create({ chat_token: null }); + const course = await CourseFactory.create(); + const organization = await OrganizationFactory.create(); - await supertest({ userId: professor.id }) - .post(`/courses/${course.id}/clone_course`) - .send({ - name: 'Cloned Course', - semesterId: 1, - }) - .expect(404); + await OrganizationUserFactory.create({ + organizationUser: professor, + organization: organization, + }); - expect(consoleError).toHaveBeenCalledWith( - ERROR_MESSAGES.profileController.accountNotAvailable, - ); - consoleError.mockRestore(); - }); + await OrganizationCourseFactory.create({ + course: course, + organization: organization, + }); - it('should return 403 if user is not a professor of the course', async () => { - const student = await UserFactory.create(); - const chatToken = await ChatTokenFactory.create({ user: student }); - student.chat_token = chatToken; - await student.save(); + await UserCourseFactory.create({ + user: professor, + role: Role.PROFESSOR, + course, + }); - const organization = await OrganizationFactory.create(); + // capture console.error + const consoleError = jest.spyOn(console, 'error').mockImplementation(); - await OrganizationUserFactory.create({ - organizationUser: student, - organization: organization, - }); + await supertest({ userId: professor.id }) + .post(`/courses/${course.id}/clone_course`) + .send({ + name: 'Cloned Course', + semesterId: 1, + }) + .expect(404); - const course = await CourseFactory.create(); - await OrganizationCourseFactory.create({ - course: course, - organization: organization, - }); - await UserCourseFactory.create({ - user: student, - role: Role.STUDENT, - course, + expect(consoleError).toHaveBeenCalledWith( + ERROR_MESSAGES.profileController.accountNotAvailable, + ); + consoleError.mockRestore(); }); - await supertest({ userId: student.id }) - .post(`/courses/${course.id}/clone_course`) - .send({ - name: 'Cloned Course', - semesterId: 1, - }) - .expect(403); - }); + it('should return 403 if user is not a professor of the course', async () => { + const student = await UserFactory.create(); + const chatToken = await ChatTokenFactory.create({ user: student }); + student.chat_token = chatToken; + await student.save(); - it('should return 201 and call cloneCourse with the right params when user is a professor', async () => { - const professor = await UserFactory.create(); - const chatToken = await ChatTokenFactory.create({ user: professor }); - professor.chat_token = chatToken; - await professor.save(); + const organization = await OrganizationFactory.create(); - const organization = await OrganizationFactory.create(); + await OrganizationUserFactory.create({ + organizationUser: student, + organization: organization, + }); - await OrganizationUserFactory.create({ - organizationUser: professor, - organization: organization, - }); + const course = await CourseFactory.create(); + await OrganizationCourseFactory.create({ + course: course, + organization: organization, + }); + await UserCourseFactory.create({ + user: student, + role: Role.STUDENT, + course, + }); - const course = await CourseFactory.create(); - await OrganizationCourseFactory.create({ - course: course, - organization: organization, - }); - await UserCourseFactory.create({ - user: professor, - role: Role.PROFESSOR, - course, + await supertest({ userId: student.id }) + .post(`/courses/${course.id}/clone_course`) + .send({ + name: 'Cloned Course', + semesterId: 1, + }) + .expect(403); }); - const cloneParams = { - name: 'Cloned Course', - semesterId: 1, - }; + it('should return 201 and call cloneCourse with the right params when user is a professor', async () => { + const professor = await UserFactory.create(); + const chatToken = await ChatTokenFactory.create({ user: professor }); + professor.chat_token = chatToken; + await professor.save(); - const response = await supertest({ userId: professor.id }) - .post(`/courses/${course.id}/clone_course`) - .send(cloneParams) - .expect(201); + const organization = await OrganizationFactory.create(); - const module = getTestModule(); - const courseService = module.get(CourseService); + await OrganizationUserFactory.create({ + organizationUser: professor, + organization: organization, + }); - expect(courseService.cloneCourse).toHaveBeenCalledWith( - course.id, - professor.id, - cloneParams, - chatToken.token, - ); + const course = await CourseFactory.create(); + await OrganizationCourseFactory.create({ + course: course, + organization: organization, + }); + await UserCourseFactory.create({ + user: professor, + role: Role.PROFESSOR, + course, + }); - expect(response.body).toEqual({ - course: { - id: course.id, - name: 'Test Sample Course', + const cloneParams = { + name: 'Cloned Course', semesterId: 1, - enabled: true, - sectionGroupName: '001', - }, - role: Role.PROFESSOR, - favourited: true, - }); - }); - - it('should return 201 when organization admin calls the endpoint', async () => { - const adminUser = await UserFactory.create(); - adminUser.chat_token = await ChatTokenFactory.create({ user: adminUser }); - await adminUser.save(); - - const organization = await OrganizationFactory.create(); - await OrganizationUserFactory.create({ - organizationUser: adminUser, - organization: organization, - role: OrganizationRole.ADMIN, + }; + + const response = await supertest({ userId: professor.id }) + .post(`/courses/${course.id}/clone_course`) + .send(cloneParams) + .expect(201); + + const module = getTestModule(); + const courseService = module.get(CourseService); + + expect(courseService.cloneCourse).toHaveBeenCalledWith( + course.id, + professor.id, + cloneParams, + chatToken.token, + ); + + expect(response.body).toEqual({ + course: { + id: course.id, + name: 'Test Sample Course', + semesterId: 1, + enabled: true, + sectionGroupName: '001', + }, + role: Role.PROFESSOR, + favourited: true, + }); }); - const course = await CourseFactory.create(); - await OrganizationCourseFactory.create({ - course: course, - organization: organization, - }); + it('should return 201 when organization admin calls the endpoint', async () => { + const adminUser = await UserFactory.create(); + adminUser.chat_token = await ChatTokenFactory.create({ + user: adminUser, + }); + await adminUser.save(); - const cloneParams = { - name: 'Cloned Course', - semesterId: 1, - }; + const organization = await OrganizationFactory.create(); + await OrganizationUserFactory.create({ + organizationUser: adminUser, + organization: organization, + role: OrganizationRole.ADMIN, + }); - const response = await supertest({ userId: adminUser.id }) - .post(`/courses/${course.id}/clone_course`) - .send(cloneParams) - .expect(201); + const course = await CourseFactory.create(); + await OrganizationCourseFactory.create({ + course: course, + organization: organization, + }); - expect(response.body).toEqual({ - course: { - id: course.id, - name: 'Test Sample Course', + const cloneParams = { + name: 'Cloned Course', semesterId: 1, - enabled: true, - sectionGroupName: '001', - }, - role: Role.PROFESSOR, - favourited: true, + }; + + const response = await supertest({ userId: adminUser.id }) + .post(`/courses/${course.id}/clone_course`) + .send(cloneParams) + .expect(201); + + expect(response.body).toEqual({ + course: { + id: course.id, + name: 'Test Sample Course', + semesterId: 1, + enabled: true, + sectionGroupName: '001', + }, + role: Role.PROFESSOR, + favourited: true, + }); }); }); - }); - // WARNING: DO NOT put any test suites BELOW clone_course integration tests because they modify the DB connection + // WARNING: DO NOT put any test suites BELOW clone_course integration tests because they modify the DB connection }); }); diff --git a/packages/server/test/organization.integration.ts b/packages/server/test/organization.integration.ts index 6bd939216..cbcab48f6 100644 --- a/packages/server/test/organization.integration.ts +++ b/packages/server/test/organization.integration.ts @@ -3349,7 +3349,7 @@ describe('Organization Integration', () => { const { supertest, getTestModule } = setupIntegrationTest( CourseModule, - modifyModule, + [modifyModule], [MailModule], ); diff --git a/packages/server/test/question.integration.ts b/packages/server/test/question.integration.ts index 9c60f15ad..78d14df90 100644 --- a/packages/server/test/question.integration.ts +++ b/packages/server/test/question.integration.ts @@ -34,7 +34,9 @@ import { QuestionTypeModel } from 'questionType/question-type.entity'; import { StudentTaskProgressModel } from 'studentTaskProgress/studentTaskProgress.entity'; describe('Question Integration', () => { - const { supertest } = setupIntegrationTest(QuestionModule, modifyMockNotifs); + const { supertest } = setupIntegrationTest(QuestionModule, [ + modifyMockNotifs, + ]); const QuestionTypes = [ { diff --git a/packages/server/test/util/testUtils.ts b/packages/server/test/util/testUtils.ts index cb1e054b5..1e70dcbdf 100644 --- a/packages/server/test/util/testUtils.ts +++ b/packages/server/test/util/testUtils.ts @@ -76,7 +76,7 @@ export class TestChatbotModule {} export function setupIntegrationTest( module: Type, - modifyModule?: ModuleModifier, + modifyModules?: ModuleModifier | ModuleModifier[], additionalModules: Type[] = [], additionalMiddlewares: (( req: express.Request, @@ -166,8 +166,14 @@ export function setupIntegrationTest( .overrideModule(ChatbotDataSourceModule) .useModule(TestChatbotDataSourceModule); - if (modifyModule) { - testModuleBuilder = modifyModule(testModuleBuilder); + if (modifyModules) { + if (Array.isArray(modifyModules)) { + for (const modifyModule of modifyModules) { + testModuleBuilder = modifyModule(testModuleBuilder); + } + } else { + testModuleBuilder = modifyModules(testModuleBuilder); + } } testModule = await testModuleBuilder.compile(); @@ -300,10 +306,17 @@ export const mockRedisQueueService = { getKey: jest.fn().mockResolvedValue([]), deleteKey: jest.fn(), }; - export const overrideRedisQueue: ModuleModifier = (builder) => builder.overrideProvider(RedisQueueService).useValue(mockRedisQueueService); +export const mockChatbotService = { + // can add more if you'd like + addDocumentChunk: jest.fn(), + deleteDocumentChunksByAsyncQuestionId: jest.fn(), +}; +export const overrideChatbotService: ModuleModifier = (builder) => + builder.overrideProvider(ChatbotApiService).useValue(mockChatbotService); + const mockSendEmail = jest.fn().mockImplementation(() => Promise.resolve()); export const mockEmailService = { sendEmail: mockSendEmail,