diff --git a/docs/DEVELOPING.md b/docs/DEVELOPING.md index a52ec157e..b18b4647c 100644 --- a/docs/DEVELOPING.md +++ b/docs/DEVELOPING.md @@ -126,10 +126,10 @@ Unit test files should be colocated with the file they test. Integration tests are located in the `test` folder. -`yarn test` at root level runs all tests, but you can also selectively run tests by running `yarn test` while inside a package. - ~~End to end (E2E) testing is in it's own folder and done with Cypress. These should be used to test core user flows. To run them headlessly (without a graphics server), do `yarn cypress run`. To watch them actually run interactively, you can use `yarn cypress open`. Be aware that this is _super slow_ on local machines.~~ (note: cypress has been removed, for now) +To run a specific unit test suite, you can run `yarn test:unit suite-name` e.g. `yarn test:unit course` + If your tests are failing with a message about "deadlock something whatever", do `yarn test --run-in-band`. This makes the tests run sequentially. If `yarn test` is not running all of the tests, navigate to `server/test` folder and run `yarn jest --config ./test/jest-integration.json -i --run-in-band` if you would like to run all the integration tests. To run the tests of a specific integration test file (e.g. course.integration.ts), you can use `yarn jest --config ./test/jest-integration.json -i --run-in-band course` diff --git a/packages/common/index.ts b/packages/common/index.ts index 6a93e2c44..a228238c8 100644 --- a/packages/common/index.ts +++ b/packages/common/index.ts @@ -351,6 +351,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', + ADMIN_NOTICE = 'admin_notice', // currently used for all prof invite admin emails. Just wanted something generic for it. } /** * Represents one of three possible user roles in a course. @@ -1078,6 +1079,56 @@ export class QueuePartial { courseId!: number } +export class GetProfInviteResponse { + course!: { + id: number + name: string + } + adminUser!: { + id: number + name: string + email: string + } + id!: number + code!: string + maxUses!: number + usesUsed!: number + @Type(() => Date) + createdAt!: Date + @Type(() => Date) + expiresAt!: Date + makeOrgProf!: boolean +} +export class CreateProfInviteParams { + @IsInt() + orgId!: number + @IsInt() + courseId!: number + + @IsOptional() + @IsInt() + maxUses?: number + + @IsOptional() + @IsDate() + @Type(() => Date) + expiresAt?: Date + + @IsOptional() + @IsBoolean() + makeOrgProf?: boolean +} + +export class AcceptProfInviteParams { + @IsString() + code!: string +} + +export type GetProfInviteDetailsResponse = { + courseId: number + orgId: number +} + /** * Used when editing QueueInvites */ @@ -2188,11 +2239,17 @@ export type OrganizationProfessor = { organizationUser: { id: number name: string + email: string } trueRole?: OrganizationRole userId: number } +export type CreateCourseResponse = { + message: string + courseId: number +} + export class UpdateOrganizationCourseDetailsParams { @IsString() @IsOptional() @@ -2480,10 +2537,10 @@ export class OrganizationCourseResponse { id?: number @IsInt() - organizationId?: number + organizationId!: number @IsInt() - courseId?: number + courseId!: number course?: GetCourseResponse @@ -2977,6 +3034,7 @@ export enum OrgRoleChangeReason { manualModification = 'manualModification', joinedOrganizationMember = 'joinedOrganizationMember', joinedOrganizationProfessor = 'joinedOrganizationProfessor', + acceptedProfInvite = 'acceptedProfInvite', unknown = 'unknown', } @@ -2984,6 +3042,7 @@ export enum OrgRoleChangeReasonMap { manualModification = 'Role was manually modified by an organization member with sufficient permissions.', joinedOrganizationMember = 'User joined the organization and gained the member role.', joinedOrganizationProfessor = 'User joined the organization and gained the professor role.', + acceptedProfInvite = 'User accepted a professor invite with makeOrgProf flag set to true given by the given admin user.', unknown = '', } @@ -4207,6 +4266,8 @@ export const ERROR_MESSAGES = { notLoggedIn: 'Must be logged in', noCourseIdFound: 'No courseId found', notInCourse: 'Not In This Course', + noOrgId: 'Organization ID not given', + invalidOrgId: 'Invalid Organization ID: Not a valid number', notAuthorized: "You don't have permissions to perform this action", userNotInOrganization: 'User not in organization', mustBeRoleToAccess: (roles: string[]): string => @@ -4340,3 +4401,41 @@ export const ERROR_MESSAGES = { `Members with role ${role} are not allowed to delete semesters`, }, } + +/* Common Query Params + Does two things: + - Allows us to easily modify the query params for error messages in 1 spot + - More importantly, it connects the backend with the frontend to make it easier to find where a particular query param is coming from + */ +export const QUERY_PARAMS = { + profInvite: { + // note that some uses of these query params will just check for .startsWith (e.g. .startsWith('prof_invite_')) + error: { + expired: 'prof_invite_expired', + expiresAt: 'expired_at', + maxUsesReached: 'prof_invite_max_uses_reached', + maxUses: 'max_uses', + notFound: 'prof_invite_not_found', + profInviteId: 'pinvite_id', + userNotFound: 'prof_invite_user_not_found', + badCode: 'prof_invite_bad_code', + unknown: 'prof_invite_unknown_error', + // It's tempting to want to re-organize this better, but it can make the urls more gross to read (e.g. /courses?error=${QUERY_PARAMS.profInviteError.notFound.queryParam}&${QUERY_PARAMS.profInviteError.notFound.extraParams.profInviteId}=${profInviteId}) + // I also considered putting the full error messages here, but they're only used in one place and I think would do more harm than good for maintainability + }, + notice: { + adminAlreadyInCourse: 'pi_admin_already_in_course', + adminAcceptedInviteNotConsumed: 'pi_admin_accepted_invite_not_consumed', + inviteAccepted: 'pi_invite_accepted', + }, + }, + queueInvite: { + error: { + notInCourse: 'queue_invite_not_in_course', + inviteNotFound: 'queue_invite_not_found', + courseNotFound: 'queue_invite_course_not_found', + badCourseInviteCode: 'queue_invite_bad_course_invite_code', + }, + }, + // TODO: add the /login redirect query params here. Avoided doing so right now since that would require middleware.ts to import this file and iirc there is errors when you try to do that +} diff --git a/packages/frontend/app/(auth)/login/layout.tsx b/packages/frontend/app/(auth)/login/layout.tsx index 094c76887..ae2c51f3a 100644 --- a/packages/frontend/app/(auth)/login/layout.tsx +++ b/packages/frontend/app/(auth)/login/layout.tsx @@ -13,23 +13,37 @@ export default async function Layout({ const cookieStore = await cookies() const queueInviteCookieString = cookieStore.get('queueInviteInfo') - if (queueInviteCookieString) { + const profInviteCookieString = cookieStore.get('profInviteInfo') + if (profInviteCookieString) { + const decodedCookie = decodeURIComponent(profInviteCookieString.value) + const cookieParts = decodedCookie.split(',') + // const profInviteId = cookieParts[0] // not used, but left here to showcase they are available + const orgId = Number(cookieParts[1]) + const courseId = Number(cookieParts[2]) + //const profInviteCode = cookieParts[3] + if (orgId) { + invitedOrgId = orgId + } + if (courseId) { + invitedCourseId = courseId + } + } else if (queueInviteCookieString) { const decodedCookie = decodeURIComponent(queueInviteCookieString.value) const cookieParts = decodedCookie.split(',') - const courseId = cookieParts[0] - const queueId = cookieParts[1] - const orgId = cookieParts[2] + const courseId = Number(cookieParts[0]) + const queueId = Number(cookieParts[1]) + const orgId = Number(cookieParts[2]) const courseInviteCode = cookieParts[3] ? Buffer.from(cookieParts[3], 'base64').toString('utf-8') : null - if (Number(orgId)) { - invitedOrgId = Number(orgId) + if (orgId) { + invitedOrgId = orgId } - if (Number(queueId)) { - invitedQueueId = Number(queueId) + if (queueId) { + invitedQueueId = queueId } - if (Number(courseId)) { - invitedCourseId = Number(courseId) + if (courseId) { + invitedCourseId = courseId } if (courseInviteCode) { // note: this will only get set if the queueInvite had willInviteToCourse set to true @@ -42,14 +56,14 @@ export default async function Layout({ const decodedCookie = decodeURIComponent( redirectCookieString.value, ).split(',') - const orgId = decodedCookie[2] - const courseId = decodedCookie[0] + const orgId = Number(decodedCookie[2]) + const courseId = Number(decodedCookie[0]) const courseInviteCode = decodedCookie[1] - if (Number(orgId)) { - invitedOrgId = Number(orgId) + if (orgId) { + invitedOrgId = orgId } - if (Number(courseId)) { - invitedCourseId = Number(courseId) + if (courseId) { + invitedCourseId = courseId } if (courseInviteCode) { invitedCourseInviteCode = courseInviteCode diff --git a/packages/frontend/app/(dashboard)/components/CourseCloneForm.tsx b/packages/frontend/app/(dashboard)/components/CourseCloneForm.tsx index ae98bbe70..66e725c1d 100644 --- a/packages/frontend/app/(dashboard)/components/CourseCloneForm.tsx +++ b/packages/frontend/app/(dashboard)/components/CourseCloneForm.tsx @@ -1,23 +1,11 @@ 'use client' import { useEffect, useState } from 'react' -import { - Checkbox, - Form, - FormInstance, - Input, - message, - Select, - Tag, - Tooltip, -} from 'antd' -import { - GetOrganizationResponse, - OrganizationProfessor, - OrganizationRole, -} from '@koh/common' +import { Checkbox, Form, FormInstance, Input, message, Select } from 'antd' +import { GetOrganizationResponse, OrganizationProfessor } from '@koh/common' import { API } from '@/app/api' import { formatSemesterDate } from '@/app/utils/timeFormatUtils' +import ProfessorSelector from './ProfessorSelector' type CourseCloneFormProps = { form: FormInstance @@ -71,59 +59,7 @@ const CourseCloneForm: React.FC = ({ className="flex-1" required > - - - {isCourseNameTooLong && ( - - )} - - - + +
{ + if (changedValues.courseName) { + if (changedValues.courseName.length > 14) { + setIsCourseNameTooLong(true) + } else { + setIsCourseNameTooLong(false) + } + } + }} + onFinish={() => updateGeneral()} > - - - +
+
+ + + + {isCourseNameTooLong && ( + + )} +
-
- - - + + + +
- - - -
+
+ + + -
- - - + + + +
- - - -
+
+ + + -
- {user.organization?.organizationRole === OrganizationRole.ADMIN && - professors ? ( - - + {organization.semesters.map((semester) => ( + + {`${semester.name}`}{' '} + + {formatSemesterDate(semester)} + + {semester.endDate && + new Date(semester.endDate) < new Date() && + !( + new Date(semester.endDate) < new Date('1971-01-01') + ) && ( + + (ended) + + )} + + ))} + + No semester + + + +
- return ( - - - {label} - - - ) - }} - /> - - ) : ( - <> - )} -
+
+ {user.organization?.organizationRole === OrganizationRole.ADMIN && + professors ? ( + + + + ) : ( + <> + )} +
- - - -
+ + + + + + ) } diff --git a/packages/frontend/app/(dashboard)/components/ProfInvites.tsx b/packages/frontend/app/(dashboard)/components/ProfInvites.tsx new file mode 100644 index 000000000..eef45dea6 --- /dev/null +++ b/packages/frontend/app/(dashboard)/components/ProfInvites.tsx @@ -0,0 +1,397 @@ +'use client' + +import { API } from '@/app/api' +import { useUserInfo } from '@/app/contexts/userContext' +import { cn, getErrorMessage } from '@/app/utils/generalUtils' +import { + CopyOutlined, + DeleteOutlined, + PlusOutlined, + QuestionCircleOutlined, +} from '@ant-design/icons' +import { + GetProfInviteResponse, + OrganizationCourseResponse, + OrganizationRole, +} from '@koh/common' +import { + Alert, + Button, + Card, + Checkbox, + DatePicker, + Form, + InputNumber, + List, + message, + Popconfirm, + Tooltip, +} from 'antd' +import { useEffect, useState } from 'react' +import dayjs from 'dayjs' +import relativeTime from 'dayjs/plugin/relativeTime' +import { useSearchParams } from 'next/navigation' +import Link from 'next/link' + +dayjs.extend(relativeTime) + +const isInviteUsable = (profInvite: GetProfInviteResponse) => { + return ( + profInvite.expiresAt > new Date() && + profInvite.usesUsed < profInvite.maxUses + ) +} + +interface FormValues { + maxUses: number + expiresAt: dayjs.Dayjs + makeOrgProf: boolean +} + +type ProfInvitesProps = { + courseData: OrganizationCourseResponse +} + +const ProfInvites: React.FC = ({ courseData }) => { + const [form] = Form.useForm() + const { userInfo } = useUserInfo() + const [profInvites, setProfInvites] = useState([]) + const [showUnusableProfInvites, setShowUnusableProfInvites] = useState(false) + const [createProfInviteLoading, setCreateProfInviteLoading] = useState(false) + const searchParams = useSearchParams() + const showCreateProfNotice = + searchParams.get('show-create-prof-notice') === 'true' + + const fetchProfInvites = async () => { + if (userInfo.organization?.organizationRole !== OrganizationRole.ADMIN) { + return // shouldn't be necessary since the component is only rendered if the user is an admin but just in case + } + if (courseData.organizationId && courseData.courseId) { + await API.profInvites + .getAll(courseData.organizationId, courseData.courseId) + .then((profInvites) => { + setProfInvites(profInvites) + }) + } else { + message.error( + 'Error fetching ProfInvites: organizationId or courseId not set', + ) + } + } + + const usableProfInvites = profInvites.filter((profInvite) => + isInviteUsable(profInvite), + ) + const unusableProfInvites = profInvites.filter( + (profInvite) => !isInviteUsable(profInvite), + ) + + useEffect(() => { + fetchProfInvites() + }, []) + + const handleCreateProfInvite = async (values: FormValues) => { + setCreateProfInviteLoading(true) + await API.profInvites + .create(courseData.organizationId, { + orgId: courseData.organizationId, + courseId: courseData.courseId, + maxUses: values.maxUses, + expiresAt: values.expiresAt.toDate(), + makeOrgProf: values.makeOrgProf, + }) + .then(async () => { + await fetchProfInvites() + }) + .catch((error) => { + message.error('Error creating ProfInvite: ' + getErrorMessage(error)) + }) + setCreateProfInviteLoading(false) + } + + useEffect(() => { + // Putting the link as organization/course/${courseId}/edit?show-create-prof-notice=true#create-prof-invite-notice + // didn't seem to work since the hash/anchor would seem to resolve before the component is rendered and not scroll + if (showCreateProfNotice) { + const el = document.getElementById('create-prof-invite-notice') + if (el) { + el.scrollIntoView({ behavior: 'smooth', block: 'start' }) + } + } + }, [showCreateProfNotice]) + + return ( + <> + {showCreateProfNotice && ( + + Or, go back to My Courses + + } + type="success" + showIcon + className="scroll-mt-4" + /> + )} + +

Professor Invites (Admin Only)

+
+ + Help + +
+ + } + > +
+
+ + + + + + + + + + + + +
+
+ {usableProfInvites.length > 0 && ( + ( + + )} + /> + )} + {unusableProfInvites.length > 0 && + (showUnusableProfInvites ? ( + ( + + )} + /> + ) : ( +
+ {unusableProfInvites.length} expired/used invites ( + + ) +
+ ))} + + + ) +} + +const ProfInviteItem: React.FC<{ + profInvite: GetProfInviteResponse + orgId: number + fetchProfInvites: () => void +}> = ({ profInvite, orgId, fetchProfInvites }) => { + const [copyLinkText, setCopyLinkText] = useState('Copy Link') + const [isDeleteLoading, setIsDeleteLoading] = useState(false) + + // used to make deletion of an invite instant only if it's < 1min since creation or < 10s after copying a link. + // Assumption is that the cases where you're usually going to be certain you will want to delete it is either: + // A: right after creating the invite (mistake, etc.) + // B: right before you go to share it (you realize something is wrong, etc.) + const [lastCopiedAt, setLastCopiedAt] = useState(null) + const [isDeletePopoverOpen, setIsDeletePopoverOpen] = useState(false) + const handleDeletePopoverOpenChange = (newOpen: boolean) => { + if (!newOpen) { + // always allow closing + setIsDeletePopoverOpen(newOpen) + return + } + + // Instant delete if: created within 1 min OR copied within last 10s + const now = Date.now() + if ( + now - profInvite.createdAt.getTime() < 60 * 1000 || + (lastCopiedAt && now - lastCopiedAt.getTime() < 10 * 1000) + ) { + handleDelete().then(() => setIsDeletePopoverOpen(false)) + } else { + setIsDeletePopoverOpen(newOpen) + } + } + + const isHttps = window.location.protocol === 'https:' + const baseURL = `${isHttps ? 'https' : 'http'}://${window.location.host}` + const inviteURL = `${baseURL}/invite/prof/${profInvite.id}?&c=${profInvite.code}` + + const handleCopy = () => { + navigator.clipboard.writeText(inviteURL).then(() => { + setCopyLinkText('Copied!') + setLastCopiedAt(new Date()) + setTimeout(() => { + setCopyLinkText('Copy Link') + }, 1000) + }) + } + + const handleDelete = async () => { + setIsDeleteLoading(true) + await API.profInvites + .delete(orgId, profInvite.id) + .then(() => { + message.success('Professor invite deleted') + }) + .catch((error) => { + message.error('Error deleting ProfInvite: ' + getErrorMessage(error)) + }) + .finally(() => { + setIsDeleteLoading(false) + fetchProfInvites() + }) + } + + const expiresAt = dayjs(profInvite.expiresAt) + const isInviteUnusable = !isInviteUsable(profInvite) + + return ( + +
+
+
+ {/* empty div for centering invite link */} +
+
+
{isInviteUnusable ? {inviteURL} : inviteURL}
+ {!isInviteUnusable && ( + + )} +
+ +
+
+
+
+ = profInvite.maxUses ? 'text-red-700' : '' + } + > + {profInvite.usesUsed} + {' '} + / {profInvite.maxUses} uses used +
+ +
+ Expire{expiresAt.isAfter(dayjs()) ? 's' : 'd'}{' '} + + {expiresAt.fromNow()} + +
+ + +
Admin: {profInvite.adminUser.name}
+
Email: {profInvite.adminUser.email}
+
UserID: {profInvite.adminUser.id}
+
+ Created At:{' '} + {dayjs(profInvite.createdAt).format('YYYY-MM-DD hh:mm A')} +
+
This admin will be notified when the invite is used.
+
+ } + > +
+
+ Created By: {profInvite.adminUser.name}{' '} + {dayjs(profInvite.createdAt).fromNow()} +
+
+ +
+ Make Org Prof:{' '} + {profInvite.makeOrgProf ? ( + Yes + ) : ( + No + )} +
+
+ + + ) +} + +export default ProfInvites diff --git a/packages/frontend/app/(dashboard)/components/ProfessorSelector.tsx b/packages/frontend/app/(dashboard)/components/ProfessorSelector.tsx new file mode 100644 index 000000000..8a6fb24ee --- /dev/null +++ b/packages/frontend/app/(dashboard)/components/ProfessorSelector.tsx @@ -0,0 +1,100 @@ +'use client' + +import { OrganizationProfessor, OrganizationRole } from '@koh/common' +import { Select, Tag, Tooltip } from 'antd' +import { CrownFilled } from '@ant-design/icons' + +import { SelectProps } from 'antd' + +type ProfessorSelectorProps = { + professors: OrganizationProfessor[] +} & SelectProps + +const ProfessorSelector: React.FC = ({ + professors, + ...selectProps +}) => { + return ( + - (optionA?.label ?? '') - .toLowerCase() - .localeCompare( - (optionB?.label ?? '').toLowerCase(), - ) - } - showSearch - optionFilterProp="label" - options={professors.map((prof) => ({ - key: prof.organizationUser.id, - label: prof.organizationUser.name, - value: prof.organizationUser.id, - }))} - /> + )} diff --git a/packages/frontend/app/(dashboard)/organization/role_history/components/OrganizationRoleHistoryList.tsx b/packages/frontend/app/(dashboard)/organization/role_history/components/OrganizationRoleHistoryList.tsx index 41cd2194a..cef0d5d22 100644 --- a/packages/frontend/app/(dashboard)/organization/role_history/components/OrganizationRoleHistoryList.tsx +++ b/packages/frontend/app/(dashboard)/organization/role_history/components/OrganizationRoleHistoryList.tsx @@ -2,6 +2,7 @@ import { Badge, Col, Input, List, Pagination, Row, Select, Tooltip } from 'antd' import { DateRangeType, OrganizationRole, + OrgRoleChangeReason, OrgRoleHistory, OrgUser, } from '@koh/common' @@ -349,8 +350,14 @@ const RoleChangeItem: React.FC<{ - +

changed role of

+ {item.changeReason === + OrgRoleChangeReason.acceptedProfInvite && ( +

+ (by prof invite) +

+ )} @@ -405,8 +412,14 @@ const RoleChangeItem: React.FC<{ - + + {item.changeReason === + OrgRoleChangeReason.acceptedProfInvite && ( +

+ (prof invite) +

+ )} diff --git a/packages/frontend/app/(dashboard)/organization/settings/components/AllProfInvites.tsx b/packages/frontend/app/(dashboard)/organization/settings/components/AllProfInvites.tsx new file mode 100644 index 000000000..cee7f6a4a --- /dev/null +++ b/packages/frontend/app/(dashboard)/organization/settings/components/AllProfInvites.tsx @@ -0,0 +1,218 @@ +import { + Button, + message, + Table, + TableColumnsType, + Tooltip, + Popconfirm, + Alert, + Card, +} from 'antd' +import { GetProfInviteResponse } from '@koh/common' +import { API } from '@/app/api' +import { getErrorMessage } from '@/app/utils/generalUtils' +import useSWRImmutable from 'swr/immutable' +import dayjs from 'dayjs' +import relativeTime from 'dayjs/plugin/relativeTime' +import { DeleteOutlined } from '@ant-design/icons' +import { useState } from 'react' +import { KeyedMutator } from 'swr' + +dayjs.extend(relativeTime) + +interface AllProfInvitesProps { + orgId: number +} + +export const AllProfInvites: React.FC = ({ orgId }) => { + // trying out useSWRImmutable as an experiment. + // It's basically the same as having 3 useState variables (data, error, isLoading) condensed into one + // Difference between "useSWR" and "useSWRImmutable" is that this disables all the extra API calls and validation n whatnot that comes loaded with swr + const { + data: profInvites, + error, + isLoading, + mutate: mutateProfInvites, + } = useSWRImmutable( + `organization/${orgId}/profInvites`, + async () => await API.profInvites.getAll(orgId), + ) + + const columns: TableColumnsType = [ + { + title: 'piid', + dataIndex: 'id', + key: 'id', + width: 20, + }, + { + title: 'Course', + children: [ + { + title: 'Name', + dataIndex: ['course', 'name'], + key: 'courseName', + sorter: (a, b) => a.course.name.localeCompare(b.course.name), + }, + { + title: 'id', + dataIndex: ['course', 'id'], + key: 'courseId', + minWidth: 20, + sorter: (a, b) => a.course.id - b.course.id, + }, + ], + }, + { + title: 'Creator', + children: [ + { + title: 'Name', + dataIndex: ['adminUser', 'name'], + key: 'adminUserName', + sorter: (a, b) => a.adminUser.name.localeCompare(b.adminUser.name), + render: (text: string, record) => ( + {text} + ), + }, + { + title: 'id', + dataIndex: ['adminUser', 'id'], + key: 'adminUserId', + minWidth: 20, + }, + ], + }, + { + title: 'Uses / MaxUses', + dataIndex: 'usesUsed', + key: 'usesUsed', + // sort by the number of uses remaining + sorter: (a, b) => a.maxUses - a.usesUsed - (b.maxUses - b.usesUsed), + render: (text: string, record) => ( + + {text} / {record.maxUses} + + ), + }, + { + title: 'Expires At', + dataIndex: 'expiresAt', + key: 'expiresAt', + sorter: (a, b) => a.expiresAt.getTime() - b.expiresAt.getTime(), + render: (_, record) => { + const expiresAt = dayjs(record.expiresAt) + return ( + + + {expiresAt.fromNow()} + + + ) + }, + }, + { + title: 'Created At', + dataIndex: 'createdAt', + key: 'createdAt', + sorter: (a, b) => a.createdAt.getTime() - b.createdAt.getTime(), + render: (_, record) => { + const createdAt = dayjs(record.createdAt) + return ( + + {createdAt.fromNow()} + + ) + }, + defaultSortOrder: 'descend', + }, + { + title: 'MakeOrgProf', + dataIndex: 'makeOrgProf', + key: 'makeOrgProf', + minWidth: 20, + render: (_, record) => <>{record.makeOrgProf ? '✓' : 'X'}, + }, + { + title: '', + key: 'action', + render: (_, record) => ( + + ), + }, + ] + if (error) { + return ( + + ) + } else { + return ( + + record.id} + tableLayout="auto" + scroll={{ x: 'max-content' }} + bordered + size="small" + /> + + ) + } +} + +const DeleteProfInviteButton: React.FC<{ + orgId: number + record: GetProfInviteResponse + mutateProfInvites: KeyedMutator +}> = ({ orgId, record, mutateProfInvites }) => { + const [isDeleteLoading, setIsDeleteLoading] = useState(false) + + return ( + { + setIsDeleteLoading(true) + API.profInvites + .delete(orgId, record.id) + .then(() => { + // remove the prof invite from the local data source + mutateProfInvites((data) => + (data ?? []).filter( + (existingInvite) => existingInvite.id !== record.id, + ), + ) + }) + .catch((err) => { + message.error(getErrorMessage(err)) + }) + .finally(() => setIsDeleteLoading(false)) + }} + okText="Yes" + okButtonProps={{ loading: isDeleteLoading }} + cancelText="Cancel" + > + + + } + /> + ) + } else { + return + } +} diff --git a/packages/server/migration/1767133750779-prof-invites.ts b/packages/server/migration/1767133750779-prof-invites.ts new file mode 100644 index 000000000..eebc3e239 --- /dev/null +++ b/packages/server/migration/1767133750779-prof-invites.ts @@ -0,0 +1,117 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +export class ProfInvites1767133750779 implements MigrationInterface { + name = 'ProfInvites1767133750779'; + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `CREATE TABLE "prof_invite_model" ("id" SERIAL NOT NULL, "orgId" integer NOT NULL, "courseId" integer NOT NULL, "maxUses" integer NOT NULL DEFAULT '1', "usesUsed" integer NOT NULL DEFAULT '0', "createdAt" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), "expiresAt" TIMESTAMP WITH TIME ZONE NOT NULL, "code" text NOT NULL, "makeOrgProf" boolean NOT NULL DEFAULT true, "adminUserId" integer NOT NULL, CONSTRAINT "PK_1ad5cda8130f1593ecd1fb3c7b1" PRIMARY KEY ("id"))`, + ); + await queryRunner.query( + `ALTER TYPE "public"."organization_role_history_model_rolechangereason_enum" RENAME TO "organization_role_history_model_rolechangereason_enum_old"`, + ); + await queryRunner.query( + `CREATE TYPE "public"."organization_role_history_model_rolechangereason_enum" AS ENUM('manualModification', 'joinedOrganizationMember', 'joinedOrganizationProfessor', 'acceptedProfInvite', 'unknown')`, + ); + await queryRunner.query( + `ALTER TABLE "organization_role_history_model" ALTER COLUMN "roleChangeReason" DROP DEFAULT`, + ); + await queryRunner.query( + `ALTER TABLE "organization_role_history_model" ALTER COLUMN "roleChangeReason" TYPE "public"."organization_role_history_model_rolechangereason_enum" USING "roleChangeReason"::"text"::"public"."organization_role_history_model_rolechangereason_enum"`, + ); + await queryRunner.query( + `ALTER TABLE "organization_role_history_model" ALTER COLUMN "roleChangeReason" SET DEFAULT 'unknown'`, + ); + await queryRunner.query( + `DROP TYPE "public"."organization_role_history_model_rolechangereason_enum_old"`, + ); + await queryRunner.query( + `ALTER TYPE "public"."mail_services_servicetype_enum" RENAME TO "mail_services_servicetype_enum_old"`, + ); + await queryRunner.query( + `CREATE TYPE "public"."mail_services_servicetype_enum" AS ENUM('async_question_human_answered', 'async_question_flagged', 'async_question_status_changed', 'async_question_upvoted', 'async_question_new_comment_on_my_post', 'async_question_new_comment_on_others_post', 'course_clone_summary', 'admin_notice')`, + ); + await queryRunner.query( + `ALTER TABLE "mail_services" ALTER COLUMN "serviceType" TYPE "public"."mail_services_servicetype_enum" USING "serviceType"::"text"::"public"."mail_services_servicetype_enum"`, + ); + await queryRunner.query( + `DROP TYPE "public"."mail_services_servicetype_enum_old"`, + ); + await queryRunner.query( + `ALTER TYPE "public"."sent_email_model_servicetype_enum" RENAME TO "sent_email_model_servicetype_enum_old"`, + ); + await queryRunner.query( + `CREATE TYPE "public"."sent_email_model_servicetype_enum" AS ENUM('async_question_human_answered', 'async_question_flagged', 'async_question_status_changed', 'async_question_upvoted', 'async_question_new_comment_on_my_post', 'async_question_new_comment_on_others_post', 'course_clone_summary', 'admin_notice')`, + ); + await queryRunner.query( + `ALTER TABLE "sent_email_model" ALTER COLUMN "serviceType" TYPE "public"."sent_email_model_servicetype_enum" USING "serviceType"::"text"::"public"."sent_email_model_servicetype_enum"`, + ); + await queryRunner.query( + `DROP TYPE "public"."sent_email_model_servicetype_enum_old"`, + ); + await queryRunner.query( + `ALTER TABLE "prof_invite_model" ADD CONSTRAINT "FK_067e61aaf7255af45013bcac7e1" FOREIGN KEY ("orgId") REFERENCES "organization_model"("id") ON DELETE CASCADE ON UPDATE NO ACTION`, + ); + await queryRunner.query( + `ALTER TABLE "prof_invite_model" ADD CONSTRAINT "FK_659d335e6d9fbbbb82ffcea77d8" FOREIGN KEY ("courseId") REFERENCES "course_model"("id") ON DELETE CASCADE ON UPDATE NO ACTION`, + ); + await queryRunner.query( + `ALTER TABLE "prof_invite_model" ADD CONSTRAINT "FK_ef5d23b5348776eece467ac9fca" FOREIGN KEY ("adminUserId") REFERENCES "user_model"("id") ON DELETE CASCADE ON UPDATE NO ACTION`, + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `ALTER TABLE "prof_invite_model" DROP CONSTRAINT "FK_ef5d23b5348776eece467ac9fca"`, + ); + await queryRunner.query( + `ALTER TABLE "prof_invite_model" DROP CONSTRAINT "FK_659d335e6d9fbbbb82ffcea77d8"`, + ); + await queryRunner.query( + `ALTER TABLE "prof_invite_model" DROP CONSTRAINT "FK_067e61aaf7255af45013bcac7e1"`, + ); + await queryRunner.query( + `CREATE TYPE "public"."sent_email_model_servicetype_enum_old" AS ENUM('async_question_flagged', 'async_question_human_answered', 'async_question_new_comment_on_my_post', 'async_question_new_comment_on_others_post', 'async_question_status_changed', 'async_question_upvoted', 'course_clone_summary')`, + ); + await queryRunner.query( + `ALTER TABLE "sent_email_model" ALTER COLUMN "serviceType" TYPE "public"."sent_email_model_servicetype_enum_old" USING "serviceType"::"text"::"public"."sent_email_model_servicetype_enum_old"`, + ); + await queryRunner.query( + `DROP TYPE "public"."sent_email_model_servicetype_enum"`, + ); + await queryRunner.query( + `ALTER TYPE "public"."sent_email_model_servicetype_enum_old" RENAME TO "sent_email_model_servicetype_enum"`, + ); + await queryRunner.query( + `CREATE TYPE "public"."mail_services_servicetype_enum_old" AS ENUM('async_question_flagged', 'async_question_human_answered', 'async_question_new_comment_on_my_post', 'async_question_new_comment_on_others_post', 'async_question_status_changed', 'async_question_upvoted', 'course_clone_summary')`, + ); + await queryRunner.query( + `ALTER TABLE "mail_services" ALTER COLUMN "serviceType" TYPE "public"."mail_services_servicetype_enum_old" USING "serviceType"::"text"::"public"."mail_services_servicetype_enum_old"`, + ); + await queryRunner.query( + `DROP TYPE "public"."mail_services_servicetype_enum"`, + ); + await queryRunner.query( + `ALTER TYPE "public"."mail_services_servicetype_enum_old" RENAME TO "mail_services_servicetype_enum"`, + ); + await queryRunner.query( + `CREATE TYPE "public"."organization_role_history_model_rolechangereason_enum_old" AS ENUM('joinedOrganizationMember', 'joinedOrganizationProfessor', 'manualModification', 'unknown')`, + ); + await queryRunner.query( + `ALTER TABLE "organization_role_history_model" ALTER COLUMN "roleChangeReason" DROP DEFAULT`, + ); + await queryRunner.query( + `ALTER TABLE "organization_role_history_model" ALTER COLUMN "roleChangeReason" TYPE "public"."organization_role_history_model_rolechangereason_enum_old" USING "roleChangeReason"::"text"::"public"."organization_role_history_model_rolechangereason_enum_old"`, + ); + await queryRunner.query( + `ALTER TABLE "organization_role_history_model" ALTER COLUMN "roleChangeReason" SET DEFAULT 'unknown'`, + ); + await queryRunner.query( + `DROP TYPE "public"."organization_role_history_model_rolechangereason_enum"`, + ); + await queryRunner.query( + `ALTER TYPE "public"."organization_role_history_model_rolechangereason_enum_old" RENAME TO "organization_role_history_model_rolechangereason_enum"`, + ); + await queryRunner.query(`DROP TABLE "prof_invite_model"`); + } +} diff --git a/packages/server/ormconfig.ts b/packages/server/ormconfig.ts index 407cdc25d..3ade485e7 100644 --- a/packages/server/ormconfig.ts +++ b/packages/server/ormconfig.ts @@ -58,6 +58,7 @@ import { LtiCourseInviteModel } from './src/lti/lti-course-invite.entity'; import { AuthStateModel } from './src/auth/auth-state.entity'; import { LtiIdentityTokenModel } from './src/lti/lti_identity_token.entity'; import { UserLtiIdentityModel } from './src/lti/user_lti_identity.entity'; +import { ProfInviteModel } from './src/course/prof-invite/prof-invite.entity'; // set .envs to their default values if the developer hasn't yet set them if (fs.existsSync('.env')) { config(); @@ -143,6 +144,7 @@ const typeorm: DataSourceOptions = { AuthStateModel, UserLtiIdentityModel, LtiIdentityTokenModel, + ProfInviteModel, ], logging: process.env.NODE_ENV !== 'production' diff --git a/packages/server/src/app.module.ts b/packages/server/src/app.module.ts index f62d42a68..5183ae5ff 100644 --- a/packages/server/src/app.module.ts +++ b/packages/server/src/app.module.ts @@ -38,6 +38,7 @@ import { LmsIntegrationModule } from './lmsIntegration/lmsIntegration.module'; import { BaseExceptionFilter } from 'exception_filters/generic-exception.filter'; import { RedisModule } from '@liaoliaots/nestjs-redis'; import { LtiModule } from './lti/lti.module'; +import { ProfInviteModule } from './course/prof-invite/prof-invite.module'; @Module({ imports: [ @@ -99,6 +100,7 @@ import { LtiModule } from './lti/lti.module'; RedisQueueModule, BackupModule, QueueChatsModule, + ProfInviteModule, // no more than 30 calls per 1 second ThrottlerModule.forRoot([ { diff --git a/packages/server/src/course/course.controller.ts b/packages/server/src/course/course.controller.ts index 3701e6a03..ebb291325 100644 --- a/packages/server/src/course/course.controller.ts +++ b/packages/server/src/course/course.controller.ts @@ -999,7 +999,7 @@ export class CourseController { return; } - // Helper function for the SELECT fields + // Helper function for the SELECT fields private getCommonSelectFields(tableAlias: string): string { return ` ${tableAlias}.user_id, @@ -1017,15 +1017,19 @@ export class CourseController { @Roles(Role.PROFESSOR, Role.TA) async exportToolUsage( @Param('id', ParseIntPipe) courseId: number, - @Query('includeQueueQuestions', new ParseBoolPipe({ optional: true })) includeQueueQuestions: boolean = true, - @Query('includeAnytimeQuestions', new ParseBoolPipe({ optional: true })) includeAnytimeQuestions: boolean = true, - @Query('includeChatbotInteractions', new ParseBoolPipe({ optional: true })) includeChatbotInteractions: boolean = true, - @Query('groupBy', new ParseEnumPipe(TimeGrouping, { optional: true })) groupBy: TimeGrouping = TimeGrouping.WEEK, + @Query('includeQueueQuestions', new ParseBoolPipe({ optional: true })) + includeQueueQuestions: boolean = true, + @Query('includeAnytimeQuestions', new ParseBoolPipe({ optional: true })) + includeAnytimeQuestions: boolean = true, + @Query('includeChatbotInteractions', new ParseBoolPipe({ optional: true })) + includeChatbotInteractions: boolean = true, + @Query('groupBy', new ParseEnumPipe(TimeGrouping, { optional: true })) + groupBy: TimeGrouping = TimeGrouping.WEEK, @Query('startDate') startDate: string, @Query('endDate') endDate: string, ): Promise { const isGroupByWeek = groupBy === TimeGrouping.WEEK; - + // Check if there are any students in the course before proceeding const studentCount = await UserCourseModel.count({ where: { @@ -1039,41 +1043,55 @@ export class CourseController { 'Cannot export tool usage data: No students are enrolled in this course.', ); } - + let startDateObj: Date; let endDateObj: Date; - + if (startDate && endDate) { startDateObj = new Date(startDate); endDateObj = new Date(endDate); } else { const dateRangeQuery = ` WITH all_dates AS ( - ${includeQueueQuestions ? ` + ${ + includeQueueQuestions + ? ` SELECT q."createdAt" as date FROM "question_model" q JOIN "queue_model" qu ON q."queueId" = qu.id WHERE qu."courseId" = $1 - ` : ''} + ` + : '' + } ${includeQueueQuestions && includeAnytimeQuestions ? 'UNION ALL' : ''} - ${includeAnytimeQuestions ? ` + ${ + includeAnytimeQuestions + ? ` SELECT aq."createdAt" as date FROM "async_question_model" aq WHERE aq."courseId" = $1 AND aq.status != 'StudentDeleted' - ` : ''} + ` + : '' + } ${(includeQueueQuestions || includeAnytimeQuestions) && includeChatbotInteractions ? 'UNION ALL' : ''} - ${includeChatbotInteractions ? ` + ${ + includeChatbotInteractions + ? ` SELECT ci.timestamp as date FROM "chatbot_interactions_model" ci WHERE ci.course = $1 - ` : ''} + ` + : '' + } ) SELECT MIN(date) as earliest_date, MAX(date) as latest_date FROM all_dates `; - - const dateRangeResult = await UserCourseModel.query(dateRangeQuery, [courseId]); + + const dateRangeResult = await UserCourseModel.query(dateRangeQuery, [ + courseId, + ]); const dateRange = dateRangeResult[0]; - + if (dateRange && dateRange.earliest_date && dateRange.latest_date) { startDateObj = new Date(dateRange.earliest_date); endDateObj = new Date(dateRange.latest_date); @@ -1085,7 +1103,7 @@ export class CourseController { try { const results = []; - + if (includeQueueQuestions) { const queueQuery = isGroupByWeek ? ` @@ -1173,10 +1191,19 @@ export class CourseController { ORDER BY sd."lastName", sd."firstName", sd.period_date `; - const queueResults = await UserCourseModel.query(queueQuery, [courseId, startDateObj, endDateObj]); - results.push(...queueResults.map(row => ({ ...row, tool_type: ToolUsageType.QUEUE_QUESTIONS }))); + const queueResults = await UserCourseModel.query(queueQuery, [ + courseId, + startDateObj, + endDateObj, + ]); + results.push( + ...queueResults.map((row) => ({ + ...row, + tool_type: ToolUsageType.QUEUE_QUESTIONS, + })), + ); } - + if (includeAnytimeQuestions) { const anytimeQuery = isGroupByWeek ? ` @@ -1264,10 +1291,19 @@ export class CourseController { ORDER BY sd."lastName", sd."firstName", sd.period_date `; - const anytimeResults = await UserCourseModel.query(anytimeQuery, [courseId, startDateObj, endDateObj]); - results.push(...anytimeResults.map(row => ({ ...row, tool_type: ToolUsageType.ANYTIME_QUESTIONS }))); + const anytimeResults = await UserCourseModel.query(anytimeQuery, [ + courseId, + startDateObj, + endDateObj, + ]); + results.push( + ...anytimeResults.map((row) => ({ + ...row, + tool_type: ToolUsageType.ANYTIME_QUESTIONS, + })), + ); } - + if (includeChatbotInteractions) { const chatbotQuery = isGroupByWeek ? ` @@ -1353,20 +1389,30 @@ export class CourseController { ORDER BY sd."lastName", sd."firstName", sd.period_date `; - const chatbotResults = await UserCourseModel.query(chatbotQuery, [courseId, startDateObj, endDateObj]); - results.push(...chatbotResults.map(row => ({ ...row, tool_type: ToolUsageType.CHATBOT_INTERACTIONS }))); + const chatbotResults = await UserCourseModel.query(chatbotQuery, [ + courseId, + startDateObj, + endDateObj, + ]); + results.push( + ...chatbotResults.map((row) => ({ + ...row, + tool_type: ToolUsageType.CHATBOT_INTERACTIONS, + })), + ); } // Return JSON data for frontend to handle CSV generation - - return results; + return results; } catch (error) { console.error('Error exporting tool usage:', error); - throw new HttpException('Failed to export tool usage data', HttpStatus.INTERNAL_SERVER_ERROR); + throw new HttpException( + 'Failed to export tool usage data', + HttpStatus.INTERNAL_SERVER_ERROR, + ); } } - @Patch(':id/set_ta_notes/:uid') @UseGuards(JwtAuthGuard, CourseRolesGuard, EmailVerifiedGuard) @Roles(Role.PROFESSOR, Role.TA) diff --git a/packages/server/src/course/course.entity.ts b/packages/server/src/course/course.entity.ts index ad6b38c56..d55b96ea6 100644 --- a/packages/server/src/course/course.entity.ts +++ b/packages/server/src/course/course.entity.ts @@ -27,6 +27,7 @@ import { ChatbotDocPdfModel } from '../chatbot/chatbot-doc-pdf.entity'; import { SuperCourseModel } from './super-course.entity'; import { CourseChatbotSettingsModel } from '../chatbot/chatbot-infrastructure-models/course-chatbot-settings.entity'; import { LtiCourseInviteModel } from '../lti/lti-course-invite.entity'; +import { ProfInviteModel } from './prof-invite/prof-invite.entity'; @Entity('course_model') export class CourseModel extends BaseEntity { @@ -170,4 +171,8 @@ export class CourseModel extends BaseEntity { @Exclude() @OneToMany(() => LtiCourseInviteModel, (ltiInvite) => ltiInvite.course) ltiInvites: LtiCourseInviteModel[]; + + @OneToMany((type) => ProfInviteModel, (profInvite) => profInvite.course) + @Exclude() + profInvites: ProfInviteModel[]; } diff --git a/packages/server/src/course/course.service.ts b/packages/server/src/course/course.service.ts index 19c7c1da7..9652145fc 100644 --- a/packages/server/src/course/course.service.ts +++ b/packages/server/src/course/course.service.ts @@ -7,6 +7,7 @@ import { GetCourseUserInfoResponse, MailServiceType, OrganizationRole, + QUERY_PARAMS, QueueConfig, QueueTypes, Role, @@ -358,7 +359,7 @@ export class CourseService { // check if the queueInvite exists and if it will invite to course const queueInvite = await QueueInviteModel.findOne({ where: { - queueId: parseInt(queueId), + queueId: Number(queueId), }, }); // get the user to see if they are in the course @@ -396,7 +397,7 @@ export class CourseService { if (!queueInvite) { // if the queueInvite doesn't exist - queryParams.set('err', 'inviteNotFound'); + queryParams.set('err', QUERY_PARAMS.queueInvite.error.inviteNotFound); return; } @@ -409,12 +410,15 @@ export class CourseService { }); if (!course) { - queryParams.set('err', 'courseNotFound'); + queryParams.set('err', QUERY_PARAMS.queueInvite.error.courseNotFound); return; } if (course.courseInviteCode !== courseInviteCode) { - queryParams.set('err', 'badCourseInviteCode'); + queryParams.set( + 'err', + QUERY_PARAMS.queueInvite.error.badCourseInviteCode, + ); return; } @@ -430,7 +434,7 @@ export class CourseService { return; } - queryParams.set('err', 'notInCourse'); + queryParams.set('err', QUERY_PARAMS.queueInvite.error.notInCourse); }; await getUrlAndParams(); diff --git a/packages/server/src/course/prof-invite/prof-invite.controller.ts b/packages/server/src/course/prof-invite/prof-invite.controller.ts new file mode 100644 index 000000000..dc3b431b1 --- /dev/null +++ b/packages/server/src/course/prof-invite/prof-invite.controller.ts @@ -0,0 +1,167 @@ +import { + AcceptProfInviteParams, + CreateProfInviteParams, + GetProfInviteResponse, + OrganizationRole, +} from '@koh/common'; +import { + BadRequestException, + Body, + ClassSerializerInterceptor, + Controller, + Delete, + Get, + NotFoundException, + Param, + ParseIntPipe, + Post, + Query, + UseGuards, + UseInterceptors, +} from '@nestjs/common'; +import { Roles } from '../../decorators/roles.decorator'; +import { UserId } from '../../decorators/user.decorator'; +import { JwtAuthGuard } from '../../guards/jwt-auth.guard'; +import { EmailVerifiedGuard } from '../../guards/email-verified.guard'; +import { ProfInviteModel } from './prof-invite.entity'; +import { OrganizationRolesGuard } from 'guards/organization-roles.guard'; +import { ProfInviteService } from './prof-invite.service'; + +@Controller('prof_invites') +@UseInterceptors(ClassSerializerInterceptor) +export class ProfInviteController { + constructor(private profInviteService: ProfInviteService) {} + + @Get('all/:orgId') + @UseGuards(JwtAuthGuard, EmailVerifiedGuard, OrganizationRolesGuard) + @Roles(OrganizationRole.ADMIN) + async getAllProfInvites( + @Param('orgId', ParseIntPipe) orgId: number, + // Providing courseId will get all prof invites for the course, otherwise it's all invites for the org + @Query('courseId') courseIdString: string, + ): Promise { + // ParseIntPipe seemed to make courseId param mandatory. + // I tried various things like using a DTO for it... didn't work. + // Honestly if you have a better idea LMK. + // I feel an "optional integer query param" is pretty common but for some reason I can't find if nestjs has a built-in way of handling it nicely + let courseId = parseInt(courseIdString); + if (!courseId || courseId <= 0) { + courseId = undefined; + } + + const profInvites = await ProfInviteModel.find({ + where: { orgId, courseId }, + relations: { course: true, adminUser: true }, + select: { + course: { + id: true, + name: true, + }, + adminUser: { + id: true, + name: true, + email: true, + }, + id: true, + maxUses: true, + usesUsed: true, + createdAt: true, + expiresAt: true, + code: true, + makeOrgProf: true, + }, + }); + return profInvites; + } + + @Post(':orgId') + @UseGuards(JwtAuthGuard, EmailVerifiedGuard, OrganizationRolesGuard) + @Roles(OrganizationRole.ADMIN) + async createProfInvite( + @Param('orgId', ParseIntPipe) orgId: number, + @UserId() userId: number, + @Body() body: CreateProfInviteParams, + ): Promise { + const newProfInvite = await this.profInviteService.createProfInvite( + orgId, + body.courseId, + userId, + body.maxUses, + body.expiresAt, + body.makeOrgProf, + ); + const profInviteResponse = await ProfInviteModel.findOne({ + where: { id: newProfInvite.id }, + relations: { course: true, adminUser: true }, + select: { + course: { + id: true, + name: true, + }, + adminUser: { + id: true, + name: true, + email: true, + }, + id: true, + maxUses: true, + usesUsed: true, + createdAt: true, + expiresAt: true, + code: true, + makeOrgProf: true, + }, + }); + return profInviteResponse; + } + + // assumed that this is mostly used to correct accidentally created invites rather than have all invites deleted + @Delete(':orgId/:piid') + @UseGuards(JwtAuthGuard, EmailVerifiedGuard, OrganizationRolesGuard) + @Roles(OrganizationRole.ADMIN) + async deleteProfInvite( + @Param('orgId', ParseIntPipe) orgId: number, + @Param('piid', ParseIntPipe) piid: number, + ): Promise { + await ProfInviteModel.delete({ id: piid }); + return; + } + + // Allow logged-in users to accept a prof invite + @Post('accept/:piid') + @UseGuards(JwtAuthGuard, EmailVerifiedGuard) + async acceptProfInvite( + @Param('piid', ParseIntPipe) piid: number, + @Body() body: AcceptProfInviteParams, + @UserId() userId: number, + ): Promise { + const url = await this.profInviteService.acceptProfInvite( + userId, + piid, + body.code, + ); + return url; + } + + // just returns the course id and org id for given prof invite (public endpoint) + @Get('details/:piid') + async getProfInviteDetails( + @Param('piid', ParseIntPipe) piid: number, + ): Promise<{ courseId: number; orgId: number }> { + const profInvite = await ProfInviteModel.findOne({ + where: { id: piid }, + relations: { + course: { + organizationCourse: true, + }, + }, + }); + if (!profInvite) { + throw new NotFoundException('Prof invite not found'); + } + return { + courseId: profInvite.courseId, + orgId: profInvite.course.organizationCourse.organizationId, + }; + } +} diff --git a/packages/server/src/course/prof-invite/prof-invite.entity.ts b/packages/server/src/course/prof-invite/prof-invite.entity.ts new file mode 100644 index 000000000..59f08bcd1 --- /dev/null +++ b/packages/server/src/course/prof-invite/prof-invite.entity.ts @@ -0,0 +1,77 @@ +import { + BaseEntity, + Column, + CreateDateColumn, + Entity, + JoinColumn, + ManyToOne, + OneToOne, + PrimaryColumn, + PrimaryGeneratedColumn, +} from 'typeorm'; +import { CourseModel } from '../course.entity'; +import { UserModel } from 'profile/user.entity'; +import { Exclude } from 'class-transformer'; +import { OrganizationModel } from 'organization/organization.entity'; + +/** + * These are temporary invite links that will automatically promote the user to professor when accepted + */ +@Entity('prof_invite_model') +export class ProfInviteModel extends BaseEntity { + @PrimaryGeneratedColumn() + id: number; + + @Column() + orgId: number; + + @ManyToOne( + (type) => OrganizationModel, + (organization) => organization.profInvites, + { + onDelete: 'CASCADE', + }, + ) + @JoinColumn({ name: 'orgId' }) + @Exclude() + organization: OrganizationModel; + + @Column() + courseId: number; + + @ManyToOne((type) => CourseModel, (course) => course.profInvites, { + onDelete: 'CASCADE', + }) + @JoinColumn({ name: 'courseId' }) + course: CourseModel; + + @Column({ default: 1 }) + maxUses: number; + + // I thought about keeping track with a relationship with the users table (to keep track of who accepted the invite) but meh I don't think it'll add much benefit + // The emails already keep track of this + @Column({ default: 0 }) + usesUsed: number; + + @CreateDateColumn({ type: 'timestamp with time zone' }) + createdAt: Date; + + @Column({ type: 'timestamp with time zone' }) + expiresAt: Date; + + @Column('text') + code: string; + + @Column('boolean', { default: true }) + makeOrgProf: boolean; + + @Column() + adminUserId: number; + + // To keep track of what admin created the invite (useful for sending them an email when the invite is used) + @ManyToOne((type) => UserModel, (user) => user.createdProfInvites, { + onDelete: 'CASCADE', + }) + @JoinColumn({ name: 'adminUserId' }) + adminUser: UserModel; +} diff --git a/packages/server/src/course/prof-invite/prof-invite.module.ts b/packages/server/src/course/prof-invite/prof-invite.module.ts new file mode 100644 index 000000000..d12d50c34 --- /dev/null +++ b/packages/server/src/course/prof-invite/prof-invite.module.ts @@ -0,0 +1,16 @@ +import { Module } from '@nestjs/common'; +import { OrganizationModule } from 'organization/organization.module'; +import { MailModule } from 'mail/mail.module'; +import { ProfInviteController } from './prof-invite.controller'; +import { ProfInviteService } from './prof-invite.service'; +import { OrganizationService } from 'organization/organization.service'; + +// Made this its own module since imo it's better organised this way than shoving it all into course module (also it helps solve nestjs import dependency issue) +// It's also probably not a bad rule of thumb that if you can CRUD it, it can probably be made into its own controller + module +@Module({ + controllers: [ProfInviteController], + imports: [OrganizationModule, MailModule], + providers: [ProfInviteService, OrganizationService], + exports: [ProfInviteService, OrganizationService], +}) +export class ProfInviteModule {} diff --git a/packages/server/src/course/prof-invite/prof-invite.service.spec.ts b/packages/server/src/course/prof-invite/prof-invite.service.spec.ts new file mode 100644 index 000000000..fd3af2cfd --- /dev/null +++ b/packages/server/src/course/prof-invite/prof-invite.service.spec.ts @@ -0,0 +1,511 @@ +import { + MailServiceType, + OrganizationRole, + QUERY_PARAMS, + Role, +} from '@koh/common'; +import { TestingModule } from '@nestjs/testing'; +import { ProfInviteModule } from './prof-invite.module'; +import { ProfInviteService } from './prof-invite.service'; +import { ProfInviteModel } from './prof-invite.entity'; +import { + expectEmailSent, + expectEmailNotSent, + overrideEmailService, + setupIntegrationTest, +} from '../../../test/util/testUtils'; +import { + CourseFactory, + OrganizationCourseFactory, + OrganizationFactory, + OrganizationUserFactory, + UserCourseFactory, + UserFactory, +} from '../../../test/util/factories'; +import { UserModel } from 'profile/user.entity'; +import { CourseModel } from 'course/course.entity'; +import { OrganizationModel } from 'organization/organization.entity'; +import { OrganizationUserModel } from 'organization/organization-user.entity'; +import { UserCourseModel } from 'profile/user-course.entity'; + +describe('ProfInviteService', () => { + const { getTestModule } = setupIntegrationTest( + ProfInviteModule, + overrideEmailService, + ); + + let service: ProfInviteService; + let module: TestingModule; + + // Shared state + let admin: UserModel; + let user: UserModel; + let org: OrganizationModel; + let course: CourseModel; + + beforeAll(async () => { + module = getTestModule(); + service = module.get(ProfInviteService); + }); + + beforeEach(async () => { + jest.clearAllMocks(); + admin = await UserFactory.create({ + firstName: 'Admin', + email: 'admin@test.com', + }); + user = await UserFactory.create({ + firstName: 'User', + email: 'user@test.com', + }); + org = await OrganizationFactory.create(); + course = await CourseFactory.create(); + await OrganizationCourseFactory.create({ + course: course, + organization: org, + }); + + const orgUserAdmin = await OrganizationUserFactory.create({ + organizationUser: admin, + organization: org, + role: OrganizationRole.ADMIN, + }); + admin.organizationUser = orgUserAdmin; + // assume that the user will always already be part of the org + const orgUserMember = await OrganizationUserFactory.create({ + organizationUser: user, + organization: org, + role: OrganizationRole.MEMBER, + }); + user.organizationUser = orgUserMember; // setting this attribute for email construction reasons + }); + + describe('createProfInvite', () => { + it('creates an invite with default values (maxUses=1, makeOrgProf=true, 7 day expiry)', async () => { + const invite = await service.createProfInvite( + org.id, + course.id, + admin.id, + ); + + expect(invite).toBeDefined(); + expect(invite.maxUses).toBe(1); + expect(invite.makeOrgProf).toBe(true); + expect(invite.usesUsed).toBe(0); + expect(invite.code).toHaveLength(12); + + // Check expiry is roughly 7 days from now + const now = new Date(); + const sevenDays = new Date(now.getTime() + 7 * 24 * 60 * 60 * 1000); + const diff = Math.abs(invite.expiresAt.getTime() - sevenDays.getTime()); + expect(diff).toBeLessThan(10000); // within 10 seconds + }); + + it('creates an invite with custom values', async () => { + const customDate = new Date(Date.now() + 100000); + const invite = await service.createProfInvite( + org.id, + course.id, + admin.id, + 5, // maxUses + customDate, + false, // makeOrgProf + ); + + expect(invite.maxUses).toBe(5); + expect(invite.makeOrgProf).toBe(false); + expect(invite.expiresAt.toISOString()).toBe(customDate.toISOString()); + }); + }); + + describe('acceptProfInvite', () => { + let invite: ProfInviteModel; + + /* Used to get the full relations of an invite for constructing the email */ + const findInviteWithRelations = async (id: number) => { + return await ProfInviteModel.findOne({ + where: { id }, + relations: { + course: true, + adminUser: { + organizationUser: true, + }, + }, + }); + }; + + beforeEach(async () => { + invite = await service.createProfInvite(org.id, course.id, admin.id); + }); + + it('Standard Acceptance: User added as Professor (and made org prof), Admin Notified', async () => { + const url = await service.acceptProfInvite( + user.id, + invite.id, + invite.code, + ); + + const params = new URLSearchParams({ + notice: QUERY_PARAMS.profInvite.notice.inviteAccepted, + }); + expect(url).toBe(`/course/${course.id}?${params.toString()}`); + + const userCourse = await UserCourseModel.findOne({ + where: { userId: user.id, courseId: course.id }, + }); + expect(userCourse).toBeDefined(); + expect(userCourse.role).toBe(Role.PROFESSOR); + + const orgUser = await OrganizationUserModel.findOne({ + where: { userId: user.id, organizationId: org.id }, + }); + expect(orgUser.role).toBe(OrganizationRole.PROFESSOR); + + const updatedInvite = await ProfInviteModel.findOneBy({ + id: invite.id, + }); + expect(updatedInvite.usesUsed).toBe(1); + + const fullInvite = await findInviteWithRelations(invite.id); + const email = service.constructSuccessEmail( + user, + fullInvite, + fullInvite.code, + 0, // remaining uses (1 max - 1 used) + ); + expectEmailSent( + [admin.email], + [MailServiceType.ADMIN_NOTICE], + email.subject, + email.content, + ); + }); + + it('No Org Role Promotion: Does not promote if makeOrgProf is false', async () => { + const noPromoteInvite = await service.createProfInvite( + org.id, + course.id, + admin.id, + 1, + undefined, + false, // makeOrgProf + ); + + const url = await service.acceptProfInvite( + user.id, + noPromoteInvite.id, + noPromoteInvite.code, + ); + + const params = new URLSearchParams({ + notice: QUERY_PARAMS.profInvite.notice.inviteAccepted, + }); + expect(url).toBe(`/course/${course.id}?${params.toString()}`); + + const userCourse = await UserCourseModel.findOne({ + where: { userId: user.id, courseId: course.id }, + }); + expect(userCourse).toBeDefined(); + expect(userCourse.role).toBe(Role.PROFESSOR); + + const orgUser = await OrganizationUserModel.findOne({ + where: { userId: user.id, organizationId: org.id }, + }); + expect(orgUser.role).toBe(OrganizationRole.MEMBER); // NOT promoted to org prof + + const updatedInvite = await ProfInviteModel.findOneBy({ + id: noPromoteInvite.id, + }); + expect(updatedInvite.usesUsed).toBe(1); + + const fullInvite = await findInviteWithRelations(noPromoteInvite.id); + const email = service.constructSuccessEmail( + user, + fullInvite, + fullInvite.code, + 0, + ); + expectEmailSent( + [admin.email], + [MailServiceType.ADMIN_NOTICE], + email.subject, + email.content, + ); + }); + + it('Multiple Uses: Invite with maxUses=2 allows two users', async () => { + const multiInvite = await service.createProfInvite( + org.id, + course.id, + admin.id, + 2, + ); + const user2 = await UserFactory.create(); + await OrganizationUserFactory.create({ + organizationUser: user2, + organization: org, + role: OrganizationRole.MEMBER, + }); + + await service.acceptProfInvite(user.id, multiInvite.id, multiInvite.code); + await service.acceptProfInvite( + user2.id, + multiInvite.id, + multiInvite.code, + ); + + const updated = await ProfInviteModel.findOneBy({ + id: multiInvite.id, + }); + expect(updated.usesUsed).toBe(2); + + // Third acceptance fails + const user3 = await UserFactory.create(); + await OrganizationUserFactory.create({ + organizationUser: user3, + organization: org, + role: OrganizationRole.MEMBER, + }); + const url = await service.acceptProfInvite( + user3.id, + multiInvite.id, + multiInvite.code, + ); + expect(url).toContain(QUERY_PARAMS.profInvite.error.maxUsesReached); + }); + + it('Invalid Invite ID: Returns NotFound error', async () => { + // capture console.error to capture the error + const consoleErrorSpy = jest + .spyOn(console, 'error') + .mockImplementation(() => {}); + const url = await service.acceptProfInvite(user.id, 99999, 'abc'); + expect(url).toContain(QUERY_PARAMS.profInvite.error.notFound); + expectEmailNotSent(); + consoleErrorSpy.mockRestore(); + }); + + it('User Not Found: Returns UserNotFound error', async () => { + const url = await service.acceptProfInvite(99999, invite.id, invite.code); + expect(url).toContain(QUERY_PARAMS.profInvite.error.userNotFound); + expectEmailNotSent(); + }); + + it('Expired Invite: Returns Expired error and emails admin', async () => { + const expiredInvite = await service.createProfInvite( + org.id, + course.id, + admin.id, + 1, + new Date(Date.now() - 10000), // Past + ); + + const url = await service.acceptProfInvite( + user.id, + expiredInvite.id, + expiredInvite.code, + ); + expect(url).toContain(QUERY_PARAMS.profInvite.error.expired); + const fullInvite = await findInviteWithRelations(expiredInvite.id); + const email = service.constructExpiredEmail( + user, + fullInvite, + fullInvite.code, + ); + expectEmailSent( + [admin.email], + [MailServiceType.ADMIN_NOTICE], + email.subject, + email.content, + ); + }); + + it('Max Uses Reached: Returns error and emails admin', async () => { + // Artificially use it up + invite.usesUsed = 1; + await invite.save(); + + const url = await service.acceptProfInvite( + user.id, + invite.id, + invite.code, + ); + expect(url).toContain(QUERY_PARAMS.profInvite.error.maxUsesReached); + const fullInvite = await findInviteWithRelations(invite.id); + const email = service.constructAlreadyUsedEmail( + user, + fullInvite, + fullInvite.code, + ); + expectEmailSent( + [admin.email], + [MailServiceType.ADMIN_NOTICE], + email.subject, + email.content, + ); + }); + + it('Bad Code: Returns BadCode error and emails admin', async () => { + const url = await service.acceptProfInvite( + user.id, + invite.id, + 'WRONG_CODE', + ); + expect(url).toContain(QUERY_PARAMS.profInvite.error.badCode); + const fullInvite = await findInviteWithRelations(invite.id); + const email = service.constructWrongCodeEmail( + user, + fullInvite, + 'WRONG_CODE', + ); + expectEmailSent( + [admin.email], + [MailServiceType.ADMIN_NOTICE], + email.subject, + email.content, + ); + }); + + it('User is Student: Does NOT consume invite, sends manual promotion email', async () => { + // Add user as student + await UserCourseFactory.create({ + user, + course, + role: Role.STUDENT, + }); + + const url = await service.acceptProfInvite( + user.id, + invite.id, + invite.code, + ); + + // Should just redirect to course + expect(url).toBe(`/course/${course.id}`); + + // Invite not used + const updated = await ProfInviteModel.findOneBy({ id: invite.id }); + expect(updated.usesUsed).toBe(0); + + const fullInvite = await findInviteWithRelations(invite.id); + const email = service.constructAlreadyStudentEmail( + user, + fullInvite, + fullInvite.code, + ); + expectEmailSent( + [admin.email], + [MailServiceType.ADMIN_NOTICE], + email.subject, + email.content, + ); + }); + + it('User is already Professor: Redirects immediately, no email, no consume', async () => { + await UserCourseFactory.create({ + user, + course, + role: Role.PROFESSOR, + }); + + const url = await service.acceptProfInvite( + user.id, + invite.id, + invite.code, + ); + + expect(url).toBe(`/course/${course.id}`); + const updated = await ProfInviteModel.findOneBy({ id: invite.id }); + expect(updated.usesUsed).toBe(0); + expectEmailNotSent(); + }); + + it('Admin accepts own invite (New to course): Adds to course, Invite NOT consumed', async () => { + // Admin creates invite, is NOT in course yet + const url = await service.acceptProfInvite( + admin.id, + invite.id, + invite.code, + ); + + expect(url).toContain( + QUERY_PARAMS.profInvite.notice.adminAcceptedInviteNotConsumed, + ); + + // Admin added + const uc = await UserCourseModel.findOne({ + where: { userId: admin.id, courseId: course.id }, + }); + expect(uc.role).toBe(Role.PROFESSOR); + + // Not consumed + const updated = await ProfInviteModel.findOneBy({ id: invite.id }); + expect(updated.usesUsed).toBe(0); + + // No email sent because admin is the creator + expectEmailNotSent(); + }); + + it('Admin accepts own invite (Already in course): Returns notice, no DB changes', async () => { + await UserCourseFactory.create({ + user: admin, + course, + role: Role.PROFESSOR, + }); + + const url = await service.acceptProfInvite( + admin.id, + invite.id, + invite.code, + ); + + expect(url).toContain( + QUERY_PARAMS.profInvite.notice.adminAlreadyInCourse, + ); + const updated = await ProfInviteModel.findOneBy({ id: invite.id }); + expect(updated.usesUsed).toBe(0); + expectEmailNotSent(); + }); + + it('Other Admin accepts invite: Added to course, Invite NOT consumed, Creator emailed', async () => { + const otherAdmin = await UserFactory.create(); + await OrganizationUserFactory.create({ + organizationUser: otherAdmin, + organization: org, + role: OrganizationRole.ADMIN, + }); + + const url = await service.acceptProfInvite( + otherAdmin.id, + invite.id, + invite.code, + ); + + expect(url).toContain( + QUERY_PARAMS.profInvite.notice.adminAcceptedInviteNotConsumed, + ); + + const uc = await UserCourseModel.findOne({ + where: { userId: otherAdmin.id, courseId: course.id }, + }); + expect(uc.role).toBe(Role.PROFESSOR); + + const updated = await ProfInviteModel.findOneBy({ id: invite.id }); + expect(updated.usesUsed).toBe(0); + + // Creator (admin) should be notified that another admin clicked it + const fullInvite = await findInviteWithRelations(invite.id); + const email = service.constructAdminAcceptedEmail( + otherAdmin, + fullInvite, + fullInvite.code, + ); + expectEmailSent( + [admin.email], + [MailServiceType.ADMIN_NOTICE], + email.subject, + email.content, + ); + }); + }); +}); diff --git a/packages/server/src/course/prof-invite/prof-invite.service.ts b/packages/server/src/course/prof-invite/prof-invite.service.ts new file mode 100644 index 000000000..94a3d1e54 --- /dev/null +++ b/packages/server/src/course/prof-invite/prof-invite.service.ts @@ -0,0 +1,437 @@ +import { + MailServiceType, + OrganizationRole, + OrgRoleChangeReason, + QUERY_PARAMS, + Role, +} from '@koh/common'; +import { Injectable } from '@nestjs/common'; +import { UserCourseModel } from 'profile/user-course.entity'; +import { UserModel } from 'profile/user.entity'; +import { OrganizationUserModel } from 'organization/organization-user.entity'; +import { ProfInviteModel } from './prof-invite.entity'; +import { OrganizationService } from 'organization/organization.service'; +import { randomBytes } from 'node:crypto'; +import { MailService } from 'mail/mail.service'; +import * as Sentry from '@sentry/nestjs'; + +@Injectable() +export class ProfInviteService { + constructor( + private readonly organizationService: OrganizationService, + private readonly mailService: MailService, + ) {} + + async createProfInvite( + orgId: number, + courseId: number, + adminUserId: number, + maxUses?: number, + expiresAt?: Date, + makeOrgProf?: boolean, + ): Promise { + return await ProfInviteModel.create({ + orgId, + courseId, + adminUserId, + code: randomBytes(6).toString('hex'), // 12 character long string. Could go longer but the invite url will look more gross + // if no expiresAt is provided, set it to 7 days from now + expiresAt: expiresAt ?? new Date(Date.now() + 1000 * 60 * 60 * 24 * 7), + makeOrgProf, // defaults to true + maxUses, // defaults to 1 + }).save(); + } + + async acceptProfInviteFromCookie( + userId: number, + profInviteCookie: string, + ): Promise { + try { + const decodedCookie = decodeURIComponent(profInviteCookie); + const splitCookie = decodedCookie.split(','); + const profInviteId = Number(splitCookie[0]); + // const orgId = splitCookie[1]; // these are part of the cookie but not used for this. + // const courseId = splitCookie[2]; + const profInviteCode = splitCookie[3]; + return this.acceptProfInvite(userId, profInviteId, profInviteCode); + } catch (error) { + Sentry.captureException(error, { + extra: { + profInviteCookie: profInviteCookie, + userId: userId, + }, + }); + console.error( + 'Unhandled error when trying to accept prof invite from cookie:', + error, + ); + const params = new URLSearchParams({ + err: QUERY_PARAMS.profInvite.error.unknown, + }); + return `/courses?${params.toString()}`; + } + } + + /* This will accept and consume a prof invite, sending any relevant emails to the admin that created it asynchronously */ + async acceptProfInvite( + userId: number, + profInviteId: number, + profInviteCode: string, + ): Promise { + // must return a string since we're using this in a redirect URL + + const profInvite = await ProfInviteModel.findOne({ + where: { id: profInviteId }, + relations: { + course: true, + adminUser: { + organizationUser: true, + }, + }, + }); + if (!profInvite) { + console.error( + `Someone tried to accept a prof invite that does not exist (invalid id). Given inputs: \nprofInviteId:${profInviteId}\nuserId:${userId}\nprofInviteCode:${profInviteCode}`, + ); + const params = new URLSearchParams({ + err: QUERY_PARAMS.profInvite.error.notFound, + [QUERY_PARAMS.profInvite.error.profInviteId]: profInviteId.toString(), + }); + return `/courses?${params.toString()}`; + } + + // check if already in course (if so, just redirect and don't consume invite) + // Note that this check happens before the invite expiry check (Meaning that as long as you are in the course, you can always use this link to navigate to the course page. Since I imagine some profs will keep using the same link to get to their course) + const user = await UserModel.findOne({ + where: { id: userId }, + relations: { + courses: true, + organizationUser: true, + }, + }); + if (!user || !user.organizationUser) { + const params = new URLSearchParams({ + err: QUERY_PARAMS.profInvite.error.userNotFound, + }); + return `/courses?${params.toString()}`; + } + const existingUserCourse = user.courses.find( + (uc) => uc.courseId === profInvite.courseId, + ); + + if ( + existingUserCourse && + user.organizationUser.role !== OrganizationRole.ADMIN + ) { + if (existingUserCourse.role === Role.PROFESSOR) { + return `/course/${profInvite.courseId}`; // no notice if you're just a prof already in the course + } else { + // Weird case: The user is already in the course as a student or TA. + // In this case, the admin messed up and should promote them manually. + + const email = this.constructAlreadyStudentEmail( + user, + profInvite, + profInviteCode, + ); + this.mailService.sendEmail({ + receiverOrReceivers: profInvite.adminUser.email, + type: MailServiceType.ADMIN_NOTICE, + subject: email.subject, + content: email.content, + }); + return `/course/${profInvite.courseId}`; + } + } + + // check "if used", "if expired", then "if invite code is correct". Checking "if used" first simply because the email notice will be more accurate (as it's likely the prof logged in with the wrong account). + if (profInvite.usesUsed >= profInvite.maxUses) { + if (user.organizationUser.role !== OrganizationRole.ADMIN) { + const email = this.constructAlreadyUsedEmail( + user, + profInvite, + profInviteCode, + ); + this.mailService.sendEmail({ + receiverOrReceivers: profInvite.adminUser.email, + type: MailServiceType.ADMIN_NOTICE, + subject: email.subject, + content: email.content, + }); + } + const params = new URLSearchParams({ + err: QUERY_PARAMS.profInvite.error.maxUsesReached, + [QUERY_PARAMS.profInvite.error.maxUses]: profInvite.maxUses.toString(), + }); + return `/courses?${params.toString()}`; + } + if (profInvite.expiresAt < new Date()) { + if (user.organizationUser.role !== OrganizationRole.ADMIN) { + const email = this.constructExpiredEmail( + user, + profInvite, + profInviteCode, + ); + this.mailService.sendEmail({ + receiverOrReceivers: profInvite.adminUser.email, + type: MailServiceType.ADMIN_NOTICE, + subject: email.subject, + content: email.content, + }); + } + const params = new URLSearchParams({ + err: QUERY_PARAMS.profInvite.error.expired, + [QUERY_PARAMS.profInvite.error.expiresAt]: + profInvite.expiresAt.toLocaleDateString(), + }); + return `/courses?${params.toString()}`; + } + if (profInvite.code !== profInviteCode) { + if (user.organizationUser.role !== OrganizationRole.ADMIN) { + const email = this.constructWrongCodeEmail( + user, + profInvite, + profInviteCode, + ); + this.mailService.sendEmail({ + receiverOrReceivers: profInvite.adminUser.email, + type: MailServiceType.ADMIN_NOTICE, + subject: email.subject, + content: email.content, + }); + } + const params = new URLSearchParams({ + err: QUERY_PARAMS.profInvite.error.badCode, + }); + return `/courses?${params.toString()}`; + } + + // Check to see if the user is already in the course AFTER other checks since it's assumed that admins only click on a prof invite link to check if it's working + if (user.organizationUser.role === OrganizationRole.ADMIN) { + if (existingUserCourse) { + const params = new URLSearchParams({ + notice: QUERY_PARAMS.profInvite.notice.adminAlreadyInCourse, + }); + return `/course/${profInvite.courseId}?${params.toString()}`; + } else { + // This covers two cases: + // A: Admin wanted to test to make sure the invite was working + // B: Another admin accepted the prof invite + + // add user to the course as a professor but don't bother changing their orgRole + await UserCourseModel.create({ + userId, + courseId: profInvite.courseId, + role: Role.PROFESSOR, + }).save(); + + if (user.id !== profInvite.adminUserId) { + const email = this.constructAdminAcceptedEmail( + user, + profInvite, + profInviteCode, + ); + this.mailService.sendEmail({ + receiverOrReceivers: profInvite.adminUser.email, + type: MailServiceType.ADMIN_NOTICE, + subject: email.subject, + content: email.content, + }); + } + const params = new URLSearchParams({ + notice: QUERY_PARAMS.profInvite.notice.adminAcceptedInviteNotConsumed, + }); + return `/course/${profInvite.courseId}?${params.toString()}`; + } + } + + // STANDARD CASE: add user to the course as a professor + await UserCourseModel.create({ + userId, + courseId: profInvite.courseId, + role: Role.PROFESSOR, + }).save(); + profInvite.usesUsed++; + await profInvite.save(); + if ( + profInvite.makeOrgProf && + user.organizationUser.role === OrganizationRole.MEMBER + ) { + await OrganizationUserModel.update( + { + userId: userId, + organizationId: user.organizationUser.organizationId, + }, + { role: OrganizationRole.PROFESSOR }, + ); + await this.organizationService.addRoleHistory( + // I didn't forget to do this because I'm one smart cookie + user.organizationUser.organizationId, + OrganizationRole.MEMBER, + OrganizationRole.PROFESSOR, + profInvite.adminUser.organizationUser.id, + user.organizationUser.id, + OrgRoleChangeReason.acceptedProfInvite, + ); + } + + const remainingUses = profInvite.maxUses - profInvite.usesUsed; + const email = this.constructSuccessEmail( + user, + profInvite, + profInviteCode, + remainingUses, + ); + this.mailService.sendEmail({ + receiverOrReceivers: profInvite.adminUser.email, + type: MailServiceType.ADMIN_NOTICE, + subject: email.subject, + content: email.content, + }); + const params = new URLSearchParams({ + notice: QUERY_PARAMS.profInvite.notice.inviteAccepted, + }); + return `/course/${profInvite.courseId}?${params.toString()}`; + } + + // + // Email Construction Helpers + // + + public getCommonEmailBody( + profInvite: ProfInviteModel, + profInviteCodeAttempt?: string, + ): string { + return ` +
+

View the course (Note that this link won't work if you're not already in the course): ${process.env.DOMAIN}/course/${profInvite.courseId}

+

You are receiving this email because you are the admin who created the prof invite.

+

Full Prof Invite Details:

+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Invite ID${profInvite.id}
Course ID${profInvite.courseId}
Course Name${profInvite.course.name}
Created At${profInvite.createdAt instanceof Date ? profInvite.createdAt.toLocaleString() : profInvite.createdAt}
Expires At${profInvite.expiresAt instanceof Date ? profInvite.expiresAt.toLocaleString() : profInvite.expiresAt}
Uses${profInvite.usesUsed} / ${profInvite.maxUses}
Make Org Prof${profInvite.makeOrgProf ? 'Yes' : 'No'}
Invite Code${profInvite.code}
+ ${profInviteCodeAttempt && profInviteCodeAttempt !== profInvite.code ? `

Note that the given prof invite code (${profInviteCodeAttempt}) does not match the real prof invite code (${profInvite.code}). This probably means the request was malicious!

` : ''} + + `; + } + + public getCommonEmailBodyUserInfo(user: UserModel): string { + return `

Acceptee User Details - ID: ${user.id}, Name: ${user.name}, Email: ${user.email}

`; + } + + public constructAlreadyStudentEmail( + user: UserModel, + profInvite: ProfInviteModel, + profInviteCode: string, + ): { subject: string; content: string } { + return { + subject: `HelpMe (Admin) - Prof Invite for ${profInvite.course.name} Could Not Be Accepted: Needs Manual Promotion`, + content: `

A user attempted to accept the professor invite for ${profInvite.course.name} but they are already in the course as a student or TA. In this case, you must promote them manually. Or, in case this was the result of a leaked invite, you should delete the old prof invite and create a new one.

+ ${this.getCommonEmailBodyUserInfo(user)} + ${this.getCommonEmailBody(profInvite, profInviteCode)}`, + }; + } + + public constructAlreadyUsedEmail( + user: UserModel, + profInvite: ProfInviteModel, + profInviteCode: string, + ): { subject: string; content: string } { + return { + subject: `HelpMe (Admin) - User Attempted to Accept Already-Used Prof Invite for ${profInvite.course.name}`, + content: `

A user attempted to accept a professor invite for ${profInvite.course.name} but the prof invite is already used. They were notified as such but the invite was not accepted.

+

In this case, you should verify the acceptee was who you were expecting and then email them a new prof invite (since the prof might've logged in with two different accounts). OR investigate if this link was leaked somewhere.

+ ${this.getCommonEmailBodyUserInfo(user)} + ${this.getCommonEmailBody(profInvite, profInviteCode)}`, + }; + } + + public constructExpiredEmail( + user: UserModel, + profInvite: ProfInviteModel, + profInviteCode: string, + ): { subject: string; content: string } { + return { + subject: `HelpMe (Admin) - User Attempted to Accept Expired Prof Invite for ${profInvite.course.name}`, + content: `

A user attempted to accept a professor invite for ${profInvite.course.name} but the prof invite is expired. They were notified as such but the invite was not accepted.

+

In this case, you should verify the acceptee was who you were expecting and then email them a new prof invite (since the prof might've forgotten). OR investigate if this link was leaked somewhere.

+ ${this.getCommonEmailBodyUserInfo(user)} + ${this.getCommonEmailBody(profInvite, profInviteCode)}`, + }; + } + + public constructWrongCodeEmail( + user: UserModel, + profInvite: ProfInviteModel, + profInviteCode: string, + ): { subject: string; content: string } { + return { + subject: `HelpMe (Admin) - User Attempted to Accept Prof Invite for ${profInvite.course.name} With Wrong Invite Code`, + content: `

A user attempted to accept a professor invite for ${profInvite.course.name} but the invite code was incorrect. They were notified as such but the invite was not accepted.

+

In this case, it is either a problem with the system or someone malicious. Please verify the acceptee details to see who is trying to access this and address it accordingly.

+ ${this.getCommonEmailBodyUserInfo(user)} + ${this.getCommonEmailBody(profInvite, profInviteCode)}`, + }; + } + + public constructAdminAcceptedEmail( + user: UserModel, + profInvite: ProfInviteModel, + profInviteCode: string, + ): { subject: string; content: string } { + return { + subject: `HelpMe (Admin) - Another Admin Accepted Your Prof Invite for ${profInvite.course.name} (Not Consumed)`, + content: `

An admin accepted your professor invite and has been added to ${profInvite.course.name} as a professor. Because they were an admin, the invite was not consumed (they were also notified of this).

+

In this case, if the user you were trying to invite to the course was the admin, you should not do this since it is expected the admin adds themselves to the course (you should also delete the prof invite in this case). Otherwise, you can disregard this email.

+ ${this.getCommonEmailBodyUserInfo(user)} + ${this.getCommonEmailBody(profInvite, profInviteCode)}`, + }; + } + + public constructSuccessEmail( + user: UserModel, + profInvite: ProfInviteModel, + profInviteCode: string, + remainingUses: number, + ): { subject: string; content: string } { + return { + subject: `HelpMe (Admin) - ${user.name} Accepted Your Prof Invite for ${profInvite.course.name}${remainingUses > 0 ? ` (${remainingUses} Uses Remaining)` : ' (Consumed)'}`, + content: `

${user.name} accepted your professor invite and has been added to ${profInvite.course.name} as a professor.

+

Doing so has consumed the professor invite, and there are ${remainingUses} uses remaining.

+ ${profInvite.makeOrgProf && user.organizationUser.role === OrganizationRole.MEMBER ? `

Because makeOrgProf was true (and they were not already an org prof), the user was also made into an organization professor.

` : ''} + ${this.getCommonEmailBodyUserInfo(user)} + ${this.getCommonEmailBody(profInvite, profInviteCode)}`, + }; + } +} diff --git a/packages/server/src/guards/organization-roles.guard.ts b/packages/server/src/guards/organization-roles.guard.ts index 0cbd4eb3f..16e3bf044 100644 --- a/packages/server/src/guards/organization-roles.guard.ts +++ b/packages/server/src/guards/organization-roles.guard.ts @@ -5,6 +5,8 @@ import { ForbiddenException, Injectable, UnauthorizedException, + BadRequestException, + InternalServerErrorException, } from '@nestjs/common'; import { Reflector } from '@nestjs/core'; import { OrganizationUserModel } from 'organization/organization-user.entity'; @@ -36,10 +38,19 @@ export class OrganizationRolesGuard implements CanActivate { } async setupData(request: any): Promise<{ user: OrganizationUserModel }> { + const oid = request.params.oid ?? request.params.orgId ?? null; + if (!oid) { + // throwing a 500 error since if this happens, it's likely the endpoint itself that's broken (that the endpoint is missing an orgId/oid param). + throw new InternalServerErrorException(ERROR_MESSAGES.roleGuard.noOrgId); + } + if (!Number(oid)) { + throw new BadRequestException(ERROR_MESSAGES.roleGuard.invalidOrgId); + } + const user = await OrganizationUserModel.findOne({ where: { userId: request.user.userId, - organizationId: request.params.oid, + organizationId: Number(oid), }, }); diff --git a/packages/server/src/login/login.module.ts b/packages/server/src/login/login.module.ts index 1f1f47d40..dadea3af3 100644 --- a/packages/server/src/login/login.module.ts +++ b/packages/server/src/login/login.module.ts @@ -9,6 +9,8 @@ import { RedisProfileService } from 'redisProfile/redis-profile.service'; import { MailModule } from 'mail/mail.module'; import { ChatbotApiService } from 'chatbot/chatbot-api.service'; import { LoginService } from './login.service'; +import { ProfInviteService } from 'course/prof-invite/prof-invite.service'; +import { ProfInviteModule } from 'course/prof-invite/prof-invite.module'; @Module({ imports: [ @@ -20,6 +22,7 @@ import { LoginService } from './login.service'; }), }), MailModule, + ProfInviteModule, ], controllers: [LoginController], providers: [ @@ -28,6 +31,8 @@ import { LoginService } from './login.service'; LoginService, RedisProfileService, ChatbotApiService, + ProfInviteService, ], + exports: [LoginService, ProfInviteService], }) export class LoginModule {} diff --git a/packages/server/src/login/login.service.spec.ts b/packages/server/src/login/login.service.spec.ts index 306fd2ef6..fc2a989cb 100644 --- a/packages/server/src/login/login.service.spec.ts +++ b/packages/server/src/login/login.service.spec.ts @@ -21,11 +21,12 @@ import { UserFactory, } from '../../test/util/factories'; import { LoginService } from './login.service'; -import { ERROR_MESSAGES } from '@koh/common'; +import { ERROR_MESSAGES, QUERY_PARAMS } from '@koh/common'; import { Request } from 'express'; import { UserModel } from '../profile/user.entity'; import { CourseService } from '../course/course.service'; import { LtiService } from '../lti/lti.service'; +import { ProfInviteService } from '../course/prof-invite/prof-invite.service'; import { CourseModel } from '../course/course.entity'; import { QueueModel } from '../queue/queue.entity'; import { OrganizationModel } from '../organization/organization.entity'; @@ -50,7 +51,15 @@ describe('LoginService', () => { }), }), ], - providers: [LoginService], + providers: [ + LoginService, + { + provide: ProfInviteService, + useValue: { + acceptProfInviteFromCookie: jest.fn(), + }, + }, + ], }).compile(); service = module.get(LoginService); @@ -498,7 +507,9 @@ describe('LoginService', () => { expect(res.headersSent).toEqual(false); if (queueInvite) { expect(res._cookies['queueInviteInfo']).toBeUndefined(); - expect(redirectUrl).toEqual(`/courses?err=notInCourse`); + expect(redirectUrl).toEqual( + `/courses?err=${QUERY_PARAMS.queueInvite.error.notInCourse}`, + ); } else if (secureRedirect) { expect(res._cookies['__SECURE_REDIRECT']).toBeUndefined(); expect(redirectUrl).toEqual( @@ -520,7 +531,7 @@ describe('LoginService', () => { expect(res._cookies['queueInviteInfo']).toBeUndefined(); expect(res._body).toHaveProperty( 'redirectUri', - `/courses?err=notInCourse`, + `/courses?err=${QUERY_PARAMS.queueInvite.error.notInCourse}`, ); } else if (secureRedirect) { expect(res._cookies['__SECURE_REDIRECT']).toBeUndefined(); diff --git a/packages/server/src/login/login.service.ts b/packages/server/src/login/login.service.ts index 22c291ec0..f94c4fa4a 100644 --- a/packages/server/src/login/login.service.ts +++ b/packages/server/src/login/login.service.ts @@ -11,6 +11,7 @@ import { LtiService } from '../lti/lti.service'; import { ERROR_MESSAGES } from '@koh/common'; import { CookieOptions, Request, Response } from 'express'; import { getCookie } from '../common/helpers'; +import { ProfInviteService } from 'course/prof-invite/prof-invite.service'; export type LoginEntryOptions = { cookieName?: string; @@ -27,6 +28,7 @@ export class LoginService { constructor( private jwtService: JwtService, private configService: ConfigService, + private profInviteService: ProfInviteService, ) {} async initLoginEnter( @@ -199,6 +201,8 @@ export class LoginService { const secureRedirectCookie = getCookie(req, '__SECURE_REDIRECT'); const queueInviteCookie = getCookie(req, 'queueInviteInfo'); + const profInviteCookie = getCookie(req, 'profInviteInfo'); + const ltiInviteCookie = getCookie(req, '__COURSE_INVITE'); const ltiIdentityCookie = getCookie(req, '__LTI_IDENTITY'); @@ -212,7 +216,16 @@ export class LoginService { ); } - if (queueInviteCookie && courseService && !redirect) { + if (profInviteCookie && !redirect) { + await this.profInviteService + .acceptProfInviteFromCookie(userId, profInviteCookie) + .then((url) => { + res.clearCookie('profInviteInfo'); + return res.status(HttpStatus.TEMPORARY_REDIRECT).send({ + redirectUri: url, + }); + }); + } else if (queueInviteCookie && courseService && !redirect) { // Ignore queueInviteInfo if there's another redirect queued await courseService .getQueueInviteRedirectURLandInviteToCourse(queueInviteCookie, userId) diff --git a/packages/server/src/organization/organization.controller.ts b/packages/server/src/organization/organization.controller.ts index 28d7e0eff..9cd86bcb7 100644 --- a/packages/server/src/organization/organization.controller.ts +++ b/packages/server/src/organization/organization.controller.ts @@ -25,6 +25,7 @@ import { COURSE_TIMEZONES, CourseResponse, CourseSettingsRequestBody, + CreateCourseResponse, ERROR_MESSAGES, GetOrganizationResponse, GetOrganizationUserResponse, @@ -300,7 +301,7 @@ export class OrganizationController { @OrgRole() orgRole: OrganizationRole, @Body() courseDetails: UpdateOrganizationCourseDetailsParams, @Res() res: Response, - ): Promise> { + ): Promise> { const orgSettings = await this.organizationService.getOrganizationSettings(oid); if ( @@ -346,9 +347,10 @@ export class OrganizationController { )}`, }); } + let newCourse: CourseModel; await this.dataSource.transaction(async (manager) => { // Create course entity - const newCourse = manager.create(CourseModel, { + newCourse = manager.create(CourseModel, { name: courseDetails.name, coordinator_email: courseDetails.coordinator_email, sectionGroupName: courseDetails.sectionGroupName, @@ -435,6 +437,7 @@ export class OrganizationController { return res.status(status).send({ message: message, + courseId: newCourse.id, }); } @@ -1504,6 +1507,7 @@ export class OrganizationController { organizationUser: { id: prof.organizationUser.id, name: prof.organizationUser.name, + email: prof.organizationUser.email, }, trueRole: prof.role, userId: prof.userId, @@ -1512,6 +1516,7 @@ export class OrganizationController { organizationUser: { id: prof.user.id, name: prof.user.name, + email: prof.user.email, }, trueRole: prof.user.organizationUser.role, userId: prof.userId, diff --git a/packages/server/src/organization/organization.entity.ts b/packages/server/src/organization/organization.entity.ts index eba79f843..0c4acf722 100644 --- a/packages/server/src/organization/organization.entity.ts +++ b/packages/server/src/organization/organization.entity.ts @@ -18,6 +18,7 @@ import { OrganizationSettingsModel } from './organization_settings.entity'; import { OrganizationRoleHistory } from './organization_role_history.entity'; import { OrganizationChatbotSettingsModel } from '../chatbot/chatbot-infrastructure-models/organization-chatbot-settings.entity'; import { AuthStateModel } from '../auth/auth-state.entity'; +import { ProfInviteModel } from 'course/prof-invite/prof-invite.entity'; @Entity('organization_model') export class OrganizationModel extends BaseEntity { @@ -118,4 +119,9 @@ export class OrganizationModel extends BaseEntity { (orgChatbotSettings) => orgChatbotSettings.organization, ) userAuthStates: AuthStateModel; + + @Exclude() + @JoinColumn({ name: 'organizationId' }) + @OneToMany((type) => ProfInviteModel, (profInvite) => profInvite.organization) + profInvites: ProfInviteModel[]; } diff --git a/packages/server/src/organization/organization.service.ts b/packages/server/src/organization/organization.service.ts index f2e1014ee..b80b1b91e 100644 --- a/packages/server/src/organization/organization.service.ts +++ b/packages/server/src/organization/organization.service.ts @@ -13,7 +13,6 @@ import { OrganizationRoleHistoryFilter, OrganizationRoleHistoryResponse, OrgRoleChangeReason, - OrgRoleChangeReasonMap, OrgRoleHistory, OrgUser, Role, @@ -435,10 +434,7 @@ export class OrganizationService { timestamp: history.OrganizationRoleHistory_timestamp, fromRole: history.OrganizationRoleHistory_fromRole, toRole: history.OrganizationRoleHistory_toRole, - changeReason: - OrgRoleChangeReasonMap[ - history.OrganizationRoleHistory_changeReason - ], + changeReason: history.OrganizationRoleHistory_roleChangeReason, toUser: { userId: history.touserid, firstName: history.touserfirstname, diff --git a/packages/server/src/profile/user.entity.ts b/packages/server/src/profile/user.entity.ts index fc631b03c..0122563e9 100644 --- a/packages/server/src/profile/user.entity.ts +++ b/packages/server/src/profile/user.entity.ts @@ -30,6 +30,7 @@ import { LMSAuthStateModel } from '../lmsIntegration/lms-auth-state.entity'; import { LMSAccessTokenModel } from '../lmsIntegration/lms-access-token.entity'; import { UserLtiIdentityModel } from '../lti/user_lti_identity.entity'; import { QueueStaffModel } from 'queue/queue-staff/queue-staff.entity'; +import { ProfInviteModel } from 'course/prof-invite/prof-invite.entity'; @Entity('user_model') export class UserModel extends BaseEntity { @@ -179,4 +180,8 @@ export class UserModel extends BaseEntity { @Exclude() @OneToMany(() => UserLtiIdentityModel, (identity) => identity.user) ltiIdentities: UserLtiIdentityModel[]; + + @OneToMany((type) => ProfInviteModel, (profInvite) => profInvite.adminUser) + @Exclude() + createdProfInvites: ProfInviteModel[]; } diff --git a/packages/server/src/question/question.subscriber.ts b/packages/server/src/question/question.subscriber.ts index d5e52f125..5e0d32e28 100644 --- a/packages/server/src/question/question.subscriber.ts +++ b/packages/server/src/question/question.subscriber.ts @@ -31,7 +31,6 @@ export class QuestionSubscriber dataSource.subscribers.push(this); } - listenTo() { return QuestionModel; } diff --git a/packages/server/test/__snapshots__/prof-invite.integration.ts.snap b/packages/server/test/__snapshots__/prof-invite.integration.ts.snap new file mode 100644 index 000000000..705b3ba3f --- /dev/null +++ b/packages/server/test/__snapshots__/prof-invite.integration.ts.snap @@ -0,0 +1,131 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +exports[`ProfInvite Integration GET /prof_invites/all/:orgId Does NOT return invites from other organizations 1`] = ` +[ + { + "adminUser": { + "email": "admin@a.com", + "id": 1, + "name": "User Person", + }, + "code": Any, + "course": { + "id": 1, + "name": "Course A", + }, + "createdAt": Any, + "expiresAt": Any, + "id": 1, + "makeOrgProf": true, + "maxUses": 1, + "usesUsed": 0, + }, +] +`; + +exports[`ProfInvite Integration GET /prof_invites/all/:orgId List Filtering: Filter by courseId 1`] = ` +{ + "allInvites": [ + { + "adminUser": { + "email": "admin@a.com", + "id": 1, + "name": "User Person", + }, + "code": Any, + "course": { + "id": 2, + "name": "Course B", + }, + "createdAt": Any, + "expiresAt": Any, + "id": 2, + "makeOrgProf": true, + "maxUses": 1, + "usesUsed": 0, + }, + { + "adminUser": { + "email": "admin@a.com", + "id": 1, + "name": "User Person", + }, + "code": Any, + "course": { + "id": 1, + "name": "Course A", + }, + "createdAt": Any, + "expiresAt": Any, + "id": 1, + "makeOrgProf": true, + "maxUses": 1, + "usesUsed": 0, + }, + ], + "filteredInvites": [ + { + "adminUser": { + "email": "admin@a.com", + "id": 1, + "name": "User Person", + }, + "code": Any, + "course": { + "id": 2, + "name": "Course B", + }, + "createdAt": Any, + "expiresAt": Any, + "id": 2, + "makeOrgProf": true, + "maxUses": 1, + "usesUsed": 0, + }, + ], +} +`; + +exports[`ProfInvite Integration GET /prof_invites/all/:orgId Retrieves invites 1`] = ` +[ + { + "adminUser": { + "email": "admin@a.com", + "id": 1, + "name": "User Person", + }, + "code": Any, + "course": { + "id": 1, + "name": "Course A", + }, + "createdAt": Any, + "expiresAt": Any, + "id": 1, + "makeOrgProf": true, + "maxUses": 1, + "usesUsed": 0, + }, +] +`; + +exports[`ProfInvite Integration POST /prof_invites/:orgId Creates an invite and returns it 1`] = ` +{ + "adminUser": { + "email": "admin@a.com", + "id": 1, + "name": "User Person", + }, + "code": Any, + "course": { + "id": 1, + "name": "Course A", + }, + "createdAt": Any, + "expiresAt": Any, + "id": 2, + "makeOrgProf": false, + "maxUses": 10, + "usesUsed": 0, +} +`; diff --git a/packages/server/test/course.integration.ts b/packages/server/test/course.integration.ts index 8171dddb2..580724e9e 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/prof-invite.integration.ts b/packages/server/test/prof-invite.integration.ts new file mode 100644 index 000000000..a82bbc86a --- /dev/null +++ b/packages/server/test/prof-invite.integration.ts @@ -0,0 +1,379 @@ +import { ProfInviteModule } from 'course/prof-invite/prof-invite.module'; +import { + overrideEmailService, + setupIntegrationTest, + expectEmailSent, +} from './util/testUtils'; +import { + CourseFactory, + OrganizationCourseFactory, + OrganizationFactory, + OrganizationUserFactory, + UserFactory, +} from './util/factories'; +import { ProfInviteModel } from 'course/prof-invite/prof-invite.entity'; +import { ProfInviteService } from 'course/prof-invite/prof-invite.service'; +import { + CreateProfInviteParams, + MailServiceType, + OrganizationRole, + QUERY_PARAMS, +} from '@koh/common'; +import { UserModel } from 'profile/user.entity'; +import { OrganizationModel } from 'organization/organization.entity'; +import { CourseModel } from 'course/course.entity'; + +describe('ProfInvite Integration', () => { + const { supertest, getTestModule } = setupIntegrationTest( + ProfInviteModule, + overrideEmailService, + ); + + // used when testing with snapshots to say "hey, don't test for exact values for these" since they're gonna be different each time + const inviteMatcher = { + code: expect.any(String), + createdAt: expect.any(String), + expiresAt: expect.any(String), + }; + + let adminUser: UserModel; // Admin of Org A + let memberUser: UserModel; // Member of Org A + let outsiderUser: UserModel; // Random user + let otherOrgAdmin: UserModel; // Admin of Org B + + let orgA: OrganizationModel; + let orgB: OrganizationModel; + let courseA: CourseModel; + let inviteA: ProfInviteModel; + + let service: ProfInviteService; + + beforeAll(() => { + service = getTestModule().get(ProfInviteService); + }); + + beforeEach(async () => { + jest.clearAllMocks(); + + adminUser = await UserFactory.create({ email: 'admin@a.com' }); + memberUser = await UserFactory.create({ email: 'member@a.com' }); + outsiderUser = await UserFactory.create({ email: 'out@side.com' }); + otherOrgAdmin = await UserFactory.create({ email: 'admin@b.com' }); + + orgA = await OrganizationFactory.create({ name: 'Org A' }); + orgB = await OrganizationFactory.create({ name: 'Org B' }); + + await OrganizationUserFactory.create({ + organizationUser: adminUser, + organization: orgA, + role: OrganizationRole.ADMIN, + }); + await OrganizationUserFactory.create({ + organizationUser: memberUser, + organization: orgA, + role: OrganizationRole.MEMBER, + }); + await OrganizationUserFactory.create({ + organizationUser: otherOrgAdmin, + organization: orgB, + role: OrganizationRole.ADMIN, + }); + + courseA = await CourseFactory.create({ + name: 'Course A', + }); + await OrganizationCourseFactory.create({ + organization: orgA, + course: courseA, + }); + // Create via service to ensure valid defaults/code + inviteA = await service.createProfInvite(orgA.id, courseA.id, adminUser.id); + }); + + describe('Permissions', () => { + const endpoints = [ + { + method: 'get', + path: () => `/prof_invites/all/${orgA.id}`, + desc: 'GET All Invites', + }, + { + method: 'post', + path: () => `/prof_invites/${orgA.id}`, + data: () => + ({ + courseId: courseA.id, + orgId: orgA.id, + }) satisfies CreateProfInviteParams, + desc: 'CREATE Invite', + }, + { + method: 'delete', + path: () => `/prof_invites/${orgA.id}/${inviteA.id}`, + desc: 'DELETE Invite', + }, + ]; + + describe('Unauthorized Access (Non-Admins)', () => { + it.each(endpoints)( + '$desc: Member should get 403', + async ({ method, path, data }) => { + // Member + await supertest({ userId: memberUser.id }) + [method](path()) + .send(typeof data === 'function' ? data() : data) + .expect(403); + }, + ); + it.each(endpoints)( + '$desc: Outsider should get 401', + async ({ method, path, data }) => { + // Outsider + await supertest({ userId: outsiderUser.id }) + [method](path()) + .send(typeof data === 'function' ? data() : data) + .expect(401); + }, + ); + }); + + describe('Cross-Org Security', () => { + it.each(endpoints)( + '$desc: Admin of Org B cannot access Org A', + async ({ method, path, data }) => { + await supertest({ userId: otherOrgAdmin.id }) + [method](path()) + .send(typeof data === 'function' ? data() : data) + .expect(401); + }, + ); + }); + + describe('Org Admin Access', () => { + it('Admin of Org A CAN access these endpoints', async () => { + await supertest({ userId: adminUser.id }) + .get(`/prof_invites/all/${orgA.id}`) + .expect(200); + + // (Need valid payload to avoid 400/500, but 201 proves auth worked) + await supertest({ userId: adminUser.id }) + .post(`/prof_invites/${orgA.id}`) + .send({ + courseId: courseA.id, + orgId: orgA.id, + }) + .expect(201); + + await supertest({ userId: adminUser.id }) + .delete(`/prof_invites/${orgA.id}/${inviteA.id}`) + .expect(200); + }); + }); + }); + + describe('GET /prof_invites/all/:orgId', () => { + it('Retrieves invites', async () => { + const res = await supertest({ userId: adminUser.id }) + .get(`/prof_invites/all/${orgA.id}`) + .expect(200); + + expect(res.body).toHaveLength(1); + const invite = res.body[0]; + expect(invite).toHaveProperty('code'); + expect(invite.course.name).toBe('Course A'); + expect(invite.adminUser.email).toBe('admin@a.com'); + + expect(invite).not.toHaveProperty('organization'); + + expect(res.body).toMatchSnapshot([inviteMatcher]); + }); + + it('List Filtering: Filter by courseId', async () => { + const courseB = await CourseFactory.create({ + name: 'Course B', + }); + await OrganizationCourseFactory.create({ + organization: orgA, + course: courseB, + }); + const inviteB = await service.createProfInvite( + orgA.id, + courseB.id, + adminUser.id, + ); + + // No courseId given - all invites + const res1 = await supertest({ userId: adminUser.id }) + .get(`/prof_invites/all/${orgA.id}`) + .expect(200); + expect(res1.body).toHaveLength(2); + + // Filter for Course B + const res2 = await supertest({ userId: adminUser.id }) + .get(`/prof_invites/all/${orgA.id}?courseId=${courseB.id}`) + .expect(200); + expect(res2.body).toHaveLength(1); + expect(res2.body[0].id).toBe(inviteB.id); + + const responses = { + allInvites: res1.body, + filteredInvites: res2.body, + }; + expect(responses).toMatchSnapshot({ + allInvites: [inviteMatcher, inviteMatcher], + filteredInvites: [inviteMatcher], + }); + }); + + it('Does NOT return invites from other organizations', async () => { + // Create invite in Org B + const courseOrgB = await CourseFactory.create(); + await OrganizationCourseFactory.create({ + organization: orgB, + course: courseOrgB, + }); + await service.createProfInvite(orgB.id, courseOrgB.id, otherOrgAdmin.id); + + const res = await supertest({ userId: adminUser.id }) + .get(`/prof_invites/all/${orgA.id}`) + .expect(200); + + // Should only see Org A invites + expect(res.body).toHaveLength(1); + expect(res.body[0].id).toBe(inviteA.id); + + expect(res.body).toMatchSnapshot([inviteMatcher]); + }); + }); + + describe('POST /prof_invites/:orgId', () => { + it('Creates an invite and returns it', async () => { + const res = await supertest({ userId: adminUser.id }) + .post(`/prof_invites/${orgA.id}`) + .send({ + courseId: courseA.id, + orgId: orgA.id, + maxUses: 10, + makeOrgProf: false, + }) + .expect(201); + + expect(res.body.maxUses).toBe(10); + expect(res.body.makeOrgProf).toBe(false); + expect(res.body.course.id).toBe(courseA.id); + + expect(res.body).toMatchSnapshot(inviteMatcher); + + const dbInvite = await ProfInviteModel.findOneBy({ id: res.body.id }); + expect(dbInvite).toBeDefined(); + }); + }); + + describe('DELETE /prof_invites/:orgId/:piid', () => { + it('Deletes the invite', async () => { + await supertest({ userId: adminUser.id }) + .delete(`/prof_invites/${orgA.id}/${inviteA.id}`) + .expect(200); + + const dbInvite = await ProfInviteModel.findOneBy({ id: inviteA.id }); + expect(dbInvite).toBeNull(); + }); + }); + + describe('GET /prof_invites/details/:piid', () => { + it('Public access: returns simple details', async () => { + // Unauthenticated request + const res = await supertest() + .get(`/prof_invites/details/${inviteA.id}`) + .expect(200); + + expect(res.body).toEqual({ + courseId: courseA.id, + orgId: orgA.id, + }); + }); + + it('Returns 404 for invalid ID', async () => { + await supertest().get(`/prof_invites/details/99999`).expect(404); + }); + }); + + describe('POST /prof_invites/accept/:piid', () => { + it('Accepts invite and redirects', async () => { + const res = await supertest({ userId: memberUser.id }) + .post(`/prof_invites/accept/${inviteA.id}`) + .send({ code: inviteA.code }) + .expect(201); // Controller returns string, Nest wraps in 201 by default for POST + + const params = new URLSearchParams({ + notice: QUERY_PARAMS.profInvite.notice.inviteAccepted, + }); + expect(res.text).toBe(`/course/${courseA.id}?${params.toString()}`); + }); + + it('Email Integration: Full flow with failure then success', async () => { + await supertest({ userId: memberUser.id }) + .post(`/prof_invites/accept/${inviteA.id}`) + .send({ code: 'BAD_CODE' }) + .expect(201); + let inviteAWithFullRelations = await ProfInviteModel.findOne({ + where: { id: inviteA.id }, + relations: { + // needs extra relations for email construction + course: true, + adminUser: { + organizationUser: true, + }, + }, + }); + const userAWithFullRelations = await UserModel.findOne({ + where: { id: memberUser.id }, + relations: { + courses: true, + organizationUser: true, + }, + }); + + const failureEmail = service.constructWrongCodeEmail( + userAWithFullRelations, + inviteAWithFullRelations, + 'BAD_CODE', + ); + expectEmailSent( + [adminUser.email], + [MailServiceType.ADMIN_NOTICE], + failureEmail.subject, + failureEmail.content, + ); + jest.clearAllMocks(); + + await supertest({ userId: memberUser.id }) + .post(`/prof_invites/accept/${inviteA.id}`) + .send({ code: inviteA.code }) + .expect(201); + inviteAWithFullRelations = await ProfInviteModel.findOne({ + // construct email with newest data + where: { id: inviteA.id }, + relations: { + course: true, + adminUser: { + organizationUser: true, + }, + }, + }); + + const successEmail = service.constructSuccessEmail( + userAWithFullRelations, + inviteAWithFullRelations, + inviteA.code, + 0, + ); + expectEmailSent( + [adminUser.email], + [MailServiceType.ADMIN_NOTICE], + successEmail.subject, + successEmail.content, + ); + }); + }); +}); diff --git a/packages/server/test/util/testUtils.ts b/packages/server/test/util/testUtils.ts index cb1e054b5..008b68156 100644 --- a/packages/server/test/util/testUtils.ts +++ b/packages/server/test/util/testUtils.ts @@ -320,10 +320,15 @@ export const mockEmailService = { export const overrideEmailService: ModuleModifier = (builder) => builder.overrideProvider(MailService).useValue(mockEmailService); -/* Takes an array of emails (receivers) and type of email (types)*/ + +/* Takes an array of emails (receivers) and type of email (types). +Also takes in subject and content (optional). If added, will check that they are correct and don't have null, undefined, etc. +*/ export const expectEmailSent = ( receivers: (string | string[])[], types: string[], + subject?: string, + content?: string, ): void => { expect(mockEmailService.sendEmail).toHaveBeenCalledTimes(receivers.length); const calls = mockEmailService.sendEmail.mock.calls.map((args) => args[0]); @@ -338,7 +343,28 @@ export const expectEmailSent = ( ), ), ); + // ensure the content and subject have no "null", "undefined", "[object Object]", "{}" + const forbidden = ['null', 'undefined', 'object Object', '{}']; + if (content) { + calls.forEach((call) => { + forbidden.forEach((term) => { + if (call.content && typeof call.content === 'string') { + expect(call.content).not.toContain(term); + } + }); + }); + } + if (subject) { + calls.forEach((call) => { + forbidden.forEach((term) => { + if (call.subject && typeof call.subject === 'string') { + expect(call.subject).not.toContain(term); + } + }); + }); + } }; + export const expectEmailNotSent = (): void => expect(mockEmailService.sendEmail).not.toHaveBeenCalled();