Skip to content

Send email digest for activities in Hypha for org level staff - #2991

Merged
frjo merged 9 commits into
mainfrom
feature/theskumar/email-notifications
Oct 19, 2022
Merged

Send email digest for activities in Hypha for org level staff#2991
frjo merged 9 commits into
mainfrom
feature/theskumar/email-notifications

Conversation

@theskumar

@theskumar theskumar commented Sep 26, 2022

Copy link
Copy Markdown
Member

Fixes #2882

This feature complements or provides an alternative to the slack notifications sent by the system to organization staff members.

  • 🏃🏼 management command to prepare and send the digest as an email python manage.py send_staff_email_digest
  • add email settings ACTIVITY_DIGEST_RECIPIENTS_EMAILS
  • Capture events of the last seven days only.
  • Port all the staff messages sent to staff to the staff notification adaptor

Screenshot 2022-10-06 at 11 06 46@2x

Screenshot 2022-10-06 at 11 08 47@2x

  • add mdmail dependency
  • add core mail send utils to wrap Django’s send_mail, which takes markdown or text as input.
  • add new ACTIVITY_DIGEST_RECIPIENT_EMAILS setting

@theskumar theskumar changed the title Add activities email digest for org level staff Send email digest for activities in Hypha for org level staff Sep 27, 2022
@theskumar
theskumar force-pushed the feature/theskumar/email-notifications branch 2 times, most recently from d00f9a5 to 5fb4455 Compare September 30, 2022 05:18
@theskumar
theskumar force-pushed the feature/theskumar/email-notifications branch 3 times, most recently from 6a4b4ba to 8532fdd Compare October 6, 2022 11:55
@theskumar
theskumar marked this pull request as ready for review October 6, 2022 11:56
@theskumar
theskumar requested a review from frjo October 6, 2022 11:57
@theskumar

Copy link
Copy Markdown
Member Author

@fourthletter please review the email format and copy

@codecov

codecov Bot commented Oct 6, 2022

Copy link
Copy Markdown

Codecov Report

Base: 80.34% // Head: 80.46% // Increases project coverage by +0.11% 🎉

Coverage data is based on head (124a45d) compared to base (58cfea3).
Patch coverage: 89.65% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2991      +/-   ##
==========================================
+ Coverage   80.34%   80.46%   +0.11%     
==========================================
  Files         378      378              
  Lines       18390    18359      -31     
==========================================
- Hits        14776    14772       -4     
+ Misses       3614     3587      -27     
Impacted Files Coverage Δ
hypha/apply/activity/adapters/slack.py 88.00% <50.00%> (-0.52%) ⬇️
hypha/apply/activity/models.py 94.16% <50.00%> (-2.33%) ⬇️
hypha/apply/api/v1/serializers.py 75.22% <50.00%> (-0.12%) ⬇️
hypha/apply/users/wagtail_hooks.py 61.11% <50.00%> (ø)
hypha/apply/activity/adapters/emails.py 83.91% <75.00%> (-0.37%) ⬇️
hypha/apply/activity/options.py 100.00% <100.00%> (ø)
.../apply/activity/templates/messages/email/base.html 100.00% <100.00%> (ø)
hypha/apply/funds/models/submissions.py 91.27% <100.00%> (ø)
hypha/apply/funds/templatetags/markdown_tags.py 100.00% <100.00%> (ø)
hypha/core/utils.py 100.00% <100.00%> (ø)
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@frjo frjo added Type: Feature This is something new (not an enhancement of an existing thing). Type: Minor Minor change, used in release drafter labels Oct 6, 2022
@theskumar
theskumar force-pushed the feature/theskumar/email-notifications branch from 8532fdd to 98c8d13 Compare October 7, 2022 04:56
@theskumar

Copy link
Copy Markdown
Member Author

/rebased with main branch

@theskumar theskumar self-assigned this Oct 7, 2022
Comment thread requirements.txt Outdated
Comment thread .env.sample Outdated
@frjo

frjo commented Oct 7, 2022

Copy link
Copy Markdown
Member

When running the send_staff_email_digest command I get these errors:

./hypha/venv/hypha/lib/python3.10/site-packages/django/db/models/fields/__init__.py:1358: RuntimeWarning: DateTimeField Event.when received a naive datetime (2022-09-30 00:00:00) while time zone support is active.
  warnings.warn("DateTimeField %s.%s received a naive datetime "

Setting USE_TZ = True fixes it but we need timezone support.

@frjo

frjo commented Oct 7, 2022

Copy link
Copy Markdown
Member

Then I get this error:

Traceback (most recent call last):
  File "./hypha/manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "./hypha/venv/hypha/lib/python3.10/site-packages/django/core/management/__init__.py", line 419, in execute_from_command_line
    utility.execute()
  File "./hypha/venv/hypha/lib/python3.10/site-packages/django/core/management/__init__.py", line 413, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "./hypha/venv/hypha/lib/python3.10/site-packages/django/core/management/base.py", line 354, in run_from_argv
    self.execute(*args, **cmd_options)
  File "./hypha/venv/hypha/lib/python3.10/site-packages/django/core/management/base.py", line 398, in execute
    output = self.handle(*args, **options)
  File "./hypha/hypha/apply/activity/management/commands/send_staff_email_digest.py", line 70, in handle
    send_mail(
  File "./hypha/hypha/core/mail.py", line 59, in send_mail
    mail.send()
  File "./hypha/venv/hypha/lib/python3.10/site-packages/django/core/mail/message.py", line 284, in send
    return self.get_connection(fail_silently).send_messages([self])
  File "./hypha/venv/hypha/lib/python3.10/site-packages/django/core/mail/backends/smtp.py", line 109, in send_messages
    sent = self._send(message)
  File "./hypha/venv/hypha/lib/python3.10/site-packages/django/core/mail/backends/smtp.py", line 125, in _send
    self.connection.sendmail(from_email, recipients, message.as_bytes(linesep='\r\n'))
  File "/usr/local/Cellar/python@3.10/3.10.7/Frameworks/Python.framework/Versions/3.10/lib/python3.10/smtplib.py", line 872, in sendmail
    self.ehlo_or_helo_if_needed()
  File "/usr/local/Cellar/python@3.10/3.10.7/Frameworks/Python.framework/Versions/3.10/lib/python3.10/smtplib.py", line 611, in ehlo_or_helo_if_needed
    if not (200 <= self.ehlo()[0] <= 299):
  File "/usr/local/Cellar/python@3.10/3.10.7/Frameworks/Python.framework/Versions/3.10/lib/python3.10/smtplib.py", line 451, in ehlo
    self.putcmd(self.ehlo_msg, name or self.local_hostname)
  File "/usr/local/Cellar/python@3.10/3.10.7/Frameworks/Python.framework/Versions/3.10/lib/python3.10/smtplib.py", line 378, in putcmd
    self.send(f'{s}{CRLF}')
  File "/usr/local/Cellar/python@3.10/3.10.7/Frameworks/Python.framework/Versions/3.10/lib/python3.10/smtplib.py", line 365, in send
    raise SMTPServerDisconnected('please run connect() first')
smtplib.SMTPServerDisconnected: please run connect() first

The new send_mail function ignore what is set as EMAIL_BACKEND from what I understand?

Locally I use EMAIL_BACKEND = 'django.core.mail.backends.filebased.EmailBackend' (see dev.py) to have e-mails written as local files.

Comment thread hypha/apply/activity/management/commands/send_staff_email_digest.py Outdated
Comment thread hypha/apply/activity/adapters/slack.py

@frjo frjo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A lot of nice functionality here! Only some concerns regarding sending out e-mails.

Comment thread hypha/core/mail.py Outdated
@theskumar

Copy link
Copy Markdown
Member Author

When running the send_staff_email_digest command I get these errors:

./hypha/venv/hypha/lib/python3.10/site-packages/django/db/models/fields/__init__.py:1358: RuntimeWarning: DateTimeField Event.when received a naive datetime (2022-09-30 00:00:00) while time zone support is active.
  warnings.warn("DateTimeField %s.%s received a naive datetime "

Setting USE_TZ = True fixes it but we need timezone support.

I got these too. I believe that because the dates are stored in the DB without timezone info. IMO, timezone support should enabled and all the time datetime object should always contain a timezone info them.

@theskumar

Copy link
Copy Markdown
Member Author

Then I get this error:

Traceback (most recent call last):
  File "./hypha/manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "./hypha/venv/hypha/lib/python3.10/site-packages/django/core/management/__init__.py", line 419, in execute_from_command_line
    utility.execute()
  File "./hypha/venv/hypha/lib/python3.10/site-packages/django/core/management/__init__.py", line 413, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "./hypha/venv/hypha/lib/python3.10/site-packages/django/core/management/base.py", line 354, in run_from_argv
    self.execute(*args, **cmd_options)
  File "./hypha/venv/hypha/lib/python3.10/site-packages/django/core/management/base.py", line 398, in execute
    output = self.handle(*args, **options)
  File "./hypha/hypha/apply/activity/management/commands/send_staff_email_digest.py", line 70, in handle
    send_mail(
  File "./hypha/hypha/core/mail.py", line 59, in send_mail
    mail.send()
  File "./hypha/venv/hypha/lib/python3.10/site-packages/django/core/mail/message.py", line 284, in send
    return self.get_connection(fail_silently).send_messages([self])
  File "./hypha/venv/hypha/lib/python3.10/site-packages/django/core/mail/backends/smtp.py", line 109, in send_messages
    sent = self._send(message)
  File "./hypha/venv/hypha/lib/python3.10/site-packages/django/core/mail/backends/smtp.py", line 125, in _send
    self.connection.sendmail(from_email, recipients, message.as_bytes(linesep='\r\n'))
  File "/usr/local/Cellar/python@3.10/3.10.7/Frameworks/Python.framework/Versions/3.10/lib/python3.10/smtplib.py", line 872, in sendmail
    self.ehlo_or_helo_if_needed()
  File "/usr/local/Cellar/python@3.10/3.10.7/Frameworks/Python.framework/Versions/3.10/lib/python3.10/smtplib.py", line 611, in ehlo_or_helo_if_needed
    if not (200 <= self.ehlo()[0] <= 299):
  File "/usr/local/Cellar/python@3.10/3.10.7/Frameworks/Python.framework/Versions/3.10/lib/python3.10/smtplib.py", line 451, in ehlo
    self.putcmd(self.ehlo_msg, name or self.local_hostname)
  File "/usr/local/Cellar/python@3.10/3.10.7/Frameworks/Python.framework/Versions/3.10/lib/python3.10/smtplib.py", line 378, in putcmd
    self.send(f'{s}{CRLF}')
  File "/usr/local/Cellar/python@3.10/3.10.7/Frameworks/Python.framework/Versions/3.10/lib/python3.10/smtplib.py", line 365, in send
    raise SMTPServerDisconnected('please run connect() first')
smtplib.SMTPServerDisconnected: please run connect() first

The new send_mail function ignore what is set as EMAIL_BACKEND from what I understand?

Locally I use EMAIL_BACKEND = 'django.core.mail.backends.filebased.EmailBackend' (see dev.py) to have e-mails written as local files.

Ok. I used an STMP email server via .env configs and didn't change anything else in the local.py. Can you share your local.py config for me to test it out?

@frjo

frjo commented Oct 10, 2022

Copy link
Copy Markdown
Member

In local.py:

# Write e-mails to local files.
LOCAL_FILE_EMAIL = True

That activate this in dev.py:

# E-mail to local files.
if LOCAL_FILE_EMAIL:
    EMAIL_BACKEND = 'django.core.mail.backends.filebased.EmailBackend'
    EMAIL_FILE_PATH = BASE_DIR + '/var/mail'

@theskumar
theskumar force-pushed the feature/theskumar/email-notifications branch 3 times, most recently from bafc375 to 619a9b0 Compare October 10, 2022 08:57
@theskumar

Copy link
Copy Markdown
Member Author

Also, please check if you set the value of DJANGO_SETTINGS_MODULE to "hypha.settings.dev"

It defaults to "hypha.settings.production".

@sandeepsajan0 sandeepsajan0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Works great for me.

image

if total_count:
email = MarkdownMail('emails/activity_summary.md')
email.send(
to=settings.ACTIVITY_DIGEST_RECIPIENT_EMAILS,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is ACTIVITY_DIGEST_RECIPIENT_EMAILS a list or a comma separated string?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ACTIVITY_DIGEST_RECIPIENT_EMAILS comes via env.list() which converts a comma separated string to python list.

ACTIVITY_DIGEST_RECIPIENT_EMAILS is a list at the end.

@frjo

frjo commented Oct 11, 2022

Copy link
Copy Markdown
Member

It works for me now, see #3004.

Comment thread requirements.txt Outdated
Comment thread hypha/apply/activity/templates/emails/activity_summary.md Outdated

@frjo frjo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is looking really nice. Some minor issues I commented on separately.

Comment thread hypha/apply/activity/templates/emails/activity_summary.md Outdated
@theskumar
theskumar force-pushed the feature/theskumar/email-notifications branch 3 times, most recently from 7fc379f to b6dda2c Compare October 13, 2022 08:27
@theskumar
theskumar requested a review from frjo October 13, 2022 08:52
Comment thread hypha/apply/activity/templates/messages/email/activity_summary.md Outdated
@theskumar

Copy link
Copy Markdown
Member Author

@frjo I've also added 'Auto-Submitted' header with 'auto-generated' value

Indicates that a message was generated by an automatic process, and is not a direct response to another message.

8a13212

https://www.iana.org/assignments/auto-submitted-keywords/auto-submitted-keywords.xhtml

@theskumar
theskumar force-pushed the feature/theskumar/email-notifications branch from 8a13212 to 124a45d Compare October 17, 2022 06:15
@theskumar

Copy link
Copy Markdown
Member Author

/rebased

@frjo
frjo merged commit 931a0f7 into main Oct 19, 2022
@theskumar
theskumar deleted the feature/theskumar/email-notifications branch December 14, 2022 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Feature This is something new (not an enhancement of an existing thing). Type: Minor Minor change, used in release drafter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make the default notification for all user roles be e-mail

4 participants