[$250] Workspace - Fallback avatar in workspace chat is clickable and leads to not here page

[lanitochka17]

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

---

Version Number: 9.0.4-0 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 staging.new.expensify.com
2. Go offline
3. Go to workspace settings > Members
4. Invite a non-existing user
5. Go to workspace chat with the invited user
6. Click on the chat header
7. Click on the fallback avatar (invited user)

Expected Result:

The fallback avatar should not be clickable

Actual Result:

The fallback avatar is clickable and leads to not here page

Workaround:

Unknown

Platforms:

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

• [x] Android: Native
• [x] Android: mWeb Chrome
• [x] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/3ae651c8-420d-49cf-832e-6b279ba781bb

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0189b738ae283ab8e4
• Upwork Job ID: 1811060566160624462
• Last Price Increase: 2024-07-10

Issue Owner

Current Issue Owner: @aimane-chnaif

[melvin-bot[bot]]

Triggered auto assignment to @miljakljajic (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]

@miljakljajic 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

[lanitochka17]

We think that this bug might be related to #wave-collect - Release 1

[daledah]

Proposal

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

The fallback avatar is clickable and leads to not here page

What is the root cause of that problem?

In offline mode, we can't call successfully openPublicProfilePage and can't get avatarURL in https://github.com/Expensify/App/blob/f73cdc44664566f9f2012cc9f7b34769391049e3/src/pages/settings/Profile/ProfileAvatar.tsx#L48

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

In the function openPublicProfilePage we can set avatarUrl to the default avatar when its call fails

What alternative solutions did you explore? (Optional)

We'll not allow user to navigate to the avatar page when it don't have data and in offline mode NA

[cretadn22]

Proposal

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

Workspace - Fallback avatar in workspace chat is clickable and leads to not here page

What is the root cause of that problem?

https://github.com/Expensify/App/blob/f73cdc44664566f9f2012cc9f7b34769391049e3/src/components/RoomHeaderAvatars.tsx#L77

In the navigateToAvatarPage function, we haven't included logic to prevent the user from navigating to the avatar page if the avatar is default

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

In the ProfileAvatar component, we currently have logic to display a "not found" page if we cannot find personalDetail matching the accountID prop. We should implement the same logic in the navigateToAvatarPage function

https://github.com/Expensify/App/blob/f73cdc44664566f9f2012cc9f7b34769391049e3/src/components/RoomHeaderAvatars.tsx#L25-L27

We need to verify whether we can find a personalDetail that matches the icon.id. If we can find a match, then we'll allow the user to navigate to the avatar page.

Even though, we can display the disable pointer when the user hovers over the default avatar

What alternative solutions did you explore? (Optional)

On the Header page, we also disable the header view, including interactions with the avatar. https://github.com/Expensify/App/blob/ec196638296045a64ced8baed6e4ba358c70f902/src/pages/home/HeaderView.tsx#L175-L180 Here, we use the function isOptimisticPersonalDetail. https://github.com/Expensify/App/blob/ec196638296045a64ced8baed6e4ba358c70f902/src/libs/ReportUtils.ts#L1133

I recommend that we do the same in RoomHeaderAvatars (there are two places that need to apply the disabled props)

disabled={ReportUtils.isOptimisticPersonalDetail(icon.id)}

Note that icon.id refers to accountID as defined

https://github.com/Expensify/App/blob/ec196638296045a64ced8baed6e4ba358c70f902/src/libs/ReportUtils.ts#L2049

[neonbhai]

Proposal

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

Workspace - Fallback avatar in workspace chat is clickable and leads to not here page

What is the root cause of that problem?

Whe navigating to avatar page here, we do not check if the avatar is the default Fallback avatar.

We should disable navigating if the avatar is the fallback avatar.

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

We should have a check that disables clicking on the avatar here and here when it is the fallback avatar: https://github.com/Expensify/App/blob/f73cdc44664566f9f2012cc9f7b34769391049e3/src/components/RoomHeaderAvatars.tsx#L39-L44

disabled={icon.source === Expensicons.FallbackAvatar}

Result

https://github.com/Expensify/App/assets/64629613/347e51ea-1423-4649-a41d-5467782f536a

[melvin-bot[bot]]

@miljakljajic Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~0189b738ae283ab8e4

[melvin-bot[bot]]

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

[aimane-chnaif]

@cretadn22 thanks for the proposal. I think it's better to disable press event (than pressable + no action) when we know that it will lead to not found page. The app performance is concerned here?

@neonbhai thanks for the proposal. Can you confirm that

• if icon.source is Expensicons.FallbackAvatar, it always leads to not found page?
• there's no other case of showing not found page?

[cretadn22]

@aimane-chnaif

Updated proposal: Adding an alternative solution

On the Header page, we also disable the header view, including interactions with the avatar. https://github.com/Expensify/App/blob/ec196638296045a64ced8baed6e4ba358c70f902/src/pages/home/HeaderView.tsx#L175-L180 Here, we use the function isOptimisticPersonalDetail. https://github.com/Expensify/App/blob/ec196638296045a64ced8baed6e4ba358c70f902/src/libs/ReportUtils.ts#L1133

I recommend that we do the same in RoomHeaderAvatars (there are two places that need to apply the disabled props)

disabled={ReportUtils.isOptimisticPersonalDetail(icon.id)}

Note that icon.id refers to accountID as defined

https://github.com/Expensify/App/blob/ec196638296045a64ced8baed6e4ba358c70f902/src/libs/ReportUtils.ts#L2049

[neonbhai]

if icon.source is Expensicons.FallbackAvatar, it always leads to not found page?

Confirmed! Shows not found in all contexts (1:1, group chats and workspace rooms)

there's no other case of showing not found page?

Confirmed, unable to find other case where the avatar leads to not found page

cc: @aimane-chnaif

[melvin-bot[bot]]

@miljakljajic, @aimane-chnaif Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[aimane-chnaif]

reviewing updated proposal

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[aimane-chnaif]

While both contributors proposed working solution, I prefer @neonbhai's proposal as icon.source check is enough. (confirmed optimistic personal detail avatar is always Expensicons.FallbackAvatar) Getting all personal details just to check avatar existence is heavy and not performant. 🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[dangrous]

Yep, that works for me!

[melvin-bot[bot]]

📣 @neonbhai 🎉 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 📖

[melvin-bot[bot]]

@dangrous @miljakljajic @neonbhai @aimane-chnaif 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!

[neonbhai]

Raising PR soon!

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[neonbhai]

Issue not reproducible during KI retests. (First week)

The flow is still broken, now we see infinite spinner on the modal. Raising PR to disable navigation according to proposal here

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

• https://github.com/Expensify/App/pull/45999

If no regressions arise, payment will be issued on 2024-08-02. :confetti_ball:

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

• @neonbhai requires payment automatic offer (Contributor)
• @aimane-chnaif requires payment through NewDot Manual Requests

[melvin-bot[bot]]

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:

• [ ] [@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
• [ ] [@aimane-chnaif] 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:
• [ ] [@aimane-chnaif] 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:
• [ ] [@aimane-chnaif] Determine if we should create a regression test for this bug.
• [ ] [@aimane-chnaif] 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.
• [ ] [@miljakljajic] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[miljakljajic]

@aimane-chnaif owed 250 for their work reviewing this issue

[miljakljajic]

@neonbhai has been paid in Upwork

[miljakljajic]

Do we need a regression test?

[dangrous]

couldn't hurt!