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.

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

Normalize User / Project Id Types #49

Closed
cstrnt opened this issue Feb 2, 2024 · 8 comments · Fixed by #61
Closed

Normalize User / Project Id Types #49

cstrnt opened this issue Feb 2, 2024 · 8 comments · Fixed by #61
Assignees
Labels
advanced Extra attention is needed typescript code is mostly backend/typescript based

Comments

@cstrnt
Copy link
Contributor

cstrnt commented Feb 2, 2024

Currently we have some ids stored as numbers, some as strings (see here for more details: #49 (comment))

Those should be normalized so we don't have to do conversions anymore when comparing them.

Also the current conversions should be removed

@cstrnt cstrnt added the typescript code is mostly backend/typescript based label Feb 2, 2024
@skushagra9 skushagra9 mentioned this issue Feb 3, 2024
Closed
16 tasks
@skushagra9
Copy link
Contributor

should I change it for providers like github, gitlab also ?

@McPizza0 McPizza0 added the advanced Extra attention is needed label Feb 3, 2024
@McPizza0
Copy link
Member

McPizza0 commented Feb 3, 2024

should I change it for providers like github, gitlab also ?

this is unrelated to the issue

@skushagra9
Copy link
Contributor

oh, I see, I was thinking to change the types

@McPizza0
Copy link
Member

McPizza0 commented Feb 3, 2024

the source of the issue:
when we insert a new db row, the response.insertId is a string so we need to cast it to a number by preceding it with a +
throughout the app/db we use number based ids for all internal linking and logic

there was an issue where the id column would be returned as a string instead of a number in db queries
so i got into the bad habit of always casting id columns to numbers

the fix: look at all db responses (from DB and from Cache storages), check the type of the returned id and remove the casting if it is no longer needed

a regex search across the codebase \+.*\.id shows there are 75 results
each one needs its type checked and the casting to be removed

full testing of the app will need to be conducted after to ensure there arnt any lingering issues

@skushagra9
Copy link
Contributor

Thanks, got it

@skushagra9
Copy link
Contributor

@McPizza0 , If you aren't working on this issue, can I work on this please ?

@McPizza0
Copy link
Member

McPizza0 commented Feb 4, 2024

sorry @skushagra9 this issue requires a lot of attention and testing

@skushagra9
Copy link
Contributor

Ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
advanced Extra attention is needed typescript code is mostly backend/typescript based
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants