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: allow multiple secondary emails #5452

Closed
wants to merge 1 commit into from

Conversation

LeoMcA
Copy link
Contributor

@LeoMcA LeoMcA commented Dec 21, 2017

While working on adding multiple email support to the email receiver (which, based on the tests I've been writing, seems to already exist... but that's a story for another PR) I was running into errors adding multiple secondary email addresses to a user.

This fixes up the validator, and replaces the index with a partial index, to allow that to happen.

@discoursebot
Copy link

You've signed the CLA, LeoMcA. Thank you! This pull request is ready for review.

@ZogStriP
Copy link
Member

ZogStriP commented Dec 21, 2017

LGTM 👍 but I'll leave it to @tgxworld to merge it.

@SamSaffron
Copy link
Member

looks good but lets wait one more week to merge till 1.9 is out

@SamSaffron SamSaffron added the 2.0 label Dec 27, 2017
@discoursebot
Copy link

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/additional-email-support/59847/13

@LeoMcA
Copy link
Contributor Author

LeoMcA commented Feb 16, 2018

@SamSaffron 1.9's been out a while now, is this ready to merge?

@gschlager
Copy link
Member

@LeoMcA Sorry for the delay. Could you please rebase your PR?

79590e4 can be reverted when this PR gets merged.

@LeoMcA LeoMcA force-pushed the multiple-alternate-emails branch from 6217dde to ebf3de0 Compare March 1, 2018 21:14
@LeoMcA
Copy link
Contributor Author

LeoMcA commented Mar 1, 2018

Sorry for the delay.

No worries!

Could you please rebase your PR?

Done 🎉

@tgxworld
Copy link
Contributor

tgxworld commented Mar 2, 2018

@LeoMcA Just to provide you with an update, this is actually on my plate but we're seeing an issue where users can be created without an email. The bad news is that I've not been able to reproduce it yet so we're still adding more logging to see if we can figure it out. This PR will be placed on hold still we fix that bug.

@tgxworld
Copy link
Contributor

tgxworld commented Jul 3, 2018

Thank you @LeoMcA ❤️ Merged in

c312944

@tgxworld tgxworld closed this Jul 3, 2018
@LeoMcA
Copy link
Contributor Author

LeoMcA commented Jul 4, 2018

@gschlager thought I'd remind you 79590e4 can be reverted now

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

Successfully merging this pull request may close these issues.

7 participants