Skip to content

fix: refactor login/logout flow and subscription handling#1436

Merged
fadi-george merged 4 commits intomainfrom
fg/400-logout
Mar 5, 2026
Merged

fix: refactor login/logout flow and subscription handling#1436
fadi-george merged 4 commits intomainfrom
fg/400-logout

Conversation

@fadi-george
Copy link
Contributor

@fadi-george fadi-george commented Mar 4, 2026

Description

1 Line Summary

Addresses: https://app.asana.com/1/780103692902078/project/1208777190614537/task/1213469123238681

Refactor login/logout to share a common user-switching path and create an empty push subscription on login so accepting permissions later patches instead of creating a second user.

Details

  • Extracted _switchUser and _resetAndGetIdentityModel in LoginManager to DRY up _login and _logout
  • Login now creates an empty push subscription model when none exists, so accepting push permissions afterward results in a subscription update (PATCH) rather than a second create-user call
  • Moved createUserOnServer into page.ts as createSubscribedUser, scoped to where it is actually used, accepting the push model directly to avoid redundant lookups
  • Inlined resetUserModels into LoginManager and deleted userDirector.ts
  • Removed unused hasCssClass and waitForAnimations from dom.ts
  • Removed stray console.log calls

Systems Affected

  • WebSDK
  • Backend
  • Dashboard

Validation

Tests

Updated tests in LoginManager.test.ts and OneSignal.test.ts to match the new behavior where login creates an empty subscription upfront and permissions acceptance triggers a subscription update.


Login flows

  • login w/o accepting push notif -> POST create user with empty sub (token: "", enabled: false), no 400 errors
    User created & fake sub created:
Screenshot 2026-03-04 at 4 04 32 PM Screenshot 2026-03-04 at 4 07 03 PM
  • login, then logout -> no additional subs created on server, anonymous user created with local ID only
    Two users now:
Screenshot 2026-03-04 at 4 08 03 PM
  • login then accept permissions -> POST create user with empty sub, then PATCH subscription with push token/keys (not a second create user call)

  • login, accept permissions, then logout -> transfer sub op fires, new anonymous user created, push sub moves to new user
Screenshot 2026-03-04 at 4 44 34 PM Screenshot 2026-03-04 at 4 44 51 PM Screenshot 2026-03-04 at 4 45 25 PM
  • login with user A, then login with user B -> first user created, models reset, second user created with transfer sub
Screenshot 2026-03-04 at 4 52 49 PM Screenshot 2026-03-04 at 4 52 42 PM

Logout flows

  • login, add email, then logout -> no 400 errors, no duplicate subs, email sub stays on logged-in user
    Before logout:
Screenshot 2026-03-04 at 4 11 52 PM

After logout (no errors):
Screenshot 2026-03-04 at 4 13 05 PM
Screenshot 2026-03-04 at 4 14 26 PM


  • login, add sms, then logout -> all subs remain on previous user, anonymous user gets push sub only
    Before logout:
Screenshot 2026-03-04 at 4 17 28 PM

After logout:
Screenshot 2026-03-04 at 4 17 42 PM


Anonymous user flows (no login)

  • accept permissions (fresh user) -> createSubscribedUser called, create user w/ sub, no 400 errors
Screenshot 2026-03-04 at 4 56 59 PM Screenshot 2026-03-04 at 4 57 20 PM
  • addEmail (fresh user) -> POST create user w/ email subscription, no 400 errors
Screenshot 2026-03-04 at 5 00 16 PM Screenshot 2026-03-04 at 5 00 25 PM
  • addSms (fresh user) -> POST create user w/ sms subscription, no 400 errors
Screenshot 2026-03-04 at 5 03 20 PM Screenshot 2026-03-04 at 5 03 57 PM
  • accept permissions, then login -> push sub transferred to logged-in user via transfer sub op
  1. identifies anon -> new external/owner
  2. transfers sub to new external id/owner
Screenshot 2026-03-04 at 5 08 25 PM Screenshot 2026-03-04 at 5 08 51 PM

Edge cases

  • login called twice rapidly with same external ID -> second call skipped ("External ID already set")
  • login called during init (quick succession) -> no duplicate identity requests due to hydrate tag
  • network failure on create user -> local ID preserved, retry on next accept permissions via createSubscribedUser

Checklist

  • All the automated tests pass or I explained why that is not possible
  • I have personally tested this on my machine or explained why that is not possible
  • I have included test coverage for these changes or explained why they are not needed

Programming Checklist
Interfaces:

  • Don't use default export
  • New interfaces are in model files

Functions:

  • Don't use default export
  • All function signatures have return types
  • Helpers should not access any data but rather be given the data to operate on.

Typescript:

  • No Typescript warnings
  • Avoid silencing null/undefined warnings with the exclamation point

Other:

  • Iteration: refrain from using elem of array syntax. Prefer forEach or use map
  • Avoid using global OneSignal accessor for context if possible. Instead, we can pass it to function/constructor so that we don't call OneSignal.context

Screenshots

Info

No UI changes.

Checklist

  • I have included screenshots/recordings of the intended results or explained why they are not needed

Related Tickets


Made with Cursor


This change is Reviewable

@fadi-george fadi-george requested a review from nan-li March 4, 2026 23:54
@fadi-george fadi-george changed the title Refactor login/logout flow and subscription handling fix: refactor login/logout flow and subscription handling Mar 4, 2026
@fadi-george fadi-george marked this pull request as ready for review March 5, 2026 01:06
Copy link

@abdulraqeeb33 abdulraqeeb33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed about the PR in detail via chat

@fadi-george fadi-george merged commit c83da4c into main Mar 5, 2026
6 checks passed
@fadi-george fadi-george deleted the fg/400-logout branch March 5, 2026 21:43
@github-actions github-actions bot mentioned this pull request Mar 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants