Skip to content

feat: set users ID to be autoincrementing - #240

Merged
allenporter merged 3 commits into
allenporter:mainfrom
6ixfalls:feat/user-increment
Sep 10, 2026
Merged

allenporter merged 3 commits into
allenporter:mainfrom
6ixfalls:feat/user-increment

Conversation

@6ixfalls

@6ixfalls 6ixfalls commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

User IDs are currently recycled as the column is an ROWID. When a user is deleted from the database, a new user can obtain the same User ID, and the old user's files are assigned to the new user who now has full access to them. This changes the column to be autoincremented and also ensures this for old deployments.

@allenporter

Copy link
Copy Markdown
Owner

I noticed there are some test failures. Does your change pass the tests locally?

@6ixfalls

6ixfalls commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

I noticed there are some test failures. Does your change pass the tests locally?

Should be fixed, but I have not deployed this to test yet.

@allenporter

Copy link
Copy Markdown
Owner

OK Looks great. How did you notice this? I was wondering if we could have a test that reproduced the original issue in a similar way that you noticed it through the API e.g.:

  • create a user
  • add a file
  • query the files for the user and see the file
  • delete a user
  • create a user
  • query the files for the user and see nothing
    etc

@codecov-commenter

codecov-commenter commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.90%. Comparing base (70a23be) to head (0b52cd0).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...bic/versions/68964804740d_prevent_user_id_reuse.py 79.31% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #240      +/-   ##
==========================================
- Coverage   90.96%   90.90%   -0.07%     
==========================================
  Files         113      114       +1     
  Lines       11749    11779      +30     
==========================================
+ Hits        10688    10708      +20     
- Misses       1061     1071      +10     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@6ixfalls

6ixfalls commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

OK Looks great. How did you notice this? I was wondering if we could have a test that reproduced the original issue in a similar way that you noticed it through the API e.g.:

  • create a user
  • add a file
  • query the files for the user and see the file
  • delete a user
  • create a user
  • query the files for the user and see nothing
    etc

It might be quite long to add a test like that. I found this issue through an AI passthrough (and a few others, but those fixes are larger).

@allenporter

Copy link
Copy Markdown
Owner

It might be quite long to add a test like that. I found this issue through an AI passthrough (and a few others, but those fixes are larger).

Please sync and rebase. I added the test in #241 setup for you to update with your fix.

@allenporter allenporter left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking request changes; update the test that should reproduce the issue.

@allenporter
allenporter merged commit f70488f into allenporter:main Sep 10, 2026
4 checks passed
striimusMiska pushed a commit to striimusMiska/supernote-private-cloud-apple-ocr that referenced this pull request Sep 11, 2026
* feat: set users ID to be autoincrementing

* chore: fix user tests

* test: update user ID reuse regression expectations
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.

3 participants