Frontend/compact states from server#716
Conversation
WalkthroughThis pull request introduces a new dependency and refines the handling of compact state data throughout the application. It removes detailed member state arrays from configuration files, updates component and test logic to properly await asynchronous operations, and adds new API methods and corresponding store actions/mutations for fetching compact states. The changes also update model and test validations to reflect the simplified member state structure, enhancing overall state management. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User Interface
participant Store as User Store
participant API as Data API
participant Backend as Server API
UI->>Store: setCurrentCompact(compact)
Store->>Store: Dispatch getCompactStatesRequest
Store->>API: getCompactStates(compact)
API->>Backend: GET /v1/compacts/{compact}/jurisdictions
Backend-->>API: Return compact states data
API-->>Store: Resolve Promise with states
Store->>Store: Commit GET_COMPACT_STATES_SUCCESS mutation
Store->>UI: Update UI with compact states
Assessment against linked issues
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
webroot/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
webroot/package.json(1 hunks)webroot/src/app.config.ts(1 hunks)webroot/src/components/CompactSelector/CompactSelector.ts(1 hunks)webroot/src/components/Licensee/LicenseeList/LicenseeList.ts(1 hunks)webroot/src/components/Page/PageContainer/PageContainer.ts(1 hunks)webroot/src/components/StateUpload/StateUpload.spec.ts(1 hunks)webroot/src/models/Compact/Compact.model.spec.ts(2 hunks)webroot/src/models/Compact/Compact.model.ts(1 hunks)webroot/src/models/StaffUser/StaffUser.model.spec.ts(1 hunks)webroot/src/network/data.api.ts(1 hunks)webroot/src/network/mocks/mock.data.api.ts(3 hunks)webroot/src/network/mocks/mock.data.ts(1 hunks)webroot/src/network/userApi/data.api.ts(2 hunks)webroot/src/router/index.ts(1 hunks)webroot/src/store/user/user.actions.ts(1 hunks)webroot/src/store/user/user.mutations.ts(2 hunks)webroot/src/store/user/user.spec.ts(6 hunks)webroot/src/store/user/user.state.ts(2 hunks)
🧰 Additional context used
🧬 Code Definitions (7)
webroot/src/network/mocks/mock.data.api.ts (2)
webroot/src/network/mocks/mock.data.ts (1)
compactStates(1055-1076)webroot/src/models/State/State.model.ts (1)
StateSerializer(54-70)
webroot/src/components/CompactSelector/CompactSelector.ts (1)
webroot/src/models/Compact/Compact.model.ts (1)
CompactSerializer(80-90)
webroot/src/network/data.api.ts (3)
webroot/src/pages/PublicLicensingDetail/PublicLicensingDetail.ts (1)
compact(48-52)webroot/src/pages/LicensingDetail/LicensingDetail.ts (1)
compact(59-63)webroot/src/network/userApi/data.api.ts (1)
userDataApi(358-358)
webroot/src/router/index.ts (1)
webroot/src/models/Compact/Compact.model.ts (1)
CompactSerializer(80-90)
webroot/src/models/Compact/Compact.model.spec.ts (5)
webroot/src/store/user/user.state.ts (1)
State(19-31)webroot/src/models/State/State.model.ts (1)
State(21-49)webroot/src/store/pagination/pagination.state.ts (1)
State(11-16)webroot/src/models/Compact/Compact.model.ts (2)
CompactSerializer(80-90)Compact(39-75)backend/compact-connect/lambdas/nodejs/lib/models/compact.ts (1)
Compact(6-17)
webroot/src/store/user/user.actions.ts (3)
webroot/src/store/user/user.state.ts (1)
state(33-45)webroot/src/network/data.api.ts (1)
dataApi(330-330)webroot/src/network/mocks/mock.data.api.ts (1)
dataApi(324-324)
webroot/src/store/user/user.mutations.ts (2)
webroot/src/store/user/user.state.ts (1)
state(33-45)webroot/src/network/mocks/mock.data.ts (1)
compactStates(1055-1076)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: CheckWebroot
🔇 Additional comments (28)
webroot/package.json (1)
19-19: LGTM: Added caniuse-lite dependencyThis addition supports the browserlist build dependency update mentioned in the PR objectives.
webroot/src/app.config.ts (1)
217-219: LGTM: Removed hardcoded member statesThe member states have been removed from the compact configurations, which aligns with the PR objective to fetch compact states from the server instead of using static configuration.
webroot/src/components/StateUpload/StateUpload.spec.ts (1)
30-30:Details
✅ Verification successful
Good improvement: Added await to setCurrentCompact dispatch
Added
awaitto properly handle the now-asynchronoussetCurrentCompactaction, preventing potential race conditions in tests.
🏁 Script executed:
#!/bin/bash # Check for any other tests that might need the same await pattern for setCurrentCompact rg "dispatch\('user/setCurrentCompact" -A 1 -B 1 --glob "*.spec.ts" | grep -v "await.*dispatch\('user/setCurrentCompact"Length of output: 152
Action Required: Verify Await Usage in Other Tests
The asynchronous dispatch fix for
setCurrentCompactinwebroot/src/components/StateUpload/StateUpload.spec.ts(line 30) is a good improvement. However, the original verification script failed due to regex-escaping issues. Please manually verify that all test files calling
dispatch('user/setCurrentCompact', …)have the correctawaitpattern.As a next step, consider running a revised command like:
Manually inspect the output to ensure no instances are missing the
awaitkeyword.
🏁 Script executed:
#!/bin/bash # Verify that all occurrences of dispatch('user/setCurrentCompact') in *.spec.ts files include "await" rg "dispatch\\('user/setCurrentCompact" --glob "*.spec.ts" -A 1 -B 1 | grep -v "await"Length of output: 658
Await Fix Verified for
setCurrentCompactDispatchThe asynchronous dispatch fix in
webroot/src/components/StateUpload/StateUpload.spec.tshas been correctly applied (line 30). Our verification—despite the extraneous context from other test files—shows no unawaited calls todispatch('user/setCurrentCompact')in the relevant test files.
- Confirmed in
StateUpload.spec.ts:
- Correct usage:
await component.$store.dispatch('user/setCurrentCompact', new Compact({ type: CompactType.ASLP }));- No evidence of other tests missing the
awaitpattern for this specific action.Please manually review the output of the revised grep command to ensure no instances were overlooked.
webroot/src/store/user/user.state.ts (1)
25-25: LGTM: Added loading state for compact statesAdded a new state property to track loading state specifically for compact states, which corresponds to the PR's implementation of a new loading state. This allows the UI to provide appropriate feedback during asynchronous operations.
Also applies to: 39-39
webroot/src/components/Page/PageContainer/PageContainer.ts (1)
70-72: LGTM: Loading state correctly extended to include compact states loadingThe addition of
isLoadingCompactStatesto theisLoadingcomputed property makes sense as it ensures the loading mask is displayed when compact states are being fetched.webroot/src/components/CompactSelector/CompactSelector.ts (1)
146-149: Improved conditional dispatch logicThe refactored code now only calls
setCurrentCompactwhen not redirecting to a different route. This avoids an unnecessary store update since the router'sbeforeEachguard will handle setting the compact after redirection.webroot/src/router/index.ts (1)
41-43: Correctly awaiting async actionGood addition of the
awaitkeyword to ensure navigation continues only after the compact states have been fully loaded. This is essential assetCurrentCompactis now an asynchronous action that may trigger network requests.webroot/src/network/mocks/mock.data.api.ts (2)
15-15: LGTM: Import statements properly updatedThe new imports for
StateSerializerand addingcompactStatesto the mock data imports are appropriate for the new functionality.Also applies to: 25-25
230-241: Mock API methods for compact states properly implementedThe new methods
getCompactStatesandgetCompactStatesPublicfollow the established patterns in the codebase:
- They use the
waitfunction to simulate network latency- They transform the mock data using the appropriate serializer
- They have both authenticated and public versions of the API endpoints
This implementation will allow for proper testing of the compact states functionality without requiring a backend.
webroot/src/models/StaffUser/StaffUser.model.spec.ts (1)
7-7: Import removed for compacts configuration.The
compacts as compactConfigsimport has been removed, which aligns with the PR objective of moving from hardcoded compact states to dynamically fetched ones from server endpoints.webroot/src/models/Compact/Compact.model.ts (1)
85-85: Simplified memberStates assignment.This change simplifies how
memberStatesare populated, now directly using server data instead of processing it through configuration. This is in line with the PR objective of fetching compact states from server endpoints rather than static configuration.webroot/src/network/data.api.ts (2)
230-237: Added new method to fetch compact states.This method provides the interface to retrieve compact states from the server through the userDataApi, which is a key component of the PR objective to integrate new server endpoints.
239-246: Added public endpoint for compact states.A corresponding public method has been added to fetch compact states without authentication requirements, enabling both authenticated and unauthenticated access paths to state data.
webroot/src/components/Licensee/LicenseeList/LicenseeList.ts (1)
202-207: Great update to handle asynchronous operation.The method signature change to
asyncand the addition ofawaitbefore the store dispatch is a necessary improvement that ensures proper control flow when resetting the search. This change properly aligns with the updated asynchronous behavior of thesetCurrentCompactaction in the user store.webroot/src/models/Compact/Compact.model.spec.ts (2)
8-8: Import simplification aligns with compact states refactoring.Removing the import for
compacts as compactConfigsreflects the architectural change to fetch compact states from the server rather than storing them in config files. This is a good cleanup.
76-76: Test updated to match new compact states implementation.The test has been correctly modified to align with the new implementation where member states are fetched from the server instead of being defined in configuration. The test now properly verifies a simpler structure with just one state in the test data.
Also applies to: 84-84
webroot/src/store/user/user.spec.ts (5)
16-16: Good use of CompactType enum import.Adding the CompactType import ensures type safety and consistency when handling compact types throughout the tests. This is a good practice that improves code maintainability.
146-179: Comprehensive test coverage for compact states mutations.The new tests thoroughly verify that the mutations for compact states correctly handle request, success, and failure states. They check both the loading state and error handling, as well as the case where member states are updated.
630-631: Use of CompactType enum instead of string literals.Replacing string literals with the CompactType enum improves type safety and makes the code more maintainable. This change ensures consistent usage of compact types throughout the application.
Also applies to: 634-635, 645-646
784-847: Thorough testing of new compact states actions.The new tests comprehensively verify that the actions for fetching compact states work correctly for both staff and non-staff users, and that the
setCurrentCompactaction properly handles null values and dispatches the fetch action when needed.
857-858: Updated getter test for consistency with CompactType.The test for the current compact getter has been updated to use CompactType instead of string literals, maintaining consistency throughout the codebase.
Also applies to: 860-860
webroot/src/store/user/user.actions.ts (2)
118-136: Well-implemented compact states fetch action.The new
getCompactStatesRequestaction intelligently selects the appropriate API method based on the user's login status, which is a good design choice for handling both authenticated and public access scenarios. The proper handling of success and failure cases is also implemented correctly.
138-144: Good enhancement to setCurrentCompact action.Making
setCurrentCompactasync and adding conditional fetching of compact states is a sensible approach. This ensures that when a compact is selected, its member states are automatically fetched when needed, providing a better user experience without additional code at call sites.webroot/src/network/userApi/data.api.ts (3)
19-19: Import addition looks good.The added import for
StateSerializeris correctly placed with other model imports and will be used by the new methods for handling compact states.
256-267: Well-implemented API method for fetching compact states.The implementation follows the established pattern in the codebase for API methods:
- Clear JSDoc comments with parameter and return type documentation
- Proper async/await usage
- Appropriate serialization of the server response
The transformation of
postalAbbreviationtoabbrevhandles the data mapping correctly between server and client models.
269-280: Public endpoint implementation matches the authenticated version.The public endpoint implementation correctly mirrors the authenticated version but uses the appropriate public API path. Both methods share the same transformation logic, which is good for consistency.
webroot/src/store/user/user.mutations.ts (2)
27-29: Mutation types follow naming convention.The new mutation types follow the established naming pattern with descriptive names that clearly indicate their purpose in the request-failure-success cycle.
109-124: Mutation handlers properly manage state transitions.The implementation correctly:
- Sets and clears loading flags appropriately across the request lifecycle
- Handles error states consistently with other mutations
- Updates the compact states only when a current compact exists
- Follows the pattern established by other request-failure-success mutations
One thing to note is the conditional check before updating
memberStates, which is a good defensive programming practice.
|
Some of the time Im logging in as a staff user and then being redirected to this page, the state list does not seem to populate. |
|
@jlkravitz This is ready for your review. |
jlkravitz
left a comment
jlkravitz
left a comment
There was a problem hiding this comment.
Looks good! @isabeleliassen Good to merge.
4 participants
Requirements List
yarn install --ignore-enginesDescription List
setCurrentCompactaction to fetch compact states as neededsetCurrentCompactcall sites to ensure async compatibilityDraft Todo
Testing List
yarn test:unit:allshould run without errors or warningsyarn serveshould run without errors or warningsyarn buildshould run without errors or warningsCloses #663
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor
Tests