Create Name model for usernames #265
Conversation
|
This change also fixes username validation in the models, in the process exposing some bugs in the unit tests. These have now been corrected. |
Also make `name`'s uniqueness case-insensitive.
|
|
||
| class SharedNameMixin(object): |
There was a problem hiding this comment.
It's not a model. It offers the name property that makes the relationship behave like a string in both User and Organization.
lastuser_oauth/forms/profile.py
Outdated
| @@ -51,7 +51,7 @@ def validate_old_password(self, field): | |||
|
|
|||
| class ProfileForm(forms.Form): | |||
| fullname = forms.StringField(__("Full name"), | |||
| validators=[forms.validators.DataRequired(), forms.validators.Length(max=80)]) | |||
| validators=[forms.validators.DataRequired(), forms.validators.Length(max=63)]) | |||
There was a problem hiding this comment.
Why the length restriction on full name? Username is created from it?
There was a problem hiding this comment.
DNS labels are restricted to 63 characters. Any longer and Talkfunnel's subdomains will break.
There was a problem hiding this comment.
No I know that and get the limit for username. full name is never used on url, right?
There was a problem hiding this comment.
Whoops, validator needed to be on the other field.
There was a problem hiding this comment.
Fixed this and a bunch of other issues. Organization views were all broken.
Undo error introduced in Alembic revision 3b17b62bf8e4.
|
|
||
| @name.expression | ||
| def name(cls): | ||
| return db.select([Name.name]).where(Name.user_id == cls.id).label('name') |
There was a problem hiding this comment.
This query is super inefficient. What it generates (with explicit column names instead of *):
SELECT * FROM "user" WHERE (SELECT name.name FROM name WHERE name.user_id = "user".id) = '<param>'EXPLAIN output:
QUERY PLAN
--------------------------------------------------------------------------------------
Seq Scan on "user" (cost=0.00..478332.40 rows=287 width=605)
Filter: (((SubPlan 1))::text = '<param>'::text)
SubPlan 1
-> Index Scan using name_user_id_key on name (cost=0.29..8.31 rows=1 width=10)
Index Cond: (user_id = "user".id)
(5 rows)
To avoid the sequential scan, we may have to write a custom comparator instead of using this expression.
| return True | ||
| @name.expression | ||
| def name(cls): | ||
| return db.select([Name.name]).where(Name.org_id == cls.id).label('name') |
|
After having tried the custom comparator approach, I've found that there is no way to invisibly insert a join condition. Whoever is performing the query must (a) perform the join, or (b) call I'm therefore merging this PR as is, and will pursue |
Merges name from User and Organization models into a single table, thereby fulfilling the unification requirement described in #91 and #232.