Skip to content
This repository has been archived by the owner on Dec 26, 2021. It is now read-only.

Add notifications #51

Merged
merged 61 commits into from
Jun 25, 2021
Merged

Add notifications #51

merged 61 commits into from
Jun 25, 2021

Conversation

girardinsamuel
Copy link
Contributor

@girardinsamuel girardinsamuel commented Feb 21, 2021

Fixes #32

Specs

  • mail notification
  • slack notifications
  • database driver to save notifications in database
  • broadcast driver to broadcast notifications
  • vonage driver (ex Nexmo) to send SMS notifications
  • add a Notifiable mixin to set app models notifiable (such as User).
  • add a way to send notifications to not Notifiable entity aka AnonymousUsers
  • be able to make notification quickly as we can make mailable
  • built so that adding a new notification driver internally (or through an external package) can be done easily

Documentation

Doc has been written here MasoniteFramework/docs#131, so most of it could be taken from this PR when writing M4 docs 🚀

TLDR

We can create a notification with craft notification Welcome.
We can send it via the different drivers either by resolving notification:

# send to anonymous entities (not a model)
self.application.make("notification")
    .route("mail", "[email protected]")
    .route("slack", "#general")
    .send(WelcomeNotification())

or by sending to a Notifiable entity (such as User):

class User(Model, Notifiable):

     def route_notification_for_mail(self):
          return self.email
          # or return [self.pro_email, self.personal_email]
   
     def route_notification_for_slack(self):
          return self.user_channel_id
# ..
user.notify(WelcomeNotification())

A notification is built this way:

class WelcomeNotification(Notification):
    def to_mail(self, notifiable):
         return Mailable().from_(..).text("Welcome !)
    
    def to_slack(self, notifiable):
         return SlackMessage().from("Sam").text("Welcome !)

    def via(self, notifiable):
         return ["mail", "slack"]

meaning that for each driver we should define a to_{driver} explaining how to build the notification. We have access to the notifiable here (e.g. our user instance for non anonymous notifications).
The via() explains through which drivers it should be sent. You can run any logic you want here because you have access to notifiable ! Of course when sending notification, it can be overriden :

user.notify(drivers=[“mail"], WelcomeNotification())

If you want to queue the notification sending operation you can subclass the notification with ShouldQueue, that's it. For each driver, the sending job will be pushed onto the queue.

If you want to store notifications in database also, it's done as a driver also. You should first create the notifs table craft notification:table. The default name is notifications but can be overriden in the config. Then update your notification:

class WelcomeNotification(Notification):
    # ...
    def to_database(self, notifiable):
          # define here what data will be serialized into database
          return {"data": "Welcome {notifiable.name}"}
    
    def via(self, notifiable):
         return ["mail", "slack", "database"]

then

user.notify(WelcomeNotification())
# and you can acccess
user.notifications.count() # == 1
user.read_notifications()
user.unread_notifications()
notification.mark_as_read()
notification.mark_as_unread()

Adding new notification channels/drivers is really easy, all we need is to create a new driver and register it. It should implement send() and queue() methods and can use config/notifications.py to store its own config.

TODO

  • globally defined dry mode
  • notification mocks for testing as done for others
  • finish adding drivers
  • add integrations tests with notification to different channels
  • add command to create notification
  • finish adding queuing for all drivers (postponed)
  • find a solution to be able to override mail driver used in mail notification

src/masonite/drivers/notification/DatabaseDriver.py Outdated Show resolved Hide resolved
src/masonite/drivers/notification/MailDriver.py Outdated Show resolved Hide resolved
src/masonite/drivers/notification/vonage/Sms.py Outdated Show resolved Hide resolved
src/masonite/notification/Notification.py Outdated Show resolved Hide resolved
src/masonite/notification/NotificationManager.py Outdated Show resolved Hide resolved
@josephmancuso josephmancuso mentioned this pull request Mar 2, 2021
@josephmancuso
Copy link
Member

fixed the tests here

@josephmancuso
Copy link
Member

@girardinsamuel I modified this a bit.

Now a notification class takes a basic Notification class it did before but that class now does really nothing but contains some of those utility methods (like should_send and ignore_errors).

Now a notification class must inherit from different utility classes to do things. To send an email it will inherit from the normal Mailable class to build a mailable.

It can also inherit from a Textable class to send those Vonage text messages.

This is an example of a notification class:

class OneTimePassword(Notification, Mailable, Textable):

    def to_mail(self, notifiable):
        return (
            self
            .to(notifiable.email)
            .subject("Masonite 4")
            .from_("[email protected]")
            .text(f"Hello {notifiable.name}")
        )

    def to_vonage(self, notifiable):
        return self.text_message("Welcome !").to('6314870798').from_("33123456789")

    def via(self, notifiable):
        return ["vonage"]

Notice it inherits from a view different classes

Copy link
Member

@josephmancuso josephmancuso left a comment

Choose a reason for hiding this comment

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

Did another complete code review. Also manually tested this feature and with some changes i already did i think it works really well. Before i do more manual testing i just did another code review where we need to talk about a few things i commented on.

Also need to remove the queuing stuff. I think we'll have to revisit that in the future

There is also some TODO we need to address.

Once those changes are made i can retest some of the other features like the database notifications and broadcast drivers

src/masonite/notification/NotificationManager.py Outdated Show resolved Hide resolved
src/masonite/notification/NotificationManager.py Outdated Show resolved Hide resolved
src/masonite/notification/NotificationManager.py Outdated Show resolved Hide resolved
src/masonite/notification/drivers/BaseDriver.py Outdated Show resolved Hide resolved
src/masonite/notification/drivers/BroadcastDriver.py Outdated Show resolved Hide resolved
src/masonite/notification/drivers/SlackDriver.py Outdated Show resolved Hide resolved
src/masonite/notification/drivers/vonage/VonageDriver.py Outdated Show resolved Hide resolved
src/masonite/notification/drivers/vonage/VonageDriver.py Outdated Show resolved Hide resolved
tests/integrations/config/notification.py Show resolved Hide resolved
@girardinsamuel
Copy link
Contributor Author

I will take a look and make changes beginning of next week !

@josephmancuso josephmancuso merged commit e0f23db into master Jun 25, 2021
@girardinsamuel
Copy link
Contributor Author

Yeaaahhhhh 🚀

@girardinsamuel girardinsamuel deleted the feat/32 branch June 25, 2021 22:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add notifications
2 participants