Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rename accounts table to user #374

Closed
Tracked by #331
Gozala opened this issue Sep 2, 2021 · 0 comments · Fixed by #485
Closed
Tracked by #331

Rename accounts table to user #374

Gozala opened this issue Sep 2, 2021 · 0 comments · Fixed by #485
Assignees
@alanshaw
Labels
kind/bug A bug in existing code (including security flaws) P0 Critical: Tackled by core team ASAP

Comments

@Gozala
Copy link
Contributor

Gozala commented Sep 2, 2021

No description provided.

@Gozala Gozala added kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization labels Sep 2, 2021
@Gozala Gozala mentioned this issue Sep 2, 2021
Closed
@dchoi27 dchoi27 added P0 Critical: Tackled by core team ASAP and removed need/triage Needs initial labeling and prioritization labels Sep 3, 2021
@alanshaw alanshaw mentioned this issue Sep 29, 2021
Closed
19 tasks
@alanshaw alanshaw assigned alanshaw and unassigned hugomrdias Sep 29, 2021
@alanshaw alanshaw mentioned this issue Sep 29, 2021
Merged
alanshaw pushed a commit that referenced this issue Oct 4, 2021
refactor: rename account table to user (#485)
a3423a1 
resolves #374

"user" needs to be quoted or namespaced in SQL because user is a reserved word in the language. Justification for doing this anyway:

1. We refer to "user" everywhere else in the JS code and we go to the trouble of ["renaming" fields from "account" to "user" in our queries](https://github.com/ipfs-shipyard/nft.storage/pull/485/files#diff-d2f63c0f47f000a546d34f098e45e9981f20eca7b8fccc3cfb35c95ab24fba47L81) and we have to [construct queries like `account_id: userId`](https://github.com/ipfs-shipyard/nft.storage/pull/485/files#diff-d2f63c0f47f000a546d34f098e45e9981f20eca7b8fccc3cfb35c95ab24fba47L81) because of it (confusing).
1. We actually don't need to quote or namespace prefix "user" when querying via postgREST and we don't often query directly in SQL (only in 1 stored procedure).
1. In my head I also find it difficult to square off one user having multiple "accounts". We don't support this right now but "account" suggests that we do.
1. IMO it is worth a small inconvenience on something we don't often do for an overall more consistent and less confusing code base.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alanshaw alanshaw

Labels
kind/bug A bug in existing code (including security flaws) P0 Critical: Tackled by core team ASAP
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor: rename account table to user nftstorage/nft.storage
4 participants
@Gozala @alanshaw @hugomrdias @dchoi27