added Script to hard - delete organization and related data #778
Conversation
WalkthroughThis change updates a migration script to handle an additional foreign key constraint on the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Script
participant PostgreSQL
User->>Script: Start script
Script->>User: Prompt for organization code and tenant code
User->>Script: Enter organization code and tenant code
Script->>PostgreSQL: Query counts of related data across multiple tables
PostgreSQL-->>Script: Return counts
Script->>User: Display summary table
Script->>User: Prompt for deletion confirmation
User->>Script: Confirm deletion
Script->>PostgreSQL: Begin transaction
Script->>PostgreSQL: Delete user organization roles
Script->>PostgreSQL: Delete user organizations (capture user IDs)
Script->>PostgreSQL: Delete orphaned users
Script->>PostgreSQL: Delete organization
Script->>PostgreSQL: Delete invites, templates, features, registration codes
Script->>PostgreSQL: Commit transaction
Script->>User: Log deleted rows and completion
Script->>PostgreSQL: Close connection
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/database/migrations/20250729064710-org-code-fix.js (1)
53-62: Missing fk_retainer.push() call for the first constraint.The code disables the foreign key constraint
fk_org_user_invites_organization_idbut doesn't add the enable query tofk_retainerarray. This means the constraint won't be re-enabled after the migration completes, potentially leaving the database in an inconsistent state.Add the missing line after line 62:
await queryInterface.sequelize.query(disableFK(table, fk_name), { type: Sequelize.QueryTypes.RAW, raw: true, transaction, }) +fk_retainer.push(enableFK(table, fk_name, fkey, refTable, refKey))
🧹 Nitpick comments (3)
src/scripts/deleted-org-data-clean-up/clean.js (3)
244-244: Remove commented code.The commented
await t.commit()is unnecessary as Sequelize automatically commits the transaction when the callback completes successfully.- // await t.commit();
252-252: Enhance error logging for better debugging.Consider logging the full error stack trace for easier debugging.
- console.error(`Error executing transaction: ${error}`) + console.error('Error executing transaction:', error)
149-151: Consider adding a dry-run mode for safety.A dry-run option would allow users to see what would be deleted without actually performing the deletion.
Would you like me to help implement a dry-run mode that shows what data would be deleted without executing the actual deletions?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/database/migrations/20250729064710-org-code-fix.js(1 hunks)src/scripts/deleted-org-data-clean-up/clean.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 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.
📚 Learning: the migration for converting tenant_code to a primary key in the entitytype model was already handle...
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.
Applied to files:
src/database/migrations/20250729064710-org-code-fix.js
🔇 Additional comments (1)
src/database/migrations/20250729064710-org-code-fix.js (1)
63-73: LGTM! Proper handling of the second foreign key constraint.The added code correctly handles the
fk_org_user_invites_org_codeconstraint by disabling it before updates and scheduling its re-enabling through thefk_retainerarray.
✅ Actions performedFull review triggered. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/scripts/deleted-org-data-clean-up/clean.js (2)
3-3: Fix dotenv configuration path.The relative path to .env file might not work when the script is executed from different directories.
54-136: Critical SQL injection vulnerability in REPORT_QUERY.Both
organizationCodeandtenantCodevariables are directly interpolated into the SQL query string, creating SQL injection vulnerabilities. User input must never be directly concatenated into SQL queries.Use parameterized queries with Sequelize's replacements feature. Replace all string interpolations with named parameters:
- WHERE organization_code = '${organizationCode}' AND tenant_code = '${tenantCode}' + WHERE organization_code = :organizationCode AND tenant_code = :tenantCodeThen update the query execution at line 137:
const report = await sequelize.query(REPORT_QUERY, { + replacements: { organizationCode, tenantCode }, type: Sequelize.QueryTypes.RAW, raw: true, })
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/database/migrations/20250729064710-org-code-fix.js(1 hunks)src/scripts/deleted-org-data-clean-up/clean.js(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 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.
📚 Learning: the migration for converting tenant_code to a primary key in the entitytype model was already handle...
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.
Applied to files:
src/database/migrations/20250729064710-org-code-fix.jssrc/scripts/deleted-org-data-clean-up/clean.js
📚 Learning: in the elevate-project/user codebase, organizationcode and tenantcode parameters passed to service m...
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#776
File: src/services/entities.js:18-23
Timestamp: 2025-07-31T08:44:36.982Z
Learning: In the ELEVATE-Project/user codebase, organizationCode and tenantCode parameters passed to service methods always come from req.decodedToken.organization_code and req.decodedToken.tenant_code, which are guaranteed to be present after token validation. Additional validation for these parameters in service methods is unnecessary as the token validation process ensures they are always available.
Applied to files:
src/scripts/deleted-org-data-clean-up/clean.js
🧬 Code Graph Analysis (1)
src/scripts/deleted-org-data-clean-up/clean.js (1)
src/scripts/delete-scripts/delete-transactional-data.js (5)
DELETE_USER_ORG_ROLES_QUERY(95-95)DELETE_USER_ORGS_QUERY(104-104)userIdsToDelete(90-90)DELETE_USERS_QUERY(113-113)DELETE_ORGS_QUERY(159-159)
🔇 Additional comments (1)
src/database/migrations/20250729064710-org-code-fix.js (1)
63-73: LGTM! Properly handles additional foreign key constraint.The implementation correctly follows the established pattern for disabling and re-enabling foreign key constraints. The constraint
fk_org_user_invites_org_codeis appropriately disabled during the migration and scheduled for re-enabling through thefk_retainerarray.
2 participants
Summary by CodeRabbit
New Features
Chores