django-allauth
django-allauth copied to clipboard
pennersr
Data model allows for having multiple primary e-mail addresses
As discussed in #210, having at most one primary e-mail address is currently not enforced at the database level. To be determined: should we fix this, e.g. by making user and primary "unique together" and using primary=NULL instead of False.
Thanks for logging this. It is indeed strange that Django allows two "Primary" email addresses. This state appears to break Django Allauth when a user tries to log in via Social media and the social media primary email . However, it seems more an idiosyncrasy of Django than Allauth. How does everyone else handle this?
delted past comment -- Misread the relationship between this and #210.
So if we add a natural key to email addresses does that mean we are at 1.0???? ;)
Meta project feedback: This is the type of issue that I will personally look at and judge a code base harshly. Now, i've been a django-allauth user for over a decade now and I know the quality of code here so it doesn't stop me personally; but if you seek to grow this project this type of issue is very bad to leave hanging around.
How do you feel about making this a priority? I've combed through bug requests that haven't been updated since 2018 and this one seems like one of the most important.
@derek-adair It simply couldn't be done back in 2013 without resorting to hacks (using NULL for False). These days, with the right Django version, conditional unique indexes are supported, so it can finally be fixed:
https://docs.djangoproject.com/en/4.2/ref/models/constraints/#django.db.models.UniqueConstraint.condition
absolutely understand. Just being cheeky.
Seems like this would be a good candidate for an upcoming release?? This may actually legit make a case for the 1.0 version as i'm a stickler for semantic versioning but am too ignorant on this code base to say so w/ certainty.
Yes. Current development branch already contains some migrations in this area, and it's best to take this step by step, but this could be done for 0.56.0. As for 1.0, I do expect more changes that will break backwards compatibility, so that won't happen soon. Or, another option is to give up on the idea of an 1.0 completely, and release say version 55.0.0 and simply iterate in semver style from there.
See #3385 - MySQL still does not support this. I would prefer to see how that ticket evolves before going forward with more of these conditional constraints.
Using null for primary=False would be a huge backwards incompatible change, that is really not worth it given that this issue has not caused any practical issues over all this time. So, opted for adding a conditional unique constraint for databases that support it. See #3791