diff --git a/packages/common/index.ts b/packages/common/index.ts index c73b50046..6c831f528 100644 --- a/packages/common/index.ts +++ b/packages/common/index.ts @@ -1329,6 +1329,11 @@ export type AsyncQuestion = { votesSum: number } +export type GetAsyncQuestionsResponse = { + questions: AsyncQuestion[] + hiddenPrivateQuestionsCount: number +} + /** * An async question is created when a student wants help from a TA. */ diff --git a/packages/frontend/app/(dashboard)/course/[cid]/async_centre/page.tsx b/packages/frontend/app/(dashboard)/course/[cid]/async_centre/page.tsx index 35cf1e228..87ef5a9c7 100644 --- a/packages/frontend/app/(dashboard)/course/[cid]/async_centre/page.tsx +++ b/packages/frontend/app/(dashboard)/course/[cid]/async_centre/page.tsx @@ -12,6 +12,7 @@ import React, { import { Button, Checkbox, + Empty, Pagination, Popover, Segmented, @@ -65,7 +66,11 @@ export default function AsyncCentrePage( const [page, setPage] = useState(1) const [pageSize, setPageSize] = useState(20) - const [asyncQuestions, mutateAsyncQuestions] = useAsyncQuestions(courseId) + const [asyncQuestionsResponse, mutateAsyncQuestions] = + useAsyncQuestions(courseId) + const asyncQuestions = asyncQuestionsResponse?.questions + const hiddenPrivateQuestionsCount = + asyncQuestionsResponse?.hiddenPrivateQuestionsCount ?? 0 const [createAsyncQuestionModalOpen, setCreateAsyncQuestionModalOpen] = useState(false) @@ -207,6 +212,8 @@ export default function AsyncCentrePage( const displayedQuestions = useMemo(() => applySort, [applySort]) const totalQuestions = displayedQuestions.length // total length after all filters applied + const totalPages = Math.max(1, Math.ceil(totalQuestions / pageSize)) + const isLastPage = page >= totalPages // reset to page 1 whenever the filtered question count changes. useEffect(() => { @@ -344,7 +351,11 @@ export default function AsyncCentrePage( if (!userInfo) { return - } else if (asyncQuestions === undefined || asyncQuestions === null) { + } else if ( + asyncQuestionsResponse === undefined || + asyncQuestionsResponse === null || + asyncQuestions === undefined // should always be defined beyond this point, just for type safety + ) { return } else { return ( @@ -450,25 +461,76 @@ export default function AsyncCentrePage( showStudents={showStudents} /> ))} + + {asyncQuestions.length === 0 && + hiddenPrivateQuestionsCount === 0 ? ( +
+ +
+ ) : ( + asyncQuestions.length === 0 && + hiddenPrivateQuestionsCount > 0 && ( +
+ +

+ No public questions or questions you created found. + Try posting a course question! +

+ {hiddenPrivateQuestionsCount > 0 && ( + +

+ +{hiddenPrivateQuestionsCount} additional + private question + {hiddenPrivateQuestionsCount === 1 ? '' : 's'} +

+
+ )} +
+ } + /> + + ) + )} + { + // Show how many total private questions that the student can't see so that the student has + // a better way to know that this system is being used + // (and students usually ask questions to the most-popular system the prof set up) + !isStaff && + hiddenPrivateQuestionsCount > 0 && + paginatedQuestions.length > 0 && + isLastPage && ( + +

+ +{hiddenPrivateQuestionsCount} additional private + question + {hiddenPrivateQuestionsCount === 1 ? '' : 's'} +

+
+ ) + } - { - setPage(newPage) - if (newPageSize !== pageSize) { - setPageSize(newPageSize) - setPage(1) // reset to page 1 when page size changes so you don't end up on a page that doesnt exist anymore + {totalQuestions > 0 && ( + { + setPage(newPage) + if (newPageSize !== pageSize) { + setPageSize(newPageSize) + setPage(1) // reset to page 1 when page size changes so you don't end up on a page that doesnt exist anymore + } + }} + showSizeChanger + showTotal={(total, range) => + `${range[0]}-${range[1]} of ${total} questions` } - }} - showSizeChanger - showTotal={(total, range) => - `${range[0]}-${range[1]} of ${total} questions` - } - className="mb-2 mt-4 text-center" - /> + className="mb-2 mt-4 text-center" + /> + )} = ({ const messagesEndRef = useRef(null) const hasAskedQuestion = useRef(false) // to track if the user has asked a question const pathname = usePathname() + const router = useRouter() const currentPageTitle = convertPathnameToPageName(pathname) const [popResetOpen, setPopResetOpen] = useState(false) // used to temporarily store what question type the user is trying to change to @@ -558,16 +559,31 @@ const Chatbot: React.FC = ({ courseFeatures.asyncQueueEnabled && chatbotQuestionType === 'Course' && messages.length > 1 && ( -
- Unhappy with your answer?{' '} - + Want to discuss or verify this with your Professor/TA?{' '} + + trigger.parentNode as HTMLElement + } + onConfirm={() => { + router.push( + `/course/${cid}/async_centre?convertChatbotQ=true`, + ) }} > - Convert to anytime question - + { + e.preventDefault() + }} + > + Convert to Anytime Question + +
)} diff --git a/packages/frontend/app/(dashboard)/course/[cid]/queue/[qid]/components/modals/PromptStudentToLeaveQueueModal.tsx b/packages/frontend/app/(dashboard)/course/[cid]/queue/[qid]/components/modals/PromptStudentToLeaveQueueModal.tsx index 2be25efa6..155aa31de 100644 --- a/packages/frontend/app/(dashboard)/course/[cid]/queue/[qid]/components/modals/PromptStudentToLeaveQueueModal.tsx +++ b/packages/frontend/app/(dashboard)/course/[cid]/queue/[qid]/components/modals/PromptStudentToLeaveQueueModal.tsx @@ -188,7 +188,7 @@ const PromptStudentToLeaveQueueModal: React.FC< icon={} size="large" > - Convert to anytime question + Convert to Anytime Question diff --git a/packages/frontend/app/api/index.ts b/packages/frontend/app/api/index.ts index 894fd8459..ec659b458 100644 --- a/packages/frontend/app/api/index.ts +++ b/packages/frontend/app/api/index.ts @@ -3,7 +3,6 @@ import { AddChatbotQuestionParams, AddDocumentChunkParams, AllStudentAssignmentProgress, - AsyncQuestion, AsyncQuestionComment, AsyncQuestionCommentParams, AsyncQuestionParams, @@ -40,6 +39,7 @@ import { EditCourseInfoParams, ExtraTAStatus, GetAlertsResponse, + GetAsyncQuestionsResponse, GetAvailableModelsBody, GetChatbotHistoryResponse, GetCourseResponse, @@ -808,7 +808,7 @@ export class APIClient { ), } asyncQuestions = { - get: async (cid: number): Promise => + get: async (cid: number): Promise => this.req('GET', `/api/v1/asyncQuestions/${cid}`, undefined), create: async (body: CreateAsyncQuestions, cid: number) => this.req( diff --git a/packages/frontend/app/hooks/useAsyncQuestions.ts b/packages/frontend/app/hooks/useAsyncQuestions.ts index 8065eabc4..46ff72f62 100644 --- a/packages/frontend/app/hooks/useAsyncQuestions.ts +++ b/packages/frontend/app/hooks/useAsyncQuestions.ts @@ -1,15 +1,17 @@ -import { AsyncQuestion } from '@koh/common' +import { GetAsyncQuestionsResponse } from '@koh/common' import useSWR from 'swr' import { API } from '../api' export function useAsyncQuestions( cid: number, ): [ - AsyncQuestion[] | undefined, + GetAsyncQuestionsResponse | undefined, ( - data?: AsyncQuestion[] | Promise, + data?: + | GetAsyncQuestionsResponse + | Promise, shouldRevalidate?: boolean, - ) => Promise, + ) => Promise, ] { const key = `/api/v1/courses/${cid}/asyncQuestions` diff --git a/packages/server/src/asyncQuestion/asyncQuestion.controller.ts b/packages/server/src/asyncQuestion/asyncQuestion.controller.ts index 203eab57f..0335bce78 100644 --- a/packages/server/src/asyncQuestion/asyncQuestion.controller.ts +++ b/packages/server/src/asyncQuestion/asyncQuestion.controller.ts @@ -6,6 +6,7 @@ import { asyncQuestionStatus, CreateAsyncQuestions, ERROR_MESSAGES, + GetAsyncQuestionsResponse, nameToRGB, Role, UnreadAsyncQuestionResponse, @@ -743,7 +744,7 @@ export class asyncQuestionController { async getAsyncQuestions( @Param('courseId', ParseIntPipe) courseId: number, @UserId() userId: number, - ): Promise { + ): Promise { const userCourse = await UserCourseModel.findOne({ where: { userId, @@ -794,6 +795,7 @@ export class asyncQuestionController { } let questions: Partial[]; + let hiddenPrivateQuestionsCount = 0; const isStaff: boolean = userCourse.role === Role.TA || userCourse.role === Role.PROFESSOR; @@ -808,23 +810,36 @@ export class asyncQuestionController { ); } else { // Students see their own questions and questions that are visible - questions = ( - await Promise.all( - all.map(async (question) => { - if ( - question.creatorId === userId || - (await this.asyncQuestionService.isVisible( - question, - courseSettings, - )) - ) { - return question; - } else { - return undefined; - } - }), - ) - ).filter((s) => s != undefined); + const visibilityResults = await Promise.all( + all.map(async (question) => { + if (question.creatorId === userId) { + return { question, isHiddenPrivate: false }; + } + + const isVisible = await this.asyncQuestionService.isVisible( + question, + courseSettings, + ); + + if (isVisible) { + return { question, isHiddenPrivate: false }; + } + + return { + question: undefined, + isHiddenPrivate: + question.status !== asyncQuestionStatus.TADeleted && + question.status !== asyncQuestionStatus.StudentDeleted, + }; + }), + ); + + questions = visibilityResults + .map((result) => result.question) + .filter((question): question is AsyncQuestionModel => !!question); + hiddenPrivateQuestionsCount = visibilityResults.filter( + (result) => result.isHiddenPrivate, + ).length; } questions = questions.map((question: AsyncQuestionModel) => { @@ -930,7 +945,10 @@ export class asyncQuestionController { return temp; }); - return questions as unknown as AsyncQuestion[]; + return { + questions: questions as unknown as AsyncQuestion[], + hiddenPrivateQuestionsCount, + }; } // Moved from userInfo context endpoint as this updates too frequently to make sense caching it with userInfo data diff --git a/packages/server/test/asyncQuestion.integration.ts b/packages/server/test/asyncQuestion.integration.ts index 49c5ffeb8..86896c811 100644 --- a/packages/server/test/asyncQuestion.integration.ts +++ b/packages/server/test/asyncQuestion.integration.ts @@ -1000,7 +1000,8 @@ describe('AsyncQuestion Integration', () => { `/asyncQuestions/${course.id}`, ); expect(response.status).toBe(200); - const questions: AsyncQuestion[] = response.body; + const questions: AsyncQuestion[] = response.body.questions; + expect(response.body.hiddenPrivateQuestionsCount).toBe(1); expect(questions).toHaveLength(2); expect(questions).toMatchSnapshot(); expect(questions).toEqual( @@ -1036,7 +1037,7 @@ describe('AsyncQuestion Integration', () => { `/asyncQuestions/${course.id}`, ); expect(response.status).toBe(200); - const questions: AsyncQuestion[] = response.body; + const questions: AsyncQuestion[] = response.body.questions; expect(questions).toHaveLength(3); expect(questions).toEqual( expect.arrayContaining([ @@ -1065,7 +1066,7 @@ describe('AsyncQuestion Integration', () => { `/asyncQuestions/${course.id}`, ); expect(response.status).toBe(200); - const questions: AsyncQuestion[] = response.body; + const questions: AsyncQuestion[] = response.body.questions; expect(questions).toHaveLength(2); expect(questions).toEqual( expect.arrayContaining([ @@ -1094,7 +1095,7 @@ describe('AsyncQuestion Integration', () => { `/asyncQuestions/${course.id}`, ); expect(response.status).toBe(200); - const questions: AsyncQuestion[] = response.body; + const questions: AsyncQuestion[] = response.body.questions; expect(questions).toHaveLength(2); expect(questions).toEqual( expect.arrayContaining([ @@ -1113,7 +1114,7 @@ describe('AsyncQuestion Integration', () => { `/asyncQuestions/${course.id}`, ); expect(response.status).toBe(200); - const questions: AsyncQuestion[] = response.body; + const questions: AsyncQuestion[] = response.body.questions; expect(questions).toEqual( expect.arrayContaining([ expect.objectContaining({ @@ -1160,7 +1161,8 @@ describe('AsyncQuestion Integration', () => { `/asyncQuestions/${course.id}`, ); expect(response.status).toBe(200); - const questions: AsyncQuestion[] = response.body; + const questions: AsyncQuestion[] = response.body.questions; + expect(response.body.hiddenPrivateQuestionsCount).toBe(0); expect(questions).toHaveLength(3); expect(questions).toEqual( expect.arrayContaining([ @@ -1191,6 +1193,63 @@ describe('AsyncQuestion Integration', () => { ]), ); }); + it('returns no visible questions but hidden count > 0 when all questions are private to another student', async () => { + const studentUser4 = await UserFactory.create(); + await UserCourseFactory.create({ + user: studentUser4, + course, + role: Role.STUDENT, + }); + + asyncQuestion2.staffSetVisible = false; + await asyncQuestion2.save(); + + const response = await supertest({ userId: studentUser4.id }).get( + `/asyncQuestions/${course.id}`, + ); + + expect(response.status).toBe(200); + expect(response.body.questions).toEqual([]); + expect(response.body.hiddenPrivateQuestionsCount).toBe(3); + }); + it('does not include TADeleted questions in hidden private count', async () => { + const studentUser4 = await UserFactory.create(); + await UserCourseFactory.create({ + user: studentUser4, + course, + role: Role.STUDENT, + }); + + asyncQuestion2.staffSetVisible = false; + await asyncQuestion2.save(); + + asyncQuestion3.status = asyncQuestionStatus.TADeleted; + await asyncQuestion3.save(); + + const response = await supertest({ userId: studentUser4.id }).get( + `/asyncQuestions/${course.id}`, + ); + + expect(response.status).toBe(200); + expect(response.body.questions).toEqual([]); + expect(response.body.hiddenPrivateQuestionsCount).toBe(2); + }); + it("does not include the requester's own private questions in hidden private count", async () => { + asyncQuestion2.staffSetVisible = false; + await asyncQuestion2.save(); + + const response = await supertest({ userId: studentUser2.id }).get( + `/asyncQuestions/${course.id}`, + ); + + expect(response.status).toBe(200); + expect(response.body.hiddenPrivateQuestionsCount).toBe(2); + expect(response.body.questions).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: asyncQuestion3.id }), + ]), + ); + }); it('does not show sensitive information for anonymous comments on questions unless they are the creator or staff', async () => { const comment1 = await AsyncQuestionCommentFactory.create({ question: asyncQuestion2, @@ -1206,7 +1265,7 @@ describe('AsyncQuestion Integration', () => { `/asyncQuestions/${course.id}`, ); expect(response.status).toBe(200); - const questions: AsyncQuestion[] = response.body; + const questions: AsyncQuestion[] = response.body.questions; expect(questions).toHaveLength(2); expect(questions).toEqual( expect.arrayContaining([ @@ -1261,7 +1320,7 @@ describe('AsyncQuestion Integration', () => { `/asyncQuestions/${course.id}`, ); expect(response2.status).toBe(200); - const questions2: AsyncQuestion[] = response2.body; + const questions2: AsyncQuestion[] = response2.body.questions; expect(questions2).toHaveLength(3); expect(questions2).toEqual( expect.arrayContaining([ @@ -1314,7 +1373,7 @@ describe('AsyncQuestion Integration', () => { `/asyncQuestions/${course.id}`, ); expect(response.status).toBe(200); - const questions: AsyncQuestion[] = response.body; + const questions: AsyncQuestion[] = response.body.questions; expect(questions).toEqual( expect.arrayContaining([ expect.objectContaining({ @@ -1383,8 +1442,8 @@ describe('AsyncQuestion Integration', () => { ); expect(response.status).toBe(200); expect(response2.status).toBe(200); - const questions: AsyncQuestion[] = response.body; - const questions2: AsyncQuestion[] = response2.body; + const questions: AsyncQuestion[] = response.body.questions; + const questions2: AsyncQuestion[] = response2.body.questions; expect(questions).toHaveLength(3); expect(questions).toEqual( expect.arrayContaining([ @@ -1430,9 +1489,9 @@ describe('AsyncQuestion Integration', () => { `/asyncQuestions/${course.id}`, ); expect(response.status).toBe(200); - const questions: AsyncQuestion[] = response.body; - const questions2: AsyncQuestion[] = response2.body; - const questions3: AsyncQuestion[] = response3.body; + const questions: AsyncQuestion[] = response.body.questions; + const questions2: AsyncQuestion[] = response2.body.questions; + const questions3: AsyncQuestion[] = response3.body.questions; const fullInfo = { id: asyncQuestion9.id, isAnonymous: true, @@ -1479,7 +1538,7 @@ describe('AsyncQuestion Integration', () => { `/asyncQuestions/${course.id}`, ); expect(response.status).toBe(200); - const questions: AsyncQuestion[] = response.body; + const questions: AsyncQuestion[] = response.body.questions; expect(questions).toEqual( expect.arrayContaining([ expect.objectContaining({