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

Apply +-normalization for all email domains #16764

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

thomas-daniels
Copy link
Member

Draft PR because this only changes the code part and does not include anything for database migrations, because I'm not sure what's the best way to write that and how to deal with potential collisions caused by them.

Zulip context topic

Prior to this commit, +- and .-normalization are only applied to Gmail/ProtonMail. However, using the + for aliasing is done by many more email providers. This commit therefore applies +-normalization to all email domains - I don't think there's any risk in doing that. Removing the dots, however, is still only done for Gmail and ProtonMail because there are email providers (I'm thinking of Outlook here, and there might be others) where dots can absolutely not be ignored.

@thomas-daniels thomas-daniels marked this pull request as draft January 12, 2025 12:43
@thomas-daniels thomas-daniels marked this pull request as ready for review January 18, 2025 13:34
@thomas-daniels
Copy link
Member Author

migration script added, think this is ready for review

@thomas-daniels
Copy link
Member Author

actually, just realized this migration script is wrong because it would undo previous normalizations since it starts from the verbatim email. I guess the right solution is not to create a new script like I did here, but to modify the old script to do all normalization at once. Will fix.

@thomas-daniels thomas-daniels marked this pull request as draft January 19, 2025 07:39
@thomas-daniels thomas-daniels marked this pull request as ready for review January 19, 2025 07:56
@thomas-daniels
Copy link
Member Author

fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant