Frontend/update mock data with privilege history#1115
Frontend/update mock data with privilege history#1115isabeleliassen merged 12 commits intocsg-org:developmentfrom
Conversation
WalkthroughReplaces hard-coded provider/jurisdiction branches with a licenseTypeMap-driven dynamic lookup using an authenticatedProviderUserIndex; expands and updates mock privilege histories and provider entries with new licenseType/privilegeId values and moment-based relative dates across multiple jurisdictions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant MockAPI as Mock API
participant Map as licenseTypeMap
participant Data as mockPrivilegeHistoryResponses
Client->>MockAPI: getPrivilegeHistory*(providerId, jurisdiction, licenseTypeAbbrev)
MockAPI->>Map: resolve licenseType = licenseTypeMap[licenseTypeAbbrev]
MockAPI->>Data: find(entry => entry.providerId==providerId && entry.jurisdiction==jurisdiction && entry.licenseType==licenseType)
alt match found
Data-->>MockAPI: matched history entry
MockAPI-->>Client: return responseData (with wait)
else no match
MockAPI-->>Client: throw "not found" (after wait)
end
Note over MockAPI: Public / Staff / Licensee flows reuse same lookup\ngetAuthenticatedLicenseeUser -> getAuthenticatedProvider() uses authenticatedProviderUserIndex
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webroot/src/network/mocks/mock.data.api.ts(6 hunks)webroot/src/network/mocks/mock.data.ts(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
webroot/src/network/mocks/mock.data.api.ts (2)
webroot/src/network/mocks/mock.data.ts (2)
mockPrivilegeHistoryResponses(1527-1911)licensees(426-1177)webroot/src/models/LicenseeUser/LicenseeUser.model.ts (1)
LicenseeUserSerializer(47-62)
webroot/src/network/mocks/mock.data.ts (1)
webroot/src/app.config.ts (1)
serverDateFormat(157-157)
⏰ 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
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webroot/src/network/mocks/mock.data.ts (1)
535-539: Align OTA-NE-11 update timestamp with its newest history eventLine 537 still reports
dateOfUpdateas seven months ago, but the matching history sequence (Lines 1713-1718) records the most recent renewal two months back. Carrying the older timestamp into the privilege record causes the privilege detail pane to disagree with the history timeline. Please sync the privilege metadata with the latest event.- dateOfUpdate: moment().subtract(7, 'months').format(serverDateFormat), + dateOfUpdate: moment().subtract(2, 'months').format(serverDateFormat),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
webroot/src/network/mocks/mock.data.ts(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
webroot/src/network/mocks/mock.data.ts (1)
webroot/src/app.config.ts (1)
serverDateFormat(157-157)
⏰ 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
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
webroot/src/network/mocks/mock.data.ts(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
webroot/src/network/mocks/mock.data.ts (1)
webroot/src/app.config.ts (1)
serverDateFormat(157-157)
⏰ 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 (1)
webroot/src/network/mocks/mock.data.ts (1)
1731-1734: Fix OTA-OH-12 history effective date mismatchLine 1732 still subtracts an extra year from
effectiveDate/createDate, so the timeline shows issuance ≈21 months ago while the privilege record on Line 553 reports 9 months. That contradiction reintroduces the accuracy bug we flagged earlier. Align the timestamps with the privilege metadata.- effectiveDate: moment().subtract(9, 'months').subtract(1, 'year').format(serverDateFormat), - createDate: moment().subtract(9, 'months').subtract(1, 'year').format(serverDateFormat) + effectiveDate: moment().subtract(9, 'months').format(serverDateFormat), + createDate: moment().subtract(9, 'months').format(serverDateFormat)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webroot/src/network/mocks/mock.data.ts (1)
543-558: Correct licenseType for OTA-OH-12
In webroot/src/network/mocks/mock.data.ts, for privilegeId 'OTA-OH-12', change licenseType in both the privileges array and the mockPrivilegeHistoryResponses entry from 'occupational therapist' to 'occupational therapy assistant'.
🧹 Nitpick comments (2)
webroot/src/network/mocks/mock.data.ts (2)
871-875: Use 'months' for consistency with Moment units.Minor: other calls use 'months'. Prefer the plural for consistency.
- dateOfExpiration: moment().add(3, 'month').format(serverDateFormat), + dateOfExpiration: moment().add(3, 'months').format(serverDateFormat),
1528-1904: Optional: centralize date offsets to avoid future driftTo prevent privilege vs history inconsistencies, consider constants/helpers (e.g., issuanceOffset('OTA-AL-10')) or generating history from privilege metadata in mocks.
I can provide a small utility to derive history arrays from privilege objects to keep dates in sync.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
webroot/src/network/mocks/mock.data.ts(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
webroot/src/network/mocks/mock.data.ts (1)
webroot/src/app.config.ts (1)
serverDateFormat(157-157)
⏰ 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 (10)
webroot/src/network/mocks/mock.data.ts (10)
539-541: LGTM: activeSince now tracked for OTA-NE-11Adding activeSince aligns the privilege with renewal timing and improves timeline accuracy.
979-987: LGTM: normalized licenseType on OCTP-AL-19The licenseType now matches OTA data and the history entry. Good.
1055-1061: LGTM: OCTP-AL-22 privilege metadata synced to renewaldateOfUpdate/dateOfRenewal now align with the renewal event. This addresses the earlier discrepancy.
1124-1173: Confirm intent: active statuses with past expirations for Jane DoeThese privileges are marked active but have dateOfExpiration in 2023/2024. If intentional for demo (per notes about incomplete details), ignore; otherwise adjust either status or expiration to avoid contradictory UI.
Do you want me to normalize these to prevent “Active but expired” states in the timeline?
1528-1557: LGTM: OTA-NE-10 history now matches privilege issuance (2y7m)Issuance timestamps align with the privilege record; this fixes the timeline inconsistency.
1656-1688: LGTM: OTA-AL-10 event sequence consistent with privilege metadataIssuance, expiration, renewal, encumbrance, and lifting events line up with the privilege’s update/renewal/activeSince fields.
1701-1718: LGTM: OTA-NE-11 history coheres with privilege fieldsIssuance (2y7m) and renewals (1y7m, 2m) are consistent with dates on the privilege.
1819-1830: LGTM: OCTP-AL-19 history deactivation flows correctlyIssuance then deactivation sequence is clear and matches the privilege’s inactive status.
1849-1853: LGTM: OCTP-AL-22 renewal history aligns with privilege metadataRenewal timing matches the privilege block updates.
1863-1901: LGTM: New issuance histories for Jane (AL/AK/AR)PrivilegeIds and licenseType fields are consistent; minimal, clean histories.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
webroot/src/network/mocks/mock.data.ts (2)
871-875: Minor: use plural time unit for consistencyPrefer 'months' for consistency with the rest of the file.
- dateOfExpiration: moment().add(3, 'month').format(serverDateFormat), + dateOfExpiration: moment().add(3, 'months').format(serverDateFormat),
1638-1643: Normalize mixed date formats within the same eventeffectiveDate uses YYYY-MM-DD while dateOfUpdate/createDate use ISO with time. Inconsistent formats can cause parsing/ordering surprises.
Consider standardizing all three fields to the same format (either serverDateFormat everywhere for date-only events or full ISO timestamps consistently).
webroot/src/network/mocks/mock.data.api.ts (5)
46-48: Rename/match testing instructions; consider storing providerId instead of indexDocs say “update authenticatedProviderUserId”, but code exposes authenticatedProviderUserIndex. Rename for clarity; storing the actual providerId and resolving via find() avoids index brittleness when the provider list changes.
-// Authenticated provider index for the current session -const authenticatedProviderUserIndex = 0; +// Authenticated provider ID for the current session (defaults to first provider) +const authenticatedProviderUserId = licensees.providers[0]?.providerId;Apply corresponding replacements at Lines 491-493 and 571-573:
- .fromServer(licensees.providers[authenticatedProviderUserIndex]) + .fromServer(licensees.providers.find(p => p.providerId === authenticatedProviderUserId) ?? licensees.providers[0])- const authenticatedUser = licensees.providers[authenticatedProviderUserIndex]; + const authenticatedUser = licensees.providers.find(p => p.providerId === authenticatedProviderUserId) ?? licensees.providers[0];
311-360: DRY: factor shared history-lookup + response shapingThe three history endpoints duplicate the same find/shape code. Extract a small helper to reduce drift.
Example helper:
- findHistory(providerId, jurisdiction, licenseTypeAbbrev)
- buildHistoryResponse(responseData)
Also applies to: 365-416, 566-612
491-493: Authenticated user selection still index-basedSafe for now, but brittle if provider ordering changes. Consider the ID-based approach in the earlier comment when convenient.
318-337: Optional: also validate mappedLicenseType presence before building responseHelps return a clean “not found” instead of matching against undefined.
- if (responseData + if (responseData && compact && providerId && jurisdiction && licenseTypeAbbrev ) { + // proceed } else { response = wait(500).then(() => { throw new Error('not found'); }); }
50-53: If enum values don’t match, map to strings instead (fallback)If verification shows LicenseType enum values differ from the mock strings, switch mapping to strings to keep mock lookups working.
-const licenseTypeMap = { - ot: LicenseType.OCCUPATIONAL_THERAPIST, - ota: LicenseType.OCCUPATIONAL_THERAPY_ASSISTANT -}; +const licenseTypeMap = { + ot: 'occupational therapist', + ota: 'occupational therapy assistant' +} as const;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webroot/src/network/mocks/mock.data.api.ts(6 hunks)webroot/src/network/mocks/mock.data.ts(9 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-21T15:23:26.019Z
Learnt from: rmolinares
PR: csg-org/CompactConnect#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/network/mocks/mock.data.ts
🧬 Code graph analysis (2)
webroot/src/network/mocks/mock.data.api.ts (2)
webroot/src/network/mocks/mock.data.ts (2)
mockPrivilegeHistoryResponses(1527-1904)licensees(426-1177)webroot/src/models/LicenseeUser/LicenseeUser.model.ts (1)
LicenseeUserSerializer(47-62)
webroot/src/network/mocks/mock.data.ts (1)
webroot/src/app.config.ts (1)
serverDateFormat(157-157)
⏰ 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 (11)
webroot/src/network/mocks/mock.data.ts (10)
550-558: OTA-OH-12 licenseType/date fixes look consistent — LGTMlicenseType set to occupational therapy assistant and issuance/renewal/update aligned at 9 months with matching activeSince/expiration.
979-987: OCTP-AL-19 licenseType corrected to OTA — LGTM
1055-1061: OCTP-AL-22: licenseType + renewal metadata sync — LGTM
1528-1557: NE/OTA-NE-10 history block aligns with privilege metadata — LGTM
1701-1718: NE/OTA-NE-11 issuance and renewals look coherent — LGTM
1722-1736: OH/OTA-OH-12 issuance history aligned to 9 months — LGTM
1769-1783: OH/OT-OH-11 deactivation sequence — LGTM
1816-1831: AL/OCTP-AL-19 issuance + deactivation — LGTM
1841-1853: AL/OCTP-AL-22 renewal matches privilege metadata — LGTM
1863-1870: AL/AK/AR one-off issuance entries for provider 4 — LGTMwebroot/src/network/mocks/mock.data.api.ts (1)
50-53: licenseTypeMap mappings are correct
Enum values for OCCUPATIONAL_THERAPIST and OCCUPATIONAL_THERAPY_ASSISTANT resolve to “occupational therapist” and “occupational therapy assistant,” matching the mock historyEntry.licenseType strings.
rmolinares
requested review from
jsandoval81
and removed request for
jsandoval81
rmolinares
requested review from
jsandoval81
and removed request for
jsandoval81
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
webroot/src/network/mocks/mock.data.api.ts (5)
46-48: Align variable name with testing instructions (add alias).Test checklist says to edit authenticatedProviderUserId, but the code defines authenticatedProviderUserIndex. Add an alias to prevent confusion.
Apply this diff:
-// Authenticated provider index for the current session -const authenticatedProviderUserIndex = 0; +// Authenticated provider index for the current session +const authenticatedProviderUserIndex = 0; +// Back-compat with testing checklist naming +const authenticatedProviderUserId = authenticatedProviderUserIndex;
49-54: Comment says “abbreviations and full names” but map only covers abbreviations.Either update the comment or expand the map to actually accept full names as well.
Apply this diff to correct the comment (minimal change):
-// License type mapping for matching abbreviations and full names to LicenseType enum values +// License type mapping for matching abbreviations to LicenseType enum values
319-330: Also filter by compact to avoid cross‑compact collisions.You require
compactbut don’t use it in the predicate. Add it to the match to prevent false positives when multiple compacts exist.Apply this diff:
- const matches = Boolean( + const matches = Boolean( historyEntry.providerId === providerId && historyEntry.jurisdiction === jurisdiction - && historyEntry.licenseType === mappedLicenseType + && historyEntry.licenseType === mappedLicenseType + && historyEntry.compact === compact );
373-385: Repeat compact filtering for staff lookup.Same as public endpoint—use the compact param to disambiguate results.
Apply this diff:
- const matches = Boolean( + const matches = Boolean( historyEntry.providerId === providerId && historyEntry.jurisdiction === jurisdiction - && historyEntry.licenseType === mappedLicenseType + && historyEntry.licenseType === mappedLicenseType + && historyEntry.compact === compact );
491-493: Guard against invalid index to avoid serializer crashing.If the index is out of bounds (or later edited), this will pass
undefinedto the serializer. Add a safe fallback or throw a clear error.Apply this diff:
- return wait(500).then(() => LicenseeUserSerializer - .fromServer(licensees.providers[authenticatedProviderUserIndex])); + return wait(500).then(() => { + const user = licensees.providers[authenticatedProviderUserIndex] ?? licensees.providers[0]; + if (!user) { throw new Error('not found'); } + return LicenseeUserSerializer.fromServer(user); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
webroot/src/network/mocks/mock.data.api.ts(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-10T19:50:56.745Z
Learnt from: jsandoval81
PR: csg-org/CompactConnect#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/network/mocks/mock.data.api.ts
🔇 Additional comments (1)
webroot/src/network/mocks/mock.data.api.ts (1)
32-32: LGTM: LicenseType import is appropriate for the new mapping.
jsandoval81
left a comment
jsandoval81
left a comment
There was a problem hiding this comment.
This feels solid and is nice to have things matching up with the mocks 👍
Just a minor item below about comments / notation.
|
@jlkravitz This is now ready for your review. |
jlkravitz
left a comment
jlkravitz
left a comment
There was a problem hiding this comment.
One question, but otherwise looks great!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
webroot/src/network/mocks/mock.data.api.ts (1)
46-63: Excellent DX improvement!This helper implements the console error approach discussed in previous reviews, providing clear feedback when the index is out of bounds. The bounds checking and fallback logic are solid.
Based on learnings
Optional: Guard against empty providers array
While unlikely in mock data context, you could add a defensive check:
const getAuthenticatedProvider = () => { const { providers } = licensees; + + if (!providers || providers.length === 0) { + console.error('Mock Data Error: No providers available in mock data.'); + return undefined; + } + const maxIndex = providers.length - 1;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
webroot/src/network/mocks/mock.data.api.ts(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
webroot/src/network/mocks/mock.data.api.ts (2)
webroot/src/network/mocks/mock.data.ts (2)
licensees(426-1177)mockPrivilegeHistoryResponses(1527-1943)webroot/src/models/LicenseeUser/LicenseeUser.model.ts (1)
LicenseeUserSerializer(47-62)
⏰ 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/network/mocks/mock.data.api.ts (6)
32-32: LGTM!Clean import addition for the license type mapping.
65-69: LGTM!Clean and maintainable mapping from abbreviations to enum values. The lowercase keys align well with the case-insensitive lookup pattern used throughout.
335-346: Dynamic lookup cleanly replaces hard-coded branches.The refactoring successfully implements a mapping-driven approach that scales better and is easier to maintain. The safe string coercion
String(licenseTypeAbbrev ?? '').toLowerCase()correctly handles null/undefined inputs.Based on learnings
389-401: LGTM!Consistent implementation of the dynamic lookup pattern. The staff endpoint correctly mirrors the public endpoint logic.
507-508: LGTM!Clean usage of the
getAuthenticatedProvider()helper, centralizing the authenticated provider resolution.
584-600: Safe authenticated user resolution.The optional chaining
authenticatedUser?.providerIdcorrectly prevents crashes when the provider index is invalid, allowing the flow to degrade gracefully to "not found". The dynamic lookup pattern is consistent with the public and staff endpoints.Based on learnings
jlkravitz
left a comment
jlkravitz
left a comment
There was a problem hiding this comment.
@isabeleliassen good to merge!
4 participants
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 warningsauthenticatedProviderUserIdinmock.data.apito 'log in' as the other mock practitioners and confirm those as wellCloses #1082
Summary by CodeRabbit
New Features
Improvements