Add wallet entity
#2264
Add wallet entity
#2264
Conversation
jmealy
force-pushed
the
add-wallet-entity
branch
from
eafbd44 to
6807c42
Compare
jmealy
force-pushed
the
add-wallet-entity
branch
from
6807c42 to
cec4604
Compare
| @PrimaryGeneratedColumn('uuid') | ||
| id!: UUID; |
There was a problem hiding this comment.
Why do we want to use a UUID as the primary key? Are we going to expose this value? I'd suggest using an auto-incrementing value if not.
There was a problem hiding this comment.
changed to auto-incrementing id 11fb705
| @ManyToOne(() => User, (user) => user.id, { | ||
| onDelete: 'CASCADE', | ||
| }) | ||
| user_id!: UUID; |
There was a problem hiding this comment.
Similar to the above, this might need changing in accordance with #2262 (comment).
There was a problem hiding this comment.
Can do auto incrementing 👍 Just to be aware, with auto incrementing IDs we would expose how many accounts have been created, but I'm not sure if this matters
There was a problem hiding this comment.
If you want to use both integer and UUID, you can make the primary key an integer and add a unique uuid column for cases where the ID shouldn’t be exposed. For foreign keys, stick to using the integer field instead of the UUID, since UUIDs are bigger and can make indexes larger, which will slow things down.
There was a problem hiding this comment.
changed User to use auto incrementing ID as well: 11fb705
| @Column({ | ||
| type: 'timestamp with time zone', | ||
| default: () => 'CURRENT_TIMESTAMP', | ||
| }) | ||
| created_at!: Date; | ||
|
|
||
| @Column({ | ||
| type: 'timestamp with time zone', | ||
| default: () => 'CURRENT_TIMESTAMP', | ||
| onUpdate: 'CURRENT_TIMESTAMP', | ||
| }) | ||
| updated_at!: Date; |
There was a problem hiding this comment.
You need to write the queries manually, but you can reuse the ones from the notification by simply updating the names.
my understanding is that this will be done when we are generating the migrations. So here I've just removed onUpdate: 'CURRENT_TIMESTAMP', since it doesn't do anything and the rest will be done later. lmk if that's correct
There was a problem hiding this comment.
@jmealy Since PostgreSQL does not have a direct ON UPDATE clause for columns, we should add a Trigger for it. Please refer to the Notification migration, as a similar approach is used there.
There was a problem hiding this comment.
yes understood 👍 but this PR is just for the entity, not the migration, so I have nowhere to add it yet. I can add it after this PR is merged and and a migration is generated
src/datasources/users/entities/__tests__/wallets.entity.db.builder.ts
Outdated
Show resolved
Hide resolved
…lder.ts Co-authored-by: Aaron Cook <[email protected]>
| @JoinColumn({ name: 'user_id' }) | ||
| user!: User; | ||
|
|
||
| @Column({ |
There was a problem hiding this comment.
Looks like we’ll be searching for wallets by their address. If that’s the case, I’d index this column to improve search performance.
Summary
Adds a new entity for Wallet. Each entry represents a signer wallet that can be used to sign in for a User.
Changes
Walletentity