Open
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds cosmetology app-mode support and licenseNumber search plumbing: new global app-mode getters, store/user compact->app-mode dispatch, UI conditional fields and validation, network/search APIs extended for licenseNumber, locale/model updates, sorting for licenses/privileges, and mock/test adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as rgba(79,129,189,0.5) User UI
participant Store as rgba(124,181,71,0.5) Vuex Store
participant Components as rgba(192,80,77,0.5) Vue Components
participant API as rgba(155,89,182,0.5) Network API
UI->>Store: dispatch setCurrentCompact(compact)
Store->>Store: dispatch setAppMode(AppModes.COSMETOLOGY|JCC)
Store->>Store: dispatch getCompactStatesRequest(compact.type)
Store-->>Components: getters isAppMode* update
UI->>Components: render mode-conditional fields (licenseNumber or privilege fields)
UI->>API: submit search params (includes licenseNumber when cosmetology)
API-->>Components: return search results
Components->>UI: render rows/columns per app mode
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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: 2
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)
127-147:⚠️ Potential issue | 🟠 Major
licenseNumber-only searches are currently ignored.
hasSearchTermsdoes not includelicenseNumber, sorequestParams.query.licenseNumberis never set unless id/first/last name is also provided.Suggested fix
- 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 - 147, The current gating variable hasSearchTerms excludes licenseNumber, so requestParams.query.licenseNumber is never set for license-number-only searches; update the logic in data.api.ts by either adding licenseNumber to the hasSearchTerms expression (e.g., include licenseNumber in the Boolean(...) that defines hasSearchTerms) or move the licenseNumber assignment (requestParams.query.licenseNumber) out of the hasSearchTerms conditional so that licenseNumber is applied independently; ensure you reference and modify the hasSearchTerms variable and the requestParams.query.licenseNumber assignment accordingly.
🧹 Nitpick comments (3)
webroot/src/store/user/user.spec.ts (1)
1086-1090: Strengthen the compact-states dispatch assertion with payload checks.Both tests verify the second dispatched action name, but not the compact type argument. Add payload checks so regressions in downstream state-fetch routing are caught.
Suggested test assertion tightening
- expect([dispatch.secondCall.args[0]]).to.matchPattern(['getCompactStatesRequest']); + expect(dispatch.secondCall.args).to.matchPattern(['getCompactStatesRequest', CompactType.ASLP]); ... - expect([dispatch.secondCall.args[0]]).to.matchPattern(['getCompactStatesRequest']); + expect(dispatch.secondCall.args).to.matchPattern(['getCompactStatesRequest', CompactType.COSMETOLOGY]);Also applies to: 1100-1103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webroot/src/store/user/user.spec.ts` around lines 1086 - 1090, The test currently only asserts the second dispatched action name; update the assertion to also verify the payload/argument passed to getCompactStatesRequest (e.g., inspect dispatch.secondCall.args[1] or the appropriate payload position) equals the expected compact type so downstream routing won't regress; change the assertion in the block referencing dispatch and getCompactStatesRequest (and mirror the same tightening for the similar assertions around lines with the other test) to assert both action name and payload value alongside existing AppModes.JCC checks.webroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.ts (2)
376-389: Consider logging unhandled app modes in the default case.The switch statement cleanly separates mode-specific search properties. However, the empty
defaultcase silently falls through. If a newAppModesvalue is added in the future without updating this switch, it would use only common search props without any indication.Consider adding a console warning for development or using TypeScript's exhaustive check pattern.
💡 Optional: Add exhaustive check or warning
case AppModes.COSMETOLOGY: allowedSearchProps.push('licenseNumber'); break; -default: - break; +default: { + // Exhaustive check - TypeScript will error if a new AppModes value is added + const _exhaustiveCheck: never = this.appMode; + console.warn(`Unhandled app mode: ${_exhaustiveCheck}`); + break; +} }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.ts` around lines 376 - 389, The switch on this.appMode (inside LicenseeSearch where allowedSearchProps is built) currently has an empty default and will silently ignore any new AppModes values; add a dev-only warning or an exhaustive check in the default branch to surface unhandled app modes (e.g., console.warn or throw in non-production), or implement a TypeScript exhaustive check using a helper that accepts the unreachable value, referencing AppModes, allowedSearchProps, and the switch block so future additions to AppModes are detected and logged.
257-265: Consider adding a placeholder for consistency.The
licenseNumberFormInput has an empty placeholder while similar fields likefirstNameandlastNameuse computed placeholders. If a placeholder would help users, consider adding one.💡 Optional: Add a placeholder
licenseNumber: new FormInput({ id: 'license-number', name: 'license-number', label: computed(() => this.$t('licensing.licenseNumber')), - placeholder: '', + placeholder: computed(() => this.$t('licensing.searchPlaceholderLicenseNumber')), validation: Joi.string().min(0).max(100).messages(this.joiMessages.string), value: this.searchParams.licenseNumber || '', enforceMax: true, }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.ts` around lines 257 - 265, Add a computed placeholder to the licenseNumber FormInput (the new FormInput instance assigned to licenseNumber) to match other fields: replace the empty placeholder with a computed translation like computed(() => this.$t('licensing.licenseNumberPlaceholder')) or an appropriate i18n key so the field shows a consistent placeholder text; update the i18n strings if needed to include that key.
🤖 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/models/License/License.model.ts`:
- Around line 33-34: Rename the misspelled enum key COSMOTOLOGIST to
COSMETOLOGIST in the License enum (License.model.ts) so the key matches its
value 'cosmetologist'; update the enum declaration (replace COSMOTOLOGIST =
'cosmetologist' with COSMETOLOGIST = 'cosmetologist') and run a quick global
search/replace for COSMOTOLOGIST to ensure no references remain, adjusting any
usages to the new COSMETOLOGIST identifier.
In `@webroot/src/network/mocks/mock.data.ts`:
- Around line 188-225: In the mock permissions objects for regions ne, oh, nv,
and ma inside the actions blocks, replace the incorrect key readSsn with the
canonical readSSN to match the rest of the mock schema; update each actions
object (e.g., the actions object nested under the ne, oh, nv, ma entries) so the
permission key is readSSN and run a quick search to ensure no other occurrences
of readSsn remain.
---
Outside diff comments:
In `@webroot/src/network/licenseApi/data.api.ts`:
- Around line 127-147: The current gating variable hasSearchTerms excludes
licenseNumber, so requestParams.query.licenseNumber is never set for
license-number-only searches; update the logic in data.api.ts by either adding
licenseNumber to the hasSearchTerms expression (e.g., include licenseNumber in
the Boolean(...) that defines hasSearchTerms) or move the licenseNumber
assignment (requestParams.query.licenseNumber) out of the hasSearchTerms
conditional so that licenseNumber is applied independently; ensure you reference
and modify the hasSearchTerms variable and the requestParams.query.licenseNumber
assignment accordingly.
---
Nitpick comments:
In `@webroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.ts`:
- Around line 376-389: The switch on this.appMode (inside LicenseeSearch where
allowedSearchProps is built) currently has an empty default and will silently
ignore any new AppModes values; add a dev-only warning or an exhaustive check in
the default branch to surface unhandled app modes (e.g., console.warn or throw
in non-production), or implement a TypeScript exhaustive check using a helper
that accepts the unreachable value, referencing AppModes, allowedSearchProps,
and the switch block so future additions to AppModes are detected and logged.
- Around line 257-265: Add a computed placeholder to the licenseNumber FormInput
(the new FormInput instance assigned to licenseNumber) to match other fields:
replace the empty placeholder with a computed translation like computed(() =>
this.$t('licensing.licenseNumberPlaceholder')) or an appropriate i18n key so the
field shows a consistent placeholder text; update the i18n strings if needed to
include that key.
In `@webroot/src/store/user/user.spec.ts`:
- Around line 1086-1090: The test currently only asserts the second dispatched
action name; update the assertion to also verify the payload/argument passed to
getCompactStatesRequest (e.g., inspect dispatch.secondCall.args[1] or the
appropriate payload position) equals the expected compact type so downstream
routing won't regress; change the assertion in the block referencing dispatch
and getCompactStatesRequest (and mirror the same tightening for the similar
assertions around lines with the other test) to assert both action name and
payload value alongside existing AppModes.JCC checks.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
webroot/.eslintrc.jswebroot/src/components/CompactSelector/CompactSelector.tswebroot/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/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
4 tasks
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
webroot/src/components/Licensee/LicenseeList/LicenseeList.ts (1)
395-397: GuardlicenseNumberserialization by app mode.For defensive correctness, only attach
licenseNumberwhen cosmetology mode is active.♻️ Suggested hardening
- if (searchParams?.licenseNumber) { + if (this.isAppModeCosmetology && searchParams?.licenseNumber) { requestConfig.licenseNumber = searchParams.licenseNumber; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webroot/src/components/Licensee/LicenseeList/LicenseeList.ts` around lines 395 - 397, Only set requestConfig.licenseNumber from searchParams?.licenseNumber when the app is running in cosmetology mode: guard the assignment by checking the app mode (e.g., call the existing isCosmetologyMode() helper or compare appMode === 'cosmetology') before assigning requestConfig.licenseNumber; leave it unset otherwise. Update the block around searchParams?.licenseNumber and requestConfig to perform that conditional check so license numbers are only serialized for cosmetology mode.
🤖 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/Licensee/LicenseeListLegacy/LicenseeListLegacy.ts`:
- Around line 299-301: Only set requestConfig.licenseNumber when the app is in
cosmetology mode: wrap the existing assignment that reads
searchParams.licenseNumber and assigns requestConfig.licenseNumber with an
additional condition that verifies cosmetology mode (e.g., check the relevant
mode flag/enum used in this component such as isCosmetologyMode or appMode ===
'cosmetology'); change the current block that uses searchParams.licenseNumber so
it only runs if both searchParams.licenseNumber is present and the
cosmetology-mode check passes, leaving all other requestConfig fields unchanged.
- Around line 127-135: The assembly of the combined search display in
searchDisplayAll should filter out empty/blank parts before joining to avoid
malformed separators; instead of building the array then joining and running
regex cleanup, filter the array elements (this.searchDisplayCompact,
this.searchDisplayFullName, this.searchDisplayState,
this.searchDisplayLicenseNumber) for truthy/non-blank values and then join them
with ', ' to produce the final string (remove the current joined variable +
regex replaces and return the filtered-then-joined result).
In
`@webroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.ts`:
- Around line 249-263: The resetForm() method currently clears
this.formData.compact.value only when enableCompactSelect is true but does not
clear the store-backed compact state; update resetForm() (within
LicenseeSearchLegacy.resetForm) to also clear the compact state in the store
whenever you reset the form (respecting enableCompactSelect), e.g., invoke the
existing action/mutation or method that sets the store compact value (the same
mechanism used elsewhere to set compact state), and keep the existing resets for
firstName, lastName, state, licenseNumber and the calls to
updateFormSubmitSuccess and updateFormSubmitError so form submit state is fully
cleared.
In
`@webroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.vue`:
- Around line 23-28: Replace the anchor used for the clear action with a native
button to restore keyboard accessibility: where the template conditionally
renders the anchor (v-if="isMockPopulateEnabled") that binds `@click` and
`@keyup.enter` to resetForm(), change it to a <button type="button"> that keeps
the same v-if, `@click`="resetForm()" and remove the redundant `@keyup.enter`
handler (buttons handle Enter/Space natively); ensure the CSS class "clear-form
search-input" is preserved and adjust any styling selectors if they relied on an
<a> element.
---
Nitpick comments:
In `@webroot/src/components/Licensee/LicenseeList/LicenseeList.ts`:
- Around line 395-397: Only set requestConfig.licenseNumber from
searchParams?.licenseNumber when the app is running in cosmetology mode: guard
the assignment by checking the app mode (e.g., call the existing
isCosmetologyMode() helper or compare appMode === 'cosmetology') before
assigning requestConfig.licenseNumber; leave it unset otherwise. Update the
block around searchParams?.licenseNumber and requestConfig to perform that
conditional check so license numbers are only serialized for cosmetology mode.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b92c7b95-1df5-4cf8-851d-e4cb1af6a234
📒 Files selected for processing (5)
webroot/src/components/Licensee/LicenseeList/LicenseeList.tswebroot/src/components/Licensee/LicenseeListLegacy/LicenseeListLegacy.tswebroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.lesswebroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.tswebroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.vue
webroot/src/components/Licensee/LicenseeListLegacy/LicenseeListLegacy.ts
Outdated
Show resolved
Hide resolved
webroot/src/components/Licensee/LicenseeListLegacy/LicenseeListLegacy.ts
Outdated
Show resolved
Hide resolved
webroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.ts
Outdated
Show resolved
Hide resolved
webroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.vue
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
webroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.ts (1)
249-264:⚠️ Potential issue | 🟡 MinorReset should also clear field-level validation state.
resetForm()clears values and submit-level flags, but it does not explicitly reset/recompute per-input touched/error state. Add a non-touched validation pass (or equivalent field reset) to avoid stale inline errors after clear.🧹 Suggested adjustment
async resetForm(): Promise<void> { if (this.enableCompactSelect) { this.formData.compact.value = ''; await this.$store.dispatch('user/setCurrentCompact', null); } this.formData.firstName.value = ''; this.formData.lastName.value = ''; this.formData.state.value = ''; this.formData.licenseNumber.value = ''; + this.validateAll({ asTouched: false }); this.isFormLoading = false; this.isFormSuccessful = false; this.isFormError = false; this.updateFormSubmitSuccess(''); this.updateFormSubmitError(''); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.ts` around lines 249 - 264, resetForm currently clears values and submit flags but leaves per-field validation state intact; update resetForm (in LicenseeSearchLegacy.resetForm) to also clear field-level validation by either calling the form validation reset API (e.g., this.$v && this.$v.$reset()) or explicitly resetting each field's validation props: for compact, firstName, lastName, state, licenseNumber set touched/dirty flags to false and clear any error messages (e.g., field.touched = false; field.error = ''), then optionally re-run a non-touched validation pass if your app relies on computed validation state.
🤖 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/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.ts`:
- Around line 219-221: handleSubmit() now emits licenseNumber for cosmetology
but the request mapper omits licenseNumber when other name/id fields are blank;
update the mapper that builds the outgoing request (the hasSearchTerms branch /
mapping logic) to explicitly check for and include query.licenseNumber (or move
the licenseNumber mapping out of the hasSearchTerms-only block) so that when
AppModes.COSMETOLOGY submits only licenseNumber it is serialized into the
request; adjust the conditional that computes hasSearchTerms or the mapping
function so licenseNumber is honored independently of firstName/lastName/id.
---
Duplicate comments:
In
`@webroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.ts`:
- Around line 249-264: resetForm currently clears values and submit flags but
leaves per-field validation state intact; update resetForm (in
LicenseeSearchLegacy.resetForm) to also clear field-level validation by either
calling the form validation reset API (e.g., this.$v && this.$v.$reset()) or
explicitly resetting each field's validation props: for compact, firstName,
lastName, state, licenseNumber set touched/dirty flags to false and clear any
error messages (e.g., field.touched = false; field.error = ''), then optionally
re-run a non-touched validation pass if your app relies on computed validation
state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 649aa13f-d463-428c-989d-30b28c18bd41
📒 Files selected for processing (2)
webroot/src/components/Licensee/LicenseeListLegacy/LicenseeListLegacy.tswebroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.ts
webroot/src/components/Licensee/LicenseeSearchLegacy/LicenseeSearchLegacy.ts
Show resolved
Hide resolved
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
eslintrcfor usingv-forkeys in<template>elementsTODO
Testing List
yarn test:unit:allshould run without errors or warningsyarn serveshould run without errors or warningsyarn buildshould run without errors or warningsCloses #
Summary by CodeRabbit
New Features
Improvements