Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.55k stars 2.9k forks source link

[Fullstory] SignUpUser does not return userMetadata #45788

Closed rushatgabhane closed 3 months ago

rushatgabhane commented 3 months ago

When an existing user signs in, the API command SignInUser is called. It returns userMetadata in its response which is then saved to onyx and used by fullstory. Without userMetadata, the user is anonymous on fullstory.

Problem

Fullstory has anonymous data

Solution

melvin-bot[bot] commented 3 months ago

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

anmurali commented 3 months ago

The problem is also present when the user is brought into New Dot in signed -in state (e.g. on classic, if I go to Support - Concierge, it brings me into ND in signed in state and I don't see the userMetadata onyx data)

rushatgabhane commented 3 months ago

The problem is also present when the user is brought into New Dot in signed -in state (e.g. on classic, if I go to Support - Concierge,

added it in issue

kadiealexander commented 3 months ago

Not overdue.

mountiny commented 3 months ago

I have got one PR for the SignInUser case up, there are some flakey tests

For the SignInWithShortLivedAuthToken its using a CreateLogin auth command, which is not returning the userMetadata yet so we need to first return that from Auth

anmurali commented 3 months ago

@mountiny thank you for prioritizing this. I was blocked on scheduling our onboarding till we fix these two things cause otherwise none of the legit accounts will have metadata for a very long time. Sign up + infinite sessions = no metadata till they are forced to sign in at some point.

mountiny commented 3 months ago

No problem, seems like it should be quite quick so given Tom explained to me this is blocking us from actually starting using FullStory happy to help while Daniel is out

anmurali commented 3 months ago

I have got one PR for the SignInUser case up, there are some flakey tests

You mean SignUpUser I think?

rushatgabhane commented 3 months ago

sign IN is correct

mountiny commented 3 months ago

SignUp, yes, the signIn already works

mountiny commented 3 months ago

I have got Auth PR up to add the user metadata to the response of CreateLogin which we need for the SignInWithShortLivedAuthToken

then the Web PR can be merged once the Auth change is deployed and that should unblock this

trjExpensify commented 3 months ago

Niceee, thanks Vit! So the Auth deploy should go out tonight on PST with Carlos as the auth deployer this week. Then we can merge the Web PR to have it go out tomorrow GST with Maria as the web deployer this week.

mountiny commented 3 months ago

Yeah that sounds good to me

melvin-bot[bot] commented 3 months ago

@mountiny, @kadiealexander Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot[bot] commented 3 months ago

@mountiny, @kadiealexander Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 3 months ago

@mountiny, @kadiealexander Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

mountiny commented 3 months ago

I believe this is done now