search

notifo-io / notifo

Multi channel notification service for collaboration tools, e-commerce, news service and more.
MIT License
746 stars 70 forks source link

Chat / Collaboration Channel #38

Open SebastianStehle opened 3 years ago

SebastianStehle commented 3 years ago

Channel for collaboration tools like Discord, Slack, Teams.

Arciiix commented 3 months ago

Hey @SebastianStehle

Hope you're all doing well! I recently came across your project and I'm really impressed by the work you've done, and I'm excited about the possibility of contributing to it.

I'm particularly interested in creating this Discord integration, but I'm open to helping out wherever needed. I'm a newbie when it comes to contributing to open source, that's why I'm eager to learn and collaborate with this wonderful community.

Is there any change my work might be merged? Or do you have any specific guidelines when it comes to contributing? Cheers

SebastianStehle commented 3 months ago

Absolutely, sounds like a great idea to work on that.

Arciiix commented 3 months ago

Great! Thank you so much! Do you have any specific requirements when it comes to this integration?

SebastianStehle commented 3 months ago

I think what the integration makes special is that you probably need an additional configuration to select a sub-channel. For example discord, teams, slack and so on.

Either you push to all channels where the user has access to or only to selected sub-channels.

This concept of sub-channels does not exist yet.

Arciiix commented 3 months ago

So for example with the Discord integration - it works similarly to the Telegram and Webhook integration - there's one bot per app. And for example when sending a notification, in the second (Channels) tab - besides setting a template, user can select the Discord guild's channel(s) to send the notification to, right?

SebastianStehle commented 3 months ago

The channels are just different communication methods that have a common behavior:

For messaging we give each extension the opportunity to add targets to the job. Depending on the values that the user has. So if the user would have a Discord username (or other identifier) we would send the notification to this identifier.

Perhaps we can just add the discord integration under the messaging channel? I think this should work fine.

Arciiix commented 3 months ago

Okay, thank you for your reply! Will try to start working on it in my free time, starting from tomorrow. I'll keep you updated and ask you here if I have any questions. Have a nice day!

Arciiix commented 3 months ago

I started working on the integration.

For the bot to be able to send messages to the user on Discord via DMs (direct messages), they either have to share the same guild (also called server) or (and that's a new feature implemented recently) user has to "install" the app/bot on their account. Therefore, I'd add the field on the user in Notifo to enter the Discord's user id (or channel id inside the guild?) + maybe provide the bot installation URL there? It looks the following: https://discord.com/oauth2/authorize?client_id=<client_id>

Users just have to confirm "installing" the bot on their accounts. It can be also used to add the bot to the guild.

User-installable apps will allow to DM the users without sharing the same guild, but on the other hand if we also implement an option to post on a given channel inside the guild and the user is in the guild with the bot, they don't have to "install the app" on their accounts (unless some privacy settings).

What do you think? Should I implement the user-installable app or just require the bot to share a common guild with every user?

More about user-installable apps: https://discord.com/developers/docs/change-log#userinstallable-apps-preview

SebastianStehle commented 3 months ago

I would start with the simple approach. But you do not have to add a property to the user.

e.g. have a look at telegram: https://github.com/notifo-io/notifo/blob/main/backend/src/Notifo.Domain.Integrations/Telegram/TelegramIntegration.cs#L45

The integration registers user properties and then sues them later: https://github.com/notifo-io/notifo/blob/main/backend/src/Notifo.Domain.Integrations/Telegram/TelegramIntegration.Messaging.cs#L139

Arciiix commented 3 months ago

Also, I just discovered a few bugs not related to my integration, e.g. the user properties aren't saved on user upsert (to reproduce - add any integration that has user properties (e.g. Telegram) and try to update any user property (here: chat id) for any user). Should I first create an issue here on GitHub and resolve it or directly make the pull request?

Sorry for asking a lot of questions, but as I said it's my first real open source contribution and I'd like to take it seriously and do my best 😉

SebastianStehle commented 3 months ago

Ideally you keep pull requests small and focuses. This is not always possible.

You can create an issue first, but in my opinion it is not needed if solve the problem immediately.