-
Notifications
You must be signed in to change notification settings - Fork 2
Add Notification Feed for per-course-Alerts #414
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
d0cefc1
a5955f2
df5f5c3
4ae661e
f78c9cf
bfd7870
4ea701e
a4b5c52
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2230,6 +2230,25 @@ export enum AlertType { | |
| REPHRASE_QUESTION = 'rephraseQuestion', | ||
| EVENT_ENDED_CHECKOUT_STAFF = 'eventEndedCheckoutStaff', | ||
| PROMPT_STUDENT_TO_LEAVE_QUEUE = 'promptStudentToLeaveQueue', | ||
| DOCUMENT_PROCESSED = 'documentProcessed', | ||
| ASYNC_QUESTION_UPDATE = 'asyncQuestionUpdate', | ||
| } | ||
|
|
||
| // Allowed combinations to enforce front/back consistency | ||
| export const FEED_ALERT_TYPES: AlertType[] = [ | ||
| AlertType.DOCUMENT_PROCESSED, | ||
| AlertType.ASYNC_QUESTION_UPDATE, | ||
| ] | ||
|
|
||
| export const MODAL_ALERT_TYPES: AlertType[] = [ | ||
| AlertType.REPHRASE_QUESTION, | ||
| AlertType.EVENT_ENDED_CHECKOUT_STAFF, | ||
| AlertType.PROMPT_STUDENT_TO_LEAVE_QUEUE, | ||
| ] | ||
|
|
||
| export enum AlertDeliveryMode { | ||
| MODAL = 'modal', | ||
| FEED = 'feed', | ||
| } | ||
|
|
||
| export class AlertPayload {} | ||
|
|
@@ -2238,12 +2257,19 @@ export class Alert { | |
| @IsEnum(AlertType) | ||
| alertType!: AlertType | ||
|
|
||
| @IsEnum(AlertDeliveryMode) | ||
| deliveryMode!: AlertDeliveryMode | ||
|
|
||
| @IsDate() | ||
| sent!: Date | ||
|
|
||
| @Type(() => AlertPayload) | ||
| payload!: AlertPayload | ||
|
|
||
| @IsOptional() | ||
| @IsDate() | ||
| readAt?: Date | ||
|
|
||
| @IsInt() | ||
| id!: number | ||
| } | ||
|
|
@@ -2266,6 +2292,35 @@ export class PromptStudentToLeaveQueuePayload extends AlertPayload { | |
| queueQuestionId?: number | ||
| } | ||
|
|
||
| export class DocumentProcessedPayload extends AlertPayload { | ||
| @IsInt() | ||
| documentId!: number | ||
|
|
||
| @IsString() | ||
| documentName!: string | ||
| } | ||
|
|
||
| export class AsyncQuestionUpdatePayload extends AlertPayload { | ||
| @IsInt() | ||
| questionId!: number | ||
|
|
||
| @IsInt() | ||
| courseId!: number | ||
|
|
||
| @IsString() | ||
| @IsOptional() | ||
| subtype?: | ||
| | 'commentOnMyPost' | ||
| | 'commentOnOthersPost' | ||
| | 'humanAnswered' | ||
| | 'statusChanged' | ||
| | 'upvoted' | ||
|
Comment on lines
+2310
to
+2317
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this (and all references to it) should 100% be put into an enum |
||
|
|
||
| @IsString() | ||
| @IsOptional() | ||
| summary?: string | ||
| } | ||
|
|
||
| export class OrganizationCourseResponse { | ||
| @IsInt() | ||
| id?: number | ||
|
|
@@ -2296,6 +2351,10 @@ export class CreateAlertParams { | |
| @IsEnum(AlertType) | ||
| alertType!: AlertType | ||
|
|
||
| @IsOptional() | ||
| @IsEnum(AlertDeliveryMode) | ||
| deliveryMode?: AlertDeliveryMode | ||
|
|
||
| @IsInt() | ||
| courseId!: number | ||
|
|
||
|
|
@@ -2311,6 +2370,9 @@ export class CreateAlertResponse extends Alert {} | |
| export class GetAlertsResponse { | ||
| @Type(() => Alert) | ||
| alerts!: Alert[] | ||
| @IsOptional() | ||
| @IsInt() | ||
| total?: number | ||
| } | ||
|
|
||
| // not used anywhere | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -7,6 +7,7 @@ import { | |||||
| UndoOutlined, | ||||||
| } from '@ant-design/icons' | ||||||
| import { | ||||||
| AlertDeliveryMode, | ||||||
| AlertType, | ||||||
| ClosedQuestionStatus, | ||||||
| ERROR_MESSAGES, | ||||||
|
|
@@ -159,6 +160,7 @@ const TAQuestionCardButtons: React.FC<TAQuestionCardButtonsProps> = ({ | |||||
| courseId, | ||||||
| payload, | ||||||
| targetUserId: question.creatorId, | ||||||
| deliveryMode: AlertDeliveryMode.FEED, | ||||||
ribhavsharma marked this conversation as resolved.
Show resolved
Hide resolved
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| }) | ||||||
| // await mutateQuestions() | ||||||
| message.success('Successfully asked student to rephrase their question.') | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,7 @@ import { Popconfirm } from 'antd' | |
| import { sortQueues } from '../(dashboard)/course/[cid]/utils/commonCourseFunctions' | ||
| import { useCourseFeatures } from '../hooks/useCourseFeatures' | ||
| import CenteredSpinner from './CenteredSpinner' | ||
| import NotificationBell from './NotificationBell' | ||
| import Image from 'next/image' | ||
| import { useOrganizationSettings } from '@/app/hooks/useOrganizationSettings' | ||
|
|
||
|
|
@@ -355,6 +356,11 @@ const NavBar = ({ | |
| ) : null} | ||
| {/* DESKTOP ONLY PART OF NAVBAR */} | ||
| <NavigationMenuItem className="!ml-auto hidden md:block"> | ||
| <div className="px-2"> | ||
| <NotificationBell /> | ||
ribhavsharma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| </div> | ||
| </NavigationMenuItem> | ||
| <NavigationMenuItem className="hidden md:block"> | ||
| <NavigationMenuTrigger | ||
| className={`!pl-4 ${isProfilePage ? 'md:border-helpmeblue md:border-b-2' : ''}`} | ||
| onFocus={setNavigationSubMenuRightSide} | ||
|
|
@@ -509,6 +515,9 @@ const HeaderBar: React.FC = () => { | |
| </h2> | ||
| )} | ||
| </div> | ||
| <div className="flex items-center"> | ||
| <NotificationBell /> | ||
| </div> | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the bell on mobile should be moved since it makes the page title/subtitle off-centre (also, for long course/queue names or on phones with increased text size, it may cause the title text to overflow/wrap easier which looks really bad) Proposed solution: Place an antd Badge on the hamburger menu itself Place the notification bell somewhere in this drawer (though ngl I'm having some trouble thinking of an area to place it. Can't place it at the top since the university name can be longer. Can't place it beside the user's name since their name can be longer) one optionPerhaps we can place it in-between the profile and logout buttons, similar to the desktop solution I proposed) But once the user clicks on it... I'm not sure what should happen, all the options I can think of are kinda gross:
another optionJust say frick it and fit the notification bell up top where the university name is.
This might get kinda iffy for future orgs, but at least now you can make a proper dropdown submenu. It is also a little more hidden up there imo so people might have a little more trouble finding it (at least in OC's case where the OC logo has a similar colour to the red "1" for notifications), but overall I think this is the better option.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i agree with hiding it in the mobile sidebar menu thing though bc of the lack of usable space on mobile
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is still not resolved
While it doesn't look that terrible, I have a few issues with it:
So I think it should be moved into the navigation menu beside the organization logo, as stated in "another option" above. Also don't forget to place the Badge on the hamburger icon too |
||
| <Drawer | ||
| direction="left" | ||
| open={isDrawerOpen} | ||
|
|
||






Uh oh!
There was an error while loading. Please reload this page.