Cosmetology - Return privileges that have aa or investigations#1314
Cosmetology - Return privileges that have aa or investigations#1314isabeleliassen merged 3 commits intocsg-org:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdated privilege generation logic to conditionally suppress privilege rows per jurisdiction when the home license is not compact-eligible, instead of skipping the entire license type. Privileges are still generated when adverse actions or investigations exist. Added corresponding unit tests and a dependency update. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/cosmetology-app/lambdas/python/common/cc_common/data_model/provider_record_util.py`:
- Around line 477-480: The current inclusion check uses inv_records (populated
by get_investigation_records_for_privilege(jurisdiction,...)) which only returns
investigations from the same jurisdiction, so change the logic to detect
investigations that target the specific privilege instead of relying on
inv_records scoped to privilege_jurisdiction: either update
get_investigation_records_for_privilege to accept and return cross-jurisdiction
investigations for the given privilege identifier(s) or replace the boolean
check with a filter over the investigation list that matches on the
privilege-identifying fields (e.g., privilege id/license number/provider id and
any privilege-specific keys) before using it in the if-condition that references
most_recent_license, CompactEligibilityStatus, and privilege_aa. Ensure you
reference the same symbols (most_recent_license,
CompactEligibilityStatus.ELIGIBLE, privilege_aa, inv_records,
get_investigation_records_for_privilege, privilege_jurisdiction) when making the
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69691227-1595-4c7f-b1dc-cd1ac4051997
📒 Files selected for processing (2)
backend/cosmetology-app/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py
814aaab to
c1804ba
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/cosmetology-app/lambdas/python/common/cc_common/data_model/provider_record_util.py (1)
485-498: Implementation looks correct; minor style suggestion.The logic correctly ensures privileges are generated when there's a matching adverse action or open investigation, even for ineligible home licenses. This addresses the core requirement from issue
#1226.One minor observation: line 486 re-checks
most_recent_license.compactEligibility != CompactEligibilityStatus.ELIGIBLEwhenis_eligible(line 473) already captures this. Consider usingnot is_eligiblefor consistency:♻️ Optional simplification
if ( - most_recent_license.compactEligibility != CompactEligibilityStatus.ELIGIBLE + not is_eligible and not include_inactive_privileges and not privilege_aa and not inv_records ):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cosmetology-app/lambdas/python/common/cc_common/data_model/provider_record_util.py` around lines 485 - 498, The conditional re-checks most_recent_license.compactEligibility when is_eligible (set earlier) already represents that value; update the if condition to use not is_eligible instead of most_recent_license.compactEligibility != CompactEligibilityStatus.ELIGIBLE and keep the rest of the checks (not include_inactive_privileges, not privilege_aa, not inv_records) unchanged, and preserve the logger.debug call and its context keys (jurisdiction, home_jurisdiction, license_type_abbr) so behavior is identical but uses the existing is_eligible flag for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@backend/cosmetology-app/lambdas/python/common/cc_common/data_model/provider_record_util.py`:
- Around line 485-498: The conditional re-checks
most_recent_license.compactEligibility when is_eligible (set earlier) already
represents that value; update the if condition to use not is_eligible instead of
most_recent_license.compactEligibility != CompactEligibilityStatus.ELIGIBLE and
keep the rest of the checks (not include_inactive_privileges, not privilege_aa,
not inv_records) unchanged, and preserve the logger.debug call and its context
keys (jurisdiction, home_jurisdiction, license_type_abbr) so behavior is
identical but uses the existing is_eligible flag for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82f674a0-31e0-4a52-b507-18195a101d63
📒 Files selected for processing (3)
backend/cosmetology-app/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/cosmetology-app/lambdas/python/common/requirements-dev.inbackend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/cosmetology-app/lambdas/python/common/requirements-dev.in
jlkravitz
left a comment
There was a problem hiding this comment.
Looks good to me! @isabeleliassen Good to merge.
Currently, if the home state license is not eligible in the system, the API does not return any privilege records for the cosmetology compact. We have identified a case where a state can place an encumbrance/investigation on a privilege record in the system, and then if the home state becomes inactive, the privilege record with an encumbrance/investigation will not be returned and the admin of that state will have no way to end the encumbrance/investigation.
To address this, we this adds a check on the backend to see if a privilege has any encumbrance/investigation placed on it, we will always return that record even if the home state license is not eligible for privileges in the system.
Closes #1226
Summary by CodeRabbit
Bug Fixes
Tests
Chores