feat: add user creation migration scripts for Hasura#83
feat: add user creation migration scripts for Hasura#83BigJohn-dev wants to merge 1 commit intosafetrustcr:mainfrom
Conversation
|
@BigJohn-dev Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughThis PR adds a Hasura database migration that creates a Changes
Possibly related issues
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
infra/hasura/migrations/safetrust/1731908676359_create_users/up.sql (2)
8-12: Avoid double-indexing
UNIQUE (email)already creates an index in Postgres, soidx_users_emailadds redundant write/storage overhead. If you need the explicitidx_users_emailname, create a unique index first and bind the constraint to it.Proposed migration adjustment
ALTER TABLE users - ADD CONSTRAINT users_email_unique UNIQUE (email); + ADD CONSTRAINT users_email_unique UNIQUE USING INDEX idx_users_email; -- Add indices for common queries -CREATE INDEX idx_users_email ON users(email); +CREATE UNIQUE INDEX idx_users_email ON users(email);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/hasura/migrations/safetrust/1731908676359_create_users/up.sql` around lines 8 - 12, The migration currently creates a UNIQUE constraint users_email_unique which already creates an index, then adds a redundant CREATE INDEX idx_users_email; remove the separate CREATE INDEX idx_users_email (or alternatively create a UNIQUE INDEX first and attach the constraint to it with ADD CONSTRAINT ... USING INDEX) so the email uniqueness index is not duplicated; update the users ALTER TABLE/constraint/index lines accordingly around users_email_unique and idx_users_email to either rely on the unique constraint's index or to create a single unique index and bind the constraint to that index.
1-12: Schema-qualify objects in migrations to avoidsearch_pathsurprises.Using bare
users/idx_users_emailcan target the wrong schema in non-default environments. Preferpublic.usersandpublic.idx_users_email.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/hasura/migrations/safetrust/1731908676359_create_users/up.sql` around lines 1 - 12, Schema-qualify the objects to avoid search_path issues: change CREATE TABLE users to CREATE TABLE public.users, change ALTER TABLE users to ALTER TABLE public.users, and change CREATE INDEX idx_users_email to CREATE INDEX public.idx_users_email (keep the constraint name users_email_unique as-is or rename if you want schema-qualified naming); update references to the table/index names in this migration (symbols: users, public.users, users_email_unique, idx_users_email, public.idx_users_email).infra/hasura/migrations/safetrust/1731908676359_create_users/down.sql (1)
5-8:DROP INDEXafterDROP TABLEis redundant.Dropping
usersalready drops its indexes. Keeping line 8 is harmless dueIF EXISTS, but unnecessary noise in rollback logic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/hasura/migrations/safetrust/1731908676359_create_users/down.sql` around lines 5 - 8, The rollback drops the users table which already removes its indexes, so remove the redundant DROP INDEX IF EXISTS idx_users_email CASCADE line from the down migration; update the down SQL in 1731908676359_create_users to only DROP TABLE IF EXISTS users CASCADE and remove the DROP INDEX statement referring to idx_users_email.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infra/hasura/migrations/safetrust/1731908676359_create_users/down.sql`:
- Around line 2-5: Remove the unnecessary CASCADEs and the redundant index drop:
delete the CASCADE modifiers from the DROP CONSTRAINT and DROP TABLE statements
for the users table (symbols: users, users_email_unique, DROP TABLE users) and
also remove the separate DROP INDEX IF EXISTS idx_users_email statement (the
table drop already removes its indexes), leaving only the simplified DROP
CONSTRAINT and DROP TABLE lines in down.sql.
---
Nitpick comments:
In `@infra/hasura/migrations/safetrust/1731908676359_create_users/down.sql`:
- Around line 5-8: The rollback drops the users table which already removes its
indexes, so remove the redundant DROP INDEX IF EXISTS idx_users_email CASCADE
line from the down migration; update the down SQL in 1731908676359_create_users
to only DROP TABLE IF EXISTS users CASCADE and remove the DROP INDEX statement
referring to idx_users_email.
In `@infra/hasura/migrations/safetrust/1731908676359_create_users/up.sql`:
- Around line 8-12: The migration currently creates a UNIQUE constraint
users_email_unique which already creates an index, then adds a redundant CREATE
INDEX idx_users_email; remove the separate CREATE INDEX idx_users_email (or
alternatively create a UNIQUE INDEX first and attach the constraint to it with
ADD CONSTRAINT ... USING INDEX) so the email uniqueness index is not duplicated;
update the users ALTER TABLE/constraint/index lines accordingly around
users_email_unique and idx_users_email to either rely on the unique constraint's
index or to create a single unique index and bind the constraint to that index.
- Around line 1-12: Schema-qualify the objects to avoid search_path issues:
change CREATE TABLE users to CREATE TABLE public.users, change ALTER TABLE users
to ALTER TABLE public.users, and change CREATE INDEX idx_users_email to CREATE
INDEX public.idx_users_email (keep the constraint name users_email_unique as-is
or rename if you want schema-qualified naming); update references to the
table/index names in this migration (symbols: users, public.users,
users_email_unique, idx_users_email, public.idx_users_email).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 359a23ac-9574-4fc6-909f-3c5712e0c043
📒 Files selected for processing (2)
infra/hasura/migrations/safetrust/1731908676359_create_users/down.sqlinfra/hasura/migrations/safetrust/1731908676359_create_users/up.sql
| ALTER TABLE IF EXISTS users | ||
| DROP CONSTRAINT IF EXISTS users_email_unique CASCADE; | ||
|
|
||
| DROP TABLE IF EXISTS users CASCADE; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, explore the migration directory structure
find infra/hasura/migrations/safetrust -type f -name "*.sql" | head -20Repository: safetrustcr/dApp-SafeTrust
Length of output: 207
🏁 Script executed:
# Read the full down.sql file to see the complete context
cat infra/hasura/migrations/safetrust/1731908676359_create_users/down.sqlRepository: safetrustcr/dApp-SafeTrust
Length of output: 285
🏁 Script executed:
# Read the corresponding up.sql file to understand what's being created
cat infra/hasura/migrations/safetrust/1731908676359_create_users/up.sqlRepository: safetrustcr/dApp-SafeTrust
Length of output: 401
🏁 Script executed:
# Search for any foreign key constraints or references to the users table in other migrations
rg "CONSTRAINT.*users|FOREIGN KEY.*users|references.*users" infra/hasura/migrations/ -iRepository: safetrustcr/dApp-SafeTrust
Length of output: 319
🏁 Script executed:
# Check all migrations to understand dependencies and ordering
git ls-files infra/hasura/migrations/safetrust/ | grep -E '\.sql$' | sortRepository: safetrustcr/dApp-SafeTrust
Length of output: 207
🏁 Script executed:
# Search for CASCADE patterns across all migrations
rg "DROP.*CASCADE|CASCADE" infra/hasura/migrations/ --type sql | head -30Repository: safetrustcr/dApp-SafeTrust
Length of output: 419
Simplify rollback: remove unused CASCADE and fix DROP INDEX ordering.
The CASCADE clauses can be removed (no foreign key dependencies exist), but the actual issue is that DROP INDEX IF EXISTS idx_users_email CASCADE; runs after DROP TABLE, making it redundant—the table drop already removes its indexes. Remove the separate index drop or reorder before the table drop if needed for clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infra/hasura/migrations/safetrust/1731908676359_create_users/down.sql` around
lines 2 - 5, Remove the unnecessary CASCADEs and the redundant index drop:
delete the CASCADE modifiers from the DROP CONSTRAINT and DROP TABLE statements
for the users table (symbols: users, users_email_unique, DROP TABLE users) and
also remove the separate DROP INDEX IF EXISTS idx_users_email statement (the
table drop already removes its indexes), leaving only the simplified DROP
CONSTRAINT and DROP TABLE lines in down.sql.
1 participant
Closes #77
Summary
This PR adds the missing Hasura migration for the
userstable required by the Firebase auth sync flow.Files Added
What it does
public.userswith:id TEXT PRIMARY KEY(Firebase UID)email TEXT NOT NULLlast_seen TIMESTAMP WITH TIME ZONE DEFAULT NOW()users_email_uniqueunique constraint onemailidx_users_emailindexdown.sqlTesting
docker compose up -dhttp://localhost:8080/consoleuserstable exists underpublicpsql\d usersid TEXTprimary keyemail TEXTuniquelast_seen timestamptz\di idx_users_emaildocker compose down -vdocker compose up -dNotes
Summary by CodeRabbit