[HOLD for payment 2023-07-28] [$1000] Web -Default avatar is changed when we select/unselect a member in Workspace invitation page #22085

kbecciv · 2023-07-02T22:56:33Z

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

Action Performed:

Go to any Workspace
Click Invite button in top header
Invite an member that you haven't chatted before
Select member in search result list
Unselect above selecting member

Expected Result:

The default avatar of member should be same every time we select/unselect

Actual Result:

The default avatar of member is changed every time we select/unselect

Workaround:

Unknown

Platforms:

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

Version Number: 1.3.35
Reproducible in staging?: y
Reproducible in production?: y
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

Screen.Recording.2023-07-02.at.00.23.12.1.mov

Gravar.2736.mp4

Expensify/Expensify Issue URL:
Issue reported by: @hoangzinh
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1688232507431689

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~018119c930be0fc798
Upwork Job ID: 1676185302459514880
Last Price Increase: 2023-07-11

melvin-bot · 2023-07-02T22:56:38Z

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

melvin-bot · 2023-07-02T22:56:40Z

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

honnamkuan · 2023-07-03T10:07:49Z

Proposal

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

When new user unregistered is being invited to a workspace via "Invite new members" page, selecting and deselecting the user row resulted in a change in avatar photo.

What is the root cause of that problem?

The userToInvite avatar icon is generated from optimisticAccountID

App/src/libs/OptionsListUtils.js

Line 801 in b0add94

source: UserUtils.getAvatar('', optimisticAccountID),

optimisticAccountID is generated using UserUtils.generateAccountID, it get randomised to a new ID every time whenever the row is deselected and the option list get rebuilt, that causes the unregistered user avatar get changed.

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

We should generate number only hashCode out of lowercase search term (so that test@gmail.com & TEST@gmail.com returns the same avatar), to get a consistent positive number only accountID based on the new user login.

function generateAccountID(login) {
    // reusing hashCode, abs because hashCode can return negative number
    return Math.abs(hashCode(login.toLowerCase()));
}

All existing usages of UserUtils.generateAccountID will be modified to pass in user login.

OptionsListUtils.js
PersonalDetailsUtils.js
Task.js
DetailsPage.js

for consistency across the app

melvin-bot · 2023-07-04T11:04:54Z

Job added to Upwork: https://www.upwork.com/jobs/~018119c930be0fc798

melvin-bot · 2023-07-04T11:04:58Z

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

melvin-bot · 2023-07-04T11:05:04Z

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

dukenv0307 · 2023-07-04T11:55:55Z

Proposal

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

The default avatar of member is changed every time we select/unselect

What is the root cause of that problem?

We're using optimisticAccountID as the basis to generate the default avatar

App/src/libs/OptionsListUtils.js

Line 801 in 3e02ef4

source: UserUtils.getAvatar('', optimisticAccountID),

, this is not ideal since the optimisticAccountID will be regenerated every time and does not have any correlation with the login that is used as search query.

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

We need to use the user login (aka the search query) to generate the default avatar so we'll see the same consistent default avatar across all searches from different accounts.

This was what was done before, reverting to it will not cause any issue with our new security practice of using the accountID (since we're not revealing any sensitive user data here, we're just showing the default avatar based on the search query/login that's typed in by the user)

What alternative solutions did you explore? (Optional)

Another way is to change the algorithm of generating the optimistic account ID a bit so that we can consistently generate the avatar.

Currently we generate a 16-digit account ID, we can add 4 more digits at the beginning which is the hash of the login
[4-digit hash of login][16-digit randomly generated as currently]
This will only increase, not decrease the randomness of the generated accountID.
Then we can use the 4-digit at the beginning to generate the default avatar rather than using the full accountID.

DrLoopFall · 2023-07-04T14:18:44Z

Proposal

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

The default avatar is changed whenever the member is unselected from the invitation page.

What is the root cause of that problem?

We use optimisticAccountID to generate the avatar, but optimisticAccountID changes whenever the member is unselected, resulting in a different avatar.

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

Generate optimisticAccountID based on login/search which would result in the same avatar for that login.

function generateAccountID(login) {
	// can't use hashText, it returns a 32bit integer
    return hash(login);
    
	// or
	const hash = hashText(login)
	return Number(`${hash}${hashText(login + hash)}`)
}

allroundexperts · 2023-07-06T07:53:32Z

I agree with @dukenv0307's proposal. I think we should generate optimistic account id based on the search term that the user entered. I also don't see any security implication of this. Before we proceed, I would like to get a confirmation about this from @Beamanator or @puneetlath since they've worked on it more extensively.

DrLoopFall · 2023-07-06T08:06:09Z

Hi @allroundexperts,
Please read the proposals again, I proposed to generate optimisticAccountID based on the search term, whereas @dukenv0307 proposed to generate the default avatar based on search term, not to generate the optimisticAccountID.

Beamanator · 2023-07-06T08:10:05Z

@allroundexperts

I think we should generate optimistic account id based on the search term that the user entered. I also don't see any security implication of this.

I agree no security implications - the optimistic account ID doesn't get to the backend and is only used on the frontend for these brand new accounts, so feel free to proceed with whichever proposal seems best 👍

honnamkuan · 2023-07-06T08:11:16Z

@allroundexperts thanks for reviewing the proposals.

Wondering if you have missed my proposal #22085 (comment) at the top which is about generating optimisticAccountID from search term (and caches it) as well.

~~I actually tried out what's proposed in @dukenv0307 's solution but it does not seems to work, the avatar still changes when row is selected and and deselected.~~

I actually tried out what's proposed in @dukenv0307 's solution but found it works only in Workspace invite page, the bug stills occurs in other pages like Bill Splitting page with new user, where the avatar still changes when user is selected and and deselected.

My proposal would fix it for all app pages.

allroundexperts · 2023-07-06T08:18:41Z

@allroundexperts thanks for reviewing the proposals.

Wondering if you have missed my proposal #22085 (comment) at the top which is about generating optimisticAccountID from search term (and caches it) as well.

I actually tried out what's proposed in @dukenv0307 's solution but it does not seems to work, the avatar still changes when row is selected and and deselected.

@honnamkuan I think with the approach you've suggested, we'll be getting different avatars for the same search term whenever the app is refreshed. Let me know if that is not the case.

honnamkuan · 2023-07-06T08:41:44Z

Yes we will get getting different avatar if user choose to search for a new user, then decide not proceed with what he is doing, refresh the app, and search for the same user again, but I think that is not something user would do in their usage workflow.

On the plus side, given my fixings are done on UserUtils.generateAccountID function, the user will be getting the consistent avatar for the same new user login everywhere in the app & not just in workspace invite, for example, CMD+K search list, personal detail page, task assignment page.

allroundexperts · 2023-07-06T08:42:50Z

Yes we will get getting different avatar if user choose to search for a new user, then decide not proceed with what he is doing, refresh the app, and search for the same user again, but I think that is not something user would do in their usage workflow.

On the plus side, given my fixings are done on UserUtils.generateAccountID function, the user will be getting the consistent avatar for the same new user login everywhere in the app & not just in workspace invite, for example, CMD+K search list, personal detail page, task assignment page.

I think we would want consistent avatars here.

honnamkuan · 2023-07-06T09:19:58Z

@allroundexperts thanks for the feedbacks, that is understandable.

I have updated my proposal #22085 (comment) to propose hashing the lowercased login to get a positive number only accountID, and update all usages of the functions, that will ensures consistent avatar shown across all EApp pages and page refresh.

Appreciate your review on it

eh2077 · 2023-07-06T09:28:49Z

@akinwale may also have a proposal for this from a previous issue.

honnamkuan · 2023-07-06T09:32:40Z

Amended my comment above #22085 (comment) to provide better clarity on my findings regarding @dukenv0307 's proposal.

~~I actually tried out what's proposed in @dukenv0307 's solution but it does not seems to work, the avatar still changes when row is selected and and deselected.~~

I actually tried out what's proposed in @dukenv0307 's solution but found it works only in Workspace invite page, the bug stills occurs in other pages like Bill Splitting page with new user, where the avatar still changes when user is selected and and deselected.

My proposal would fix it for all app pages.

dukenv0307 · 2023-07-06T09:50:49Z

@honnamkuan The approach is the only thing that's needed in the proposal, whichever approach is selected will of course be applied across the app. I'm the first to suggest to generate optimisticAccountID based on login/search.

honnamkuan · 2023-07-06T10:14:59Z

I read your proposal again, sorry to say I have the same understanding as @DrLoopFall #22085 (comment) when it comes to your proposal.

... whereas @dukenv0307 proposed to generate the default avatar based on search term, not to generate the optimisticAccountID.

If you ask me, the approach in mine (both previous and current one) and DrLoopFall's proposal is to generate optimisticAccountID based on login/search; whereas yours is about generating avatar based on login/search

Quoting summarised text from your proposal..

What is the root cause of that problem?
We're using optimisticAccountID as the basis to generate the default avatar

What changes do you think we should make in order to solve the problem?
We need to use the user login (aka the search query) to generate the default avatar

puneetlath · 2023-07-06T20:38:18Z

I agree with @dukenv0307's #22085 (comment). I think we should generate optimistic account id based on the search term that the user entered.

That seems reasonable to me.

akinwale · 2023-08-03T11:25:19Z

@akinwale mind applying here?? 🙇‍♂️

Applied! Thanks.

allroundexperts · 2023-08-03T15:40:23Z

@dylanexpensify Can you please add payments summary here?

Julesssss · 2023-08-07T10:25:52Z

Bump for payment summary @dylanexpensify

dylanexpensify · 2023-08-10T10:18:33Z

Payment Summary:

Issue reporter: @akinwale $250 @hoangzinh $250
Contributor +: @allroundexperts $1,500
Contributor: @dukenv0307 $1,500

Upwork job here

allroundexperts · 2023-08-10T10:21:45Z

@dylanexpensify I think this is eligible for bonus because it was reviewed / approved within 3 days. Can you please confirm?

dylanexpensify · 2023-08-10T10:22:42Z

reviewing!

dylanexpensify · 2023-08-10T10:23:43Z

Correct! Updating prices!

dylanexpensify · 2023-08-10T10:24:02Z

Updated! Thanks @allroundexperts!

dylanexpensify · 2023-08-10T10:27:19Z

@akinwale sent offer!

akinwale · 2023-08-10T10:29:49Z

@akinwale sent offer!

Accepted, thanks!

dylanexpensify · 2023-08-10T10:32:25Z

Everything done here - just pending C+ ND payout!

hoangzinh · 2023-08-10T10:34:15Z

@dylanexpensify can I get Issue reporter payout as well? Just back read chats. I agree that two issue are same root cause, but different steps to reproduce issue.

dylanexpensify · 2023-08-10T11:09:54Z

That's fair, yes you can. Updated comment

dylanexpensify · 2023-08-10T11:10:56Z

Paid @hoangzinh

hoangzinh · 2023-08-10T14:08:34Z

Thanks @dylanexpensify

dylanexpensify · 2023-08-11T19:55:34Z

Waiting for @allroundexperts to be paid

dylanexpensify · 2023-08-14T08:27:55Z

Reviewing today!

dylanexpensify · 2023-08-14T12:17:50Z

@allroundexperts you're paid in NewDot right?

allroundexperts · 2023-08-14T12:19:52Z

@dylanexpensify Not yet. Usually, someone from the team posts a confirmation message here once paid.

dylanexpensify · 2023-08-14T12:21:00Z

Ahh got it! Mind applying at the above job then? I'll get you sorted today!

allroundexperts · 2023-08-14T12:22:10Z

Thanks Dylan but I'll get paid through ND. I'm not in a rush so its fine if they're taking time to approve.

dylanexpensify · 2023-08-14T12:23:31Z

Ah I see, I thought you meant not yet as in not paid via new dot yet 😅 sounds good!

JmillsExpensify · 2023-08-15T16:28:19Z

Reviewed the details for @allroundexperts. Approved for payment in NewDot based on the BZ summary above.

kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 2, 2023

melvin-bot bot assigned dylanexpensify Jul 2, 2023

dylanexpensify added the External Added to denote the issue can be worked on by a contributor label Jul 4, 2023

melvin-bot bot changed the title ~~Web -Default avatar is changed when we select/unselect a member in Workspace invitation page~~ [$1000] Web -Default avatar is changed when we select/unselect a member in Workspace invitation page Jul 4, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 4, 2023

melvin-bot bot assigned allroundexperts Jul 4, 2023

melvin-bot bot added the Overdue label Jul 5, 2023

melvin-bot bot removed the Overdue label Jul 6, 2023

melvin-bot bot added the Overdue label Aug 7, 2023

melvin-bot bot added Overdue and removed Overdue labels Aug 7, 2023

melvin-bot bot removed the Overdue label Aug 10, 2023

melvin-bot bot added the Overdue label Aug 14, 2023

melvin-bot bot removed the Overdue label Aug 14, 2023

dylanexpensify closed this as completed Aug 15, 2023

[HOLD for payment 2023-07-28] [$1000] Web -Default avatar is changed when we select/unselect a member in Workspace invitation page #22085

[HOLD for payment 2023-07-28] [$1000] Web -Default avatar is changed when we select/unselect a member in Workspace invitation page #22085

Comments

kbecciv commented Jul 2, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Jul 2, 2023

melvin-bot bot commented Jul 2, 2023 • edited by dylanexpensify Loading

Bug0 Triage Checklist (Main S/O)

honnamkuan commented Jul 3, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

melvin-bot bot commented Jul 4, 2023

melvin-bot bot commented Jul 4, 2023

melvin-bot bot commented Jul 4, 2023

dukenv0307 commented Jul 4, 2023

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

DrLoopFall commented Jul 4, 2023

Proposal

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

What is the root cause of that problem?

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

allroundexperts commented Jul 6, 2023

DrLoopFall commented Jul 6, 2023

Beamanator commented Jul 6, 2023

honnamkuan commented Jul 6, 2023 • edited Loading

allroundexperts commented Jul 6, 2023

honnamkuan commented Jul 6, 2023

allroundexperts commented Jul 6, 2023

honnamkuan commented Jul 6, 2023

eh2077 commented Jul 6, 2023

honnamkuan commented Jul 6, 2023 • edited Loading

dukenv0307 commented Jul 6, 2023 • edited Loading

honnamkuan commented Jul 6, 2023

puneetlath commented Jul 6, 2023

akinwale commented Aug 3, 2023

allroundexperts commented Aug 3, 2023

Julesssss commented Aug 7, 2023

dylanexpensify commented Aug 10, 2023 • edited Loading

allroundexperts commented Aug 10, 2023

dylanexpensify commented Aug 10, 2023

dylanexpensify commented Aug 10, 2023

dylanexpensify commented Aug 10, 2023

dylanexpensify commented Aug 10, 2023

akinwale commented Aug 10, 2023

dylanexpensify commented Aug 10, 2023

hoangzinh commented Aug 10, 2023

dylanexpensify commented Aug 10, 2023

dylanexpensify commented Aug 10, 2023

hoangzinh commented Aug 10, 2023

dylanexpensify commented Aug 11, 2023

dylanexpensify commented Aug 14, 2023

dylanexpensify commented Aug 14, 2023

allroundexperts commented Aug 14, 2023

dylanexpensify commented Aug 14, 2023

allroundexperts commented Aug 14, 2023

dylanexpensify commented Aug 14, 2023

JmillsExpensify commented Aug 15, 2023

kbecciv commented Jul 2, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Jul 2, 2023 •

edited by dylanexpensify

Loading

honnamkuan commented Jul 3, 2023 •

edited

Loading

honnamkuan commented Jul 6, 2023 •

edited

Loading

honnamkuan commented Jul 6, 2023 •

edited

Loading

dukenv0307 commented Jul 6, 2023 •

edited

Loading

dylanexpensify commented Aug 10, 2023 •

edited

Loading