-
Notifications
You must be signed in to change notification settings - Fork 4
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
Discord webhooks #69
base: v2
Are you sure you want to change the base?
Discord webhooks #69
Conversation
CreedsCode
commented
Feb 20, 2024
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
embeds: [embed], | ||
}; | ||
|
||
fetch(webhookDetails?.discordWebhook, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be good to seperate the preparation of the content and the fetch to not duplicate the whole part here and add some more strict typing
// }, | ||
}; | ||
|
||
const payload = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need typing
prisma/schema.prisma
Outdated
@@ -15,6 +15,7 @@ model BuilderPlace { | |||
cover String? | |||
customDomain String? @unique | |||
icon String? | |||
discordWebhook String? @default("") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
discordWebhookUrl seems more clear
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also need to generate your db migration
@@ -392,6 +393,36 @@ export const getBuilderPlaceById = async (id: string) => { | |||
} | |||
}; | |||
|
|||
|
|||
export const getDiscordWebhookDetailsByBuilderPlaceId = async (id: string) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for maintanability, i think it's good to not create a new get function everytime we want some info on a BP. We can use a single getById that returns all the datas
@@ -574,6 +605,11 @@ export const addBuilderPlaceCollaborator = async (body: AddBuilderPlaceCollabora | |||
export const updateBuilderPlace = async (builderPlace: UpdateBuilderPlace) => { | |||
let errorMessage; | |||
try { | |||
|
|||
if (builderPlace.discordWebhook != "" && builderPlace.discordWebhook != undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 things here:
- both conditon are falsy, so just do: if (builderPlace.discordWebhook)
- it's better to move this check logical inside your function, else you have to duplicate it everywhere we need to use it
|
||
|
||
export const sendNewServiceNotification = async (builderPlaceId: string, serviceId: string, cid: string) => { | ||
const service: { title: string, about: string, keywords: string, rateToken: string, rateAmount: string } = await readFromIPFS(cid); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in term of architecture, we now make any create service API post success depending on an IPFS call, and discord call. This is not good for maintability. Would be better to imagine something non bloquant with a message broker by example. Any suggestion?
}, | ||
{ | ||
name: "Budget", | ||
value: `${(Number(service.rateAmount) / Math.pow(10, token.decimals)).toString()} ${token.symbol}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have helper normally for that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i could not find one, i tried searching the code base for "Math.pow" and nothing come up.