Skip to content
This repository has been archived by the owner on May 1, 2023. It is now read-only.

feat: user model #74

Merged
merged 3 commits into from
Aug 16, 2021
Merged

feat: user model #74

merged 3 commits into from
Aug 16, 2021

Conversation

maxneuvians
Copy link
Contributor

Closes #69. Adds a user model based on the proposal in #9.

sa.Column("email_address", sa.String(length=255), nullable=False, unique=True),
sa.Column("password_hash", sa.String(length=255), nullable=False),
sa.Column(
"access_token", postgresql.UUID(as_uuid=True), nullable=False, unique=True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably want an index on this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought unique columns don't need to be indexed because unique creates an implicit index? Happy to add one if this is not correct.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know alchemy, so maybe it does? We should verify that the index gets created though as we'll probably want to lookup by access_token a lot.

sa.Column("id", postgresql.UUID(as_uuid=True), primary_key=True),
sa.Column("organisation_id", postgresql.UUID(as_uuid=True), nullable=False),
sa.Column("name", sa.String(), nullable=False),
sa.Column("email_address", sa.String(length=255), nullable=False, unique=True),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

index? (I'm assuming login is email/password)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above

session.rollback()


def test_user_empty_email_address_fails(organisation_fixture, session):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test_user_duplicate_email_address test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! Fixed

Copy link
Contributor

@dj2 dj2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can always fixup the index later if it doesn't get created.

@CalvinRodo CalvinRodo merged commit 9e897d6 into main Aug 16, 2021
@CalvinRodo CalvinRodo deleted the feat/user_model branch August 16, 2021 13:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add User model
3 participants