-
Notifications
You must be signed in to change notification settings - Fork 7
Frontend/military status update #1250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Frontend/military status update #1250
Conversation
- Renamed unusable list / paging components as Legacy - Added new blank components that will hold new core list / paging functionality
- Implement roughed out version of standard paging for staff licensee search
- Implement roughed out version of the admin search form - Implement roughed out version of server payload prep for the opensearch provider query
- Add search terms display to list view - Style the new paging UI - Updates to opensearch queries
- Added the provider vs. privilege search UI variants - Added an export-only UI flow to the privilege search
- Moved the search API to a new network folder - Added env param for search API URI - Added env param for specifically disabling statsig - Updated opensearch syntax to keyword for sort fields
- Updated queries for more complex nested search scenarios - Started testing with privilege export CSVs - Disabled the military status search until the next PR
- Added window function to log query output for helping with query debugging as needed - Minor updates to pagination
- PR review updates
- PR review updates
- PR review updates
📝 WalkthroughWalkthroughAdds a military audit workflow: new enum and model fields, UI icon/component and modals, audit endpoints (real & mock), Vuex actions/mutations/tests, locale updates, mock data updates, and adjustments to component usages and search form. Changes
Sequence Diagram(s)sequenceDiagram
actor User as Compact Director
participant UI as MilitaryAffiliationInfoBlock
participant Modal as Audit Modal
participant Store as Vuex Store
participant API as DataApi
participant Backend as Backend
User->>UI: Click Approve/Decline
UI->>Modal: open modal
Modal->>User: show form (notes if decline)
User->>Modal: Submit (action + notes)
Modal->>UI: auditSubmit(action, notes)
UI->>Store: dispatch updateMilitaryAuditRequest(compact, licenseeId, data)
Store->>Store: commit UPDATE_MILITARY_AUDIT_REQUEST
Store->>API: dataApi.updateMilitaryAudit(compact, licenseeId, data)
API->>Backend: PATCH /v1/compacts/{compact}/providers/{licenseeId}/militaryAudit
Backend-->>API: response
API-->>Store: response
Store->>Store: commit UPDATE_MILITARY_AUDIT_SUCCESS / FAILURE
Store-->>UI: state updated
UI->>Modal: close modal
UI->>User: reflect updated audit status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-08-13T18:24:06.298ZApplied to files:
📚 Learning: 2025-08-21T16:36:48.723ZApplied to files:
📚 Learning: 2025-06-24T00:02:39.944ZApplied to files:
📚 Learning: 2025-09-08T20:19:46.094ZApplied to files:
📚 Learning: 2025-11-06T18:11:58.272ZApplied to files:
⏰ 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)
🔇 Additional comments (2)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
webroot/src/locales/en.json (1)
954-971: Frontend migration to new military status keys appears complete.The key changes have been consistently applied across the codebase:
- Mock data uses the new keys (
'approved','declined','notApplicable')- Component logic correctly maps the
isAuditStatusPendingcheck to theTENTATIVEenum value- No references to old keys (
'pending','denied') remain in TypeScript/JavaScript files- Localization strings in the enum match the new keys
Verify that the backend API is returning these same key values in militaryStatus fields. If not, the frontend will fail to properly interpret the status.
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vue (1)
96-106: Potential runtime/a11y bug:@keyup.enter="focusOnModalCancelButton()"may run before the modal exists.Pressing Enter on a button often triggers click; with both handlers,
focusOnModalCancelButton()can fire beforeshouldShowEndAffiliationModalrenders the modal/refs.Proposed fix (remove key handler; focus after open in TS via watcher/nextTick)
<InputButton v-if="shouldShowEndButton" :label="$t('military.endMilitaryAffiliation')" :aria-label="$t('military.endMilitaryAffiliation')" :isTextLike="true" :shouldHideMargin="true" class="end-affiliation-button" - @keyup.enter="focusOnModalCancelButton()" @click="startEndAffiliationFlow" />webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts (1)
85-115:currentCompactType/licenseeIdcan be null; downstream calls should not assume otherwise.Right now
auditSubmit()can dispatch withcompact: nulland/orlicenseeId: null, which is likely to fail (and could fail non-obviously).
🤖 Fix all issues with AI agents
In
@webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts:
- Around line 250-275: The approve/decline modal toggles
(toggleMilitaryAuditApproveModal and toggleMilitaryAuditDeclineModal) must be
made mutually exclusive and must clear stale error state when opening; update
each method so when opening its modal it explicitly sets the other modal flag to
false, resets modalErrorMessage and isFormError (or call a clearError helper)
and then initializes inputs (initFormInputs), and when closing it should only
toggle its own flag without leaving error state behind.
- Around line 294-331: In auditSubmit, wrap the asynchronous store dispatches
('users/updateMilitaryAuditRequest' and 'license/getLicenseeRequest') in a
try/catch/finally block so endFormLoading() always runs; call startFormLoading()
before the try, perform the first dispatch then the second inside try, set
isFormSuccessful and hide shouldShowMilitaryAuditApproveModal/DeclineModal when
both succeed, set modalErrorMessage and isFormError in catch using the caught
error, and call endFormLoading() in finally to guarantee the loading state is
cleared.
In
@webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vue:
- Around line 11-55: The modal toggle is being bound twice on the InputButton
components (both @click and @keyup.enter), which can double-trigger if the
InputButton renders a native <button>; remove the redundant @keyup.enter
handlers on the two InputButton instances so only @click invokes
toggleMilitaryAuditApproveModal and toggleMilitaryAuditDeclineModal (or
alternatively ensure InputButton itself handles Enter internally and keep only a
single activation path).
In @webroot/src/network/data.api.ts:
- Around line 162-171: The JSDoc incorrectly documents the HTTP method as PATCH
for updateMilitaryAudit; update the comment above the public
updateMilitaryAudit(compact, licenseeId, data) method to state the correct HTTP
method (POST) to match licenseDataApi.updateMilitaryAudit(compact, licenseeId,
data), so the description and @return/@param remain unchanged but the summary
line should read "POST Update Military Audit Status for a licensee."
In @webroot/src/network/licenseApi/data.api.ts:
- Around line 255-266: The JSDoc for updateMilitaryAudit incorrectly says
"PATCH" while the implementation calls this.api.post(); update the JSDoc summary
to indicate a POST request (e.g., "POST Update Military Audit Status for a
licensee.") so the comment matches the actual HTTP method used by
updateMilitaryAudit and this.api.post.
🧹 Nitpick comments (8)
webroot/src/components/Icons/ClockStatus/ClockStatus.spec.ts (1)
16-17: Remove redundant assertion.Line 17 is redundant since
wrapperis already the mounted ClockStatus component. Whenwrapper.exists()is true,wrapper.findComponent(ClockStatus)will always find itself.♻️ Simplify to single assertion
expect(wrapper.exists()).to.equal(true); - expect(wrapper.findComponent(ClockStatus).exists()).to.equal(true);webroot/src/components/Icons/ClockStatus/ClockStatus.ts (1)
18-18: Remove commented-out code.This commented export should be removed to keep the codebase clean.
♻️ Remove dead code
export default toNative(ClockStatus); - -// export default ClockStatus;webroot/src/network/mocks/mock.data.api.ts (1)
228-238: LGTM! Mock implementation follows established patterns.The
updateMilitaryAuditmock method is consistent with other mock API methods in this file, using the same response structure and simulated delay pattern.💡 Optional: Consider adding mock validation
If you want to make the mock more realistic, you could add validation or data transformation to simulate actual server behavior:
public updateMilitaryAudit(compact, licenseeId, data) { + if (!compact || !licenseeId || !data) { + return wait(500).then(() => { + throw new Error('Invalid parameters'); + }); + } + const response = { message: 'success', compact, licenseeId, data, }; return wait(500).then(() => response); }However, this is entirely optional—the current implementation is sufficient for testing purposes.
webroot/src/models/Licensee/Licensee.model.ts (1)
53-54: Consider using an enum type for militaryStatus.The new fields are properly integrated, but
militaryStatusis typed asstring | null. For better type safety, consider using theMilitaryAuditStatusTypesenum (referenced in related files) instead of a plain string.💡 Optional type safety improvement
Import and use the enum type:
+import { MilitaryAuditStatusTypes } from '@/app.config'; + export interface InterfaceLicensee { ... - militaryStatus?: string | null; + militaryStatus?: MilitaryAuditStatusTypes | null; militaryStatusNote?: string | null;And in the class:
- public militaryStatus? = null; + public militaryStatus?: MilitaryAuditStatusTypes | null = null;Also applies to: 86-87
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vue (1)
115-242:TransitionGroupshould have a stable wrapper/tag and keyed children (prevents warnings + odd DOM/layout).In Vue 3,
TransitionGroupdefaults to aspanwrapper and expects keyed children to animate/move predictably. Wrapping modals with aspancan also affect layout/styling.Proposed fix
-<TransitionGroup> +<TransitionGroup tag="div" name="modal-stack"> <Modal v-if="shouldShowMilitaryAuditApproveModal" + :key="'audit-approve-modal'" modalId="audit-approve-modal" class="audit-modal audit-approve-modal" ... > ... <Modal v-if="shouldShowMilitaryAuditDeclineModal" + :key="'audit-decline-modal'" modalId="audit-decline-modal" class="audit-modal audit-decline-modal" ... > ... <Modal v-if="shouldShowEndAffiliationModal" + :key="'end-affiliation-modal'" modalId="end-affiliation-modal" class="end-affiliation-modal" ... > -</TransitionGroup> +</TransitionGroup>webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts (3)
211-219:initFormInputs()doesn’t initialize defaultformData; ensure consumers can’t run before modal init.This is mostly OK because the modal templates are gated, but focus-trap methods assume
this.formData.submitEnd.idexists. If any event fires unexpectedly beforeinitFormInputs*()ran, it will throw.
367-373: Prefer focusing after render via watcher/nextTickvs immediate DOM refs.Given the project preference to avoid rAF retry loops, a
@Watch('shouldShowEndAffiliationModal')+await this.$nextTick()focusingnoBackButtonwill be more reliable than callingfocusOnModalCancelButton()from a key handler. Based on learnings, prefer watchers over rAF.
374-383:confirmEndMilitaryAffiliation()lacks error handling (possible silent failures).If either dispatch fails, the user gets no feedback. Consider mirroring the audit modal error pattern (or a toast) so staff/licensees aren’t left guessing.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
webroot/src/app.config.tswebroot/src/components/Icons/ClockStatus/ClockStatus.lesswebroot/src/components/Icons/ClockStatus/ClockStatus.spec.tswebroot/src/components/Icons/ClockStatus/ClockStatus.tswebroot/src/components/Icons/ClockStatus/ClockStatus.vuewebroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.tswebroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.lesswebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.tswebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/locales/en.jsonwebroot/src/locales/es.jsonwebroot/src/models/Licensee/Licensee.model.spec.tswebroot/src/models/Licensee/Licensee.model.tswebroot/src/network/data.api.tswebroot/src/network/licenseApi/data.api.tswebroot/src/network/mocks/mock.data.api.tswebroot/src/network/mocks/mock.data.tswebroot/src/pages/LicensingDetail/LicensingDetail.vuewebroot/src/pages/MilitaryStatus/MilitaryStatus.vuewebroot/src/store/users/users.actions.tswebroot/src/store/users/users.mutations.tswebroot/src/store/users/users.spec.ts
🧰 Additional context used
🧠 Learnings (19)
📓 Common learnings
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/network/mocks/mock.data.ts:1919-1944
Timestamp: 2025-11-06T18:11:58.272Z
Learning: In CompactConnect, the investigation event type (`updateType: 'investigation'`) in privilege/license history is intentionally not yet fully supported in the frontend UI translation and categorization logic (webroot/src/locales/*.json and webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts). The server license history API needs to be updated first to return sufficient information about investigation events. Until then, the UI gracefully falls back to displaying "Unknown" for investigation events, which is acceptable as an interim solution.
<!--
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 922
File: webroot/src/components/UserAccount/UserAccount.ts:0-0
Timestamp: 2025-07-10T19:50:56.745Z
Learning: In the UserAccount component (webroot/src/components/UserAccount/UserAccount.ts), the email field should be disabled for staff users (`isDisabled: this.isStaff`) to maintain existing restrictions, while licensees should be able to change their email address through the new verification process. This is the intended behavior per PR #922 requirements.
📚 Learning: 2025-12-19T16:04:23.341Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1234
File: webroot/src/network/searchApi/data.api.ts:83-95
Timestamp: 2025-12-19T16:04:23.341Z
Learning: In the CompactConnect codebase, all network API files under webroot/src/network/*/data.api.ts use a specific axios.create header configuration pattern with nested get/post/put objects inside the headers config. Reviewers should not flag this pattern as an issue, since it aligns with the project’s Axios version and conventions. If enforcing general axios patterns, treat this as a project-specific exception and focus on consistency within this path.
Applied to files:
webroot/src/network/licenseApi/data.api.tswebroot/src/network/data.api.ts
📚 Learning: 2025-08-21T15:23:26.019Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 1025
File: webroot/src/models/Licensee/Licensee.model.ts:242-254
Timestamp: 2025-08-21T15:23:26.019Z
Learning: In the CompactConnect codebase, the LicenseSerializer.fromServer() method maps the server response field `dateOfIssuance` to the License model's `issueDate` property. After serialization, License instances only contain `issueDate` and never have `dateOfIssuance`. When reviewing date comparisons in reducers that work with License objects, check if they're working with serialized License instances (which use `issueDate`) rather than raw server data (which uses `dateOfIssuance`).
Applied to files:
webroot/src/models/Licensee/Licensee.model.tswebroot/src/models/Licensee/Licensee.model.spec.ts
📚 Learning: 2025-08-21T15:23:30.649Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 1025
File: webroot/src/models/Licensee/Licensee.model.ts:242-254
Timestamp: 2025-08-21T15:23:30.649Z
Learning: In the CompactConnect frontend codebase, the LicenseSerializer.fromServer() method maps server response field `dateOfIssuance` to the License model field `issueDate`. After serialization, License objects only contain `issueDate` and never `dateOfIssuance`, so reducer logic only needs to check for null `issueDate` values.
Applied to files:
webroot/src/models/Licensee/Licensee.model.ts
📚 Learning: 2025-08-13T18:24:06.298Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1010
File: webroot/src/components/MilitaryDocumentRow/MilitaryDocumentRow.vue:19-21
Timestamp: 2025-08-13T18:24:06.298Z
Learning: In MilitaryAffiliationInfoBlock, the militaryDocumentHeader() getter returns an object with wrapper functions that provide translated labels for header display, including firstDownloadLink: () => this.$t('common.downloadFile'). When used as a header item in MilitaryDocumentRow, these wrapper functions return appropriate column labels rather than actual data values.
Applied to files:
webroot/src/models/Licensee/Licensee.model.tswebroot/src/pages/LicensingDetail/LicensingDetail.vuewebroot/src/pages/MilitaryStatus/MilitaryStatus.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/network/mocks/mock.data.tswebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-09-09T19:28:53.260Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1071
File: webroot/tests/helpers/setup.ts:153-155
Timestamp: 2025-09-09T19:28:53.260Z
Learning: In the CompactConnect project, component tests are kept minimal and focus primarily on basic functionality. The main testing effort is concentrated on the data layer (models, store, etc.). For component tests, API stub methods only need to exist to prevent runtime errors - they don't need to return specific mock data since components aren't being tested for full API integration scenarios.
Applied to files:
webroot/src/components/Icons/ClockStatus/ClockStatus.spec.ts
📚 Learning: 2025-05-28T16:09:24.547Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 822
File: webroot/src/components/StateSettingsConfig/StateSettingsConfig.spec.ts:12-19
Timestamp: 2025-05-28T16:09:24.547Z
Learning: The CompactConnect app uses mount testing as their general component testing strategy, focusing more on testing the data layer (models and store) rather than comprehensive component testing. This meets their test coverage needs and is their preferred approach.
Applied to files:
webroot/src/components/Icons/ClockStatus/ClockStatus.spec.ts
📚 Learning: 2025-05-28T16:09:12.906Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 822
File: webroot/src/components/Forms/InputEmailList/InputEmailList.spec.ts:12-19
Timestamp: 2025-05-28T16:09:12.906Z
Learning: The CompactConnect application uses mount testing as the general component testing strategy. They focus their testing efforts on the data layer (models and store) rather than extensive component testing, and mount tests meet their coverage needs.
Applied to files:
webroot/src/components/Icons/ClockStatus/ClockStatus.spec.ts
📚 Learning: 2025-08-29T18:22:23.275Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1042
File: webroot/src/pages/MfaResetConfirmLicensee/MfaResetConfirmLicensee.spec.ts:12-12
Timestamp: 2025-08-29T18:22:23.275Z
Learning: In the CompactConnect frontend codebase, async describe blocks are widely used and supported in their version of Mocha. The pattern `describe('Test Suite', async () => {` is an accepted and working approach throughout their test files, so suggestions to remove the async keyword from describe blocks should not be made for this codebase.
Applied to files:
webroot/src/components/Icons/ClockStatus/ClockStatus.spec.ts
📚 Learning: 2025-08-12T22:51:48.937Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 1009
File: webroot/src/pages/LicenseeProof/LicenseeProof.less:158-165
Timestamp: 2025-08-12T22:51:48.937Z
Learning: In webroot/src/pages/LicenseeProof/LicenseeProof.vue, the .max-gap elements inside .licenses-container are intentionally hard-coded as empty elements that serve as space placeholders for tablet+ screen widths. On mobile, they are hidden completely using the :empty pseudo-class. This is an intentional design pattern where the developers have full control over keeping these elements truly empty.
Applied to files:
webroot/src/pages/LicensingDetail/LicensingDetail.vuewebroot/src/pages/MilitaryStatus/MilitaryStatus.vuewebroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.vue
📚 Learning: 2025-07-03T15:35:57.893Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 905
File: webroot/src/components/UpdateHomeJurisdiction/UpdateHomeJurisdiction.vue:32-41
Timestamp: 2025-07-03T15:35:57.893Z
Learning: In the CompactConnect frontend codebase, the team prefers to keep non-dynamic text directly in Vue templates rather than moving it to computed properties in TypeScript modules, as this approach prevents cluttering the TS files with template labels.
Applied to files:
webroot/src/pages/LicensingDetail/LicensingDetail.vuewebroot/src/pages/MilitaryStatus/MilitaryStatus.vuewebroot/src/components/Icons/ClockStatus/ClockStatus.tswebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-08-29T18:16:12.078Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1042
File: webroot/src/store/user/user.mutations.ts:269-272
Timestamp: 2025-08-29T18:16:12.078Z
Learning: The CONFIRM_MFA_LICENSEE_ACCOUNT_REQUEST mutation intentionally sets isLoadingAccount = false (rather than true like other REQUEST mutations) because the MfaResetConfirmLicensee page handles its own loading UI with a custom LoadingSpinner component and isLoading state, rather than triggering the global app loading state.
Applied to files:
webroot/src/store/users/users.mutations.ts
📚 Learning: 2025-11-06T18:11:58.272Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/network/mocks/mock.data.ts:1919-1944
Timestamp: 2025-11-06T18:11:58.272Z
Learning: In CompactConnect, the investigation event type (`updateType: 'investigation'`) in privilege/license history is intentionally not yet fully supported in the frontend UI translation and categorization logic (webroot/src/locales/*.json and webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts). The server license history API needs to be updated first to return sufficient information about investigation events. Until then, the UI gracefully falls back to displaying "Unknown" for investigation events, which is acceptable as an interim solution.
<!--
Applied to files:
webroot/src/locales/en.json
📚 Learning: 2025-07-10T19:50:56.745Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 922
File: webroot/src/components/UserAccount/UserAccount.ts:0-0
Timestamp: 2025-07-10T19:50:56.745Z
Learning: In the UserAccount component (webroot/src/components/UserAccount/UserAccount.ts), the email field should be disabled for staff users (`isDisabled: this.isStaff`) to maintain existing restrictions, while licensees should be able to change their email address through the new verification process. This is the intended behavior per PR #922 requirements.
Applied to files:
webroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.vuewebroot/src/models/Licensee/Licensee.model.spec.tswebroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.tswebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-06-24T00:02:39.944Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 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/Licensee/LicenseeSearch/LicenseeSearch.vue
📚 Learning: 2025-08-21T16:36:48.723Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 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/Licensee/LicenseeSearch/LicenseeSearch.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-11-12T21:06:06.981Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/components/LicenseCard/LicenseCard.ts:388-396
Timestamp: 2025-11-12T21:06:06.981Z
Learning: In the CompactConnect investigation creation flow, the backend APIs for creating investigations (both `createLicenseInvestigation` and `createPrivilegeInvestigation` in webroot/src/network/licenseApi/data.api.ts) do not accept a `startDate` parameter. The backend automatically sets the investigation creation date to the timestamp when the API request is received, so the frontend UI does not need to capture or submit an investigation start date during the creation workflow.
Applied to files:
webroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.ts
📚 Learning: 2025-08-29T17:54:57.683Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1042
File: webroot/src/locales/es.json:958-958
Timestamp: 2025-08-29T17:54:57.683Z
Learning: Spanish translations in webroot/src/locales/es.json are currently generated using Google Translate and are considered nice-to-have. The team plans to use professional translation services if formal Spanish support is needed in the future. Don't spend time on nuanced translation quality issues for Spanish locale files.
Applied to files:
webroot/src/locales/es.json
📚 Learning: 2025-06-09T19:57:51.519Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 851
File: webroot/src/pages/RegisterLicensee/RegisterLicensee.ts:0-0
Timestamp: 2025-06-09T19:57:51.519Z
Learning: In the RegisterLicensee component, when handling DOM element availability issues, the developer prefers using Vue Watchers over retry mechanisms with requestAnimationFrame to avoid infinite recursion risks and maintain Vue's reactive patterns.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
🧬 Code graph analysis (7)
webroot/src/models/Licensee/Licensee.model.ts (1)
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts (1)
auditStatusTypes(160-162)
webroot/src/components/Icons/ClockStatus/ClockStatus.spec.ts (1)
webroot/tests/helpers/setup.ts (1)
mountShallow(265-265)
webroot/src/network/data.api.ts (8)
webroot/src/pages/LicensingDetail/LicensingDetail.ts (2)
compact(63-67)licenseeId(92-94)webroot/src/pages/PublicLicensingDetail/PublicLicensingDetail.ts (2)
compact(51-55)licenseeId(57-59)webroot/src/pages/PublicPrivilegeDetail/PublicPrivilegeDetail.ts (2)
compact(60-62)licenseeId(56-58)webroot/src/pages/PrivilegeDetail/PrivilegeDetail.ts (2)
compact(83-87)licenseeId(75-77)webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts (1)
licenseeId(113-115)webroot/src/components/PrivilegeCard/PrivilegeCard.ts (1)
licenseeId(135-137)webroot/src/components/LicenseCard/LicenseCard.ts (1)
licenseeId(147-149)webroot/src/network/licenseApi/data.api.ts (1)
licenseDataApi(654-654)
webroot/src/store/users/users.mutations.ts (1)
webroot/src/store/users/users.state.ts (1)
state(17-24)
webroot/src/store/users/users.actions.ts (2)
webroot/src/network/data.api.ts (1)
dataApi(723-723)webroot/src/network/mocks/mock.data.api.ts (1)
dataApi(848-848)
webroot/src/store/users/users.spec.ts (1)
webroot/src/store/users/users.state.ts (1)
state(17-24)
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts (5)
webroot/src/pages/LicenseeDashboard/LicenseeDashboard.ts (2)
Component(26-192)currentCompactType(67-69)webroot/src/components/PrivilegePurchaseInformationConfirmation/PrivilegePurchaseInformationConfirmation.ts (2)
Component(35-292)currentCompactType(71-73)webroot/src/pages/MilitaryStatus/MilitaryStatus.ts (2)
Component(17-72)currentCompactType(53-55)webroot/src/components/PrivilegePurchaseSelect/PrivilegePurchaseSelect.ts (2)
Component(37-465)currentCompactType(100-102)webroot/src/components/PrivilegePurchaseAttestation/PrivilegePurchaseAttestation.ts (2)
Component(43-338)currentCompactType(114-116)
⏰ 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 (19)
webroot/src/components/Icons/ClockStatus/ClockStatus.less (1)
1-6: LGTM - Empty stylesheet is acceptable.The file contains only header metadata with no styling rules, which is fine for a new icon component that relies on inherited styles.
webroot/src/components/Icons/ClockStatus/ClockStatus.spec.ts (1)
8-14: LGTM - Test setup follows team patterns.The imports and test structure align with the team's minimal component testing approach.
webroot/src/components/Icons/ClockStatus/ClockStatus.vue (1)
1-61: LGTM - Well-structured icon component.The SVG icon component follows standard patterns with proper template structure and external script/style references.
webroot/src/components/Icons/ClockStatus/ClockStatus.ts (1)
8-16: LGTM - Component structure is correct.The component follows the standard vue-facing-decorator pattern with proper imports, decorator, and toNative export.
webroot/src/network/mocks/mock.data.ts (3)
461-469: LGTM! Well-structured military affiliation and audit status data.The
downloadLinksstructure properly maintains consistency with thefileNamesarray, and the audit status fields are well-formed. The 'approved' status with a descriptive note provides good test coverage for the approval workflow.
600-608: Verify the combination of 'initializing' affiliation status with 'declined' audit status.The military affiliation has
status: 'initializing'(line 604) while the overallmilitaryStatusis set to'declined'(line 607). While the note suggests documents couldn't be viewed, please confirm this combination is intentional test data for a specific workflow scenario—for example, testing when document processing is incomplete but an audit decision has already been made.
1028-1029: LGTM! Consistent handling of providers without military affiliations.The
'notApplicable'status with empty notes is appropriate for providers with no military affiliations, providing complete test coverage across different user scenarios.Also applies to: 1105-1106, 1182-1183
webroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.vue (1)
97-102: LGTM! Military status search field successfully enabled.The uncommented search input is properly structured and consistent with other search form fields. The binding to
formData.militaryStatusaligns with the backend implementation and completes the military audit status workflow.webroot/src/pages/LicensingDetail/LicensingDetail.vue (1)
130-130: LGTM! Clean formatting improvement.The self-closing tag format is more concise while maintaining the same functionality and prop binding.
webroot/src/components/Licensee/LicenseeSearch/LicenseeSearch.ts (1)
386-386: LGTM! Mock population correctly enables military status testing.The uncommented line properly sets the military status to 'approved' during mock data population, enabling development testing of the new military audit workflow.
webroot/src/pages/MilitaryStatus/MilitaryStatus.vue (1)
33-33: LGTM! Prop removal aligns with component refactoring.The removal of the
currentCompactTypeprop is consistent with the refactoring ofMilitaryAffiliationInfoBlock, which now computes the compact type internally. This simplifies the component API and reduces prop drilling.webroot/src/store/users/users.mutations.ts (1)
31-33: LGTM! Military audit mutations follow established patterns.The new mutation types and handlers are consistent with existing code structure and properly manage loading and error states.
Also applies to: 151-162
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.less (1)
18-26: LGTM! Responsive layout and audit UI styling well-structured.The changes enhance responsiveness by defaulting to column layout on mobile and add comprehensive styling for the new audit status display and modal workflows. The structure follows existing design patterns in the codebase.
Also applies to: 38-97, 158-267
webroot/src/models/Licensee/Licensee.model.ts (2)
221-227: LGTM! Display name helper follows existing patterns.The
militaryAuditStatusName()method correctly uses i18n to map status keys to display names, consistent with similar helpers likelicenseTypeName().
437-438: LGTM! Serialization properly maps new fields.The new fields are correctly mapped from the server response in the serializer.
webroot/src/store/users/users.actions.ts (1)
127-144: LGTM! Military audit actions follow established patterns.The new actions are structured consistently with existing actions in this file, properly handling the request/success/failure flow and maintaining the same return/throw pattern for promise handling.
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts (2)
233-248: No action required.this.joiMessages.stringis properly compatible with Joi v17 and follows the correct message format. The error codes ('string.empty', 'string.min', 'string.max', etc.) are standard Joi v17 codes, and the structure will work correctly with the.messages()method.Likely an incorrect or invalid review comment.
385-401: These callbacks won't be invoked—excludeSortingandexcludeTopPaginationare bothtrue.Since ListContainer won't call these methods, the signature mismatch (expected parameters, returning
anyvs. returningbooleanwith no parameters) is moot. The code is fine as-is.webroot/src/locales/es.json (1)
923-954: The military status key renaming is complete and consistent across the codebase.Both locale files correctly use
tentativeanddeclinedkeys inmilitaryStatusOptions. The server-sideLicensee.model.tsproperly mapsmilitaryStatusvalues against these translated keys, and the Vue component correctly references the audit status throughisAuditStatusPending,isAuditStatusDeclined, andisAuditStatusApproved. The app config defines the matching enum values (TENTATIVEandDECLINED). No cross-codebase inconsistencies exist for military status handling.
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
Show resolved
Hide resolved
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vue
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
@webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts:
- Around line 211-219: The modal control is not mutually exclusive and doesn’t
keep setModalIsOpen in sync; update initFormInputs and each modal-toggle path
(e.g., initFormInputsAuditApprove, initFormInputsAuditDecline,
initFormInputsEndAffiliation and the corresponding toggle handlers) to
explicitly close the other modals and call setModalIsOpen(true) when opening and
setModalIsOpen(false) when closing, and ensure each toggle sets the other
shouldShow... flags to false so formData isn’t rendered by multiple modals
simultaneously; apply the same change pattern to the other modal branches
mentioned (the blocks around lines 250-279, 351-356, 388-391) so all modal
open/close flows remain mutually exclusive and keep setModalIsOpen in sync.
- Around line 298-337: In auditSubmit, guard against null/undefined
currentCompactType or licenseeId before calling
this.$store.dispatch('users/updateMilitaryAuditRequest'); check the
currentCompactType and licenseeId (from this.currentCompactType and
this.licenseeId) after validating the form and before building the payload, and
if either is missing set this.modalErrorMessage to a clear message (or
this.$t('common.error')), set this.isFormError = true, stop loading (call
this.endFormLoading() if started) and return early so no dispatch occurs; keep
the subsequent success handling intact for the valid-path.
🧹 Nitpick comments (3)
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vue (1)
19-55: Audit icon rendering should be mutually exclusive (avoid mixedv-if/v-else-if).
Today it works because the predicates are exclusive, but making the icon chain explicitly exclusive prevents accidental double-rendering if logic changes.Proposed tweak
- <CheckIcon v-if="isAuditStatusApproved" class="icon approved" /> - <AlertIcon v-else-if="isAuditStatusDeclined" class="icon declined" /> - <ClockIcon v-if="isAuditStatusPending" class="icon pending" /> + <CheckIcon v-if="isAuditStatusApproved" class="icon approved" /> + <AlertIcon v-else-if="isAuditStatusDeclined" class="icon declined" /> + <ClockIcon v-else-if="isAuditStatusPending" class="icon pending" />webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts (2)
85-87: Getter-basedcurrentCompactTypeis a nice cleanup over prop threading.
382-391: End-affiliation close flow is OK, but consider error handling for the async dispatches.
If either dispatch fails, the UI will silently “succeed” (modal already closed). Consider surfacing a toast/banner if that’s the app convention.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.tswebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/network/licenseApi/data.api.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- webroot/src/network/licenseApi/data.api.ts
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/network/mocks/mock.data.ts:1919-1944
Timestamp: 2025-11-06T18:11:58.272Z
Learning: In CompactConnect, the investigation event type (`updateType: 'investigation'`) in privilege/license history is intentionally not yet fully supported in the frontend UI translation and categorization logic (webroot/src/locales/*.json and webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts). The server license history API needs to be updated first to return sufficient information about investigation events. Until then, the UI gracefully falls back to displaying "Unknown" for investigation events, which is acceptable as an interim solution.
<!--
📚 Learning: 2025-08-13T18:24:06.298Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1010
File: webroot/src/components/MilitaryDocumentRow/MilitaryDocumentRow.vue:19-21
Timestamp: 2025-08-13T18:24:06.298Z
Learning: In MilitaryAffiliationInfoBlock, the militaryDocumentHeader() getter returns an object with wrapper functions that provide translated labels for header display, including firstDownloadLink: () => this.$t('common.downloadFile'). When used as a header item in MilitaryDocumentRow, these wrapper functions return appropriate column labels rather than actual data values.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-08-21T16:36:48.723Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 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/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-06-24T00:02:39.944Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 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/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-09-08T20:19:46.094Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 1041
File: webroot/src/components/StateSettingsList/StateSettingsList.vue:71-90
Timestamp: 2025-09-08T20:19:46.094Z
Learning: In the CompactConnect codebase, modal button IDs should remain hard-coded (e.g., "confirm-modal-submit-button", "confirm-modal-cancel-button") rather than being dynamically generated from modalId, because modals are mutually exclusive and only one will exist in the DOM at any given time.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-06-24T00:07:10.463Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 873
File: webroot/src/components/LicenseCard/LicenseCard.ts:509-528
Timestamp: 2025-06-24T00:07:10.463Z
Learning: In the CompactConnect frontend codebase, the project prefers to avoid early returns in frontend code when possible, as mentioned by jsandoval81 in webroot/src/components/LicenseCard/LicenseCard.ts.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-07-03T15:35:57.893Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 905
File: webroot/src/components/UpdateHomeJurisdiction/UpdateHomeJurisdiction.vue:32-41
Timestamp: 2025-07-03T15:35:57.893Z
Learning: In the CompactConnect frontend codebase, the team prefers to keep non-dynamic text directly in Vue templates rather than moving it to computed properties in TypeScript modules, as this approach prevents cluttering the TS files with template labels.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-06-09T19:57:51.519Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 851
File: webroot/src/pages/RegisterLicensee/RegisterLicensee.ts:0-0
Timestamp: 2025-06-09T19:57:51.519Z
Learning: In the RegisterLicensee component, when handling DOM element availability issues, the developer prefers using Vue Watchers over retry mechanisms with requestAnimationFrame to avoid infinite recursion risks and maintain Vue's reactive patterns.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-07-10T19:50:56.745Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 922
File: webroot/src/components/UserAccount/UserAccount.ts:0-0
Timestamp: 2025-07-10T19:50:56.745Z
Learning: In the UserAccount component (webroot/src/components/UserAccount/UserAccount.ts), the email field should be disabled for staff users (`isDisabled: this.isStaff`) to maintain existing restrictions, while licensees should be able to change their email address through the new verification process. This is the intended behavior per PR #922 requirements.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
⏰ 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 (5)
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vue (1)
11-18: Status/affiliation rendering updates look safe.
Translation key swap + usingaffiliationTypecomputed reads cleanly.webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts (4)
14-45: Icon + form deps integration looks coherent.
Imports/registration for new icons +InputTextarea/Joimatch the new decline-notes UX.
401-408:editInfo()routing looks fine (compact-param guarded).
113-187: No issue found—audit status wiring is correct.The field
militaryStatuson the Licensee model is indeed the audit status field, despite its name. ThemilitaryAuditStatusName()method in the Licensee model confirms this by looking up translations based onthis.militaryStatus. The component correctly reads audit status fromthis.licensee?.militaryStatusand compares it againstMilitaryAuditStatusTypes.*enum values.Likely an incorrect or invalid review comment.
257-296: This review concern is not applicable—InputSubmit already binds the id attribute.InputSubmit.vue (line 32) explicitly sets
:id="formInput.id"on the rendered input element, sodocument.getElementById(this.formData.submitEnd.id)will correctly resolve the submit button. The focus trap implementation using optional chaining (?.focus()) provides adequate null safety for DOM access, consistent with accepted patterns in the codebase.Likely an incorrect or invalid review comment.
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
Show resolved
Hide resolved
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
Show resolved
Hide resolved
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vue
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vue (2)
11-57: Make audit-status icon rendering mutually exclusive (avoid mixedv-if/v-else-if).
Right nowClockIcon(Line 28) usesv-ifwhile the others usev-if/v-else-if; it’s clearer/safer to keep these exclusive even if the computed flags “should” be exclusive.Proposed tweak
- <CheckIcon v-if="isAuditStatusApproved" class="icon approved" /> - <AlertIcon v-else-if="isAuditStatusDeclined" class="icon declined" /> - <ClockIcon v-if="isAuditStatusPending" class="icon pending" /> + <CheckIcon v-if="isAuditStatusApproved" class="icon approved" /> + <AlertIcon v-else-if="isAuditStatusDeclined" class="icon declined" /> + <ClockIcon v-else-if="isAuditStatusPending" class="icon pending" />
117-247: Prefer explicitTransitionGrouptag + verify audit modals close behavior parity.
- Consider
tag="div"onTransitionGroup(Line 117) to avoid an inline wrapper impacting layout/stacking in edge cases.- End-affiliation modal listens for
@close-modal(Line 220) but audit modals don’t; please confirm audit modals still close correctly on background click / internal close events (if supported byModal).webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts (3)
184-187: Add a defensivecurrentCompactType/licenseeIdguard insideauditSubmit(don’t rely on disabled buttons).
isAuditReadyexists (Line 184), butauditSubmitwill still dispatch with null-ish values if triggered indirectly.Also applies to: 302-341
254-283: Make audit modals mutually exclusive when opening (prevent double-open states).
toggleMilitaryAuditApproveModalandtoggleMilitaryAuditDeclineModalonly flip their own boolean; consider explicitly closing the other modal when opening one.
346-395: Modal open-state signaling looks inconsistent (setModalIsOpen(false)on close only).
closeEndAffiliationModaldispatchessetModalIsOpen(false)(Line 394), but opening paths (end-affiliation + both audit modals) don’t dispatch the “open” signal. If the app relies on this for scroll lock / global key handling, consider setting it on open as well (and for audit modals too).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.tswebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vue
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/network/mocks/mock.data.ts:1919-1944
Timestamp: 2025-11-06T18:11:58.272Z
Learning: In CompactConnect, the investigation event type (`updateType: 'investigation'`) in privilege/license history is intentionally not yet fully supported in the frontend UI translation and categorization logic (webroot/src/locales/*.json and webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts). The server license history API needs to be updated first to return sufficient information about investigation events. Until then, the UI gracefully falls back to displaying "Unknown" for investigation events, which is acceptable as an interim solution.
<!--
📚 Learning: 2025-08-13T18:24:06.298Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1010
File: webroot/src/components/MilitaryDocumentRow/MilitaryDocumentRow.vue:19-21
Timestamp: 2025-08-13T18:24:06.298Z
Learning: In MilitaryAffiliationInfoBlock, the militaryDocumentHeader() getter returns an object with wrapper functions that provide translated labels for header display, including firstDownloadLink: () => this.$t('common.downloadFile'). When used as a header item in MilitaryDocumentRow, these wrapper functions return appropriate column labels rather than actual data values.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-08-21T16:36:48.723Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 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/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-06-24T00:02:39.944Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 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/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-09-08T20:19:46.094Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 1041
File: webroot/src/components/StateSettingsList/StateSettingsList.vue:71-90
Timestamp: 2025-09-08T20:19:46.094Z
Learning: In the CompactConnect codebase, modal button IDs should remain hard-coded (e.g., "confirm-modal-submit-button", "confirm-modal-cancel-button") rather than being dynamically generated from modalId, because modals are mutually exclusive and only one will exist in the DOM at any given time.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-11-06T18:11:58.272Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/network/mocks/mock.data.ts:1919-1944
Timestamp: 2025-11-06T18:11:58.272Z
Learning: In CompactConnect, the investigation event type (`updateType: 'investigation'`) in privilege/license history is intentionally not yet fully supported in the frontend UI translation and categorization logic (webroot/src/locales/*.json and webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts). The server license history API needs to be updated first to return sufficient information about investigation events. Until then, the UI gracefully falls back to displaying "Unknown" for investigation events, which is acceptable as an interim solution.
<!--
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-06-24T00:17:31.188Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 873
File: webroot/src/styles.common/_inputs.less:115-118
Timestamp: 2025-06-24T00:17:31.188Z
Learning: The team intentionally uses broad CSS selectors like `.dp__input_wrap div { position: static; }` to fix styling issues with the Vue Date-Picker library. They have experience with these styles working well, keep the library version pinned in yarn.lock, and have established processes to re-test everything when updating the library version. This approach is acceptable given their testing discipline and version control.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-07-10T19:50:56.745Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 922
File: webroot/src/components/UserAccount/UserAccount.ts:0-0
Timestamp: 2025-07-10T19:50:56.745Z
Learning: In the UserAccount component (webroot/src/components/UserAccount/UserAccount.ts), the email field should be disabled for staff users (`isDisabled: this.isStaff`) to maintain existing restrictions, while licensees should be able to change their email address through the new verification process. This is the intended behavior per PR #922 requirements.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-07-10T19:52:40.366Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 922
File: webroot/src/components/UserAccount/UserAccount.ts:250-267
Timestamp: 2025-07-10T19:52:40.366Z
Learning: In focus trap implementations for modals, when there's only one focusable element (like in a success state), it's correct for both firstTabIndex and lastTabIndex to reference the same element. This keeps focus appropriately trapped on that single element. Optional chaining operators (?.focus()) provide adequate null safety for DOM element access in focus management code.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-06-24T00:07:10.463Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 873
File: webroot/src/components/LicenseCard/LicenseCard.ts:509-528
Timestamp: 2025-06-24T00:07:10.463Z
Learning: In the CompactConnect frontend codebase, the project prefers to avoid early returns in frontend code when possible, as mentioned by jsandoval81 in webroot/src/components/LicenseCard/LicenseCard.ts.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-06-17T19:05:36.255Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 848
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/registration.py:111-117
Timestamp: 2025-06-17T19:05:36.255Z
Learning: In CompactConnect PR #848, the user landonshumway-ia decided to leave timezone handling code in _should_allow_reregistration function as-is after testing in sandbox environment confirmed it works correctly. The user's reasoning was that reregistration is an edge case, the code has been tested and verified, and AWS is unlikely to change behavior that would break many clients. This represents a pragmatic engineering decision based on testing and risk assessment.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-07-03T15:35:57.893Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 905
File: webroot/src/components/UpdateHomeJurisdiction/UpdateHomeJurisdiction.vue:32-41
Timestamp: 2025-07-03T15:35:57.893Z
Learning: In the CompactConnect frontend codebase, the team prefers to keep non-dynamic text directly in Vue templates rather than moving it to computed properties in TypeScript modules, as this approach prevents cluttering the TS files with template labels.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-06-09T19:57:51.519Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 851
File: webroot/src/pages/RegisterLicensee/RegisterLicensee.ts:0-0
Timestamp: 2025-06-09T19:57:51.519Z
Learning: In the RegisterLicensee component, when handling DOM element availability issues, the developer prefers using Vue Watchers over retry mechanisms with requestAnimationFrame to avoid infinite recursion risks and maintain Vue's reactive patterns.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
🧬 Code graph analysis (1)
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts (4)
webroot/src/pages/MilitaryStatus/MilitaryStatus.ts (2)
Component(17-72)currentCompactType(53-55)webroot/src/components/LicenseCard/LicenseCard.ts (1)
currentCompactType(118-120)webroot/src/components/PrivilegeCard/PrivilegeCard.ts (1)
currentCompactType(102-104)webroot/src/components/PrivilegePurchaseLicense/PrivilegePurchaseLicense.ts (1)
currentCompactType(79-81)
⏰ 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 (2)
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts (2)
160-190: No changes needed—the code correctly usesmilitaryStatusfield for audit status.The Licensee model defines only
militaryStatusandmilitaryStatusNotefields. There are no separatemilitaryAuditStatusormilitaryAuditStatusNotefields. TheauditStatusgetter,auditStatusName()method call,auditStatusNotegetter, and theauditSubmitpayload all correctly reference these same fields. ThemilitaryAuditStatusName()method implementation internally reads frommilitaryStatus, which holds audit status values (approved, declined, notApplicable, etc.), so the naming is consistent with the implementation.
261-300: No action needed—focus trap IDs are correctly forwarded to the DOM.InputSubmit (line 32) and InputTextarea (line 25) both properly bind
formInput.idto the DOMidattribute. All referenced IDs (submit-approve,decline-notes,submit-decline,submit-end,audit-approve-cancel-button,no-back-button) exist in the template and are accessible viadocument.getElementById(). The focus trap implementations will work as intended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts:
- Around line 285-300: The focus trap in focusTrapAuditDecline currently loops
between elements referenced by formData.auditDeclineNotes.id and
formData.submitEnd.id but skips the cancel button (audit-decline-cancel-button)
which sits between them in DOM order; update the function to include the cancel
button in the trap (for example, set lastTabIndex to
document.getElementById('audit-decline-cancel-button') or build an ordered list
of tabbable elements [formData.auditDeclineNotes.id,
'audit-decline-cancel-button', formData.submitEnd.id] and cycle focus between
the first and last items so Shift+Tab from the first goes to the cancel button
and Tab from the last returns to the first.
🧹 Nitpick comments (2)
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vue (1)
26-28: Consider usingv-elsefor the ClockIcon.Since the container already guards for all three states and lines 26-27 use
v-if/v-else-if, line 28 should usev-elseinstead of a standalonev-ifto complete the conditional chain. This makes the mutual exclusivity explicit and slightly more efficient.Suggested change
<CheckIcon v-if="isAuditStatusApproved" class="icon approved" /> <AlertIcon v-else-if="isAuditStatusDeclined" class="icon declined" /> - <ClockIcon v-if="isAuditStatusPending" class="icon pending" /> + <ClockIcon v-else class="icon pending" />webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts (1)
386-390: Consider adding error handling for the end affiliation flow.Unlike
auditSubmit, which properly catches errors and displays them in the modal,confirmEndMilitaryAffiliationcloses the modal first and then fires async actions without error handling. If either dispatch fails, the user won't see feedback.Suggested improvement
async confirmEndMilitaryAffiliation(): Promise<void> { - this.closeEndAffiliationModal(); - await this.$store.dispatch('user/endMilitaryAffiliationRequest'); - await this.$store.dispatch('user/getLicenseeAccountRequest'); + this.startFormLoading(); + this.modalErrorMessage = ''; + + await this.$store.dispatch('user/endMilitaryAffiliationRequest') + .catch((err) => { + this.modalErrorMessage = err?.message || this.$t('common.error'); + this.isFormError = true; + }); + + if (!this.isFormError) { + this.closeEndAffiliationModal(); + await this.$store.dispatch('user/getLicenseeAccountRequest').catch(() => { + // Continue + }); + } + + this.endFormLoading(); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.tswebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vue
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/network/mocks/mock.data.ts:1919-1944
Timestamp: 2025-11-06T18:11:58.272Z
Learning: In CompactConnect, the investigation event type (`updateType: 'investigation'`) in privilege/license history is intentionally not yet fully supported in the frontend UI translation and categorization logic (webroot/src/locales/*.json and webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts). The server license history API needs to be updated first to return sufficient information about investigation events. Until then, the UI gracefully falls back to displaying "Unknown" for investigation events, which is acceptable as an interim solution.
<!--
📚 Learning: 2025-08-13T18:24:06.298Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1010
File: webroot/src/components/MilitaryDocumentRow/MilitaryDocumentRow.vue:19-21
Timestamp: 2025-08-13T18:24:06.298Z
Learning: In MilitaryAffiliationInfoBlock, the militaryDocumentHeader() getter returns an object with wrapper functions that provide translated labels for header display, including firstDownloadLink: () => this.$t('common.downloadFile'). When used as a header item in MilitaryDocumentRow, these wrapper functions return appropriate column labels rather than actual data values.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-08-21T16:36:48.723Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 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/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-06-24T00:02:39.944Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 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/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-09-08T20:19:46.094Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 1041
File: webroot/src/components/StateSettingsList/StateSettingsList.vue:71-90
Timestamp: 2025-09-08T20:19:46.094Z
Learning: In the CompactConnect codebase, modal button IDs should remain hard-coded (e.g., "confirm-modal-submit-button", "confirm-modal-cancel-button") rather than being dynamically generated from modalId, because modals are mutually exclusive and only one will exist in the DOM at any given time.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-11-06T18:11:58.272Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/network/mocks/mock.data.ts:1919-1944
Timestamp: 2025-11-06T18:11:58.272Z
Learning: In CompactConnect, the investigation event type (`updateType: 'investigation'`) in privilege/license history is intentionally not yet fully supported in the frontend UI translation and categorization logic (webroot/src/locales/*.json and webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts). The server license history API needs to be updated first to return sufficient information about investigation events. Until then, the UI gracefully falls back to displaying "Unknown" for investigation events, which is acceptable as an interim solution.
<!--
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-06-24T00:17:31.188Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 873
File: webroot/src/styles.common/_inputs.less:115-118
Timestamp: 2025-06-24T00:17:31.188Z
Learning: The team intentionally uses broad CSS selectors like `.dp__input_wrap div { position: static; }` to fix styling issues with the Vue Date-Picker library. They have experience with these styles working well, keep the library version pinned in yarn.lock, and have established processes to re-test everything when updating the library version. This approach is acceptable given their testing discipline and version control.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vuewebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-07-10T19:50:56.745Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 922
File: webroot/src/components/UserAccount/UserAccount.ts:0-0
Timestamp: 2025-07-10T19:50:56.745Z
Learning: In the UserAccount component (webroot/src/components/UserAccount/UserAccount.ts), the email field should be disabled for staff users (`isDisabled: this.isStaff`) to maintain existing restrictions, while licensees should be able to change their email address through the new verification process. This is the intended behavior per PR #922 requirements.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-07-10T19:52:40.366Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 922
File: webroot/src/components/UserAccount/UserAccount.ts:250-267
Timestamp: 2025-07-10T19:52:40.366Z
Learning: In focus trap implementations for modals, when there's only one focusable element (like in a success state), it's correct for both firstTabIndex and lastTabIndex to reference the same element. This keeps focus appropriately trapped on that single element. Optional chaining operators (?.focus()) provide adequate null safety for DOM element access in focus management code.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-06-24T00:07:10.463Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 873
File: webroot/src/components/LicenseCard/LicenseCard.ts:509-528
Timestamp: 2025-06-24T00:07:10.463Z
Learning: In the CompactConnect frontend codebase, the project prefers to avoid early returns in frontend code when possible, as mentioned by jsandoval81 in webroot/src/components/LicenseCard/LicenseCard.ts.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-06-17T19:05:36.255Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 848
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/registration.py:111-117
Timestamp: 2025-06-17T19:05:36.255Z
Learning: In CompactConnect PR #848, the user landonshumway-ia decided to leave timezone handling code in _should_allow_reregistration function as-is after testing in sandbox environment confirmed it works correctly. The user's reasoning was that reregistration is an edge case, the code has been tested and verified, and AWS is unlikely to change behavior that would break many clients. This represents a pragmatic engineering decision based on testing and risk assessment.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-07-08T18:40:24.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 882
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/compact_configuration_client.py:287-359
Timestamp: 2025-07-08T18:40:24.408Z
Learning: In the CompactConnect codebase, landonshumway-ia prefers to avoid extraneous unit tests when existing test coverage is already sufficient to catch bugs. For the get_privilege_purchase_options method's live-jurisdiction filtering logic, the existing tests in the purchases test suite provide adequate coverage without needing additional edge case tests.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-10-15T19:56:58.050Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1135
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py:60-0
Timestamp: 2025-10-15T19:56:58.050Z
Learning: In the CompactConnect codebase, when migrating from a single field to a list field (e.g., clinicalPrivilegeActionCategory to clinicalPrivilegeActionCategories), both fields are intentionally kept as optional during the migration period to support backwards compatibility. Mutual exclusivity validation is not enforced during this phase, as the deprecated field will be removed in a follow-up PR with migration scripts.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-08-25T19:04:48.800Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1014
File: backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_account_recovery.py:288-293
Timestamp: 2025-08-25T19:04:48.800Z
Learning: In the CompactConnect project, landonshumway-ia has already provided feedback on patch cleanup patterns in test setup methods for the account recovery tests, so similar suggestions about stopping patches in setUp/tearDown should not be repeated.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-07-03T15:35:57.893Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 905
File: webroot/src/components/UpdateHomeJurisdiction/UpdateHomeJurisdiction.vue:32-41
Timestamp: 2025-07-03T15:35:57.893Z
Learning: In the CompactConnect frontend codebase, the team prefers to keep non-dynamic text directly in Vue templates rather than moving it to computed properties in TypeScript modules, as this approach prevents cluttering the TS files with template labels.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
📚 Learning: 2025-06-09T19:57:51.519Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 851
File: webroot/src/pages/RegisterLicensee/RegisterLicensee.ts:0-0
Timestamp: 2025-06-09T19:57:51.519Z
Learning: In the RegisterLicensee component, when handling DOM element availability issues, the developer prefers using Vue Watchers over retry mechanisms with requestAnimationFrame to avoid infinite recursion risks and maintain Vue's reactive patterns.
Applied to files:
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
⏰ 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 (13)
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.vue (5)
11-18: LGTM!The status and affiliation type chunks are properly structured with appropriate i18n keys and conditional styling.
35-54: LGTM!The audit buttons are properly guarded by
isCompactAdmin && shouldShowAuditButtons, have appropriate accessibility attributes, and follow established event handling patterns.
115-157: LGTM!The audit approve modal is well-structured with proper focus trapping, error handling display, and form submission. The modal ID is appropriately hard-coded per project conventions.
158-207: LGTM!The audit decline modal properly includes a textarea for decline notes, maintains consistent error handling, and uses the
:isWarning="true"prop appropriately for the decline action's visual emphasis.
208-245: LGTM!The end-affiliation modal is correctly moved into the TransitionGroup while preserving its existing functionality including
closeOnBackgroundClickand the@close-modalhandler.webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts (8)
14-31: LGTM!All new imports are utilized: Vue reactivity for form data, Joi for validation, icons for status display, and InputTextarea for decline notes.
33-45: LGTM!Component registration correctly includes all newly imported components.
137-158: LGTM!The
affiliationsgetter provides clean access to military affiliations, andaffiliationTypeproperly handles the empty state before licensee data is available.
160-190: LGTM!The audit status getters provide clean abstractions. The
shouldShowAuditButtonscorrectly requires all necessary data (compact type, licensee ID, and affiliations) before enabling the audit workflow.
215-252: LGTM!Form initialization is cleanly separated per modal type. The Joi validation on decline notes with
max(256)andenforceMax: trueprovides proper input constraints.
302-341: LGTM!The
auditSubmitmethod follows a clean pattern: validate, start loading, dispatch action, handle errors, refresh data on success, and end loading. The silent catch ongetLicenseeRequestis acceptable since the primary audit operation succeeded.
405-412: LGTM!The
editInfomethod properly guards against nullcurrentCompactTypebefore navigation, and the route parameters are correctly structured.
85-87: Removal ofcurrentCompactTypeprop is safe—no parent components pass this as a prop.The change from prop to computed getter is verified safe. Parent components (MilitaryStatus.vue, LicensingDetail.vue) that use MilitaryAffiliationInfoBlock do not pass currentCompactType as a prop. The getter correctly derives the value from currentCompact?.type, eliminating unnecessary prop duplication.
webroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.ts
Show resolved
Hide resolved
|
@jlkravitz This PR is ready for review. |
jlkravitz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one small nit!
@jsandoval81 I'm trying to test locally with the mock API, is that possible? I can't seem to find a user where the audit buttons show up.
| > | ||
| <CheckIcon v-if="isAuditStatusApproved" class="icon approved" /> | ||
| <AlertIcon v-else-if="isAuditStatusDeclined" class="icon declined" /> | ||
| <ClockIcon v-if="isAuditStatusPending" class="icon pending" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v-else-if?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Fixed!
|
@jlkravitz This is ready for re-review. For mock data users where the new audit buttons are displayed, if you use the staff login (which has compact admin for mock) you should be able to use:
You won't expect see the buttons if you login as a practitioner. |

Requirements List
Description List
Testing List
yarn test:unit:allshould run without errors or warningsyarn serveshould run without errors or warningsyarn buildshould run without errors or warningsCloses #1171
Summary by CodeRabbit
New Features
Localization
Tests
✏️ Tip: You can customize this high-level summary in your review settings.