Skip to content
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

Notification Bug Fix #930

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Notification Bug Fix #930

wants to merge 3 commits into from

Conversation

anchen9
Copy link
Collaborator

@anchen9 anchen9 commented Mar 3, 2025

Summary

Debugging PR based on this ticket. Beginning work on addressing notification bug fixes for browser notifications, sms notifications, delays in notification bell updates, delays from professor -> ta question updates.
Added console statements for frontend and backend to address all listed areas of notification bugs.

  • Currently worked on figuring out notification bell UI update delays --> seems to be that after I toggle hasViewed after clicking off the notification bell, another function or useEffect toggles it back.

Some examples of console logs:
Screenshot 2025-03-02 at 10 29 57 PM
Screenshot 2025-03-02 at 10 30 45 PM

Screenshot 2025-03-02 at 10 31 22 PM
  • Fix Notification bell red dot indicator removal delay
  • Fix Browser notification spamming after TA marked as done
  • Fix lack of SMS notifications
  • Fix delay in update of student view after TA marked question as done

Test Plan

  • Simulate being a TA and a Student and monitor console output and Firebase logs

Notes

  • Fixes still in progress, still determining where the issue is stemming from through console statements

Breaking Changes

None

  • I have updated the documentation accordingly.
  • My PR adds a @ts-ignore

@anchen9 anchen9 requested a review from a team as a code owner March 3, 2025 03:32
@dti-github-bot
Copy link
Member

dti-github-bot commented Mar 3, 2025

[diff-counting] Significant lines: 90.

* and if so, closes the dropdown and calls updateTrackable if the dropdown was open.
*
* @param e The MouseEvent that is triggered by the click action.
*/
Copy link
Contributor

@NIDHI2023 NIDHI2023 Mar 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good documentation - if you find any problems along the way during debugging it would be helpful if you make note of what the issue was

@NIDHI2023 NIDHI2023 changed the title Notification Bug Fix (In Progress) Notification Bug Fix Mar 5, 2025
@NIDHI2023
Copy link
Contributor

Just renaming the title so the WIP check passes lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants