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

[$1000] After creating a group with a new user, user is automatically removed from the group (or we navigate away) #20812

Closed
6 tasks
Beamanator opened this issue Jun 15, 2023 · 75 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@Beamanator
Copy link
Contributor

Beamanator commented Jun 15, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. click on "+" icon
  2. then go to create group section
  3. select user from list
  4. add new user from search
  5. select the random email and select that email
  6. Click on create group
  7. Observe the group

Expected Result:

The optimistic report should remain, and you should be able to type in the chat

Actual Result:

The new user is removed for the group, or maybe we navigate to a different report

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: main
Reproducible in staging?: N
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
Issue reported by: @niravkakadiya25
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686831616374889

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01e256c96a0de2398f
  • Upwork Job ID: 1670720417402716160
  • Last Price Increase: 2023-06-19
@Beamanator Beamanator added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 15, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 15, 2023

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Jun 15, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@BeeMargarida
Copy link
Contributor

I can't reproduce this with web but in iOS I can 🤔

@BeeMargarida
Copy link
Contributor

BeeMargarida commented Jun 16, 2023

@Beamanator This seems to be something on the backend, the new report data is sent correctly, the params with the emailList too, the optimistic data is also correct, but when it receives the updated data from the backend, the new user data is not in the participantsAccountIDs nor participants

@dylanexpensify
Copy link
Contributor

@Beamanator I'm not able to reproduce on my side with iOS - can you elaborate a bit more what we should see?

@melvin-bot melvin-bot bot added the Overdue label Jun 19, 2023
@dylanexpensify
Copy link
Contributor

Bumping this @Beamanator!

@melvin-bot melvin-bot bot removed the Overdue label Jun 19, 2023
@Beamanator
Copy link
Contributor Author

Yes sorry for the delay! 🙏

@dylanexpensify you're also not able to reproduce on iOS? 😅 Basically the user was seeing that, if they create a group with 2 users - 1 existing and 1 brand new one that doesn't exist yet (a.k.a. creating the group will create their account), they saw that:

  1. At first, the group was created successfully
  2. After a few seconds, the brand new user was removed from the group 😅

If you're not seeing that on staging iOS, maaaaaaaybe we can close this out? Or ask for better reproduction steps?

@dylanexpensify
Copy link
Contributor

Ahh I'll try again for good measure, but may need to see about repro steps bein better!! Thanks man!

@dylanexpensify
Copy link
Contributor

Ahh was able to repro! Thanks for the check @Beamanator!!

@dylanexpensify dylanexpensify added the External Added to denote the issue can be worked on by a contributor label Jun 19, 2023
@melvin-bot melvin-bot bot changed the title After creating a group with a new user, user is automatically removed from the group (or we navigate away) [$1000] After creating a group with a new user, user is automatically removed from the group (or we navigate away) Jun 19, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 19, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01e256c96a0de2398f

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 19, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 19, 2023

Current assignee @dylanexpensify is eligible for the External assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Jun 19, 2023

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

@waterim
Copy link
Contributor

waterim commented Jun 19, 2023

Hi!
I'm Artem from Callstack - expert contributor group
Maybe I could help with this one?

@melvin-bot
Copy link

melvin-bot bot commented Jun 19, 2023

📣 @waterim! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@AbeerAhmad
Copy link

how to join Expensify slack account

@melvin-bot
Copy link

melvin-bot bot commented Jun 19, 2023

📣 @AbeerAhmad! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@AbeerAhmad
Copy link

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/abeerahmad204

@melvin-bot
Copy link

melvin-bot bot commented Jun 19, 2023

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@nitin-kukreti
Copy link

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01cab64261c95a66c5

@melvin-bot
Copy link

melvin-bot bot commented Jun 19, 2023

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@waterim
Copy link
Contributor

waterim commented Jun 26, 2023

@nitin-kukreti As I understand, we cant create group for just 1 user. @s77rt Am I right?

Because Im getting something like this after creating a group just with 1 new user and trying to send a message:
image

Updated!
With old users it works fine(we can create a group with 1 user), just for a new user it looks like on the screenshot

@nitin-kukreti
Copy link

@waterim no we can create group with one person (also with person which not exist)

@waterim
Copy link
Contributor

waterim commented Jun 26, 2023

image participantAccountIDs array is empty in this case

@waterim
Copy link
Contributor

waterim commented Jun 26, 2023

@s77rt
Here is what Im getting as a response for a newly created user after creating a group
image

@Beamanator
Copy link
Contributor Author

I'd recommend looking in personalDetailsList, not personalDetails (keyed by accountID, not email) - I think we fixed the -1 accountID problem in personalDetailsList, hopefully :D

@waterim
Copy link
Contributor

waterim commented Jun 26, 2023

@Beamanator Here I dont have even my newly created user :D
image

@nitin-kukreti
Copy link

@Beamanator when we are requesting from mobile devices for gerrating new Report with a user which not exit giving accounId=-1 in api response but when we are performaing same operation from desktop web we are getting accurate response in which accountId is not -1.
does we are using different API end points for mobile and web ?

@Beamanator
Copy link
Contributor Author

@waterim aahhhh this is the response to an API command, sorry i thought you were looking in Onyx directly :D

@nitin-kukreti that's quite strange, it's very unlikely that the data "should" be sent / returned different depending on the platform (mobile vs web) - we strive to make it all the same so we don't have weird edge cases for 1 platform

@s77rt
Copy link
Contributor

s77rt commented Jun 27, 2023

@Beamanator Is the issue still reproducible from your end? (test against staging server). Currently it appears that only @nitin-kukreti is able to reproduce which is odd

@nitin-kukreti
Copy link

nitin-kukreti commented Jun 28, 2023

@Beamanator and @s77rt
It is a server-side error
I attaching some response portion of both mWeb and Web
here accountId=-1
this response from mWeb when first time we create Group with new user which not exit yet but after reloading response is OK.

MWeb

Before Reloading

image

After Reloading

Response Is totally different (Before it was array of 7 but after reloading on new request its only array of 5 as In previous response two entry were duplicate ) but accoudId exist for newly created user its not -1

image

Web

response in web is similar to response of mWeb but here account Id is not -1 but their also have two duplicate entry

image

I hope this will be helpful to resolve this one 😊.

@s77rt
Copy link
Contributor

s77rt commented Jun 28, 2023

@nitin-kukreti I really doubt we get different responses based on the device. I'm still unable to reproduce the bug. Also the personalDetails is deprecated. You should just check personalDetailsList. As for the duplicated entries I have reported this here as well.

@Beamanator
Copy link
Contributor Author

I really doubt we get different responses based on the device.

Agreed response shouldn't be different based on device - check you're on the same env (staging / production) on both 🙏

Also the personalDetails is deprecated. You should just check personalDetailsList.

Agreed 👍 We'll be removing all personalDetails data very soon b/c we don't use it in the App anymore - we will use personalDetailsList from now on

As for the duplicated entries I have reported this #21600 (comment) as well.

👍

@nitin-kukreti
Copy link

@Beamanator
I am rechecking that asap

@nitin-kukreti
Copy link

@Beamanator @s77rt

MWeb

not getting detail of newly created user

{
            "onyxMethod": "merge",
            "key": "personalDetailsList",
            "value": {
                "14923263": {
                    "accountID": 14923263,
                    "avatar": "https:\/\/d2k5nsl2zxldvw.cloudfront.net\/images\/avatars\/default-avatar_21.png",
                    "displayName": "[email protected]"
                }
            }
        }

after refreshing page

same report open but this time response is correct

{
          "onyxMethod": "merge",
          "key": "personalDetailsList",
          "value": {
              "14943429": {
                  "accountID": 14943429,
                  "avatar": "https:\/\/d2k5nsl2zxldvw.cloudfront.net\/images\/avatars\/default-avatar_6.png",
                  "displayName": "[email protected]"
              }
          }
      },

Web

Every thing is ok

 {
            "onyxMethod": "merge",
            "key": "personalDetailsList",
            "value": {
                "14943466": {
                    "accountID": 14943466,
                    "avatar": "https:\/\/d2k5nsl2zxldvw.cloudfront.net\/images\/avatars\/default-avatar_16.png",
                    "displayName": "[email protected]"
                },
                "14893351": {
                    "accountID": 14893351,
                    "avatar": "https:\/\/d1wpcgnaa73g0y.cloudfront.net\/00e8d12809980f19e9830837925eba7f04c7b31c_128.jpeg",
                    "displayName": "Nitin Kukreti"
                }
            }
        },

after Reload


{
            "onyxMethod": "merge",
            "key": "personalDetailsList",
            "value": {
                "14943466": {
                    "accountID": 14943466,
                    "avatar": "https:\/\/d2k5nsl2zxldvw.cloudfront.net\/images\/avatars\/default-avatar_16.png",
                    "displayName": "[email protected]"
                }
            }
        },

Apart form this problem I notice a inconsistent API response after reloading page we are getting different response for same report in key=personalDetailsList

@melvin-bot
Copy link

melvin-bot bot commented Jun 29, 2023

@s77rt @waterim @dylanexpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@s77rt
Copy link
Contributor

s77rt commented Jun 29, 2023

@nitin-kukreti That's weird 😅 Just double check that all those request are being sent to the correct server and that is the staging server. I don't know why you are the only one who can reproduce this. From my end both web and mweb return same data

@s77rt
Copy link
Contributor

s77rt commented Jun 29, 2023

Melvin

Are we close to approving a proposal?

We are not even close to reproducing the bug 😅 That's a good thing I hope

@melvin-bot melvin-bot bot added the Overdue label Jul 3, 2023
@dylanexpensify
Copy link
Contributor

reviewing today and posting next steps!

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jul 3, 2023
@dylanexpensify
Copy link
Contributor

@s77rt can you repro? Or should we close this out? cc @Beamanator

@melvin-bot melvin-bot bot removed the Overdue label Jul 6, 2023
@Beamanator
Copy link
Contributor Author

Hoping we can close this out 🙏

@s77rt
Copy link
Contributor

s77rt commented Jul 6, 2023

@dylanexpensify I can't repro. Let's close this

@melvin-bot
Copy link

melvin-bot bot commented Jul 6, 2023

@s77rt @waterim @dylanexpensify this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

@s77rt
Copy link
Contributor

s77rt commented Jul 7, 2023

Melvin we will probably close this one

@dylanexpensify
Copy link
Contributor

Closing!

@dylanexpensify dylanexpensify closed this as not planned Won't fix, can't repro, duplicate, stale Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

8 participants