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

feat(api): Migrating to ULID instead of 64-bit integer IDs #1740

Merged
merged 67 commits into from
May 19, 2024

Conversation

elliotcourant
Copy link
Member

This will make writing data in bulk easier in the future, it will also
make this data more portable between environments or between databases.

@elliotcourant
Copy link
Member Author

I think instead of generating the initial ULIDs in go they should be done via this: https://github.com/geckoboard/pgulid

This way the migration can be done as a single SQL migration rather than multiple.

@codecov-commenter
Copy link

codecov-commenter commented May 5, 2024

Codecov Report

Attention: Patch coverage is 58.14536% with 334 lines in your changes are missing coverage. Please review.

Project coverage is 52.13%. Comparing base (8d6d757) to head (7c56726).
Report is 20 commits behind head on main.

Current head 7c56726 differs from pull request most recent head aa93122

Please upload reports for the commit aa93122 to get more accurate results.

Files Patch % Lines
server/controller/helpers.go 44.66% 41 Missing and 16 partials ⚠️
server/controller/transactions.go 13.46% 43 Missing and 2 partials ⚠️
server/controller/plaid.go 40.47% 25 Missing ⚠️
server/controller/links.go 23.33% 21 Missing and 2 partials ⚠️
server/controller/spending.go 53.48% 20 Missing ⚠️
server/controller/forecast.go 0.00% 17 Missing ⚠️
server/controller/bank_accounts.go 0.00% 15 Missing ⚠️
server/controller/files.go 0.00% 13 Missing ⚠️
server/controller/funding_schedules.go 73.91% 6 Missing and 6 partials ⚠️
server/models/transaction_cluster.go 18.18% 9 Missing ⚠️
... and 33 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1740      +/-   ##
==========================================
+ Coverage   50.46%   52.13%   +1.67%     
==========================================
  Files         366      358       -8     
  Lines       20363    18525    -1838     
  Branches      500      502       +2     
==========================================
- Hits        10276     9658     -618     
+ Misses       9523     8325    -1198     
+ Partials      564      542      -22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@elliotcourant
Copy link
Member Author

The code side is complete, now its just fixing tests or fixing code depending on how the test fails.

Also the fixtures I had generated a long time ago for forecasting need to be regenerated somehow.

@elliotcourant elliotcourant marked this pull request as ready for review May 19, 2024 22:00
@elliotcourant elliotcourant merged commit 8b673b6 into main May 19, 2024
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants