104: Add Call Activity Details Section for Outbound Calls#96
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds SQL migration V61 that creates Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/resources/db/migration/dbiemr/V61__Outbound_CallActivity.sql`:
- Around line 66-80: The prepared statement for modifying CzentrixCallID is
using a hardcoded table name `db_iemr.t_104CoMoOutboundCallDetails` instead of
the dynamic schema/table variables; update the construction of
`@preparedStatement` (the SELECT/IF that builds the ALTER) to use
CONCAT(`@dbname`, '.', `@tablename`) for the ALTER TABLE target (consistent with the
other blocks using `@dbname` and `@tablename`) so the ALTER operates on the intended
database/table at runtime.
- Around line 3-11: Add a UNIQUE constraint on ActivityName in the
m_outbound_call_activity table to prevent duplicates (modify the CREATE TABLE in
V61__Outbound_CallActivity.sql to include UNIQUE on ActivityName), insert the
required seed rows for the master data immediately after the CREATE TABLE (use
INSERT INTO m_outbound_call_activity (...) VALUES (...) for the expected
activities), and fix the hardcoded ALTER TABLE statement that references
t_104CoMoOutboundCallDetails by replacing the literal SQL string with a CONCAT
using `@dbname` and `@tablename` (e.g., change the `'ALTER TABLE
db_iemr.t_104CoMoOutboundCallDetails ...'` entry to use CONCAT('ALTER TABLE
',`@dbname`,'.',`@tablename`,' ...') so it follows the same variable-driven
pattern).
- Around line 17-30: The new ActivityID column on t_104CoMoOutboundCallDetails
needs a conditional foreign key creation block added immediately after the ADD
COLUMN step: check INFORMATION_SCHEMA.TABLE_CONSTRAINTS for the existence of
constraint fk_104CoMoOutboundCallDetails_ActivityID and, if missing, run an
ALTER TABLE t_104CoMoOutboundCallDetails ADD CONSTRAINT
fk_104CoMoOutboundCallDetails_ActivityID FOREIGN KEY (ActivityID) REFERENCES
m_outbound_call_activity(ActivityID); use the same PREPARE/EXECUTE/DEALLOCATE
PREPARE pattern as the ADD COLUMN block so the FK is created only when absent.
src/main/resources/db/migration/dbiemr/V61__Outbound_CallActivity.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/main/resources/db/migration/dbiemr/V61__Outbound_CallActivity.sql (3)
17-32: Missing FK constraint forActivityID.Already raised in a previous review — a conditional
ALTER TABLE … ADD CONSTRAINT … FOREIGN KEY (ActivityID) REFERENCES m_outbound_call_activity(ActivityID)block using the same PREPARE/EXECUTE pattern is still absent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/dbiemr/V61__Outbound_CallActivity.sql` around lines 17 - 32, Add a conditional foreign-key constraint creation for ActivityID similar to the existing column-add block: after the ALTER/ADD COLUMN logic for ActivityID, build a `@preparedStatement` that checks INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS/KEY_COLUMN_USAGE for an existing FK and, if absent, CONVERT it into an ALTER TABLE ... ADD CONSTRAINT fk_outbound_call_activity FOREIGN KEY (ActivityID) REFERENCES m_outbound_call_activity(ActivityID); then PREPARE stmt FROM `@preparedStatement`; EXECUTE stmt; and DEALLOCATE PREPARE stmt; so the code using `@preparedStatement`, PREPARE stmt, EXECUTE stmt, and DEALLOCATE PREPARE stmt mirrors the column creation pattern but enforces the FK.
73-85: Hardcoded table reference on line 80.Already flagged in a previous review — the
'ALTER TABLE db_iemr.t_104CoMoOutboundCallDetails MODIFY COLUMN CzentrixCallID VARCHAR(30);'literal should useCONCAT(@dbname, '.',@tablename, ' MODIFY COLUMN CzentrixCallID VARCHAR(30);')to match the pattern used in every other block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/dbiemr/V61__Outbound_CallActivity.sql` around lines 73 - 85, The ALTER TABLE statement currently hardcodes the schema and table name in the `@preparedStatement` string; update the `@preparedStatement` construction (where `@columnname` = 'CzentrixCallID' and PREPARE stmt / EXECUTE stmt are used) to build the ALTER statement with CONCAT(`@dbname`, '.', `@tablename`, ' MODIFY COLUMN CzentrixCallID VARCHAR(30);') so it matches the pattern used elsewhere and avoids the literal 'db_iemr.t_104CoMoOutboundCallDetails'.
3-11: No UNIQUE constraint onActivityNameand no seed data.Already flagged in a previous review — both the missing
UNIQUEconstraint and absent seedINSERTstatements should be addressed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/dbiemr/V61__Outbound_CallActivity.sql` around lines 3 - 11, Add a UNIQUE constraint on the ActivityName column and include seed INSERTs for the m_outbound_call_activity table: modify the CREATE TABLE m_outbound_call_activity statement to define ActivityName as UNIQUE (either inline UNIQUE key or a separate UNIQUE INDEX on ActivityName) and append idempotent seed INSERT statements (e.g., INSERT IGNORE or INSERT ... ON DUPLICATE KEY UPDATE) that populate default activities with values for ActivityName, IsActive, CreatedBy, CreatedDate, ModifiedBy, LastModDate so the migration creates the constraint and inserts initial rows for m_outbound_call_activity.
🧹 Nitpick comments (1)
src/main/resources/db/migration/dbiemr/V61__Outbound_CallActivity.sql (1)
87-199: InconsistentDEFAULT NULLacross ADD COLUMN statements.The first three blocks (lines 28, 46, 64) declare columns with
DEFAULT NULLexplicitly, while all ten columns in the "Additional Columns" section omit it entirely. Functionally equivalent, but inconsistent within the same script.♻️ Proposed fix (representative example)
- CONCAT('ALTER TABLE ',`@dbname`,'.',`@tablename`,' ADD COLUMN BeneficiaryRegID BIGINT;') + CONCAT('ALTER TABLE ',`@dbname`,'.',`@tablename`,' ADD COLUMN BeneficiaryRegID BIGINT DEFAULT NULL;')Apply the same
DEFAULT NULLsuffix to all tenADD COLUMNstrings in this section.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/dbiemr/V61__Outbound_CallActivity.sql` around lines 87 - 199, The ADD COLUMN statements for BeneficiaryRegID, ProviderServiceMapID, PrefferedDateTime, BenCallID, RequestedFor, CallTypeID, SubServiceID, AssignedUserID, RequestedFeature, and IsSelf are missing the "DEFAULT NULL" suffix and should match the earlier blocks; update the CONCAT(...) strings that build `@preparedStatement` (the dynamic ALTER TABLE statements) to append " DEFAULT NULL;" for each column so the generated ALTER TABLE for each column includes DEFAULT NULL, keeping the same pattern used in the first three column blocks and using the same `@columnname/`@preparedStatement flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/resources/db/migration/dbiemr/V61__Outbound_CallActivity.sql`:
- Around line 113-122: The migration V61__Outbound_CallActivity.sql currently
creates/checks for the misspelled column name PrefferedDateTime; update the
migration to use the correct column name PreferredDateTime everywhere (change
the `@columnname` value and the ALTER TABLE ADD COLUMN clause), and also handle
existing databases that may already have the misspelled PrefferedDateTime by
adding logic to detect PrefferedDateTime and RENAME it to PreferredDateTime (or
add PreferredDateTime if rename is not desired), keeping the PREPARE
stmt/EXECUTE stmt pattern consistent.
---
Duplicate comments:
In `@src/main/resources/db/migration/dbiemr/V61__Outbound_CallActivity.sql`:
- Around line 17-32: Add a conditional foreign-key constraint creation for
ActivityID similar to the existing column-add block: after the ALTER/ADD COLUMN
logic for ActivityID, build a `@preparedStatement` that checks
INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS/KEY_COLUMN_USAGE for an existing FK
and, if absent, CONVERT it into an ALTER TABLE ... ADD CONSTRAINT
fk_outbound_call_activity FOREIGN KEY (ActivityID) REFERENCES
m_outbound_call_activity(ActivityID); then PREPARE stmt FROM `@preparedStatement`;
EXECUTE stmt; and DEALLOCATE PREPARE stmt; so the code using `@preparedStatement`,
PREPARE stmt, EXECUTE stmt, and DEALLOCATE PREPARE stmt mirrors the column
creation pattern but enforces the FK.
- Around line 73-85: The ALTER TABLE statement currently hardcodes the schema
and table name in the `@preparedStatement` string; update the `@preparedStatement`
construction (where `@columnname` = 'CzentrixCallID' and PREPARE stmt / EXECUTE
stmt are used) to build the ALTER statement with CONCAT(`@dbname`, '.',
`@tablename`, ' MODIFY COLUMN CzentrixCallID VARCHAR(30);') so it matches the
pattern used elsewhere and avoids the literal
'db_iemr.t_104CoMoOutboundCallDetails'.
- Around line 3-11: Add a UNIQUE constraint on the ActivityName column and
include seed INSERTs for the m_outbound_call_activity table: modify the CREATE
TABLE m_outbound_call_activity statement to define ActivityName as UNIQUE
(either inline UNIQUE key or a separate UNIQUE INDEX on ActivityName) and append
idempotent seed INSERT statements (e.g., INSERT IGNORE or INSERT ... ON
DUPLICATE KEY UPDATE) that populate default activities with values for
ActivityName, IsActive, CreatedBy, CreatedDate, ModifiedBy, LastModDate so the
migration creates the constraint and inserts initial rows for
m_outbound_call_activity.
---
Nitpick comments:
In `@src/main/resources/db/migration/dbiemr/V61__Outbound_CallActivity.sql`:
- Around line 87-199: The ADD COLUMN statements for BeneficiaryRegID,
ProviderServiceMapID, PrefferedDateTime, BenCallID, RequestedFor, CallTypeID,
SubServiceID, AssignedUserID, RequestedFeature, and IsSelf are missing the
"DEFAULT NULL" suffix and should match the earlier blocks; update the
CONCAT(...) strings that build `@preparedStatement` (the dynamic ALTER TABLE
statements) to append " DEFAULT NULL;" for each column so the generated ALTER
TABLE for each column includes DEFAULT NULL, keeping the same pattern used in
the first three column blocks and using the same `@columnname/`@preparedStatement
flow.
src/main/resources/db/migration/dbiemr/V61__Outbound_CallActivity.sql
Outdated
Show resolved
Hide resolved
ed10e25 to
5a5238b
Compare
|



📋 Description
JIRA ID:
AMM-2083
✅ Type of Change
Summary by CodeRabbit
New Features
Chores