Frontend/multistate license scopes#1708
Open
jsandoval81 wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR introduces a ChangesMultistate License Scope Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant LicenseCard
participant UsersActions
participant DataApi
participant LicenseDataApi
LicenseCard->>UsersActions: dispatch encumberLicenseRequest(licenseScope)
UsersActions->>DataApi: encumberLicense(..., licenseScope)
DataApi->>LicenseDataApi: encumberLicense(..., licenseScope)
LicenseDataApi-->>DataApi: POST payload including licenseScope
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webroot/src/network/licenseApi/data.api.ts (1)
376-408: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
encumbrance.licenseScopeis declared but never read.The
encumbranceparameter type declares an independentlicenseScope?: stringfield (Line 388), but the request-body construction always uses the outerlicenseScopeparameter instead ofencumbrance.licenseScope, both for the top-level field (Line 393) and inside the nestedencumbranceobject (Line 400). If a caller ever passes a different value inencumbrance.licenseScopethan the outerlicenseScope, it will be silently dropped.Currently harmless because
LicenseCard.tsalways passes the same value to both, but the type contract is misleading and error-prone for future callers.🐛 Proposed fix
encumbrance: { encumbranceType: encumbrance.encumbranceType, clinicalPrivilegeActionCategories: encumbrance.npdbCategories, encumbranceEffectiveDate: encumbrance.startDate, - ...(licenseScope && { licenseScope }), + ...((encumbrance.licenseScope || licenseScope) && { licenseScope: encumbrance.licenseScope || licenseScope }), },🤖 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 `@webroot/src/network/licenseApi/data.api.ts` around lines 376 - 408, The updateLicenseInvestigation method in data.api.ts defines encumbrance.licenseScope but never uses it, since the request body always reads the outer licenseScope instead. Update the payload construction in updateLicenseInvestigation so it consistently uses the intended scope source, either by removing encumbrance.licenseScope from the encumbrance type or by wiring it into both the top-level and nested encumbrance objects where appropriate. Make sure the fields in the patch body match the contract used by callers such as LicenseCard.
🧹 Nitpick comments (7)
webroot/src/models/License/License.model.ts (1)
158-182: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting a shared translate-lookup helper.
licenseTypeDisplay(),licenseTypeAbbreviation(), and the newlicenseScopeDisplay()all repeat the same$tm(...).find(...)lookup pattern. A small private helper (e.g.translateLookup(tmKey, matchValue)) would remove the duplication.♻️ Example refactor
+ private translateLookupEntry(tmKey: string, matchValue: any): any { + const translations = this.$tm(tmKey) || []; + + return translations.find((translate) => translate.key === matchValue); + } + public licenseTypeDisplay(): string { - const licenseTypes = this.$tm('licensing.licenseTypes') || []; - const licenseType = licenseTypes.find((translate) => translate.key === this.licenseType); - const typeDisplay = licenseType?.name || ''; - - return typeDisplay; + return this.translateLookupEntry('licensing.licenseTypes', this.licenseType)?.name || ''; }🤖 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 `@webroot/src/models/License/License.model.ts` around lines 158 - 182, The three methods licenseTypeDisplay(), licenseTypeAbbreviation(), and licenseScopeDisplay() duplicate the same $tm(...).find(...) lookup logic. Add a small private helper in License.model.ts that takes the translation key and matching value, performs the lookup once, and reuse it from these methods so the repeated pattern is centralized and easier to maintain.webroot/src/components/PrivilegeCard/PrivilegeCard.ts (2)
280-290: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMerge identical cosmetology/social-work branches.
The
isAppModeCosmetologyandisAppModeSocialWorkbranches (Lines 280-290) are byte-for-byte identical. Combining them avoids future divergence bugs (e.g., someone updates one list but forgets the other).♻️ Proposed consolidation
- } else if (isAppModeCosmetology) { - isMultiSelect = false; - includeList.push('fraud'); - includeList.push('consumer harm'); - includeList.push('other'); - } else if (isAppModeSocialWork) { + } else if (isAppModeCosmetology || isAppModeSocialWork) { isMultiSelect = false; includeList.push('fraud'); includeList.push('consumer harm'); includeList.push('other'); }🤖 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 `@webroot/src/components/PrivilegeCard/PrivilegeCard.ts` around lines 280 - 290, Merge the duplicate handling in PrivilegeCard’s app-mode branch logic: the isAppModeCosmetology and isAppModeSocialWork cases do the exact same work, so consolidate them into one shared branch in the relevant conditional block. Keep the existing behavior in PrivilegeCard intact by setting isMultiSelect and pushing the same includeList values from the combined branch, so future edits only need to be made in one place.
293-299: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove leftover debug
console.log.
console.log('???')at Line 297 is a debug artifact and shouldn't ship.🧹 Proposed cleanup
// For a single-select, include the blank option if (!isMultiSelect) { - console.log('???'); options.unshift({ value: '', name: computed(() => this.$t('common.selectOption')) }); }🤖 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 `@webroot/src/components/PrivilegeCard/PrivilegeCard.ts` around lines 293 - 299, Remove the leftover debug logging from PrivilegeCard’s single-select branch: the console.log('???') inside the option setup should be deleted from the code path in PrivilegeCard so only the blank option insertion remains. Keep the existing behavior in the isMultiSelect check and the options.unshift call, but eliminate the debug artifact before shipping.webroot/src/components/LicenseCard/LicenseCard.less (1)
200-220: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGlobe icon stroke color relies on cascade order, not specificity.
.scope-icon.stroke-fill-type(Line 212-215, matched via the class hardcoded in Globe.vue's template) and.scope-icon.globe-icon(Line 217-219) have identical specificity. The globe stroke correctly ends up as@fontColoronly because this rule is declared after.stroke-fill-typein the stylesheet — if these rules are ever reordered (e.g. during a later refactor), the globe icon would silently revert to a white stroke on the light multi-state pill background, hurting contrast/readability.Consider increasing specificity explicitly instead of relying on declaration order:
♻️ Suggested fix
- &.stroke-fill-type, - &.stroke-type { - stroke: `@white`; - } - - &.globe-icon { - stroke: `@fontColor`; - } + &.globe-icon { + stroke: `@fontColor`; + } + + &.stroke-fill-type, + &.stroke-type { + &:not(.globe-icon) { + stroke: `@white`; + } + }🤖 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 `@webroot/src/components/LicenseCard/LicenseCard.less` around lines 200 - 220, The globe icon stroke color in LicenseCard.less is depending on rule order instead of specificity, so make the `.scope-icon.globe-icon` styling explicitly override the shared `.scope-icon.stroke-fill-type` / `.scope-icon.stroke-type` rules. Update the `.scope-icon` block in LicenseCard.less so the Globe.vue icon class wins by specificity rather than declaration order, keeping the stroke as `@fontColor` regardless of future CSS reordering.webroot/src/components/LicenseCard/LicenseCard.ts (2)
596-604: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
licenseScopeinside the nestedencumbranceobject.
licenseScopeis already forwarded at the top level of theupdateInvestigationLicenseRequestpayload (Line 596). Based on theLicenseDataApi.updateLicenseInvestigationimplementation, the nested encumbrance object is built using the outerlicenseScopeparam, notencumbrance.licenseScope, so the value set here at Line 603 has no effect and is dead weight in the payload.🧹 Proposed cleanup
encumbrance: { encumbranceType: formData.encumberModalDisciplineAction.value, npdbCategories: (Array.isArray(formData.encumberModalNpdbCategories.value)) ? formData.encumberModalNpdbCategories.value : [formData.encumberModalNpdbCategories.value], startDate: formData.encumberModalStartDate.value, - licenseScope, },🤖 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 `@webroot/src/components/LicenseCard/LicenseCard.ts` around lines 596 - 604, The nested licenseScope inside the encumbrance payload in LicenseCard is redundant because updateInvestigationLicenseRequest already passes licenseScope at the top level and LicenseDataApi.updateLicenseInvestigation uses the outer value. Remove the inner licenseScope from the encumbrance object in the LicenseCard request builder so the payload only includes the effective top-level field.
310-350: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsolidate the Cosmetology/SocialWork branch. Both paths set the same include list and
isMultiSelect = false; merge them to avoid divergence.🤖 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 `@webroot/src/components/LicenseCard/LicenseCard.ts` around lines 310 - 350, The npdbCategoryOptions getter has duplicated logic for the cosmetology and social work modes, which should be consolidated to avoid divergence. Update LicenseCard.ts so the isAppModeCosmetology and isAppModeSocialWork branches share the same includeList and single-select behavior, keeping the unique behavior in npdbCategoryOptions while removing the redundant branch split.webroot/src/components/Icons/MapPin/MapPin.ts (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove leftover commented-out export.
Dead code artifact from scaffolding.
🧹 Proposed cleanup
export default toNative(MapPin); - -// export default MapPin;🤖 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 `@webroot/src/components/Icons/MapPin/MapPin.ts` at line 18, Remove the leftover commented-out export in MapPin and keep the module clean by deleting the dead scaffold artifact. Update the MapPin component file so only the active MapPin export remains, and leave no commented-out export statements behind.
🤖 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 `@webroot/src/locales/es.json`:
- Around line 967-981: Update the Spanish locale entries used by
PrivilegeCard.ts’s npdbCategoryOptions so the option keys match the lowercase
identifiers expected by the COSM/SocialWork includeList. In es.json, align the
"Fraude" and "Daños al consumidor" items to use the same stable keys as the new
en.json entries (for example the lowercase fraud/consumer harm/other
identifiers), and remove the duplicate value collision caused by reusing "Fraud,
Deception, or Misrepresentation" for multiple Spanish labels. Ensure the
localized names remain Spanish while the keys are unique and consistent with the
option filtering logic.
---
Outside diff comments:
In `@webroot/src/network/licenseApi/data.api.ts`:
- Around line 376-408: The updateLicenseInvestigation method in data.api.ts
defines encumbrance.licenseScope but never uses it, since the request body
always reads the outer licenseScope instead. Update the payload construction in
updateLicenseInvestigation so it consistently uses the intended scope source,
either by removing encumbrance.licenseScope from the encumbrance type or by
wiring it into both the top-level and nested encumbrance objects where
appropriate. Make sure the fields in the patch body match the contract used by
callers such as LicenseCard.
---
Nitpick comments:
In `@webroot/src/components/Icons/MapPin/MapPin.ts`:
- Line 18: Remove the leftover commented-out export in MapPin and keep the
module clean by deleting the dead scaffold artifact. Update the MapPin component
file so only the active MapPin export remains, and leave no commented-out export
statements behind.
In `@webroot/src/components/LicenseCard/LicenseCard.less`:
- Around line 200-220: The globe icon stroke color in LicenseCard.less is
depending on rule order instead of specificity, so make the
`.scope-icon.globe-icon` styling explicitly override the shared
`.scope-icon.stroke-fill-type` / `.scope-icon.stroke-type` rules. Update the
`.scope-icon` block in LicenseCard.less so the Globe.vue icon class wins by
specificity rather than declaration order, keeping the stroke as `@fontColor`
regardless of future CSS reordering.
In `@webroot/src/components/LicenseCard/LicenseCard.ts`:
- Around line 596-604: The nested licenseScope inside the encumbrance payload in
LicenseCard is redundant because updateInvestigationLicenseRequest already
passes licenseScope at the top level and
LicenseDataApi.updateLicenseInvestigation uses the outer value. Remove the inner
licenseScope from the encumbrance object in the LicenseCard request builder so
the payload only includes the effective top-level field.
- Around line 310-350: The npdbCategoryOptions getter has duplicated logic for
the cosmetology and social work modes, which should be consolidated to avoid
divergence. Update LicenseCard.ts so the isAppModeCosmetology and
isAppModeSocialWork branches share the same includeList and single-select
behavior, keeping the unique behavior in npdbCategoryOptions while removing the
redundant branch split.
In `@webroot/src/components/PrivilegeCard/PrivilegeCard.ts`:
- Around line 280-290: Merge the duplicate handling in PrivilegeCard’s app-mode
branch logic: the isAppModeCosmetology and isAppModeSocialWork cases do the
exact same work, so consolidate them into one shared branch in the relevant
conditional block. Keep the existing behavior in PrivilegeCard intact by setting
isMultiSelect and pushing the same includeList values from the combined branch,
so future edits only need to be made in one place.
- Around line 293-299: Remove the leftover debug logging from PrivilegeCard’s
single-select branch: the console.log('???') inside the option setup should be
deleted from the code path in PrivilegeCard so only the blank option insertion
remains. Keep the existing behavior in the isMultiSelect check and the
options.unshift call, but eliminate the debug artifact before shipping.
In `@webroot/src/models/License/License.model.ts`:
- Around line 158-182: The three methods licenseTypeDisplay(),
licenseTypeAbbreviation(), and licenseScopeDisplay() duplicate the same
$tm(...).find(...) lookup logic. Add a small private helper in License.model.ts
that takes the translation key and matching value, performs the lookup once, and
reuse it from these methods so the repeated pattern is centralized and easier to
maintain.
🪄 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: c144699e-7266-4ce2-998b-f6140750d4ab
📒 Files selected for processing (23)
webroot/src/components/Icons/Globe/Globe.lesswebroot/src/components/Icons/Globe/Globe.spec.tswebroot/src/components/Icons/Globe/Globe.tswebroot/src/components/Icons/Globe/Globe.vuewebroot/src/components/Icons/MapPin/MapPin.lesswebroot/src/components/Icons/MapPin/MapPin.spec.tswebroot/src/components/Icons/MapPin/MapPin.tswebroot/src/components/Icons/MapPin/MapPin.vuewebroot/src/components/LicenseCard/LicenseCard.lesswebroot/src/components/LicenseCard/LicenseCard.tswebroot/src/components/LicenseCard/LicenseCard.vuewebroot/src/components/PrivilegeCard/PrivilegeCard.lesswebroot/src/components/PrivilegeCard/PrivilegeCard.tswebroot/src/components/PrivilegeCard/PrivilegeCard.vuewebroot/src/locales/en.jsonwebroot/src/locales/es.jsonwebroot/src/models/License/License.model.spec.tswebroot/src/models/License/License.model.tswebroot/src/network/data.api.tswebroot/src/network/licenseApi/data.api.tswebroot/src/network/mocks/mock.data.tswebroot/src/store/users/users.actions.tswebroot/src/styles.common/_colors.less
Comment on lines
+967
to
+981
| { | ||
| "name": "Fraude", | ||
| "key": "Fraud, Deception, or Misrepresentation" | ||
| }, | ||
| { | ||
| "name": "Daños al consumidor", | ||
| "key": "Consumer Harm" | ||
| }, | ||
| { | ||
| "name": "Otro", | ||
| "key": "Other" | ||
| }, | ||
| { | ||
| "name": "Otro", | ||
| "key": "other" |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Locale key mismatch breaks NPDB basis-for-action options for Spanish-locale COSM/SOCW users.
PrivilegeCard.ts's npdbCategoryOptions filters by lowercase keys 'fraud', 'consumer harm', 'other' for COSM/SocialWork modes (matching en.json's new entries). Here in es.json, the "Fraude" and "Daños al consumidor" entries use keys "Fraud, Deception, or Misrepresentation" and "Consumer Harm" instead — they will never match the includeList, so those options silently disappear from the Spanish-locale dropdown. The reused key "Fraud, Deception, or Misrepresentation" also now maps to two differently-named entries in the same array, producing a duplicate-valued option for JCC mode.
🌐 Proposed fix to align keys with en.json
{
"name": "Fraude",
- "key": "Fraud, Deception, or Misrepresentation"
+ "key": "fraud"
},
{
"name": "Daños al consumidor",
- "key": "Consumer Harm"
+ "key": "consumer harm"
},📝 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.
Suggested change
| { | |
| "name": "Fraude", | |
| "key": "Fraud, Deception, or Misrepresentation" | |
| }, | |
| { | |
| "name": "Daños al consumidor", | |
| "key": "Consumer Harm" | |
| }, | |
| { | |
| "name": "Otro", | |
| "key": "Other" | |
| }, | |
| { | |
| "name": "Otro", | |
| "key": "other" | |
| { | |
| "name": "Fraude", | |
| "key": "fraud" | |
| }, | |
| { | |
| "name": "Daños al consumidor", | |
| "key": "consumer harm" | |
| }, | |
| { | |
| "name": "Otro", | |
| "key": "Other" | |
| }, | |
| { | |
| "name": "Otro", | |
| "key": "other" |
🤖 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 `@webroot/src/locales/es.json` around lines 967 - 981, Update the Spanish
locale entries used by PrivilegeCard.ts’s npdbCategoryOptions so the option keys
match the lowercase identifiers expected by the COSM/SocialWork includeList. In
es.json, align the "Fraude" and "Daños al consumidor" items to use the same
stable keys as the new en.json entries (for example the lowercase fraud/consumer
harm/other identifiers), and remove the duplicate value collision caused by
reusing "Fraud, Deception, or Misrepresentation" for multiple Spanish labels.
Ensure the localized names remain Spanish while the keys are unique and
consistent with the option filtering logic.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requirements List
Description List
licenseScopelicenseScopeTesting List
yarn test:unit:allshould run without errors or warningsyarn serveshould run without errors or warningsyarn buildshould run without errors or warningsCloses #1707
Summary by CodeRabbit
New Features
Bug Fixes
Tests