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

fix: Send after event emails and notifs to unique receivers #7126

Merged
merged 5 commits into from
Jul 10, 2020

Conversation

Haider8
Copy link
Contributor

@Haider8 Haider8 commented Jul 9, 2020

Fixes #6731

Short description of what this resolves:

Multiple emails can be sent if the same user is owner, speaker or organizer which is wrong. Only single email should be sent in that case.

Changes proposed in this pull request:

Send after event emails to unique email ids only by creating a set of unique emails.

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream development branch.
  • The unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • All the functions created/modified in this PR contain relevant docstrings.

@auto-label auto-label bot added the fix label Jul 9, 2020
@Haider8 Haider8 requested a review from iamareebjamal July 9, 2020 08:07
@codecov
Copy link

codecov bot commented Jul 9, 2020

Codecov Report

Merging #7126 into development will decrease coverage by 0.05%.
The diff coverage is 10.52%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #7126      +/-   ##
===============================================
- Coverage        62.84%   62.79%   -0.06%     
===============================================
  Files              262      262              
  Lines            13011    13023      +12     
===============================================
+ Hits              8177     8178       +1     
- Misses            4834     4845      +11     
Impacted Files Coverage Δ
app/api/helpers/scheduled_jobs.py 47.77% <7.14%> (-2.23%) ⬇️
app/api/helpers/utilities.py 69.87% <20.00%> (-3.20%) ⬇️

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 74fa0c3...44a52b7. Read the comment docs.

send_notif_after_event(organizer.user, event.name)
if owner:
send_email_after_event(owner.user.email, event.name, frontend_url)
unique_emails.add(owner.user.email)
send_notif_after_event(owner.user, event.name)
Copy link
Member

Choose a reason for hiding this comment

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

Same with notification

@iamareebjamal
Copy link
Member

Change in all places where repeatedly sending mails

@Haider8 Haider8 changed the title fix: Send after event emails to unique email ids only fix: Send after event emails and notifs to unique receivers Jul 9, 2020
@@ -54,18 +54,23 @@ def send_after_event_mail():
frontend_url = get_settings()['frontend_url']
if current_time > event.ends_at and time_difference_minutes < 1440:
unique_emails = set()
user_objects = list()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
user_objects = list()
user_objects = []

This will return a dict containing unique keys
mapped to the object which contains that key.
"""
mapped_dict = dict()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mapped_dict = dict()
mapped_dict = {}

@Haider8 Haider8 requested a review from iamareebjamal July 10, 2020 07:23
@iamareebjamal iamareebjamal merged commit 2c84d77 into fossasia:development Jul 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

After-Event Email: Several identical mails sent out, if user has several sessions/roles
2 participants