Skip to content

Notification Schema#5

Merged
arnavjk007 merged 18 commits into
mainfrom
notifications-schema
May 6, 2026
Merged

Notification Schema#5
arnavjk007 merged 18 commits into
mainfrom
notifications-schema

Conversation

@emmanishikawa
Copy link
Copy Markdown
Collaborator

@emmanishikawa emmanishikawa commented Feb 10, 2026

Describe your changes

  • Created Notification Schema including id, type, labId, resourceId, recipients, and createdAt date.

Issue ticket number and link

https://docs.google.com/document/d/1cAICVmUirgPTL-fq3dkHVtx4QkyrG0XCJO7VUxnibdA/edit?usp=sharing

Checklist before requesting a review

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • This change requires a documentation update

@emmanishikawa emmanishikawa changed the title created notification schema Created notification schema Feb 10, 2026
@emmanishikawa emmanishikawa changed the title Created notification schema Notification Schema Feb 10, 2026
Comment thread models/Notification.ts Outdated
Comment thread services/notifications/watchUpdates.ts Outdated
Comment thread services/notifications/watchUpdates.ts Outdated
Comment thread services/notifications/watchUpdates.ts
Copy link
Copy Markdown
Collaborator

@arnavjk007 arnavjk007 left a comment

Choose a reason for hiding this comment

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

Just some minor changes

Comment thread models/Notification.ts Outdated
Comment thread models/Notification.ts Outdated
Comment thread models/Notification.ts Outdated
Copy link
Copy Markdown
Collaborator

@arnavjk007 arnavjk007 left a comment

Choose a reason for hiding this comment

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

Small comment change but everything else looks good

Comment thread services/notifications/watchUpdates.ts Outdated
Copy link
Copy Markdown
Collaborator

@arnavjk007 arnavjk007 left a comment

Choose a reason for hiding this comment

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

Small schema type change

Comment thread models/Notification.ts Outdated
Copy link
Copy Markdown
Collaborator

@arnavjk007 arnavjk007 left a comment

Choose a reason for hiding this comment

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

Minor structure change

Comment thread models/Notification.ts
Comment thread models/Notification.ts Outdated
Copy link
Copy Markdown
Collaborator

@arnavjk007 arnavjk007 left a comment

Choose a reason for hiding this comment

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

Looks good! I think the last step is to write a few simple unit tests to make sure that this works. I have already done the setup for the jest testing env in the main branch so just switch to that and do git pull then go back to the feat and do git merge main to get the updated changes.

Copy link
Copy Markdown
Collaborator

@arnavjk007 arnavjk007 left a comment

Choose a reason for hiding this comment

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

More specific testing + ts-node depencency not installed so unit tests are failing


import mongoose from "mongoose";

describe("startNotificationWatcher", () => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we have a simple test that checks if the watcher is triggered over a specific threshold?

@arnavjk007 arnavjk007 merged commit 61fd00d into main May 6, 2026
2 checks passed
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