Frontend/adverse action NPDB multi select #1161
Conversation
- Added multi-select NPDB category to encumber UI - Updated model / store / network modules - Update unencumber UI to display encumbrance type rather than NPDB category - Updated webpack config to suppress css load order warnings - @todo: Make final UI updates once designs are complete - @todo: Use a feature flag
WalkthroughAdds a feature-flagged multi-select NPDB category capability: new InputSelectMultiple component, UI and form model changes in LicenseCard/PrivilegeCard/ExampleForm, model and serializer updates, store and API signatures extended for Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as Card (License/Privilege)
participant S as Store
participant API as data.api
participant LA as licenseApi
participant SV as Server/Mock
U->>C: Open encumber modal / select category(ies)
note over C: Check FeatureGates.ENCUMBER_MULTI_CATEGORY
alt Gate ON
C->>C: Render InputSelectMultiple
U->>C: Select multiple -> npdbCategories [...]
C->>S: encumber...Request({ npdbCategories: [...] })
else Gate OFF
C->>C: Render InputSelect (single)
U->>C: Select one -> npdbCategory "..."
C->>S: encumber...Request({ npdbCategory: "..." })
end
S->>API: encumber...(…, npdbCategory, npdbCategories)
API->>LA: encumber...(…, npdbCategory, npdbCategories)
note over LA: Read $features at runtime to decide payload field
alt Gate ON
LA->>SV: POST { clinicalPrivilegeActionCategories: [...] }
else Gate OFF
LA->>SV: POST { clinicalPrivilegeActionCategory: "..." }
end
SV-->>LA: Response
LA-->>API: Response
API-->>S: Response
S-->>C: Update UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ 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}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (12)
webroot/src/components/Forms/InputSelectMultiple/InputSelectMultiple.spec.ts (1)
12-19: Remove unnecessary async on describe and assert multiple-select semantics
- describe need not be async.
- Add an assertion that the select has multiple enabled to verify core behavior.
Apply this diff:
-describe('InputSelectMultiple component', async () => { +describe('InputSelectMultiple component', () => { it('should mount the component', async () => { const wrapper = await mountShallow(InputSelectMultiple); expect(wrapper.exists()).to.equal(true); expect(wrapper.findComponent(InputSelectMultiple).exists()).to.equal(true); + expect(wrapper.find('select[multiple]').exists()).to.equal(true); }); });webroot/src/components/Forms/InputSelectMultiple/InputSelectMultiple.less (1)
19-21: Select resize likely has no effectresize on a native select is unsupported in most browsers. Consider setting a size attribute on the select or a custom control if vertical resizing is needed.
webroot/src/models/AdverseAction/AdverseAction.model.ts (1)
86-92: Deduplicate NPDB type name resolutionnpdbTypeName and getNpdbTypeName duplicate lookup logic. Extract a single resolver and reuse to reduce drift.
Also applies to: 94-100
webroot/src/components/Forms/InputSelectMultiple/InputSelectMultiple.vue (2)
24-34: Expose description to AT via aria-describedbyGive the helper text an id and reference it.
Apply this diff:
- <div class="multi-select-description">{{ $t('common.selectMultipleKeys') }}</div> + <div + class="multi-select-description" + :id="`${formInput.id}-desc`" + > + {{ $t('common.selectMultipleKeys') }} + </div> @@ - :aria-describedby="`${formInput.id}-error`" + :aria-describedby="`${formInput.id}-desc ${formInput.id}-error`"
44-51: Use stable keys for optionsIndex keys are fragile. Use option.value.
Apply this diff:
- :key="index" + :key="option.value"webroot/src/store/users/users.actions.ts (1)
135-147: npdbCategories plumbed correctly; add safe defaults and typesLooks good. Consider defaulting to [] to avoid undefined and improve typing of the payload.
Apply this minimal change at both action payload destructures:
- npdbCategories, + npdbCategories = [],Also applies to: 233-245
webroot/src/components/StyleGuide/ExampleForm/ExampleForm.ts (1)
79-86: Option builder duplication; consider centralizingnpdbCategoryOptions mirrors logic in LicenseCard. Extract a shared helper to avoid divergence.
webroot/src/components/Forms/InputSelectMultiple/InputSelectMultiple.ts (1)
32-37: Normalize display value (optional)If templates expect strings, consider unref-ing ComputedRef names before returning.
- getValueDisplay(value = ''): string | ComputedRef<string> { + getValueDisplay(value = ''): string { const selectedOption: SelectOption = this.formInput?.valueOptions?.find((option: SelectOption) => option.value === value) || { value, name: '' }; - return selectedOption?.name || ''; + const name = selectedOption?.name ?? ''; + return typeof name === 'object' && 'value' in name ? (name as ComputedRef<string>).value : String(name); }webroot/src/components/LicenseCard/LicenseCard.ts (1)
458-460: Remove debug loggingDrop console.log before merge (or guard by env).
- // @TODO - console.log(response); + // no-opwebroot/src/network/licenseApi/data.api.ts (1)
277-288: Avoid global window feature lookup in data layer (optional)Prefer passing the gate decision from callers to keep network layer pure/testable.
- const { $features } = (window as any).Vue?.config?.globalProperties || {}; - ...($features?.checkGate(FeatureGates.ENCUMBER_MULTI_CATEGORY) ? { ... } : { ... }) + // Pass a boolean flag or infer from provided params: + const useMulti = Array.isArray(npdbCategories) && npdbCategories.length > 0; + ...(useMulti ? { clinicalPrivilegeActionCategories: npdbCategories } : { clinicalPrivilegeActionCategory: npdbCategory })webroot/src/models/AdverseAction/AdverseAction.model.spec.ts (1)
101-101: Consider testing the actual lookup behavior.While this test validates the method signature with an optional category parameter, it appears to test a pass-through scenario (
getNpdbTypeName('Other')returns'Other'). Consider adding a test that validates the method's lookup logic when it needs to resolve a category from stored data or translations.webroot/src/components/PrivilegeCard/PrivilegeCard.ts (1)
275-295: LGTM! Proper conditional form input initialization.The feature-gated logic correctly initializes:
- Multi-select mode:
encumberModalNpdbCategorieswith array validation (Joi.array().min(1)) and empty array default- Single-select mode:
encumberModalNpdbCategorywith string validation (Joi.string().required())The validation schemas and initial values are appropriate for each mode.
Optional refactor: Consider extracting the conditional form input creation to a helper method (e.g.,
createNpdbCategoryInput()) to improve testability and reduce the cognitive load of the initialization method.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
webroot/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (20)
webroot/src/app.config.ts(1 hunks)webroot/src/components/Forms/InputSelectMultiple/InputSelectMultiple.less(1 hunks)webroot/src/components/Forms/InputSelectMultiple/InputSelectMultiple.spec.ts(1 hunks)webroot/src/components/Forms/InputSelectMultiple/InputSelectMultiple.ts(1 hunks)webroot/src/components/Forms/InputSelectMultiple/InputSelectMultiple.vue(1 hunks)webroot/src/components/LicenseCard/LicenseCard.ts(8 hunks)webroot/src/components/LicenseCard/LicenseCard.vue(1 hunks)webroot/src/components/PrivilegeCard/PrivilegeCard.ts(8 hunks)webroot/src/components/PrivilegeCard/PrivilegeCard.vue(1 hunks)webroot/src/components/StyleGuide/ExampleForm/ExampleForm.ts(5 hunks)webroot/src/components/StyleGuide/ExampleForm/ExampleForm.vue(1 hunks)webroot/src/locales/en.json(2 hunks)webroot/src/locales/es.json(2 hunks)webroot/src/models/AdverseAction/AdverseAction.model.spec.ts(8 hunks)webroot/src/models/AdverseAction/AdverseAction.model.ts(4 hunks)webroot/src/network/data.api.ts(4 hunks)webroot/src/network/licenseApi/data.api.ts(5 hunks)webroot/src/network/mocks/mock.data.api.ts(4 hunks)webroot/src/store/users/users.actions.ts(4 hunks)webroot/vue.config.js(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-21T16:36:48.723Z
Learnt from: jsandoval81
PR: csg-org/CompactConnect#1019
File: webroot/src/components/PrivilegeCard/PrivilegeCard.vue:270-278
Timestamp: 2025-08-21T16:36:48.723Z
Learning: In PrivilegeCard component's unencumber modal (webroot/src/components/PrivilegeCard/PrivilegeCard.vue), the unencumber-select wrapper element intentionally uses space key handling to catch bubbled events from child InputCheckbox elements for custom handling actions. When adverseAction.hasEndDate() is true, items show an inactive-category div and are designed to be non-interactive (not focusable), while items without end dates contain focusable InputCheckbox child elements. This design pattern is consistent with prior implementation and represents intentional UX behavior.
Applied to files:
webroot/src/components/PrivilegeCard/PrivilegeCard.vuewebroot/src/components/LicenseCard/LicenseCard.vue
📚 Learning: 2025-06-24T00:02:39.944Z
Learnt from: jsandoval81
PR: csg-org/CompactConnect#873
File: webroot/src/components/LicenseCard/LicenseCard.ts:414-443
Timestamp: 2025-06-24T00:02:39.944Z
Learning: In LicenseCard component's clickUnencumberItem method (webroot/src/components/LicenseCard/LicenseCard.ts), complex event handling for checkbox interactions is intentionally designed to ensure consistent behavior across checkbox input, wrapper label, and outer selection parent elements for custom UI requirements. This complexity should be preserved rather than simplified.
Applied to files:
webroot/src/components/LicenseCard/LicenseCard.ts
🧬 Code graph analysis (5)
webroot/src/components/LicenseCard/LicenseCard.ts (1)
webroot/src/models/FormInput/FormInput.model.ts (1)
FormInput(58-168)
webroot/src/network/data.api.ts (1)
webroot/src/network/licenseApi/data.api.ts (1)
licenseDataApi(499-499)
webroot/src/components/StyleGuide/ExampleForm/ExampleForm.ts (1)
webroot/src/models/FormInput/FormInput.model.ts (1)
FormInput(58-168)
webroot/src/components/Forms/InputSelectMultiple/InputSelectMultiple.spec.ts (1)
webroot/tests/helpers/setup.ts (1)
mountShallow(265-265)
webroot/src/components/PrivilegeCard/PrivilegeCard.ts (1)
webroot/src/models/FormInput/FormInput.model.ts (1)
FormInput(58-168)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CheckWebroot
🔇 Additional comments (26)
webroot/vue.config.js (1)
93-107: LGTM! Suppressing CSS order warnings is reasonable for code-split apps.The ignoreOrder flag prevents noisy warnings from mini-css-extract-plugin in applications using code splitting/chunking, where CSS load order may vary but doesn't affect functionality. This is a standard practice.
webroot/src/app.config.ts (1)
292-292: LGTM! Feature gate addition follows the existing pattern.The new ENCUMBER_MULTI_CATEGORY flag enables conditional multi-category NPDB support throughout the application.
webroot/src/network/mocks/mock.data.api.ts (1)
237-238: LGTM! Feature gate access pattern works but is unconventional.The mock API accesses the feature gate via
(window as any).Vue?.config?.globalPropertiesto determine payload shape at runtime. While this approach works and includes safe fallback via optional chaining, it's an unusual pattern. The conditional spread operator correctly includes eithernpdbCategories(when flag is enabled) ornpdbCategory(when disabled).Also applies to: 250-257
webroot/src/components/StyleGuide/ExampleForm/ExampleForm.vue (1)
21-21: LGTM! Demonstrates the new multi-select component in the style guide.The InputSelectMultiple component is correctly integrated for demonstration purposes.
webroot/src/locales/es.json (1)
21-21: LGTM! Spanish translations for multi-select UI are appropriate.The added keys
selectMultipleKeysandstatesMultipleprovide proper Spanish translations for the multi-select functionality.Also applies to: 73-73
webroot/src/locales/en.json (1)
21-21: LGTM! English translations clearly communicate multi-select usage.The added keys provide clear instructions ("Hold Ctrl / Cmd key to select multiple") and appropriate labels for the multi-select UI.
Also applies to: 72-72
webroot/src/components/PrivilegeCard/PrivilegeCard.vue (1)
193-198: LGTM! Clean feature-gated UI switching between single and multi-select.The conditional rendering correctly displays InputSelectMultiple for multi-category mode when the ENCUMBER_MULTI_CATEGORY feature flag is enabled, and falls back to the single InputSelect otherwise. This provides backward compatibility while enabling the new functionality.
webroot/src/components/LicenseCard/LicenseCard.vue (1)
133-137: Gate-driven switch looks good; verify validators, value types, and payloadsConfirm:
- encumberModalNpdbCategories initializes to [] and has validators aligned to array values.
- Hidden field’s validators don’t block submit (only the visible input should be required).
- Store/network paths send npdbCategories vs npdbCategory per gate.
webroot/src/components/Forms/InputSelectMultiple/InputSelectMultiple.vue (1)
25-31: Verify v-model array contract and validatorsEnsure formInput.value is always an array ([]) for multi-select, and validators/transformers expect arrays. Also ensure tokens render correctly when value is empty.
Also applies to: 63-67
webroot/src/components/StyleGuide/ExampleForm/ExampleForm.ts (1)
136-143: Multi-select FormInput config LGTMValidation, options, and default [] align with multi-select usage.
webroot/src/components/LicenseCard/LicenseCard.ts (4)
283-303: Gate-aware form inputs for NPDB categories: solid implementationGood split between single vs multi with correct validations and options.
Ensure the template renders InputSelectMultiple when the gate is on and binds to encumberModalNpdbCategories.value (array).
444-451: Payload gating LGTMCorrectly selects npdbCategories vs npdbCategory based on the feature gate.
245-250: Placeholder option only in single-select mode: goodPrevents empty selection token in multi-select.
655-659: Date format in mockPopulate may not match Joi patternJoi validation uses MM/DD/YYYY, but mockPopulate sets YYYY-MM-DD. Align formats or relax validation.
- this.formData.encumberModalStartDate.value = moment().format('YYYY-MM-DD'); + this.formData.encumberModalStartDate.value = moment().format('MM/DD/YYYY');webroot/src/network/licenseApi/data.api.ts (2)
277-288: Confirm required payload keys with backend
Model and specs map bothclinicalPrivilegeActionCategoryandclinicalPrivilegeActionCategories, but mocks only include the singular field. Verify via API docs that both keys are expected on the license and privilege encumbrance endpoints, and update mocks to include the plural field for multi-category scenarios. Applies to lines 277–288 and 364–375.
262-276: All encumberLicense and encumberPrivilege call sites updated
All callers now supply the newnpdbCategoriesargument.webroot/src/components/Forms/InputSelectMultiple/InputSelectMultiple.ts (1)
39-47: TypeScript compile error: assignment to a type assertionLeft-hand assignment uses a type assertion, which TS disallows. Also, tighten typing and mark input as edited.
Apply:
- removeSelectedValue(value): void { + removeSelectedValue(value: string | number): void { const { formInput } = this; if (Array.isArray(formInput.value)) { - (formInput.value as Array<string>) = formInput.value.filter((selected) => selected !== value); + formInput.value = (formInput.value as Array<string | number>) + .filter((selected) => selected !== value); } - formInput.validate(); + // Maintain input semantics (edited + validate) + formInput.input(); }⛔ Skipped due to learnings
Learnt from: jsandoval81 PR: csg-org/CompactConnect#822 File: webroot/src/components/Forms/InputEmailList/InputEmailList.ts:77-89 Timestamp: 2025-05-28T19:57:53.185Z Learning: In TypeScript, when dealing with union types that include arrays, direct assignment after filtering can cause compilation errors. The pattern `(formInput.value as Array<string>) = ...` is valid syntax for type assertion on the left-hand side of assignments and can be used as a workaround when TypeScript struggles with type inference on complex nested properties.webroot/src/models/AdverseAction/AdverseAction.model.spec.ts (3)
50-50: LGTM! Good test coverage for default initialization.The tests correctly validate that
npdbTypesinitializes as an empty array andgetNpdbTypeName()returns an empty string when called without arguments.Also applies to: 62-62
74-74: LGTM! Constructor properly handles npdbTypes array.The test validates that
npdbTypesis correctly populated from constructor data, ensuring data-driven initialization works as expected.Also applies to: 89-89
156-156: LGTM! Comprehensive serializer test coverage.The tests properly validate that:
- Server data (
clinicalPrivilegeActionCategories) is correctly deserialized tonpdbTypesgetNpdbTypeName()works correctly with categories from deserialized dataThis provides good coverage of the serialization pathway.
Also applies to: 172-172, 188-188
webroot/src/components/PrivilegeCard/PrivilegeCard.ts (6)
19-19: LGTM! Proper component setup for multi-select feature.The imports and component registration correctly add the necessary dependencies for the feature-gated NPDB multi-select functionality.
Also applies to: 24-24, 49-49
78-80: LGTM! Feature gate exposure for template access.The public
featureGatesgetter appropriately exposesFeatureGatesfor template-based conditional rendering. This is a standard pattern for feature-gated UI components.
217-222: LGTM! Appropriate UX handling for single vs. multi-select.The conditional placeholder logic correctly adds a "Select an option" entry only for single-select mode. Multi-select mode naturally starts with an empty selection, eliminating the need for a placeholder option.
499-506: LGTM! Proper conditional payload construction.The feature-gated payload logic correctly includes either:
npdbCategories(array) when multi-category is enablednpdbCategory(single value) when multi-category is disabledThis matches the expected API contract for each mode.
709-713: LGTM! Mock populate correctly handles both modes.The mock populate logic properly sets:
[this.npdbCategoryOptions[1]?.value](array) for multi-select modethis.npdbCategoryOptions[1]?.value(single value) for single-select modeThis ensures the mock data matches the expected structure for each mode.
327-327: Approve change:encumbranceTypeName()is implemented and tested: method exists inAdverseAction.model.ts(line 78) and is covered by unit tests, so the update aligns with requirements.
webroot/src/components/Forms/InputSelectMultiple/InputSelectMultiple.less
Show resolved
Hide resolved
webroot/src/components/Forms/InputSelectMultiple/InputSelectMultiple.vue
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
webroot/src/models/AdverseAction/AdverseAction.model.ts (1)
94-100: Consider refactoring to eliminate duplication.The new
getNpdbTypeNamemethod duplicates the logic of the existingnpdbTypeName()method (lines 86-92). Both methods perform identical lookups in thelicensing.npdbTypestranslation array.Apply this diff to refactor
npdbTypeName()to delegate togetNpdbTypeName():public npdbTypeName(): string { - const npdbTypes = this.$tm('licensing.npdbTypes') || []; - const npdbType = npdbTypes.find((translate) => translate.key === this.npdbType); - const typeName = npdbType?.name || ''; - - return typeName; + return this.getNpdbTypeName(this.npdbType || ''); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webroot/src/models/AdverseAction/AdverseAction.model.spec.ts(8 hunks)webroot/src/models/AdverseAction/AdverseAction.model.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
webroot/src/models/AdverseAction/AdverseAction.model.spec.ts (1)
webroot/src/models/AdverseAction/AdverseAction.model.ts (2)
AdverseActionSerializer(127-147)AdverseAction(35-122)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CheckWebroot
🔇 Additional comments (6)
webroot/src/models/AdverseAction/AdverseAction.model.ts (2)
26-26: LGTM!The addition of
npdbTypesas an optional array field in both the interface and class supports the multi-select NPDB category feature.Also applies to: 44-44
137-139: LGTM!The serializer correctly coerces
clinicalPrivilegeActionCategoriesto an array, preventing UI loops when the server returns null or undefined. This implementation matches the resolution of the past review comment.Based on past review comment.
webroot/src/models/AdverseAction/AdverseAction.model.spec.ts (4)
50-50: LGTM!The test coverage correctly validates the default empty array for
npdbTypesand the ability to construct anAdverseActionwith specificnpdbTypesvalues.Also applies to: 74-74, 89-89
101-101: LGTM!The tests correctly validate
getNpdbTypeNamewith proper arguments, demonstrating the method's ability to map NPDB type keys to display names.Also applies to: 188-188
156-156: LGTM!The test data and assertions correctly validate that the serializer populates
npdbTypesfrom the server'sclinicalPrivilegeActionCategoriesarray.Also applies to: 172-172
191-200: LGTM!This test case provides valuable coverage for the serializer's handling of invalid data types from the server, confirming that non-array values are coerced to an empty array as expected.
- Update for UI design changes - @todo: Wire up backend once ready
- Wire up backend
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
webroot/src/components/PrivilegeCard/PrivilegeCard.ts (1)
78-80: Same patterns as LicenseCard - see type safety concern.The feature gate exposure, conditional options, and form initialization follow the same patterns as LicenseCard. The type safety concern regarding
FormInput.valuebeing a string while multi-select assigns an array applies here as well (see LicenseCard.ts review).Also applies to: 217-222, 275-295
🧹 Nitpick comments (2)
webroot/src/components/LicenseCard/LicenseCard.ts (2)
94-96: Consider limiting FeatureGates exposure in templates.While functional, exposing the entire
FeatureGatesobject couples templates to the full gate structure. For better encapsulation, consider exposing only the specific gate state or check method you need.Example alternative:
-get featureGates(): typeof FeatureGates { - return FeatureGates; -} +get isMultiCategoryEnabled(): boolean { + return this.$features.checkGate(FeatureGates.ENCUMBER_MULTI_CATEGORY); +}
283-303: Consider extracting shared encumbrance logic into a mixin.The feature-gated NPDB category logic is duplicated between
LicenseCard.tsandPrivilegeCard.ts. Since both components share identical patterns for form initialization, payload construction, and mock population, consider extracting this logic into a shared mixin to improve maintainability.This would reduce duplication and ensure consistent behavior across both license and privilege encumbrance workflows.
Also applies to: 444-451, 651-655
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webroot/src/components/LicenseCard/LicenseCard.ts(8 hunks)webroot/src/components/PrivilegeCard/PrivilegeCard.ts(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-24T00:02:39.944Z
Learnt from: jsandoval81
PR: csg-org/CompactConnect#873
File: webroot/src/components/LicenseCard/LicenseCard.ts:414-443
Timestamp: 2025-06-24T00:02:39.944Z
Learning: In LicenseCard component's clickUnencumberItem method (webroot/src/components/LicenseCard/LicenseCard.ts), complex event handling for checkbox interactions is intentionally designed to ensure consistent behavior across checkbox input, wrapper label, and outer selection parent elements for custom UI requirements. This complexity should be preserved rather than simplified.
Applied to files:
webroot/src/components/LicenseCard/LicenseCard.ts
🧬 Code graph analysis (2)
webroot/src/components/LicenseCard/LicenseCard.ts (1)
webroot/src/models/FormInput/FormInput.model.ts (1)
FormInput(58-168)
webroot/src/components/PrivilegeCard/PrivilegeCard.ts (1)
webroot/src/models/FormInput/FormInput.model.ts (1)
FormInput(58-168)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CheckWebroot
🔇 Additional comments (9)
webroot/src/components/LicenseCard/LicenseCard.ts (6)
19-19: LGTM! Feature gate and multi-select component imports are correct.The addition of
FeatureGatesandInputSelectMultipleproperly supports the conditional multi-select NPDB category feature.Also applies to: 23-23
245-250: LGTM! Correct conditional empty option for single-select.The logic correctly omits the placeholder "Select option" entry for multi-select mode, as native multi-select controls don't require an empty placeholder.
335-335: LGTM! Label change aligns with PR objectives.The switch from
npdbTypeName()toencumbranceTypeName()correctly implements the requirement that the unencumber workflow displays Discipline Type rather than NPDB Category.
444-451: LGTM! Payload construction correctly branches on feature gate.The conditional spread properly sends either
npdbCategories(array) ornpdbCategory(string) based on the feature gate state, maintaining backward compatibility.
651-655: LGTM! Mock populate logic correctly handles both modes.The conditional logic properly assigns an array for multi-select and a string for single-select, matching the respective form input value types.
283-303: ****The review comment is based on an incorrect premise. The FormInput type definition explicitly supports array values:
value?: string | number | boolean | null | Array<File> | Array<string>(FormInput.model.ts, line 22). The multi-select pattern usingvalue: []is type-safe and compatible with the FormInput class methods (enforceLength()andvalidate()), which both use generic operations (.length,.slice(), and Joi validation) that work correctly with arrays. The LicenseCard code properly initializes, validates, and uses array values for the multi-category form input.Likely an incorrect or invalid review comment.
webroot/src/components/PrivilegeCard/PrivilegeCard.ts (3)
19-19: LGTM! Required imports for feature gate functionality.The
FeatureGatesandInputSelectMultipleimports properly enable the conditional multi-select NPDB category feature for privileges.Also applies to: 24-24
327-327: LGTM! Label change aligns with PR requirements.The switch to
encumbranceTypeName()correctly displays Discipline Type in the unencumber workflow for privileges, consistent with the changes in LicenseCard.
499-506: LGTM! Payload and mock logic correctly handle both modes.The conditional payload construction and mock populate logic properly branch on the feature gate, maintaining consistency with LicenseCard's implementation.
Also applies to: 706-710
|
The encumbrance event detail is no longer displaying correctly in the history. Previously, this would match to an npbd category. Now its coming back as a comma seperated list so its not translating correctly: 
|
|
Not sure if this your problem per se but holding command and then pressing space is the default to bring up spotlight search on mac which overwrites the select action. Clicking control direction on mac appears to be a different hotkey. I can make this work if I command arrow, release command, space etc. Just wanted to bring this up. This may be standard. |
|
@ChiefStief Good catch. We had a similar situation with the unencumber workflow - where it was previously showing the NPDB category as the label. In that case, I confirmed with Product that we could change that label to the Discipline Type. That seems appropriate for me to do here as well - any concerns with that? |
|
@jsandoval81 That would be fine the problem is we aren't returning the whole adverse action there, just the nbpd categories, so we dont have that data for every history item |
|
Paper trail for @ChiefStief comments above:
|
|
@jlkravitz This is ready for your review. |
jlkravitz
left a comment
jlkravitz
left a comment
There was a problem hiding this comment.
Looks good to me! @isabeleliassen Good to merge.
5 participants
Requirements List
yarn install --ignore-enginesDescription List
ENCUMBER_MULTI_CATEGORYtmptransitive dev dependencyTesting List
yarn test:unit:allshould run without errors or warningsyarn serveshould run without errors or warningsyarn buildshould run without errors or warningsSome notes regarding the new multi-select input and a11y:
<select>element (e.g., using Tab).<select>element (e.g., using Tab).Closes #1091
Summary by CodeRabbit
New Features
Tests
Chores