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

[PAID] [$250] [Collect] Workspace - User name and avatar disappears when opening profile immediately after invite #38925

Closed
6 tasks done
izarutskaya opened this issue Mar 25, 2024 · 92 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@izarutskaya
Copy link

izarutskaya commented Mar 25, 2024

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: 1.4.56-2
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Precondition:

  • User is admin of Collect workspace.
  1. Go to staging.new.expensify.com.
  2. Go to Profile > Workspaces > Collect workspace.
  3. Go to Members.
  4. Invite a new member that does not have Expensify account.
  5. Immediately after inviting the member, click on the highlighted new user in the list.

Expected Result:

The user name and avatar will not disappear.

Actual Result:

The user name disappears and avatar changes to placeholder avatar.

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6425580_1711344257027.20240325_132027.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~011d6aa43dad80012f
  • Upwork Job ID: 1778901063514075136
  • Last Price Increase: 2024-04-26
  • Automatic offers:
    • ikevin127 | Reviewer | 0
    • nkdengineer | Contributor | 0
    • bernhardoj | Contributor | 102955490
Issue OwnerCurrent Issue Owner: @strepanier03
@izarutskaya izarutskaya added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 25, 2024
Copy link

melvin-bot bot commented Mar 25, 2024

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

@izarutskaya
Copy link
Author

We think that this bug might be related to #wave-control.

@bernhardoj
Copy link
Contributor

bernhardoj commented Mar 25, 2024

Proposal

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

The user name and avatar disappear if we view the recently added workspace member. We can easily reproduce this by adding the member while offline, viewing the member detail, and go online.

What is the root cause of that problem?

When we start adding a new member, we generate an optimistic personal detail for that user and clear it when the request is successful.

App/src/libs/actions/Policy.ts

Lines 1130 to 1138 in b3baa68

const optimisticData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: membersListKey,
// Convert to object with each key containing {pendingAction: ‘add’}
value: optimisticMembersState,
},
...newPersonalDetailsOnyxData.optimisticData,

App/src/libs/actions/Policy.ts

Lines 1143 to 1161 in b3baa68

const successData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: membersListKey,
// Convert to object with each key clearing pendingAction, when it is an existing account.
// Remove the object, when it is a newly created account.
value: accountIDs.reduce((accountIDsWithClearedPendingAction, accountID) => {
let value = null;
const accountAlreadyExists = !isEmptyObject(allPersonalDetails?.[accountID]);
if (accountAlreadyExists) {
value = {pendingAction: null, errors: null};
}
return {...accountIDsWithClearedPendingAction, [accountID]: value};
}, {}),
},
...newPersonalDetailsOnyxData.finallyData,

That's why after the request is completed, the name and avatar disappear because we are viewing the optimistic personal detail data that is already gone.

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

We can disable it when the personal detail is still an optimistic one until we receive the correct personal detail from the server. This is what we do in several places too, for example:

{ReportUtils.isOptimisticPersonalDetail(accountID) ? (
<Text style={[styles.textStrong]}>{displayName}</Text>
) : (
<Text
style={[styles.textStrong]}
onPress={() => Navigation.navigate(ROUTES.PROFILE.getRoute(accountID))}
suppressHighlighting
>
{displayName}
</Text>
)}

formattedParticipantsList = _.map(formattedParticipantsList, (participant) => ({
...participant,
isDisabled: ReportUtils.isOptimisticPersonalDetail(participant.accountID),
}));

First, add isOptimisticPersonalDetail: true to getNewPersonalDetailsOnyxData.

function getNewPersonalDetailsOnyxData(logins: string[], accountIDs: number[]): Required<Pick<OnyxData, 'optimisticData' | 'finallyData'>> {
const personalDetailsNew: PersonalDetailsList = {};
const personalDetailsCleanup: PersonalDetailsList = {};
logins.forEach((login, index) => {
const accountID = accountIDs[index];
if (allPersonalDetails && Object.keys(allPersonalDetails?.[accountID] ?? {}).length === 0) {
personalDetailsNew[accountID] = {
login,
accountID,
avatar: UserUtils.getDefaultAvatarURL(accountID),
displayName: LocalePhoneNumber.formatPhoneNumber(login),
};

Then, disable it when isOptimisticPersonalDetail is true.

isDisabled:
isPolicyAdmin &&
(accountID === session?.accountID ||
accountID === policy?.ownerAccountID ||
policyMember.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ||
!isEmptyObject(policyMember.errors)),

This will disable the checkbox too, we can customize to not disable the checkbox, but I don't think that's a good idea separating the checkbox disabled state and the row disabled state. Or we can keep it enable, but do nothing when clicked (or just toggle the checkbox when clicked).

optionally we can show not found page on member detail page when it's an optimistic personal detail, but I see we don't do that for profile page, so not sure whether we want to do this or not

What alternative solutions did you explore? (Optional)

Don't clear the optimistic personal detail, but this would keep stacking unnecessary data every time we add a new member. Also, it's useless for the user to see the outdated optimistic personal detail when the server already sends us the correct data.

or don't rerender the detail page when personal details change. we can use memo for this case.

Another alternative is, save the member optimistic login in a ref, then if the member personal details become empty, iterate through the policy members collection and find the one that has the same login. If found, close the current member page and navigate to the new one. But this would fail if we invite the user with their secondary login.

@bernhardoj
Copy link
Contributor

Just FYI, we don't need to perform the 5th step quickly. We can perform it while offline to reproduce it easier.

  1. Go offline
  2. Add a new member (any user that doesn't exist in your contact, it can be a registered expensify user or non-registered, doesn't matter)
  3. Click the member to open the detail
  4. Go online
  5. Wait for a few seconds until the apps reconnecting and the name and avatar will disappear

@strepanier03
Copy link
Contributor

Hmmm, I'm having a slightly different experience with the avatar, but the username isn't an issue when testing Mac/Chrome and Android mobile web for me.


On both here's what I am doing and seeing:

  1. Go to staging.new.expensify.com.
  2. Go to Profile > Workspaces > Collect workspace.
  3. Go to Members.
  4. Invite a new member that does not have Expensify account.
  5. Immediately after inviting the member, click on the highlighted new user in the list.

Expected Result:
The user name and avatar will not disappear and remain as they appear until the user updates them..

Actual Result:
In the RHP the username and avatar remain as they appear initially, but in the member table the avatar changes to a different default avatar. Pressing back and then clicking on the new member, shows the updated avatar.

2024-03-26_16-12-53.mp4

@bernhardoj
Copy link
Contributor

@strepanier03 did you test it on a collect workspace? Looking at the left side of the workspace menu, I guess it's a free workspace. The new RHP for workspace member detail (instead of a profile page that is shown in your video) is only available for collect workspace.

@strepanier03
Copy link
Contributor

strepanier03 commented Mar 27, 2024

I'll retest, I made a collect workspace but maybe clicked the wrong button afterward and added on the free workspace.

@strepanier03
Copy link
Contributor

Running into an issue recreating this. I get a "Hmm ... its not here" in the RHP when clicking on any Collect workspace member.

image

I'm going to ask for a hand repro'ing and then tie to a project.

@strepanier03
Copy link
Contributor

Not sure what was different on the second collect workspace I made but I was able to repro with a fresh slate.

2024-03-27_12-53-12.mp4

@strepanier03 strepanier03 changed the title Workspace - User name and avatar disappears when opening profile immediately after invite [Collect] Workspace - User name and avatar disappears when opening profile immediately after invite Mar 27, 2024
@melvin-bot melvin-bot bot added the Overdue label Apr 1, 2024
Copy link

melvin-bot bot commented Apr 8, 2024

@strepanier03 this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

Copy link

melvin-bot bot commented Apr 8, 2024

@strepanier03 10 days overdue. Is anyone even seeing these? Hello?

Copy link

melvin-bot bot commented Apr 10, 2024

@strepanier03 12 days overdue now... This issue's end is nigh!

@strepanier03 strepanier03 added the External Added to denote the issue can be worked on by a contributor label Apr 12, 2024
@melvin-bot melvin-bot bot changed the title [Collect] Workspace - User name and avatar disappears when opening profile immediately after invite [$250] [Collect] Workspace - User name and avatar disappears when opening profile immediately after invite Apr 12, 2024
Copy link

melvin-bot bot commented Apr 12, 2024

Job added to Upwork: https://www.upwork.com/jobs/~011d6aa43dad80012f

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 12, 2024
Copy link

melvin-bot bot commented Apr 12, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Apr 12, 2024
@nkdengineer
Copy link
Contributor

Proposal

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

The user name disappears and avatar changes to placeholder avatar.

What is the root cause of that problem?

The problem here happens because when we add a new workspace member, if that member doesn't exist in personal details list yet, we'll create optimistic personal details for them and then clearing the personal details after the request is done, see here.

This leads to the user seeing user name and avatar disappearing because the optimistic personal details is gone now.

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

We had this same problem with the optimistic report: we create a new optimistic report with a user, but the report with that user already exists, so what we did is we'll navigate to the new report when the API request is successful.

Here's exactly how we implement it, we should do the same for the optimistic personal detail in this case

  • Do not clear the personal details of the existing users when the API request is successful
  • Modify the back-end to merge preexistingAccountID into the personalDetailsList item of the current optimistic personal detail created in front end. For example, if the optimisticAccountID created in front-end is 123456, and the actual accountID of the existing user is 654321, then the backend will return this Onyx data:
{
  "key": "personalDetailsList",
  "onyxMethod": "merge",
  "value": {
    "123456": {
      "preexistingAccountID": 654321
  }
}
  • Now, in the front-end, when we detect that there's the preexistingAccountID being merged into personalDetailsList, we'll do the following (same as here for report):
    • Check if the user is on the workspace member URL with optimistic account ID, if yes, navigate the user to the workspace member URL with the preexistingAccountID
    • Clear the optimistic personal details

This is the same approach as we did with the optimistic report, just that we do for optimistic personal details now. This will make sure the user has a seamless experience when using the product.

What alternative solutions did you explore? (Optional)

NA

@melvin-bot melvin-bot bot added the Overdue label Apr 15, 2024
@strepanier03
Copy link
Contributor

@ikevin127 - Friendly bump on the proposal here, thanks!

@melvin-bot melvin-bot bot removed the Overdue label Apr 15, 2024
@bernhardoj
Copy link
Contributor

PR is ready

cc: @ikevin127

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jul 10, 2024
@melvin-bot melvin-bot bot changed the title [$250] [Collect] Workspace - User name and avatar disappears when opening profile immediately after invite [HOLD for payment 2024-07-17] [$250] [Collect] Workspace - User name and avatar disappears when opening profile immediately after invite Jul 10, 2024
Copy link

melvin-bot bot commented Jul 10, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jul 10, 2024
Copy link

melvin-bot bot commented Jul 10, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.5-13 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-07-17. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Jul 10, 2024

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@ikevin127] The PR that introduced the bug has been identified. Link to the PR:
  • [@ikevin127] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@ikevin127] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@ikevin127] Determine if we should create a regression test for this bug.
  • [@ikevin127] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@strepanier03] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@bernhardoj
Copy link
Contributor

I'll request in ND once payment is due.

@ikevin127
Copy link
Contributor

Regression Test Proposal

Precondition: User is admin of a Collect workspace.

  1. Go to Profile > Workspaces > Collect workspace.
  2. Go to Members.
  3. Invite a new member that does not have an Expensify account, using an email address.
  4. Immediately after inviting the member, verify that the newly added member row is disabled until the BE returns the created user information. While this happens, the default avatar should be shown.

Do we agree 👍 or 👎.

@nkdengineer
Copy link
Contributor

We can discuss the compensation for @nkdengineer once the issue is fixed.

@techievivek Could you help with the discussion here, thanks 🙇

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jul 15, 2024
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2024-07-17] [$250] [Collect] Workspace - User name and avatar disappears when opening profile immediately after invite [HOLD for payment 2024-07-22] [HOLD for payment 2024-07-17] [$250] [Collect] Workspace - User name and avatar disappears when opening profile immediately after invite Jul 15, 2024
Copy link

melvin-bot bot commented Jul 15, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.6-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-07-22. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Jul 15, 2024

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@ikevin127] The PR that introduced the bug has been identified. Link to the PR:
  • [@ikevin127] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@ikevin127] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@ikevin127] Determine if we should create a regression test for this bug.
  • [@ikevin127] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@strepanier03] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@ikevin127
Copy link
Contributor

ikevin127 commented Jul 15, 2024

@strepanier03 This was deployed to production 5 days ago (reference) meaning payment is due on 17th of July.
BZ checklist already completed in #38925 (comment).

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 16, 2024
@bernhardoj
Copy link
Contributor

As @ikevin127 mentioned, the payment is due on the 17, so I've requested it in ND.

@strepanier03
Copy link
Contributor

@bernhardoj - I closed the contract in Upwork without payment since you're requesting in Newdot now.

@ikevin127 - I've paid your contract and closed it as well.


@nkdengineer and @techievivek - What payment should I authorize here? The full $250?

@strepanier03
Copy link
Contributor

strepanier03 commented Jul 17, 2024

Payment summary

@JmillsExpensify - Request incoming.

@JmillsExpensify
Copy link

$250 approved for @bernhardoj

@strepanier03 strepanier03 changed the title [HOLD for payment 2024-07-22] [HOLD for payment 2024-07-17] [$250] [Collect] Workspace - User name and avatar disappears when opening profile immediately after invite [PAID] [$250] [Collect] Workspace - User name and avatar disappears when opening profile immediately after invite Jul 19, 2024
@github-project-automation github-project-automation bot moved this from Polish to Done in [#whatsnext] #wave-collect Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
No open projects
Archived in project
Development

No branches or pull requests