Conversation
WalkthroughAdds a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/scripts/insertDefaultOrg.js (2)
42-46: Parameterize INSERT; pass env via replacements; remove dead mapping.This eliminates injection risk and makes the replacements list explicit.
-const insertQuery = ` - INSERT INTO organizations (name, code, description, status, updated_at, created_at , tenant_code) - VALUES (?, '${process.env.DEFAULT_ORGANISATION_CODE}', ?, ?, NOW(), NOW(), '${process.env.DEFAULT_TENANT_CODE}') - RETURNING id; -` +const insertQuery = ` + INSERT INTO organizations (name, code, description, status, updated_at, created_at, tenant_code) + VALUES (?, ?, ?, ?, NOW(), NOW(), ?) + RETURNING id; +`-const defaultValues = ['Default Organization', 'Default Organisation', 'ACTIVE'] -const queryParams = defaultValues.map((value, index) => (value === 'default' ? null : value)) +const insertParams = [ + 'Default Organization', // name + process.env.DEFAULT_ORGANISATION_CODE, // code + 'Default Organisation', // description + 'ACTIVE', // status + process.env.DEFAULT_TENANT_CODE, // tenant_code +]-const [result] = await sequelize.query(insertQuery, { replacements: queryParams, raw: true }) +const [result] = await sequelize.query(insertQuery, { replacements: insertParams, raw: true })Also applies to: 66-68, 82-85
3-4: Fix .env loading path (resolve from __dirname)src/scripts/insertDefaultOrg.js (lines 3–4) — '../.env' is cwd-dependent and can silently fail; resolve an absolute path from __dirname.
-require('dotenv').config({ path: '../.env' }) +require('dotenv').config({ path: require('path').resolve(__dirname, '../../.env') })
🧹 Nitpick comments (6)
src/scripts/insertDefaultOrg.js (6)
11-11: Exit with non‑zero status on fatal env validation failure.-process.exit() +process.exit(1)
35-35: Remove debug log leaking env value.Not sensitive, but avoid printing env in normal runs.
-console.log(process.env.DEFAULT_ORGANISATION_CODE)
49-51: Feature seeding may need tenant scoping.If features are tenant‑scoped, filter by tenant_code to avoid cross‑tenant copies; otherwise confirm they’re global.
Example (only if features has tenant_code):
SELECT * FROM features WHERE tenant_code = ? OR tenant_code IS NULL ORDER BY display_order;…and pass [process.env.DEFAULT_TENANT_CODE] as replacement.
Also applies to: 92-102
112-112: Clarify error message.-console.error(`Error creating function: ${error}`) +console.error(`Error seeding default organization and features: ${error}`)
71-111: Optional: make org + features insert atomic via a transaction.Keeps DB consistent if a feature insert fails.
const t = await sequelize.transaction() try { const [existingRow] = await sequelize.query(checkQuery, { replacements: [process.env.DEFAULT_ORGANISATION_CODE, process.env.DEFAULT_TENANT_CODE], transaction: t, raw: true }) if (existingRow.length > 0) { await t.rollback(); return } const [rows] = await sequelize.query(insertQuery, { replacements: insertParams, transaction: t, raw: true }) const insertedRowId = rows[0].id const [features] = await sequelize.query(fetchFeaturesQuery, { transaction: t, raw: true }) for (const feature of features) { await sequelize.query(insertOrgFeatureQuery, { replacements: [process.env.DEFAULT_ORGANISATION_CODE, feature.code, true, feature.label, feature.icon, process.env.DEFAULT_TENANT_CODE, feature.display_order], transaction: t, raw: true }) } await t.commit() } catch (e) { await t.rollback(); throw e }
72-73: Optional: specify QueryTypes for clarity and stable shapes.const { QueryTypes } = require('sequelize') // ... const [existingRow] = await sequelize.query(checkQuery, { replacements: [...], type: QueryTypes.SELECT }) const [result] = await sequelize.query(insertQuery, { replacements: insertParams, type: QueryTypes.INSERT }) const [features] = await sequelize.query(fetchFeaturesQuery, { type: QueryTypes.SELECT })Also applies to: 82-83, 90-91
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/scripts/insertDefaultOrg.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#776
File: src/database/models/entityType.js:38-38
Timestamp: 2025-07-31T08:43:35.971Z
Learning: The migration for converting tenant_code to a primary key in the EntityType model was already handled in a previous PR, not in the current refactoring PR that focuses on organization codes instead of organization IDs.
🔇 Additional comments (1)
src/scripts/insertDefaultOrg.js (1)
37-41: ```shell
#!/bin/bash
set -euo pipefailecho "PWD: $(pwd)"
FILE=src/scripts/insertDefaultOrg.js
if [ -f "$FILE" ]; then
echo "FOUND: $FILE"
echo "----- Showing file (1-300) with line numbers -----"
awk '{printf "%4d: %s\n", NR, $0}' "$FILE" | sed -n '1,300p'
else
echo "MISSING: $FILE"
fisearch helper
run_search() {
local pattern="$1"
if command -v rg >/dev/null 2>&1; then
rg -n --hidden -S --no-ignore-vcs "$pattern" || true
else
grep -RIn --exclude-dir=node_modules --exclude-dir=.git --exclude-dir=.venv "$pattern" . || true
fi
}echo "----- Search: DEFAULT_ORGANISATION_CODE -----"
run_search "DEFAULT_ORGANISATION_CODE"echo "----- Search: DEFAULT_TENANT_CODE -----"
run_search "DEFAULT_TENANT_CODE"echo "----- Search: process.env.DEFAULT_ occurrences -----"
run_search "process.env.DEFAULT_"echo "----- Search: sequelize.query occurrences (repo-wide) -----"
run_search "sequelize\.query"echo "----- Search: tenant_code across repo -----"
run_search "tenant_code"echo "----- Search: organizations (table/model/migration references) -----"
run_search "organizations"echo "----- Search: createTable('organizations' in migrations) -----"
if command -v rg >/dev/null 2>&1; then
rg -nP "createTable\s*\(\s*['"]organizations['"]" || true
else
grep -RIn "createTable" . | grep -i "organizations" || true
fiecho "----- Search: model definition for organizations (sequelize.define / model file) -----"
run_search "define\(\s*['"]organizations|module\.exports\s*=\s*\(sequelize, DataTypes\)\s*=>\s*\{|exports\.default.*organizations"echo "----- Search: unique/index constraints mentioning code and tenant -----"
run_search "UNIQUE|unique|index.*code|fields.*code|code.*unique|code.*tenant|unique.*tenant|addIndex\(|addConstraint\("echo "----- Show package.json (if present) -----"
if [ -f package.json ]; then
sed -n '1,200p' package.json
else
echo "no package.json at repo root"
fi</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
2 participants
Summary by CodeRabbit