-
Notifications
You must be signed in to change notification settings - Fork 10
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
Remove of crud from users table #73
Conversation
…ybe evaluate if we can fix CRUD
WalkthroughThe pull request modifies the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (4)src/user.rs (4)Line range hint The Line range hint The removal of the UUID field and switch to pubkey as primary key requires careful consideration of:
Consider documenting the migration process and any necessary steps for handling existing data in the PR description.
The removal of Consider documenting the SQL queries that replace the CRUD operations, either in the code or in a separate migration guide. ✅ Verification successfulThe initial results show that while CRUD references exist in other files, we need to:
Removal of SqlxCrud from User struct is safe The verification shows that:
The removal of SqlxCrud derive macro from the User struct appears to be an intentional architectural change that doesn't negatively impact other parts of the system. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for SQL queries that handle user operations
rg -l "SELECT|INSERT|UPDATE|DELETE.*FROM.*users?" --type rust
# Look for any remaining CRUD-related code
rg -l "SqlxCrud|Crud" --type rust
Length of output: 127 Script: #!/bin/bash
# Let's examine the content of the files that still contain CRUD references
rg "SqlxCrud|Crud" src/order.rs src/dispute.rs -B 2 -A 2
# Check if there are any direct user table operations in the codebase
rg "users?.*(\.|,|\(|\))" --type rust -B 2 -A 2
# Look for any sqlx query operations related to users
rg "sqlx::query.*users?" --type rust -B 2 -A 2
Length of output: 1385
Using
Consider adding a unique constraint and index on the pubkey column if not already present in the schema migrations. ✅ Verification successfulThe previous search didn't yield results, so I'm broadening the search to:
This will help us understand how the database schema is defined and verify the primary key changes. I'm narrowing down the search to:
This will help us understand if this is actually a database-backed model and how it's being used in the system. Primary key change appears to be a safe architectural decision Based on the code analysis:
The removal of the UUID field and using 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Look for schema definitions or migrations
fd -e sql | xargs grep -l "CREATE TABLE.*users"
# Check for foreign key references to users table
rg "REFERENCES.*users?" --type sql
Length of output: 235 Script: #!/bin/bash
# Look for schema definitions or migrations in various common locations and extensions
fd -e sql -e prisma -e dbml -e rs "schema|migration|model|entity"
# Search for database-related code patterns in Rust files
ast-grep --pattern 'struct User {
$$$
}'
# Look for ORM-related annotations or attributes
rg "#\[.*Table|Entity|Model" -A 5
# Search for database initialization or setup code
rg "::schema|::model|::entity" --type rust
Length of output: 193 Script: #!/bin/bash
# Look for the User struct definition in src/user.rs
cat src/user.rs
# Search for any database-related dependencies in Cargo.toml
cat Cargo.toml
# Look for database configuration files
fd -e toml -e yaml -e json "database|db"
# Search for database setup code in main.rs or lib.rs
fd "main.rs|lib.rs" | xargs cat
Length of output: 9298 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hi @grunch ,
with this removal of
CRUD
from users table inmostro-core
I was able to have pubkey as primary key working. Crud crate was not working with String, butsqlx
does.We can think to improve CRUD later imo. Also using id as primary key was not meaningful in my opinion, this makes more sense.
Summary by CodeRabbit
New Features
Bug Fixes