-
Notifications
You must be signed in to change notification settings - Fork 4
Added support for Queue V3 #78
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
Conversation
* develop: Bump version to 3.14.0 Addresses PR feedback. Added exitClick handling
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.
Non-blocking comment. Looks good.
|
||
function setQueueAPIVersion(response) { | ||
if (response && response.headers) { | ||
var queueVersion = response.headers["x-cio-queue-version"]; |
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.
Not a blocker to merge but I was wondering if the "x-cio-queue-version" should be a constant. That was the one hard coded value that made me pause. It isn't in multiple places or anything. I gave it another thought as something that isn't hardcoded thinking it was important and shouldn't change.
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.
If we were using it in many places yeah, but in this case I think its easier to read.
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.
Pull Request Overview
This PR updates the network and queue services to support Queue V3. Key changes include reducing the network request timeout, switching API endpoints based on the queue version, and implementing session ID generation/renewal for V3 requests.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/services/network.js | Updated timeout configuration and API endpoint switching for V3. |
src/services/queue-service.js | Added session ID support and adjusted API request logic to support V3. |
src/services/settings.js | Introduced GIST_QUEUE_V3_API_ENDPOINT and updated queue version getters/setters. |
Comments suppressed due to low confidence (2)
src/services/network.js:9
- The network timeout has been reduced from 20000ms to 5000ms. If this change is intentional, please update the inline comment or documentation to clarify the new timeout value and its rationale.
timeout: 5000
src/services/queue-service.js:56
- [nitpick] The function name getSessionId implies a simple retrieval but it also renews the session ID TTL. Consider renaming it (e.g., renewSessionId) or splitting the logic for clarity.
function getSessionId() {
Part of: https://linear.app/customerio/issue/INAPP-13059/implement-web-sdk-changes-to-enable-dynamic-switching