Skip to content
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

[user model] fix bug in identify user: need to fetch #1210

Merged

Conversation

nan-li
Copy link
Contributor

@nan-li nan-li commented Jan 12, 2023

Description

One Line Summary

When the anonymous user has been identified, we still need to Fetch User as we cleared local data.

Details

Motivation

need to fetch user after successfully identifying user

  • The anonymous user has been identified, but we still need to Fetch User as we cleared local data
  • This will hydrate the onesignal_id again as well as other user properties

Testing

Manual testing

  • Better manual testing this time with identifying user flows and checking the local data before and after. Also checked data on server with REST API.
  1. Install app, see an anon user is created
  2. Add a tag
  3. Check local state: see tag, see the push subscription object
  4. Login to a new external_id
  5. See the identify user request and fetch user request are sent
  6. Check the local data and see the the tag is there, the onesignal_id has not changed, the external_id is there, and pushsubscription model is same as before.
  7. Check this user via REST API: see the same data as exists locally, and see it has the push subscription that we expect.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
    • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

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

Final pass

  • Code is as readable as possible.
    • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
  • I have reviewed this PR myself, ensuring it meets each checklist item
    • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

This change is Reviewable

* The anonymous user has been identified, but we still need to Fetch User as we cleared local data
* This will hydrate the onesignal_id again as well as other user properties
@nan-li nan-li requested review from emawby, fhboswell and jennantilla and removed request for emawby January 12, 2023 20:58
@nan-li nan-li force-pushed the user_model/fix_identify_user_and_push_observer branch from 43d35eb to 504eace Compare January 12, 2023 21:01
@nan-li nan-li changed the title [user model] fix bug in identify user and firing the push subscription observer [user model] fix bug in identify user Jan 12, 2023
* calculateIsOptedIn and calculateIsEnabled were not actually using the passed in arguments
@nan-li nan-li changed the title [user model] fix bug in identify user [user model] fix bug in identify user: need to fetch Jan 12, 2023
[user model] Fix firing the push subscription observer
@nan-li nan-li merged commit 0fe3a1a into major_release_5.0.0 Jan 12, 2023
@nan-li nan-li deleted the user_model/fix_identify_user_and_push_observer branch January 12, 2023 21:41
nan-li added a commit that referenced this pull request Oct 30, 2023
…and_push_observer

[user model] fix bug in identify user: need to fetch
nan-li added a commit that referenced this pull request Oct 30, 2023
…and_push_observer

[user model] fix bug in identify user: need to fetch
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