Open
Conversation
Contributor
|
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 due to trivial changes (1)
📝 WalkthroughWalkthroughAdds application-mode (JCC vs Cosmetology) awareness across store, components, templates, network, models, locales, mocks, and tests; introduces mode-specific getters, conditional rendering/validation/payloads, licenseNumber/dob search plumbing, deterministic sorting, and small styling and lint tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CompactSelector as CompactSelector (component)
participant Store
participant API as getCompactStatesRequest (action/API)
participant UI as Mode-aware Components
User->>CompactSelector: select compact
CompactSelector->>Store: dispatch setCurrentCompact(compact)
Store->>Store: commit current compact
Store->>Store: determine AppMode (COSMETOLOGY / JCC)
Store->>Store: commit setAppMode(AppMode)
Store->>API: dispatch getCompactStatesRequest(compact.type)
API-->>Store: return states
Store-->>UI: state updated (appMode, compactStates)
UI->>UI: components re-evaluate getters (isAppModeJcc/isAppModeCosmetology)
UI-->>User: render mode-conditional UI (search fields, fees, columns)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
![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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
webroot/src/store/license/license.mutations.ts (1)
107-127:⚠️ Potential issue | 🟡 MinorAdd
licenseNumber: ''to the initial search state inlicense.state.ts.The mutations reset
licenseNumberto an empty string, but the initial state (lines 29-34 inlicense.state.ts) doesn't include this field, creating an inconsistency. On initial load,state.search.licenseNumberisundefined, but after reset it becomes''. To maintain a consistent state shape, addlicenseNumber: ''to the initial search object alongside the other search fields.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webroot/src/store/license/license.mutations.ts` around lines 107 - 127, The initial search state object is missing the licenseNumber key so state.search.licenseNumber is undefined on load; update the initial state (the exported state object / initialState used in license.state.ts where search is defined) to include licenseNumber: '' alongside compact, firstName, lastName, and state so the shape matches the reset mutations (MutationTypes.STORE_RESET_SEARCH and MutationTypes.STORE_RESET_LICENSE).webroot/src/components/CompactSettingsConfig/CompactSettingsConfig.vue (1)
18-38:⚠️ Potential issue | 🟠 MajorHidden JCC-specific fields have required/min validation that blocks Cosmetology mode form submission.
The fields
compactFeeandsummaryReportNotificationEmailsare hidden whenisAppModeJccis false (Cosmetology mode), but their FormInput declarations include validation rules that will fail:
compactFee:Joi.number().required()— fails if undefinedsummaryReportNotificationEmails:Joi.array().min(1)— fails on empty arraySince
validateAll()in the form mixin validates all form inputs regardless of CSS visibility, Cosmetology admins cannot save settings. Additionally, the always-visible email fields (opsNotificationEmails,adverseActionNotificationEmails) also have.min(1)validation but initialize as empty arrays, creating the same blocking issue.Fix by:
- Conditionally initialize fields only when
isAppModeJccis true, or- Make validation rules conditional based on app mode, or
- Populate empty arrays with placeholder values before validation for visible fields
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webroot/src/components/CompactSettingsConfig/CompactSettingsConfig.vue` around lines 18 - 38, Hidden JCC-only fields (formData.compactFee and formData.summaryReportNotificationEmails) are being validated even when isAppModeJcc is false, blocking Cosmetology saves; update the component so these fields are only initialized and/or validated when isAppModeJcc is true and loosen .min(1)/.required() rules for always-visible email lists: change data() so compactFee and summaryReportNotificationEmails are added to formData only when isAppModeJcc is true (or set to null/no-op), and update the Joi validation schema used by validateAll (or the form mixin) to conditionally require Joi.number().required() and Joi.array().min(1) only when isAppModeJcc is true (e.g., use Joi.when('isAppModeJcc', ...) or branch the schema creation), or alternatively allow empty arrays for opsNotificationEmails/adverseActionNotificationEmails unless the UI shows they must be populated.webroot/src/network/licenseApi/data.api.ts (1)
127-146:⚠️ Potential issue | 🟠 Major
licenseNumbernot included inhasSearchTerms, may cause search-only-by-license-number to fail.The
hasSearchTermscheck on line 127 does not includelicenseNumber:const hasSearchTerms = Boolean(licenseeId || licenseeFirstName || licenseeLastName);This means if a user searches only by
licenseNumber(without providing any other search terms),hasSearchTermswill befalse, and thelicenseNumberwill never be added to the query (lines 144-146 are inside theif (hasSearchTerms)block).🐛 Proposed fix to include licenseNumber in hasSearchTerms
- const hasSearchTerms = Boolean(licenseeId || licenseeFirstName || licenseeLastName); + const hasSearchTerms = Boolean(licenseeId || licenseeFirstName || licenseeLastName || licenseNumber);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webroot/src/network/licenseApi/data.api.ts` around lines 127 - 146, The hasSearchTerms boolean currently omits licenseNumber so searches by licenseNumber alone won't populate requestParams.query; update the hasSearchTerms definition (used around requestParams/query population) to include licenseNumber (e.g., include licenseNumber in the Boolean expression that defines hasSearchTerms) so that the subsequent block which sets requestParams.query.licenseNumber executes when only a license number is provided.
🧹 Nitpick comments (1)
webroot/src/models/License/License.model.ts (1)
33-34: Add mock data mappings for new license types when they are used in features.The new
COSMETOLOGISTandESTHETICIANenum values are correctly defined. However,licenseTypeMapinwebroot/src/network/mocks/mock.data.api.tsonly includes mappings forotandota. When these new license types are used in actual features, add corresponding abbreviation mappings tolicenseTypeMapand test data entries tomockPrivilegeHistoryResponsesto ensure mock API calls work correctly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webroot/src/models/License/License.model.ts` around lines 33 - 34, licenseTypeMap is missing mappings for the new enum values COSMETOLOGIST and ESTHETICIAN and mockPrivilegeHistoryResponses lacks test entries for them; add entries in the licenseTypeMap (e.g., map the string values 'cosmetologist' and 'esthetician' or the enum LicenseType.COSMETOLOGIST / LicenseType.ESTHETICIAN to the same abbreviation style used for 'ot'/'ota') and add corresponding mock objects in mockPrivilegeHistoryResponses so API tests and UI features that reference COSMETOLOGIST and ESTHETICIAN return realistic mock data; ensure keys match the enum values and the mock entries include the same fields/shape as existing privilege history responses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@webroot/.eslintrc.js`:
- Line 100: Remove the global disable of the vue/no-v-for-template-key rule and
fix the single violation in LicensingDetail.vue by moving the :key from the
<template v-for="(privilege, index) in licenseePrivileges" :key="index"> to the
child element (e.g., the inner <div>) so the key is applied to the rendered
element, keeping the rule enabled for future checks; if necessary as a minimal
alternative, add an inline eslint-disable-next-line comment immediately above
that template line instead of turning the rule off globally.
In `@webroot/src/components/Licensee/LicenseeList/LicenseeList.ts`:
- Around line 388-390: searchDisplayAll is being left empty when only
searchParams.licenseNumber is set, which triggers the edge-case that reopens the
search modal later; update the code that builds the search summary (the logic
that composes searchDisplayAll in the LicenseeList component) to include
licenseNumber whenever requestConfig.licenseNumber or searchParams.licenseNumber
is present. Specifically, when you set requestConfig.licenseNumber (in the same
block using searchParams), append a human-readable entry for licenseNumber into
the searchDisplayAll aggregation so the component recognizes the search as
non-empty and avoids the modal-reopen edge-case.
In `@webroot/src/components/Licensee/LicenseeListLegacy/LicenseeListLegacy.ts`:
- Around line 324-326: Update the legacy summary and modal-reopen logic to
account for license-number-only searches by including searchParams.licenseNumber
when composing the legacy summary string (the same place you set
requestConfig.licenseNumber) and by changing the modal-reopen condition that
currently only checks searchDisplayAll to also treat a non-empty
searchParams.licenseNumber as a valid search; i.e., when deciding whether to
reopen the search modal, consider both searchDisplayAll and
searchParams.licenseNumber so a license-number-only search does not incorrectly
reopen the modal.
In `@webroot/src/components/StateSettingsConfig/StateSettingsConfig.vue`:
- Around line 18-46: The summaryReportNotificationEmails field defaults to []
but uses Joi.array().min(1) and is always validated by validateAll() even when
hidden via v-if="isAppModeJcc", causing Cosmetology submissions to fail; fix by
making validation conditional: either remove or exclude
summaryReportNotificationEmails from the formInputs list when isAppModeJcc is
false (so validateAll() won't iterate it), or update validateAll() to skip
validation for fields not currently rendered (check isAppModeJcc / the component
visibility flag before validating), and ensure references to
summaryReportNotificationEmails, validateAll(), formInputs, InputEmailList and
isAppModeJcc are updated accordingly.
---
Outside diff comments:
In `@webroot/src/components/CompactSettingsConfig/CompactSettingsConfig.vue`:
- Around line 18-38: Hidden JCC-only fields (formData.compactFee and
formData.summaryReportNotificationEmails) are being validated even when
isAppModeJcc is false, blocking Cosmetology saves; update the component so these
fields are only initialized and/or validated when isAppModeJcc is true and
loosen .min(1)/.required() rules for always-visible email lists: change data()
so compactFee and summaryReportNotificationEmails are added to formData only
when isAppModeJcc is true (or set to null/no-op), and update the Joi validation
schema used by validateAll (or the form mixin) to conditionally require
Joi.number().required() and Joi.array().min(1) only when isAppModeJcc is true
(e.g., use Joi.when('isAppModeJcc', ...) or branch the schema creation), or
alternatively allow empty arrays for
opsNotificationEmails/adverseActionNotificationEmails unless the UI shows they
must be populated.
In `@webroot/src/network/licenseApi/data.api.ts`:
- Around line 127-146: The hasSearchTerms boolean currently omits licenseNumber
so searches by licenseNumber alone won't populate requestParams.query; update
the hasSearchTerms definition (used around requestParams/query population) to
include licenseNumber (e.g., include licenseNumber in the Boolean expression
that defines hasSearchTerms) so that the subsequent block which sets
requestParams.query.licenseNumber executes when only a license number is
provided.
In `@webroot/src/store/license/license.mutations.ts`:
- Around line 107-127: The initial search state object is missing the
licenseNumber key so state.search.licenseNumber is undefined on load; update the
initial state (the exported state object / initialState used in license.state.ts
where search is defined) to include licenseNumber: '' alongside compact,
firstName, lastName, and state so the shape matches the reset mutations
(MutationTypes.STORE_RESET_SEARCH and MutationTypes.STORE_RESET_LICENSE).
---
Nitpick comments:
In `@webroot/src/models/License/License.model.ts`:
- Around line 33-34: licenseTypeMap is missing mappings for the new enum values
COSMETOLOGIST and ESTHETICIAN and mockPrivilegeHistoryResponses lacks test
entries for them; add entries in the licenseTypeMap (e.g., map the string values
'cosmetologist' and 'esthetician' or the enum LicenseType.COSMETOLOGIST /
LicenseType.ESTHETICIAN to the same abbreviation style used for 'ot'/'ota') and
add corresponding mock objects in mockPrivilegeHistoryResponses so API tests and
UI features that reference COSMETOLOGIST and ESTHETICIAN return realistic mock
data; ensure keys match the enum values and the mock entries include the same
fields/shape as existing privilege history responses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b5ec9f66-bf2a-4bce-871f-0e24eb7fc578
📒 Files selected for processing (33)
webroot/.eslintrc.jswebroot/src/components/CompactSelector/CompactSelector.tswebroot/src/components/CompactSettingsConfig/CompactSettingsConfig.lesswebroot/src/components/CompactSettingsConfig/CompactSettingsConfig.tswebroot/src/components/CompactSettingsConfig/CompactSettingsConfig.vuewebroot/src/components/Licensee/LicenseeList/LicenseeList.tswebroot/src/components/Licensee/LicenseeListLegacy/LicenseeListLegacy.tswebroot/src/components/Licensee/LicenseeRow/LicenseeRow.tswebroot/src/components/Licensee/LicenseeRow/LicenseeRow.vuewebroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.tswebroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.vuewebroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.tswebroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.vuewebroot/src/components/PrivilegeCard/PrivilegeCard.tswebroot/src/components/PrivilegeCard/PrivilegeCard.vuewebroot/src/components/StateSettingsConfig/StateSettingsConfig.lesswebroot/src/components/StateSettingsConfig/StateSettingsConfig.tswebroot/src/components/StateSettingsConfig/StateSettingsConfig.vuewebroot/src/locales/en.jsonwebroot/src/locales/es.jsonwebroot/src/models/License/License.model.tswebroot/src/models/Licensee/Licensee.model.tswebroot/src/network/licenseApi/data.api.tswebroot/src/network/mocks/mock.data.tswebroot/src/network/searchApi/data.api.tswebroot/src/pages/LicensingDetail/LicensingDetail.lesswebroot/src/pages/LicensingDetail/LicensingDetail.tswebroot/src/pages/LicensingDetail/LicensingDetail.vuewebroot/src/store/global/global.getters.tswebroot/src/store/license/license.mutations.tswebroot/src/store/license/license.spec.tswebroot/src/store/user/user.actions.tswebroot/src/store/user/user.spec.ts
webroot/src/components/Licensee/LicenseeListLegacy/LicenseeListLegacy.ts
Outdated
Show resolved
Hide resolved
Contributor
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 `@webroot/src/components/CompactSettingsConfig/CompactSettingsConfig.ts`:
- Around line 255-261: The payload builder in CompactSettingsConfig is
unconditionally setting configuredStates to
this.initialCompactConfig?.configuredStates || [], which will wipe configured
states if initialCompactConfig is missing or malformed; change the logic so
configuredStates is only included on the payload when
this.initialCompactConfig?.configuredStates is defined (or valid), otherwise
omit configuredStates from the CompactConfig payload (or preserve the current
form value/source) to avoid accidentally clearing stored states; update the code
that constructs payload (referencing payload, configuredStates,
this.initialCompactConfig, and getCompactConfig) to perform this presence check
before assigning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4695b966-2c70-4a4b-99f6-dc75f75a26ed
📒 Files selected for processing (3)
webroot/src/components/CompactSettingsConfig/CompactSettingsConfig.tswebroot/src/components/StateSettingsConfig/StateSettingsConfig.tswebroot/src/models/Compact/Compact.model.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
webroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.vue (1)
15-29: Gate the dev-only wrapper rows withisMockPopulateEnabled.The controls themselves are mock-mode only, but the two
.search-form-rowwrappers still render unconditionally. Moving the condition to the wrappers keeps the production form DOM/layout identical when mock mode is off.Based on learnings, the mock-populate and clear controls are intended to render only in local development mock mode.♻️ Suggested template cleanup
- <div class="search-form-row"> + <div v-if="isMockPopulateEnabled" class="search-form-row"> <MockPopulate :isEnabled="isMockPopulateEnabled" `@selected`="mockPopulate" class="mock-populate search-input" /> </div> - <div class="search-form-row"> - <a - v-if="isMockPopulateEnabled" + <div v-if="isMockPopulateEnabled" class="search-form-row"> + <a `@click`="resetForm()" `@keyup.enter`="resetForm()" class="clear-form search-input" >{{ $t('common.clear') }}</a> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.vue` around lines 15 - 29, The two wrapper divs with class "search-form-row" are rendered even when mock mode is off; move the mock-mode guard to those wrappers so the MockPopulate component and the clear link are not present in production DOM. Concretely, add v-if="isMockPopulateEnabled" to the outer divs that contain <MockPopulate ... /> and the <a ...> that calls resetForm(), removing or no longer needing the v-if on the <a> itself, while keeping the existing props/events (MockPopulate, isMockPopulateEnabled, mockPopulate, resetForm) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@webroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.vue`:
- Around line 15-29: The two wrapper divs with class "search-form-row" are
rendered even when mock mode is off; move the mock-mode guard to those wrappers
so the MockPopulate component and the clear link are not present in production
DOM. Concretely, add v-if="isMockPopulateEnabled" to the outer divs that contain
<MockPopulate ... /> and the <a ...> that calls resetForm(), removing or no
longer needing the v-if on the <a> itself, while keeping the existing
props/events (MockPopulate, isMockPopulateEnabled, mockPopulate, resetForm)
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2755fe26-e048-472a-812e-344d984e0e02
📒 Files selected for processing (10)
webroot/src/components/Licensee/LicenseeList/LicenseeList.tswebroot/src/components/Licensee/LicenseeListLegacy/LicenseeListLegacy.tswebroot/src/components/Licensee/LicenseeRow/LicenseeRow.vuewebroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.tswebroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.vuewebroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.lesswebroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.tswebroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.vuewebroot/src/network/licenseApi/data.api.tswebroot/src/network/searchApi/data.api.ts
✅ Files skipped from review due to trivial changes (1)
- webroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.less
🚧 Files skipped from review as they are similar to previous changes (3)
- webroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.vue
- webroot/src/network/searchApi/data.api.ts
- webroot/src/components/Licensee/LicenseeList/LicenseeList.ts
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
maininto this branchDescription List
TODO
maininto this branchTesting List
yarn test:unit:allshould run without errors or warningsyarn serveshould run without errors or warningsyarn buildshould run without errors or warningsCloses #1307
Summary by CodeRabbit
New Features
Improvements
Tests
Chores