FIX: Use correct group out of multiple for SMTP sender #14957
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When there are multiple groups on a topic, we were selecting the first from the topic allowed groups to act as the sender email address when sending group SMTP replies via PostAlerter. However, this was not ordered, and since there is no created_at column on TopicAllowedGroup we cannot order this nicely, which caused just a random group to be used (based on whatever postgres decided it felt like that morning). This commit changes the group used for SMTP sending to be the group using the email_username of the to address of the first incoming email for the topic, if there are more than one allowed groups on the topic. Otherwise it just uses the only SMTP enabled group.
CvX
approved these changes
Nov 16, 2021
|
Thanks @CvX , thinned out the soup now ;) |
martin-brennan
deleted the
issue/find-correct-group-for-smtp-in-post-alerter
branch
CvX
reviewed
Nov 16, 2021
| return post.topic.first_smtp_enabled_group | ||
| end | ||
|
|
||
| group = Group.find_by_email(post.topic.incoming_email.first.to_addresses) |
There was a problem hiding this comment.
@martin-brennan Looking at the error logs: post.topic.incoming_email.first can be nil
There was a problem hiding this comment.
Sigh x1000, of course it can. Will make a fix.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When there are multiple groups on a topic, we were selecting
the first from the topic allowed groups to act as the sender
email address when sending group SMTP replies via PostAlerter.
However, this was not ordered, and since there is no created_at
column on TopicAllowedGroup we cannot order this nicely, which
caused just a random group to be used (based on whatever postgres
decided it felt like that morning).
This commit changes the group used for SMTP sending to be the
group using the email_username of the to address of the first
incoming email for the topic, if there are more than one allowed
groups on the topic. Otherwise it just uses the only SMTP enabled
group.