Adds changeset to fix Edit conditions privilege with small c - #325
Adds changeset to fix Edit conditions privilege with small c#325sahupadma wants to merge 3 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request adds a single Liquibase database migration that corrects the casing of two privilege names ( ChangesPrivilege Case Correction
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@bahmnicore-omod/src/main/resources/liquibase.xml`:
- Around line 4691-4693: The precondition currently checks only role_privilege
and thus skips the change if role mappings are missing even when privilege rows
exist; update the <preConditions>/<sqlCheck expectedResult="1"> to also detect
the target privilege rows in the privilege table (or use an OR/UNION/EXISTS to
check both role_privilege and privilege for BINARY privilege IN ('Edit
conditions','Get conditions')) so the changeset runs when privilege rows are
present even if role_privilege rows are absent; adjust the sqlCheck expression
inside the preConditions block accordingly.
🪄 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 Plus
Run ID: 327f7b78-20a9-4e12-964f-988de92d6cfe
📒 Files selected for processing (1)
bahmnicore-omod/src/main/resources/liquibase.xml
| <preConditions onFail="MARK_RAN"> | ||
| <sqlCheck expectedResult="1">SELECT IF(count(*) > 0, 1, 0) FROM role_privilege WHERE BINARY privilege IN ('Edit conditions', 'Get conditions')</sqlCheck> | ||
| </preConditions> |
There was a problem hiding this comment.
Broaden the precondition beyond role_privilege.
The old privilege records were created independently at Line 3688 and Line 3699, while the role mappings were added later at Line 3709 and Line 3719. If an installation still has privilege rows but no matching role_privilege rows, this MARK_RAN precondition skips the migration entirely and the casing bug remains in privilege.
Suggested fix
- <preConditions onFail="MARK_RAN">
- <sqlCheck expectedResult="1">SELECT IF(count(*) > 0, 1, 0) FROM role_privilege WHERE BINARY privilege IN ('Edit conditions', 'Get conditions')</sqlCheck>
- </preConditions>
+ <preConditions onFail="MARK_RAN">
+ <sqlCheck expectedResult="1">
+ SELECT IF(
+ EXISTS (SELECT 1 FROM privilege WHERE BINARY privilege IN ('Edit conditions', 'Get conditions'))
+ OR EXISTS (SELECT 1 FROM role_privilege WHERE BINARY privilege IN ('Edit conditions', 'Get conditions')),
+ 1,
+ 0
+ )
+ </sqlCheck>
+ </preConditions>📝 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.
| <preConditions onFail="MARK_RAN"> | |
| <sqlCheck expectedResult="1">SELECT IF(count(*) > 0, 1, 0) FROM role_privilege WHERE BINARY privilege IN ('Edit conditions', 'Get conditions')</sqlCheck> | |
| </preConditions> | |
| <preConditions onFail="MARK_RAN"> | |
| <sqlCheck expectedResult="1"> | |
| SELECT IF( | |
| EXISTS (SELECT 1 FROM privilege WHERE BINARY privilege IN ('Edit conditions', 'Get conditions')) | |
| OR EXISTS (SELECT 1 FROM role_privilege WHERE BINARY privilege IN ('Edit conditions', 'Get conditions')), | |
| 1, | |
| 0 | |
| ) | |
| </sqlCheck> | |
| </preConditions> |
🤖 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 `@bahmnicore-omod/src/main/resources/liquibase.xml` around lines 4691 - 4693,
The precondition currently checks only role_privilege and thus skips the change
if role mappings are missing even when privilege rows exist; update the
<preConditions>/<sqlCheck expectedResult="1"> to also detect the target
privilege rows in the privilege table (or use an OR/UNION/EXISTS to check both
role_privilege and privilege for BINARY privilege IN ('Edit conditions','Get
conditions')) so the changeset runs when privilege rows are present even if
role_privilege rows are absent; adjust the sqlCheck expression inside the
preConditions block accordingly.
|
# Conflicts: # bahmnicore-omod/src/main/resources/liquibase.xml



Summary by CodeRabbit