Support websocket workspace #1376
Conversation
ws service supports models now.
Part of the filters work, this can currently always be added
We didn’t ever use this. Likely never will
if a socket just opens by default it’s not an issue.
It reopens during changing tests
WalkthroughThis pull request introduces modifications to the WebSocket service and related components across multiple files. The changes focus on enhancing WebSocket subscription handling, simplifying resource request mechanisms, and improving test infrastructure. Key modifications include adding workspace context to subscriptions, refactoring message handling in test support utilities, and streamlining the way resources are requested and processed during WebSocket interactions. Changes
Sequence DiagramsequenceDiagram
participant Client
participant WebSocketService
participant Server
Client->>WebSocketService: Initialize Connection
WebSocketService->>Server: Open Connection
WebSocketService->>Server: Subscribe with Workspace Context
Server-->>WebSocketService: Acknowledge Subscription
WebSocketService-->>Client: Ready for Notifications
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/js/services/ws.js (1)
98-98: Consider adding a constant for the test environment flag.The
_TEST_variable should be defined as a constant at the module level for better maintainability.+const IS_TEST_ENV = typeof _TEST_ !== 'undefined'; + onClose() { this.stopHeartbeat(); - if (!_TEST_ && this.resources.length) this._subscribe(); + if (!IS_TEST_ENV && this.resources.length) this._subscribe(); }test/integration/patients/patient/patient-flow.js (2)
2681-2681: Fix indentation.The line is not properly indented compared to the surrounding code.
- .wait('@routeFlowActions'); + .wait('@routeFlowActions');
2688-2696: Verify WebSocket subscription resources.The test verifies that the WebSocket subscription includes both the flow and action resources. However, the test could be more robust:
- The assertion uses a deep equality check which might be too strict
- The order of resources in the array might not matter
Consider making the test more resilient:
- .then(stub => { - expect(stub.getCall(0).args[0].data.resources).to.deep.equal([ - getRelationship(testSocketFlow).data, - getRelationship(testSocketAction).data, - ]); + .then(stub => { + const resources = stub.getCall(0).args[0].data.resources; + const expectedResources = [ + getRelationship(testSocketFlow).data, + getRelationship(testSocketAction).data, + ]; + + expect(resources).to.have.length(2); + expect(resources).to.deep.include.members(expectedResources); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/js/apps/patients/patient/flow/flow_app.js(2 hunks)src/js/base/model.js(0 hunks)src/js/services/ws.cy.js(6 hunks)src/js/services/ws.js(3 hunks)test/integration/patients/patient/patient-flow.js(1 hunks)test/support/websockets.js(2 hunks)
💤 Files with no reviewable changes (1)
- src/js/base/model.js
🔇 Additional comments (7)
test/support/websockets.js (2)
Line range hint
7-19: LGTM! Simplified message handling improves test maintainability.The refactoring from complex message routing to direct message handling reduces test complexity and potential timing issues.
Line range hint
25-39: LGTM! Streamlined mock WebSocket setup.The removal of
connectionResponseMessageand simplified command structure makes the test setup more straightforward and reduces potential points of failure.src/js/services/ws.js (2)
39-48: LGTM! Enhanced subscription context with workspace ID.The addition of workspace context to subscriptions aligns with the PR objectives and improves the subscription payload structure.
77-78: LGTM! Improved data handling robustness.The conditional check for data existence prevents potential issues with undefined data during connection open.
src/js/services/ws.cy.js (2)
8-8: LGTM! Consistent workspace handling in tests.The addition of the workspace constant and its integration with Radio replies maintains consistency across tests.
Also applies to: 25-25, 36-36
170-179: LGTM! Well-structured test data helper.The
testDatahelper function improves code reusability and maintains consistent test data structure.src/js/apps/patients/patient/flow/flow_app.js (1)
101-101: LGTM! Simplified subscription logic.The direct array spread approach is cleaner and more maintainable than the previous invoke method.
Shortcut Story ID: [sc-58278]
For the moment at least all websocket subscriptions.. similar to api requests can by default pass the current workspace.
We might run into issue in the future where we want to monitor across workspace, but we'll tackle that when we get there as it will likely coincide with changes to how we interact with the API as well. (ie: get a notification from a different workspace.. make a request on that different workspace)
In addition to this is a bit of cleanup of unused features or flake-related issues
getResourceto model in order to make it easier to feed the subscription service{ id: type }but later we just made the subscription service support an array of models so it's no longer needed.connectionResponseMessagefrom the mock websocket server as it was never used and likely never will beonOpensometimes passed data and sometimes didn't depending on timing.Summary by CodeRabbit
Release Notes
Bug Fixes
Refactor
Tests