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.32k stars 2.76k forks source link

[HOLD for payment 2024-09-16][$250] mWeb - Chat - In contacts a random guest user is displayed after login via public room link #47806

Open lanitochka17 opened 3 weeks ago

lanitochka17 commented 3 weeks ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.22 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to this link as signed out user https://staging.new.expensify.com/r/7075912447943023
  2. Tap sign in
  3. Enter credentials and log in
  4. Navigate to LHN
  5. Tap fab -- start chat
  6. In contacts, note a guest user is added.

Expected Result:

In contacts, a random guest user must not be displayed after login via public room link

Actual Result:

In contacts, a random guest user is displayed after login via public room link

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/55f56373-f43a-467d-822e-2343ad068345

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0148f0358fba85f9ac
  • Upwork Job ID: 1828135607297326184
  • Last Price Increase: 2024-08-26
  • Automatic offers:
    • jjcoffee | Reviewer | 103703199
    • dominictb | Contributor | 103703202
Issue OwnerCurrent Issue Owner: @jjcoffee
melvin-bot[bot] commented 3 weeks ago

Triggered auto assignment to @CortneyOfstad (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.

lanitochka17 commented 3 weeks ago

@CortneyOfstad FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

dominictb commented 3 weeks ago

Edited by proposal-police: This proposal was edited at 2023-10-07T14:32:22Z.

Proposal

Please re-state the problem that we are trying to solve in this issue.

In contacts, a random guest user is displayed after login via public room link

What is the root cause of that problem?

When opening a public room as an anonymous user, the session data is:

accountID: A,
email: x@expensify.anon

and we create a personal detail for it:

accountID: A,
displayName: "Guest",
lastName: "Guest"

and after signing, we do not clear these data, so these data appear in contact list.

What changes do you think we should make in order to solve the problem?

Can use useEffect:

    const isAnonymousUser = session?.authTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS;
    const prevIsAnonymousUser = usePrevious(isAnonymousUser);
    useEffect(() => {
        if (prevIsAnonymousUser && !isAnonymousUser) {
            removeAnonFromPersonalDetails();
        }
    }, [prevIsAnonymousUser, isAnonymousUser]);

Can cleanup the anon personal details when the sign in API is called successfully, such as in: https://github.com/Expensify/App/blob/f12c1fff5f5344efb20cc48e2805e766c6b5523d/src/libs/actions/Session/index.ts#L520 and https://github.com/Expensify/App/blob/f12c1fff5f5344efb20cc48e2805e766c6b5523d/src/libs/actions/Session/index.ts#L588

dominictb commented 3 weeks ago

Proposal Updated

CortneyOfstad commented 2 weeks ago

I am confused by the steps, as it specifically mentions In contacts, note a guest user is added.

The contact was not added to the chat, but rather a suggestion. And this is by design.

@lanitochka17 does the tester recognize that email in any capacity in regards to Expensify or testing within Expensify?

dominictb commented 2 weeks ago

@CortneyOfstad If we do not sign in and then visit the public room, a random personal detail data is assigned to us. That data is something like:

accountID: A,
displayName: "Guest",
lastName: "Guest",
login: random@expensify.anon,

Then when signing in, the above random user data is not cleared, so it is displayed in the contact list.

CortneyOfstad commented 2 weeks ago

Thank you @dominictb for the clarification!

I am having trouble recreating this, so I am going to have this retest by QA πŸ‘

melvin-bot[bot] commented 2 weeks ago

Job added to Upwork: https://www.upwork.com/jobs/~0148f0358fba85f9ac

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @jjcoffee (External)

melvin-bot[bot] commented 2 weeks ago

@CortneyOfstad Whoops! This issue is 2 days overdue. Let's get this updated quick!

jjcoffee commented 2 weeks ago

@dominictb's proposal LGTM! I think cleaning up on the API success callback seems the neatest way to do this as we can then pass the accountID to clear from the session data. Happy to listen to arguments otherwise if I've missed anything!

:ribbon::eyes::ribbon: C+ reviewed

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @dangrous, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

dangrous commented 2 weeks ago

Yeah updating it in the success data makes sense to me, then we already know which account ID to remove.

melvin-bot[bot] commented 2 weeks ago

πŸ“£ @jjcoffee πŸŽ‰ An offer has been automatically sent to your Upwork account for the Reviewer role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 2 weeks ago

πŸ“£ @dominictb πŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Keep in mind: Code of Conduct | Contributing πŸ“–

CortneyOfstad commented 1 week ago

PR is still being reviewed!

bfitzexpensify commented 6 days ago

@dominictb @jjcoffee can you please take a look at https://github.com/Expensify/App/issues/48715 to see if it is related to your changes? It was found while executing https://github.com/Expensify/App/pull/48144

mvtglobally commented 3 days ago

Issue not reproducible during KI retests. (First week)

VictoriaExpensify commented 3 days ago

Bumping Ben's message above - @jjcoffee I think you were looking into this:

@dominictb @jjcoffee can you please take a look at https://github.com/Expensify/App/issues/48715 to see if it is related to your changes? It was found while executing https://github.com/Expensify/App/pull/48144

It looks like these are probably related issues

jjcoffee commented 3 days ago

I can't repro after reverting the changes from the PR, so I don't think it's related.

dominictb commented 3 days ago

I can reproduce that bug in prod, so I think it is not related to PR, because it is not deployed to prod.

cc @bfitzexpensify

jjcoffee commented 1 day ago

Looks like the automation failed again! This hit production 2024-09-09, so this should be held for payment 2024-09-16. cc @CortneyOfstad

CortneyOfstad commented 21 hours ago

Thanks @jjcoffee!