Skip to content
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

WIP: add notifications and reply-by-email #486

Merged
merged 13 commits into from Feb 25, 2018
Merged

Conversation

@tiltec
Copy link
Member

tiltec commented Feb 23, 2018

Users get notification e-mails for new conversation messages and can write follow-up messages by replying to the notification e-mail

to do

  • add updated local_settings to ansible and run setup_sparkpost after deploy
  • make a nice e-mail template and make it translatable
  • send emails in the receivers' language
  • fix failing test that seems unrelated
  • add test for translations
  • implement notification settings in frontend
  • add click-to-unsubscribe link in backend and frontend
  • add fancy inboxmarkup to notification mails
  • add task runner to not send tons of emails in request #242
tiltec added 10 commits Feb 23, 2018
Users get notification e-mails for new conversation messages and can write follow-up messages via e-mail reply
@tiltec tiltec mentioned this pull request Feb 25, 2018
11 of 14 tasks complete
@tiltec tiltec requested a review from nicksellen Feb 25, 2018
@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 25, 2018

Codecov Report

Merging #486 into master will decrease coverage by 0.78%.
The diff coverage is 74.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #486      +/-   ##
==========================================
- Coverage   97.87%   97.08%   -0.79%     
==========================================
  Files         209      216       +7     
  Lines        6405     6627     +222     
  Branches      283      300      +17     
==========================================
+ Hits         6269     6434     +165     
- Misses        108      159      +51     
- Partials       28       34       +6
Impacted Files Coverage Δ
foodsaving/conversations/models.py 100% <100%> (ø) ⬆️
foodsaving/subscriptions/tests/test_receivers.py 99.69% <100%> (ø) ⬆️
...009_conversationparticipant_email_notifications.py 100% <100%> (ø)
foodsaving/conversations/tests/test_api.py 100% <100%> (ø) ⬆️
foodsaving/conversations/serializers.py 100% <100%> (ø) ⬆️
foodsaving/webhooks/__init__.py 100% <100%> (ø)
foodsaving/webhooks/models.py 100% <100%> (ø)
foodsaving/conversations/api.py 100% <100%> (ø) ⬆️
foodsaving/webhooks/migrations/0001_initial.py 100% <100%> (ø)
foodsaving/webhooks/tests/test_api.py 100% <100%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a5105c...d734c0a. Read the comment docs.

DEFAULT_FROM_EMAIL = "testing@example.com"
SPARKPOST_RELAY_DOMAIN = 'replies.karrot.localhost'
HOSTNAME = 'https://localhost:8000'
SITE_NAME = 'karrot.localhost'

This comment has been minimized.

Copy link
@nicksellen

nicksellen Feb 25, 2018

Member

What SITE_NAME and DEFAULT_FROM_EMAIL should contain is a bit unclear right now, but will be addressed in #490 ... can get validated in the check command, etc.

)

for participant in participants_to_notify:
with translation.override(participant.user.language):

This comment has been minimized.

Copy link
@nicksellen

nicksellen Feb 25, 2018

Member

I would maybe put this in the prepare_conversation_message_notification function

@skipCI
Copy link
Member

nicksellen left a comment

Great :) Really nice feature!

@tiltec tiltec merged commit 974b8ba into master Feb 25, 2018
1 of 2 checks passed
1 of 2 checks passed
ci/circleci: test Your CircleCI tests were canceled
Details
pyup.io/safety-ci No dependencies with known security vulnerabilities.
Details
@tiltec tiltec deleted the email-notifications-and-replies branch Feb 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.