chore(bounces): Use a normalized and aliased email to search bounces #19865
Conversation
nshirley
force-pushed
the
double-match-on-bounce-check
branch
from
4eb095a to
546dd8a
Compare
| const normalizedEmail = emailNormalization.normalizeEmailAliases(email); | ||
| // we need to check for two variants of the email if the normalization | ||
| // uses a replace on the alias (e.g. '+' to '%') | ||
| const aliasedEmail = emailNormalization.normalizeEmailAliases(email, '+%'); |
There was a problem hiding this comment.
a couple problems with this, it's hard coded so we can't easily change it on the fly, and we're assuming all aliases should be replaced with a +.
Perhaps a follow up should be to update the bounce config to have a delimiter property for the normalization to reference. i.e. { domain: 'gmail', regex: '...', replace: '', delimiter: '+' } This would allow us to, on the fly, change the regex matching and replace per domain
There was a problem hiding this comment.
I also don't love this since we will need to update the BOUNCES_EMAIL_ALIAS_NORMALIZATION config to remove the replace value. Since we're trying to create two versions of the email we have to first run it through normalization to remove the aliasing all together, and again to 'wildcard' it:
[email protected] -> [email protected] and test+%@domain.com
nshirley
force-pushed
the
double-match-on-bounce-check
branch
3 times, most recently
from
e70b04c to
e17fead
Compare
| return applyRules(bounces); | ||
| } | ||
|
|
||
| async function checkBouncesWithAliases(email) { |
There was a problem hiding this comment.
Looks good to me! Thanks for fixing that.
Because: - There is an edge case where aliased email look ups can return wrong bounces This Commit: - Updates the bounce lookup to use a fully normalized email, and an aliased email with wildcard Closes: FXA-12867
nshirley
force-pushed
the
double-match-on-bounce-check
branch
from
e17fead to
6e1f849
Compare
|
last push was just a commit --amend |
Because:
This Commit:
Closes: FXA-12867
Checklist
Put an
xin the boxes that applyScreenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
Any other information that is important to this pull request.