cleanup(seed): remove organization seeding from scripts/seed.ts#29009
cleanup(seed): remove organization seeding from scripts/seed.ts#29009romitg2 wants to merge 6 commits intocalcom:mainfrom
Conversation
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
romitg2
added
run-ci
📝 WalkthroughWalkthroughThe seeding script ( 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 (1)
scripts/seed.ts (1)
138-145: Tighten the Prisma query and preferfindUniquewith an explicitselect.
User.emailcarries a unique constraint, sofindUniqueis the correct lookup primitive here, and per the repo guideline Prisma reads shouldselectonly the fields they consume (justidin this case) to avoid pulling the full row (including sensitive columns). Also consider logging when the user is missing so failed seeds are visible rather than silently skipped.♻️ Proposed refactor
async function ensureProUserHasApiKeySeeded() { - const proUser = await prisma.user.findFirst({ - where: { email: "pro@example.com" }, - }); - if (proUser) { - await seedApiKey(proUser.id, "0123456789abcdef0123456789abcdef"); - } + const proUser = await prisma.user.findUnique({ + where: { email: "pro@example.com" }, + select: { id: true }, + }); + if (!proUser) { + console.log("⚠️ pro@example.com not found, skipping API key seeding."); + return; + } + await seedApiKey(proUser.id, "0123456789abcdef0123456789abcdef"); }As per coding guidelines: "Use
selectinstead ofincludein Prisma queries for performance and security" and "Use early returns to reduce nesting".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/seed.ts` around lines 138 - 145, In ensureProUserHasApiKeySeeded replace prisma.user.findFirst with prisma.user.findUnique and use an explicit select { id: true } so only the id is fetched; use an early return if no user is found and log that fact (e.g., via processLogger or existing logger) instead of silently skipping, and continue to call seedApiKey(proUser.id, "0123456789abcdef0123456789abcdef") when the user exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/seed.ts`:
- Line 719: The removal of the Acme organization and owner1-acme user from
main() breaks two downstream scripts; either restore the Acme seeding in main()
or update the dependent scripts/docs to stop assuming those entities exist. Fix
option A: re-add the Acme org + owner1-acme seeding where main() runs other
seeds (so ensureProUserHasApiKeySeeded() and peers still run against a
consistent seed surface), recreating the org/team with slug "acme" and the user
"owner1-acme" (and its org profile). Fix option B: modify scripts
seed-booking-audit.ts and prepare-local-for-delegation-credentials-testing.js to
either create their required org/user fixtures at runtime or to reference an
existing seeded test user/org (remove hard-coded "owner1-acme" / "acme"
assumptions) and update any docs referencing those scripts; ensure error checks
that previously threw "User owner1-acme not found..." and "has no organization
profile" are removed or replaced with fixture-creation logic.
---
Nitpick comments:
In `@scripts/seed.ts`:
- Around line 138-145: In ensureProUserHasApiKeySeeded replace
prisma.user.findFirst with prisma.user.findUnique and use an explicit select {
id: true } so only the id is fetched; use an early return if no user is found
and log that fact (e.g., via processLogger or existing logger) instead of
silently skipping, and continue to call seedApiKey(proUser.id,
"0123456789abcdef0123456789abcdef") when the user exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
|
||
| await ensureAcmeOwnerHasApiKeySeeded(); | ||
| await seedPerHostLocationsInAcmeOrg(); | ||
| await ensureProUserHasApiKeySeeded(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find any remaining references to the Acme org / owner1-acme user / dunder org
# across scripts, docs, and tests so we can tell whether they need updates too.
rg -nP --hidden -g '!**/node_modules/**' \
-e '\bowner1-acme\b' \
-e '"acme"' \
-e "'acme'" \
-e '\bdunder\b' \
-e 'isOrganization\s*:\s*true' \
-e 'createOrganizationAndAddMembersAndTeams'Repository: calcom/cal.diy
Length of output: 10728
🏁 Script executed:
# Check if these two scripts were modified/removed in the PR and verify their current state
git ls-files scripts/ | grep -E "(seed-booking-audit|prepare-local-for-delegation)"Repository: calcom/cal.diy
Length of output: 148
🏁 Script executed:
# Examine the current state of both scripts to see if they've been updated
head -50 scripts/seed-booking-audit.ts
tail -30 scripts/seed-booking-audit.tsRepository: calcom/cal.diy
Length of output: 2543
🏁 Script executed:
# Check the prepare-local-for-delegation-credentials-testing.js script
head -50 scripts/prepare-local-for-delegation-credentials-testing.jsRepository: calcom/cal.diy
Length of output: 1859
🏁 Script executed:
# Look at the specific area in seed.ts mentioned in the review (line 719)
sed -n '700,750p' scripts/seed.tsRepository: calcom/cal.diy
Length of output: 781
🏁 Script executed:
# Search for where Acme org was created in seed.ts to confirm it's been removed
rg -n 'createOrganizationAndAddMembersAndTeams|Acme|owner1-acme' scripts/seed.ts | head -20Repository: calcom/cal.diy
Length of output: 40
🏁 Script executed:
# Check the git diff to see what was actually changed in seed.ts
git diff --no-index /dev/null scripts/seed.ts 2>/dev/null | grep -A5 -B5 'ensureProUserHasApiKeySeeded' | head -30Repository: calcom/cal.diy
Length of output: 634
Two seed scripts will break after Acme org/owner1-acme seeding is removed.
After this PR, scripts/seed-booking-audit.ts and scripts/prepare-local-for-delegation-credentials-testing.js will fail. Both scripts have hard-coded dependencies on the Acme organization and owner1-acme user that are no longer produced by main():
scripts/seed-booking-audit.ts(lines 389–407) fails with"User owner1-acme not found. Run the main seed first."or"User owner1-acme has no organization profile."scripts/prepare-local-for-delegation-credentials-testing.js(lines 1–42) fails when it cannot findowner1-acme@example.comor the team withslug: "acme", isOrganization: true.
Either update or remove these scripts (and any docs referencing them) in this PR, or restore the Acme org seeding to keep the seed surface consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/seed.ts` at line 719, The removal of the Acme organization and
owner1-acme user from main() breaks two downstream scripts; either restore the
Acme seeding in main() or update the dependent scripts/docs to stop assuming
those entities exist. Fix option A: re-add the Acme org + owner1-acme seeding
where main() runs other seeds (so ensureProUserHasApiKeySeeded() and peers still
run against a consistent seed surface), recreating the org/team with slug "acme"
and the user "owner1-acme" (and its org profile). Fix option B: modify scripts
seed-booking-audit.ts and prepare-local-for-delegation-credentials-testing.js to
either create their required org/user fixtures at runtime or to reference an
existing seeded test user/org (remove hard-coded "owner1-acme" / "acme"
assumptions) and update any docs referencing those scripts; ensure error checks
that previously threw "User owner1-acme not found..." and "has no organization
profile" are removed or replaced with fixture-creation logic.
Pull request was converted to draft
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
2 participants
What This Does
scripts/seed.ts.Seed Verification
Command run:
Result:
Log excerpt: