worthwhile / django-herald Goto Github PK

for delnotifs command with start and end args, doc says:

end is up to, but not including that date.

I'd think if I specify the end date being yesterday without start date, it will delete notifications sent before yesterday.

the actual code is

start_date = options['start'] if options['start'] else today

This will set the start date to be today, and end date to be yesterday, it's going to be an invalid range.

Suggestion:

• if only end date is specified, delete anything sent before date_sent
• if only start date is specified, delete anything sent after date_sent
• if both start and end dates are specified, delete the range

It is impossible to install herald in in an environment that does not have django installed yet. This is because setup.py imports herald.__version__ which then imports from django.utils.module_loading import autodiscover_modules.

I think all we need to do is move the django import down inside the autodiscover function.

As per the readme section, the attachment can be in the form of Django File object.
Now, I have created a Django File object as follows:

    with open('docs/my.pdf', 'r') as f:
        file = File(f)

I am having a hard time figuring out how to use this object in the code to send the email. Anything done outside the with block obviously creates an error as the file is closed.

Here is the full code:

@registry.register_decorator()
class SendThisMail(SomeBaseClass, EmailNotification):
    def __init__(self, user, my_modal: models.MyModal):
        super().__init__(user, my_modal)

        self.subject = "abc"

        with open('docs/my.pdf', 'r') as f:
            file = File(f)

            self.attachments = (
                'abc.pdf', file,
                'application/pdf')

        self.context = {
            'subject': self.subject,
            **self.context,
        }

        self.to_emails = [user.email]

def sendMail():
    SendThisMail(user, my_modal).send(user=my_user) # creates an error on this line as the file object is closed and inaccessible.

This is how you register now:

class MyEmail(EmailNotification):
    ...
registry.register(MyEmail)

Would be nice if you could also do this:

@registry.register
class MyEmail(EmailNotification):
    ...

Hi ,
Have you thought about adding support for push notifications for app.
Basically it would be good to add a template for adding a transmission method.

Thanks
Anand

I've been struggling for days to make it work but the lack of documentation and silent failures made it impossible to debug the issues. It seems that the mails are not being sent if I include an attachment in it.

def sendMail():
    SendThisMail(user, my_modal).send(user=my_user) # creates an error on this line as the file object is closed and inaccessible.

@registry.register_decorator()
class SendThisMail(SomeBaseClass, EmailNotification):
    def __init__(self, user, my_modal: models.MyModal):
        super().__init__(user, my_modal)

        self.subject = "abc"

        file = open('.staticfiles/assets/some.pdf', 'rb')

        self.attachments = [('attachment_1', File(file))]

        self.context = {
            **self.context,
            'subject': self.subject,
            'attachment': self.attachments,
        }

        self.to_emails = [user.email]

What's wrong with it?

Would require setting up celery tasks

At the moment, Herald is squarely a Django package. However, there may be some benefit in making it less dependent on Django specifically and more dependent on having specific components that adhere to Django's API interface for various things.

Should this repo be forked to work on that, or is there openness to supporting this as a long-running branch?

Right now, you are forced to make sure your templates are in the following directory structure: herald/html/{notifcation_name}.html or herald/text/{notifcation_name}.txt.

First of all, that should definitely be overridable. Maybe rename NotificationBase.template_name to NotificationBase.template_base_name to avoid confusion, then add the ability to define absolute template paths per render_type. (but the current default would still remain). like the following:

class MyEmail(NotificationBase):
    template_names = {
        'text': 'my/custom/template/path.txt',
        'html': 'my/custom/template/path.html'
    }

Secondly, is the current default even a good default? Rather than herald/html/{notifcation_name}.html or herald/text/{notifcation_name}.txt should it maybe be {app name}/{notifcation_name}.html or {app name}/{notifcation_name}.txt similar to how the django generic class based views find templates by default (for example, {app_name}/{model_name}_create.html is the default for CreateViews.)

Could be readthedocs, etc.

I'm trying to send an email to a group of users with the same template and context and they can't be in the same message
so I tried using bcc only with to_emails = []
but it's not sending any emails
here is the init:

 def __init__(self, obj, emails):
        self.context = {'obj': obj}
        self.bcc = emails
        self.to_emails = []

is there any way to make this work?

Hello! I ask for help! It’s not clear from the description how to set up an email account from where messages will be sent. For example, I have a gmail mailbox. How do I send from there? Where can I set login parameters for my account? Thank!

I'm wondering how you would recommended using django-herald so that notifications are sent via both SMS and email.

The users in my system will always have an email address, and 90% of them will also have a mobile phone number; both of these are set on my 'User' model. I will have many different types of notifications, and it would be good if I could define them once and have the notification always sent via email, and also via SMS if the user has a mobile phone number set. I plan to use Twilio for sending SMS messages.

Ideally I would want to be able to define the message/content of the notification in just one one place too, with the SMS message being just this plain text message, while the email gets rendered into a HTML email template with the message rendered in this template.

I'm trying to figure out the best way to approach this so that I don't have to create 2 copies of every notification type (one for email and one for SMS) and trigger 2 notifications every time I want to send a notification to a user. Any recommendations?

I'm building an application that allows users to customize email templates before sending out messages to users.

Given that the docs only allow for specifying a directory for the email template, is there a way to utilize this library for pulling a template record directly from the database instead?

When viewing the HTML content of an Email class, the encoding is not properly set (utf8 in my case) so the text is garbled.

In text view, it's the content-type which seems abnormal, text/text should be text/plain, so the content gets downloaded as unknown instead of displayed as raw text.

@raiderrobert @georgmzimmer @mands

We are sprinting on Django-Herald at DjangoCon 2022 latter this week. Could one of you make peterfarrell-atdsaa a maintainer on PyPi so we can make a release?

https://pypi.org/project/django-herald/

If we don't hear from you, we will fork this project to django-heralder and consider the efforts on this repo / PyPi no longer maintained.

@raiderrobert I was wondering if we could get a release of 0.3.0 with the new HTML2Text support pushed to PyPI?

Document TwilioTextNotification class and configuration

TwilioTextNotification().send() causes AssertionError: Invalid Render Type

The EmailNotification class put the subject in the context when get_context_data is called. However, fetching the subject from self.subject can be incorrect if the developer has implemented and overrides the get_subject method. PR forth coming.

Add support for slack as a backend

Right now, each notification class basically does it all. It specifies WHAT to send and HOW to send it. I think it would be a much more flexible API if the notification classes just specified WHAT to send. Then we would have different backends that could be specified on the notification that tell HOW to send it.

That way you could, for example, have one notification that gets sent a bunch of different ways:

class BlogPostLikedNotification(Notification):
    backends = [EmailBackend, TwillioTextBacked, APNSBackend, GCMBackend]

Of course, that will come with a bunch of complications. Each backend will need separate info for who to send to, (the push notification backends need device identifiers, the twilio backend needs a phone number, and the email backend needs an email, etc.) and what to send. (The email backend needs html, the others just plain text, etc)

Something to think about... would of course be a huge refactor.

Would be nice if herald could provide a "view in browser" link so that users can view the email in a browser like most email services do like mail chimp, etc.

Since the email html is cached in the database, it shouldn't be too difficult to add that feature.

The Notification model has a field notification_class = models.CharField(max_length=80, unique=True). This max_length restriction of 80 characters is causing an issue for me, because I'm trying to create a notification class that happens to be 83 characters long, so I'm getting an error django.db.utils.DataError: (1406, "Data too long for column 'notification_class' at row 1").

The SentNotification model also has a field notification_class, but with a less restrictive max_length of 255 characters.

Is there a reason why the Notification model has an inconsistent and much more restrictive max_length for its notification_class field than that of the SentNotification model? If not, can we increase it to match the SentNotification model, otherwise you have to artificially limit the names of your notification classes in order to fit within this restriction.

Hello everyone! Great project!

I'm using it in production for one our SaaS and it's working nicely.

One of the things I've just learned about today is https://pypi.python.org/pypi/html2text.

There is a big hassle in creating and maintaining two sets of templates, one for html and one for txt. Perhps we could integrate with html2text and auto-generate the text emails, when the txt template is not available?

I can contribute, but I would like to know how you feel about that, before starting it.

default_app_config was deprecated in Django 3.2 . Given this project now only supports a minimum of Django 3.2 the default_app_config should be removed.

Note, I came across this because Django was emitting a warning. I was able to silence this warning by changing my INSTALLED_APPS from "herald" to "herald.apps.HeraldConfig".

Django allows passing in the connection param to use single connection for sending multiple emails. It'll be helpful for django-herald to add the support

The issue occurs when herald.contrib.auth is added to INSTALLED_APPS. The issue only occurs in django versions 3.2 or above.
Here's the traceback when running the tests:

Traceback (most recent call last):
  File "/Users/apoorva/Desktop/django-herald/runtests.py", line 14, in <module>
    runtests()
  File "/Users/apoorva/Desktop/django-herald/runtests.py", line 10, in runtests
    execute_from_command_line(argv)
  File "/Users/apoorva/Desktop/venvdjango3/lib/python3.9/site-packages/django/core/management/__init__.py", line 419, in execute_from_command_line
    utility.execute()
  File "/Users/apoorva/Desktop/venvdjango3/lib/python3.9/site-packages/django/core/management/__init__.py", line 395, in execute
    django.setup()
  File "/Users/apoorva/Desktop/venvdjango3/lib/python3.9/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/Users/apoorva/Desktop/venvdjango3/lib/python3.9/site-packages/django/apps/registry.py", line 91, in populate
    app_config = AppConfig.create(entry)
  File "/Users/apoorva/Desktop/venvdjango3/lib/python3.9/site-packages/django/apps/config.py", line 255, in create
    return app_config_class(app_name, app_module)
  File "/Users/apoorva/Desktop/venvdjango3/lib/python3.9/site-packages/django/apps/config.py", line 38, in __init__
    raise ImproperlyConfigured(
django.core.exceptions.ImproperlyConfigured: The app label 'herald.contrib.auth' is not a valid Python identifier.

I wished someone could document this plugin. Is the cc field also accepts array similar to the to_emails field?