feat: Create the identites module
#121
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @Blckbrry-Pi and the rest of your teammates on |
Blckbrry-Pi
force-pushed
the
06-27-feat_tokens_add_a_modify_meta_script
branch
from
06cf55a to
d2012c6
Compare
Blckbrry-Pi
force-pushed
the
06-29-feat_create_the_auth_provider_module
branch
from
ae1fc49 to
7d177f8
Compare
Blckbrry-Pi
force-pushed
the
06-27-feat_tokens_add_a_modify_meta_script
branch
from
d2012c6 to
2832c4c
Compare
Blckbrry-Pi
force-pushed
the
06-29-feat_create_the_auth_provider_module
branch
from
7d177f8 to
30af057
Compare
Blckbrry-Pi
force-pushed
the
06-27-feat_tokens_add_a_modify_meta_script
branch
from
2832c4c to
bbb15a8
Compare
Blckbrry-Pi
force-pushed
the
06-29-feat_create_the_auth_provider_module
branch
from
30af057 to
553a363
Compare
Blckbrry-Pi
force-pushed
the
06-27-feat_tokens_add_a_modify_meta_script
branch
from
bbb15a8 to
4411d94
Compare
Blckbrry-Pi
force-pushed
the
06-29-feat_create_the_auth_provider_module
branch
2 times, most recently
from
aa4b390 to
c02a755
Compare
Blckbrry-Pi
changed the title
auth_provider moduleidentites module
Blckbrry-Pi
force-pushed
the
06-29-feat_create_the_auth_provider_module
branch
from
c02a755 to
efc852f
Compare
Blckbrry-Pi
force-pushed
the
06-27-feat_tokens_add_a_modify_meta_script
branch
from
4411d94 to
f3e0b8d
Compare
Blckbrry-Pi
force-pushed
the
06-29-feat_create_the_auth_provider_module
branch
from
efc852f to
bfd5d25
Compare
Blckbrry-Pi
force-pushed
the
06-27-feat_tokens_add_a_modify_meta_script
branch
from
f3e0b8d to
ad062ed
Compare
Blckbrry-Pi
force-pushed
the
06-29-feat_create_the_auth_provider_module
branch
from
bfd5d25 to
ce34e98
Compare
modules/identities/scripts/get.ts
Outdated
| return { data: null }; | ||
| } | ||
|
|
||
| const { uniqueData, additionalData } = data; |
There was a problem hiding this comment.
is this because of our ajv validation issue or something else? ideally we don't have to write ugly code like this.
this might already be fixed on main with the zod migration.
There was a problem hiding this comment.
It's because uniqueData and additionalData are JSON fields in the psql database.
The null reassignment shouldn't be necessary, that was an accident.
However, it doesn't guarantee there's a matching row, and the JSON type doesn't guarantee it's a non-nullable object.
This essentially just says invalid/missing entries have no data.
There was a problem hiding this comment.
am i missing something: if there's no matching row, identities will be null. it's not going to return an object with undefined properties. our db constraint has not null on jsonb so all of this code is redundant.
There was a problem hiding this comment.
It sadly is not. The DB constraint says that if there is a row, there is a valid JSON value in that column. null is a valid JSON value, along with "hello", 42, etc.
Prisma ensures that the value returned will be JSON, but not whether or not it's an object or not.
There was a problem hiding this comment.
to clarify: there is no case where we intentionally set anything that's not an object to the additional data, right? if that's a case, let's write an assertion using the deno assert lib (which throws an internal error, not runtimeerror) & crash instead of failing silentl.
There was a problem hiding this comment.
otherwise, some downstream script will likely break from unintended state afaik.
There was a problem hiding this comment.
Good point. Will do!
| await ctx.modules.rateLimit.throttlePublic({}); | ||
|
|
||
| // Ensure the user token is valid and get the user ID | ||
| const { userId } = await ctx.modules.users.authenticateToken({ userToken: req.userToken } ); |
There was a problem hiding this comment.
we should let the caller authenticate the token and pass a raw user id. allows for better composability.
There was a problem hiding this comment.
This is a public script— are you sure we want that to be public by user ID?
Blckbrry-Pi
force-pushed
the
06-29-feat_create_the_auth_provider_module
branch
2 times, most recently
from
7a38516 to
a119950
Compare
modules/identities/scripts/get.ts
Outdated
| return { data: null }; | ||
| } | ||
|
|
||
| const { uniqueData, additionalData } = data; |
There was a problem hiding this comment.
am i missing something: if there's no matching row, identities will be null. it's not going to return an object with undefined properties. our db constraint has not null on jsonb so all of this code is redundant.
Blckbrry-Pi
force-pushed
the
06-29-feat_create_the_auth_provider_module
branch
2 times, most recently
from
7a04e0a to
68f1b5e
Compare
modules/identities/scripts/get.ts
Outdated
| return { data: null }; | ||
| } | ||
|
|
||
| const { uniqueData, additionalData } = data; |
There was a problem hiding this comment.
to clarify: there is no case where we intentionally set anything that's not an object to the additional data, right? if that's a case, let's write an assertion using the deno assert lib (which throws an internal error, not runtimeerror) & crash instead of failing silentl.
modules/identities/scripts/get.ts
Outdated
| return { data: null }; | ||
| } | ||
|
|
||
| const { uniqueData, additionalData } = data; |
There was a problem hiding this comment.
otherwise, some downstream script will likely break from unintended state afaik.
NathanFlurry
force-pushed
the
06-27-feat_tokens_add_a_modify_meta_script
branch
from
ad062ed to
1caae6f
Compare
NathanFlurry
force-pushed
the
06-29-feat_create_the_auth_provider_module
branch
from
68f1b5e to
faed173
Compare
Merge activity
|
NathanFlurry
changed the base branch from
06-27-feat_tokens_add_a_modify_meta_script
to
graphite-base/121
NathanFlurry
force-pushed
the
06-29-feat_create_the_auth_provider_module
branch
from
faed173 to
171fdbe
Compare
No description provided.