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

feat: notification #148

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

Aashish-Upadhyay-101
Copy link
Contributor

@Aashish-Upadhyay-101 Aashish-Upadhyay-101 commented Nov 11, 2023

@mfts ready for review, for "document_viewed" event !

Copy link

vercel bot commented Nov 11, 2023

@Aashish-Upadhyay-101 is attempting to deploy a commit to the mftsio Team on Vercel.

A member of the Team first needs to authorize it.

@Aashish-Upadhyay-101 Aashish-Upadhyay-101 marked this pull request as ready for review November 18, 2023 17:21
@mfts mfts added the WTD whatthediff.ai pull request summary label Nov 21, 2023
Copy link
Owner

@mfts mfts left a comment

Choose a reason for hiding this comment

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

Looks really good @Aashish-Upadhyay-101 !
I think this feature will enable a lot of great use cases.
I made in-line notes.

Another thing that I noticed, you currently scoped Notification and Webhook by User. I think we need to scope it by Team instead.
I don't think you need to change the API routes, you just need to request the current teamId from the user inside the API functions.

Let me know if you have any questions about it

lib/types.ts Show resolved Hide resolved
pages/api/notifications/webhooks.ts Outdated Show resolved Hide resolved
pages/api/notifications/webhooks.ts Outdated Show resolved Hide resolved
pages/api/notifications/webhooks.ts Outdated Show resolved Hide resolved
pages/api/notifications/webhooks.ts Outdated Show resolved Hide resolved
prisma/schema.prisma Outdated Show resolved Hide resolved
prisma/schema.prisma Outdated Show resolved Hide resolved
lib/notifications/notification-handlers.ts Outdated Show resolved Hide resolved
prisma/schema.prisma Outdated Show resolved Hide resolved
prisma/schema.prisma Outdated Show resolved Hide resolved
@Aashish-Upadhyay-101 Aashish-Upadhyay-101 marked this pull request as draft December 6, 2023 18:00
@Aashish-Upadhyay-101 Aashish-Upadhyay-101 marked this pull request as ready for review December 7, 2023 19:45
@Aashish-Upadhyay-101
Copy link
Contributor Author

@mfts webhook and notification now have 3 events i.e. document.uploaded, document.deleted and document.viewed.
waiting for your review ; )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WTD whatthediff.ai pull request summary
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants