Upgrades django to 4.2#4073
Conversation
joemull
left a comment
joemull
left a comment
There was a problem hiding this comment.
I'm a bit unclear on the way the email is transformed or not transformed, and there's one line of code that I am a bit confused by, so just want to make it clear for others as well.
| email=email, | ||
| # Lowercasing the email into the username gets around duplicate | ||
| # accounts due to case-sensitivity. We still preserve the original | ||
| # casing for the email field and rely on username for CI constraint |
There was a problem hiding this comment.
I don't entirely follow this comment, so others might have difficulty too. What does the last part mean?
There was a problem hiding this comment.
I tried to explain exisitng behaviour because I thought it wasn't clear, seems I didn't do that well. The basic idea is that we need a case-sensitive email field, but we need a case-insensitive constraint to avoid accounts being created with variations in case e.g.: [email protected]. The way this is achieved is by setting a unique constraint on username while making it case insensitive. Any ideas on how I can reword it to make it clear?
I'm not very happy with that level of indirection, so perhaps an issue for setting the constraint on email and ditching the username field would be a good one to raise
There was a problem hiding this comment.
OK, now I get it, thanks! Maybe something like this?
diff --git a/src/core/models.py b/src/core/models.py
index aba452e2..d480da59 100644
--- a/src/core/models.py
+++ b/src/core/models.py
@@ -196,10 +196,12 @@ class AccountManager(BaseUserManager):
raise ValueError(f'{email} not a valid email address.')
account = self.model(
+ # The original case of the email is preserved
+ # in the email field
email=email,
- # Lowercasing the email into the username gets around duplicate
- # accounts due to case-sensitivity. We still preserve the original
- # casing for the email field and rely on username for CI constraint
+ # The email is lowercased in the username field
+ # so that we can perform case-insensitive checking
+ # and avoid creating duplicate accounts
username=email.lower(),
)
joemull
left a comment
joemull
left a comment
There was a problem hiding this comment.
I understand it now, thanks--I put some possible text that is clear to me above. Does it work for you?
3 participants
Updates to make Janeway compatible with Django 4.2
closes #4072