-
Notifications
You must be signed in to change notification settings - Fork 27
chore: Enable SSE only for authenticated users #973
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
Sample app builds 📱Below you will find the list of the latest versions of the sample apps. |
SDK binary size reports 📊SDK binary size of this PRSDK binary size diff report vs. main branch |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/inapp-sse #973 +/- ##
====================================================
Coverage ? 68.20%
====================================================
Files ? 182
Lines ? 9400
Branches ? 0
====================================================
Hits ? 6411
Misses ? 2989
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // logger.logWithModuleTag("State changes after action '\(action)': \(changes)", level: .debug) | ||
| } else { | ||
| logger.logWithModuleTag("No state changes after action '\(action)'", level: .debug) | ||
| // logger.logWithModuleTag("No state changes after action '\(action)'", level: .debug) |
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.
Commented out logging statements appear accidentally committed
Three debug logging statements in inAppMessageReducer have been commented out rather than removed. The PR title mentions enabling SSE only for authenticated users, not disabling logging. The function's docstring still states it "logs the action and state before and after the reducer is called" but the actual logging is now disabled, creating inconsistency. Other debug logging throughout the codebase remains active, suggesting these were temporarily disabled during development and accidentally committed.
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.
➕ 1
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.
Fixed!
32a1c68 to
9d6c72d
Compare
504250f to
8672ff8
Compare
9d6c72d to
8bec6ac
Compare
c797f69 to
158290e
Compare
| // logger.logWithModuleTag("State changes after action '\(action)': \(changes)", level: .debug) | ||
| } else { | ||
| logger.logWithModuleTag("No state changes after action '\(action)'", level: .debug) | ||
| // logger.logWithModuleTag("No state changes after action '\(action)'", level: .debug) |
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.
➕ 1
158290e to
097c09b
Compare
097c09b to
618301f
Compare
618301f to
5bf1ff9
Compare
This PR ensures that SSE connections are only enabled for authenticated users and not anonymous users.
Note
Aligns SSE usage with eligibility rules and streamlines polling control.
InAppMessageState.isUserIdentifiedandshouldUseSseto centralize SSE eligibilityGistto subscribe topollInterval,useSse, anduserId; start/stop polling based onshouldUseSseand user identification; skip polling when SSE is activeSseLifecycleManagerto also subscribe touserId, check eligibility before starting SSE, and always stop SSE on background; refine foreground/background and flag change handling to avoid racesSseLifecycleManagerTestto cover anonymous vs identified users, foreground/background transitions, SSE flag changes, and combined flowsWritten by Cursor Bugbot for commit 5bf1ff9. This will update automatically on new commits. Configure here.