Conversation
…lter Changed isoutbound filter from "is false" to "IS NOT TRUE" in Pr_104BeneficiaryReport stored procedure to include NULL records. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds database migrations to establish a facility hierarchy schema. The first migration introduces a hierarchy level reference table, extends facility and facility-type tables with state/district/block/village hierarchy columns, creates facility-to-location and ASHA-to-supervisor mapping tables, and scopes user service roles to facilities and locations. The second migration adds facility tracking to the outreach flow table. ChangesFacility Hierarchy Schema and Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 |
…as V64__Facility_Hierarchy.sql Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/main/resources/db/migration/dbiemr/V81__i_ben_flow_outreach_facilityID.sql (1)
15-15: ⚡ Quick winConsider indexing
facilityIDfor common filter/join paths.If outreach reports will query by facility, indexing
facilityIDwill prevent full scans as data grows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/resources/db/migration/dbiemr/V81__i_ben_flow_outreach_facilityID.sql` at line 15, The migration adds facilityID to table i_ben_flow_outreach but doesn’t create an index; update V81__i_ben_flow_outreach_facilityID.sql to create an index on column facilityID (e.g., add a CREATE INDEX / ALTER TABLE ... ADD INDEX statement named something like idx_i_ben_flow_outreach_facilityID) immediately after the ALTER TABLE so common filter/join paths on i_ben_flow_outreach.facilityID avoid full table scans.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/resources/db/migration/dbiemr/V64_Facility_Hierarchy.sql`:
- Around line 136-139: Change the mapping FK columns to be non-nullable: update
the column definitions for supervisorUserID, ashaUserID, and facilityID in this
migration (V64_Facility_Hierarchy.sql) from INT to INT NOT NULL so relationship
rows cannot be created with NULL keys; ensure any existing seed data in this
migration satisfies the NOT NULL constraint before applying (or add appropriate
DEFAULT/cleanup steps) and keep the deleted BOOLEAN DEFAULT FALSE line as-is.
In
`@src/main/resources/db/migration/dbiemr/V81__i_ben_flow_outreach_facilityID.sql`:
- Around line 1-3: Remove the hardcoded "USE db_iemr;" and make the migration
schema-agnostic by relying on the dynamic database name already captured by "SET
`@dbname` = DATABASE();" (or a migration parameter), then update any subsequent
object references in this script to use the `@dbname` variable or unqualified
names so the migration runs against the current connection's schema instead of
forcing db_iemr.
- Around line 13-21: The migration adds facilityID to i_ben_flow_outreach but
does not add a foreign key; modify the IF branch that runs when `@col_exists` = 0
so the generated SQL both adds the column and adds a foreign key constraint
referencing m_facility(FacilityID) (e.g., ALTER TABLE i_ben_flow_outreach ADD
COLUMN facilityID INT NULL, ADD CONSTRAINT fk_i_ben_flow_outreach_facilityID
FOREIGN KEY (facilityID) REFERENCES m_facility(FacilityID)); ensure the prepared
statement uses that combined ALTER and choose a clear constraint name
(fk_i_ben_flow_outreach_facilityID) and keep the existing behavior when the
column already exists.
---
Nitpick comments:
In
`@src/main/resources/db/migration/dbiemr/V81__i_ben_flow_outreach_facilityID.sql`:
- Line 15: The migration adds facilityID to table i_ben_flow_outreach but
doesn’t create an index; update V81__i_ben_flow_outreach_facilityID.sql to
create an index on column facilityID (e.g., add a CREATE INDEX / ALTER TABLE ...
ADD INDEX statement named something like idx_i_ben_flow_outreach_facilityID)
immediately after the ALTER TABLE so common filter/join paths on
i_ben_flow_outreach.facilityID avoid full table scans.
🪄 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: adf09e23-725c-4317-9234-11b42458b4bf
📒 Files selected for processing (2)
src/main/resources/db/migration/dbiemr/V64_Facility_Hierarchy.sqlsrc/main/resources/db/migration/dbiemr/V81__i_ben_flow_outreach_facilityID.sql
| supervisorUserID INT, | ||
| ashaUserID INT, | ||
| facilityID INT, | ||
| deleted BOOLEAN DEFAULT FALSE, |
There was a problem hiding this comment.
Make mapping foreign-key columns non-nullable.
Line 136-139 allows supervisorUserID, ashaUserID, and facilityID to be NULL, which permits relationship rows that are not valid mappings.
Suggested fix
CREATE TABLE IF NOT EXISTS asha_supervisor_mapping (
id BIGINT AUTO_INCREMENT PRIMARY KEY,
- supervisorUserID INT,
- ashaUserID INT,
- facilityID INT,
+ supervisorUserID INT NOT NULL,
+ ashaUserID INT NOT NULL,
+ facilityID INT NOT NULL,
deleted BOOLEAN DEFAULT FALSE,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/resources/db/migration/dbiemr/V64_Facility_Hierarchy.sql` around
lines 136 - 139, Change the mapping FK columns to be non-nullable: update the
column definitions for supervisorUserID, ashaUserID, and facilityID in this
migration (V64_Facility_Hierarchy.sql) from INT to INT NOT NULL so relationship
rows cannot be created with NULL keys; ensure any existing seed data in this
migration satisfies the NOT NULL constraint before applying (or add appropriate
DEFAULT/cleanup steps) and keep the deleted BOOLEAN DEFAULT FALSE line as-is.
| SET @sql = IF( | ||
| @col_exists = 0, | ||
| 'ALTER TABLE i_ben_flow_outreach ADD COLUMN facilityID INT NULL', | ||
| 'SELECT "Column facilityID already exists"' | ||
| ); | ||
|
|
||
| PREPARE stmt FROM @sql; | ||
| EXECUTE stmt; | ||
| DEALLOCATE PREPARE stmt; |
There was a problem hiding this comment.
Add a foreign key for facilityID to preserve referential integrity.
Line 15 adds facilityID but no FK to m_facility(FacilityID), so invalid facility references can be inserted.
Suggested fix
SET `@sql` = IF(
`@col_exists` = 0,
'ALTER TABLE i_ben_flow_outreach ADD COLUMN facilityID INT NULL',
'SELECT "Column facilityID already exists"'
);
PREPARE stmt FROM `@sql`;
EXECUTE stmt;
DEALLOCATE PREPARE stmt;
+
+SET `@fk_exists` = (
+ SELECT COUNT(*)
+ FROM information_schema.TABLE_CONSTRAINTS
+ WHERE TABLE_SCHEMA = `@dbname`
+ AND TABLE_NAME = 'i_ben_flow_outreach'
+ AND CONSTRAINT_NAME = 'fk_ibfo_facility'
+);
+
+SET `@sql` = IF(
+ `@fk_exists` = 0,
+ 'ALTER TABLE i_ben_flow_outreach ADD CONSTRAINT fk_ibfo_facility FOREIGN KEY (facilityID) REFERENCES m_facility(FacilityID)',
+ 'SELECT "FK fk_ibfo_facility already exists"'
+);
+
+PREPARE stmt FROM `@sql`;
+EXECUTE stmt;
+DEALLOCATE PREPARE stmt;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SET @sql = IF( | |
| @col_exists = 0, | |
| 'ALTER TABLE i_ben_flow_outreach ADD COLUMN facilityID INT NULL', | |
| 'SELECT "Column facilityID already exists"' | |
| ); | |
| PREPARE stmt FROM @sql; | |
| EXECUTE stmt; | |
| DEALLOCATE PREPARE stmt; | |
| SET `@sql` = IF( | |
| `@col_exists` = 0, | |
| 'ALTER TABLE i_ben_flow_outreach ADD COLUMN facilityID INT NULL', | |
| 'SELECT "Column facilityID already exists"' | |
| ); | |
| PREPARE stmt FROM `@sql`; | |
| EXECUTE stmt; | |
| DEALLOCATE PREPARE stmt; | |
| SET `@fk_exists` = ( | |
| SELECT COUNT(*) | |
| FROM information_schema.TABLE_CONSTRAINTS | |
| WHERE TABLE_SCHEMA = `@dbname` | |
| AND TABLE_NAME = 'i_ben_flow_outreach' | |
| AND CONSTRAINT_NAME = 'fk_ibfo_facility' | |
| ); | |
| SET `@sql` = IF( | |
| `@fk_exists` = 0, | |
| 'ALTER TABLE i_ben_flow_outreach ADD CONSTRAINT fk_ibfo_facility FOREIGN KEY (facilityID) REFERENCES m_facility(FacilityID)', | |
| 'SELECT "FK fk_ibfo_facility already exists"' | |
| ); | |
| PREPARE stmt FROM `@sql`; | |
| EXECUTE stmt; | |
| DEALLOCATE PREPARE stmt; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/resources/db/migration/dbiemr/V81__i_ben_flow_outreach_facilityID.sql`
around lines 13 - 21, The migration adds facilityID to i_ben_flow_outreach but
does not add a foreign key; modify the IF branch that runs when `@col_exists` = 0
so the generated SQL both adds the column and adds a foreign key constraint
referencing m_facility(FacilityID) (e.g., ALTER TABLE i_ben_flow_outreach ADD
COLUMN facilityID INT NULL, ADD CONSTRAINT fk_i_ben_flow_outreach_facilityID
FOREIGN KEY (facilityID) REFERENCES m_facility(FacilityID)); ensure the prepared
statement uses that combined ALTER and choose a clear constraint name
(fk_i_ben_flow_outreach_facilityID) and keep the existing behavior when the
column already exists.



📋 Description
JIRA ID: AMM-2206
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit