[HOLD for payment 2023-06-23] [HOLD for payment 2023-06-21] [$1000] profile pic does not update in tooltip in offline mode #20672

kavimuru · 2023-06-13T12:49:00Z

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 offline
go to Settings >Profile
change the profile pic
go to any chat and hover on email to see user details in tooltip

Expected Result:

updated profile pic should show

Actual Result:

shows old profile pic

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?

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

Screen.Recording.2023-06-13.at.3.00.50.PM.mov

Recording.966.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0163d5011f7c3a7e5f
Upwork Job ID: 1668632232209276928
Last Price Increase: 2023-06-13

melvin-bot · 2023-06-13T12:49:06Z

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

melvin-bot · 2023-06-13T12:49:08Z

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

OSBotify · 2023-06-13T12:50:35Z

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

Identify the pull request that introduced this issue and revert it.
Find someone who can quickly fix the issue.
Fix the issue yourself.

melvin-bot · 2023-06-13T12:50:38Z

Triggered auto assignment to @pecanoro (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

mdneyazahmad · 2023-06-13T14:38:46Z

Proposal

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

Avatar is not updated in tooltip in offline mode

What is the root cause of that problem?

When user upload a new profile picture. It is updated in Onyx with this key ONYXKEYS.PERSONAL_DETAILS to immediately reflect the result. In the chat we are getting the avatar from ONYXKEYS.PERSONAL_DETAILS_LIST which is not updated until it is stored on the server. Therefore, it keeps displaying the old avatar. We are also using the avatar url from ONYXKEYS.PERSONAL_DETAILS wherever it immediately shows the updated avatar.

App/src/components/UserDetailsTooltip/index.js

Line 14 in 93957f0

    
           const userDetails = lodashGet(props.personalDetailsList, props.accountID, props.fallbackUserDetails);

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

Get url from ONYXKEYS.PERSONAL_DETAILS

Import required files

import compose from '../../libs/compose';
import * as UserUtils from '../../libs/UserUtils';
import {withPersonalDetails} from '../OnyxProvider';

Add withPersonalDetails

export default compose(
    withPersonalDetails(), 
    withOnyx({
        personalDetailsList: {
            key: ONYXKEYS.PERSONAL_DETAILS_LIST,
        },
    })
)(UserDetailsTooltip);

Get url from ONYXKEYS.PERSONAL_DETAILS

    const personalDetail = props.personalDetails[userDetails.login] || {};

    const avatarSource = UserUtils.getAvatar(personalDetail.avatar, userDetails.login);

Update the prop in Avatar component

What alternative solutions did you explore? (Optional)

None

commit: eef7da9

melvin-bot · 2023-06-13T14:51:43Z

Job added to Upwork: https://www.upwork.com/jobs/~0163d5011f7c3a7e5f

melvin-bot · 2023-06-13T14:51:45Z

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

melvin-bot · 2023-06-13T14:51:51Z

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

melvin-bot · 2023-06-13T14:51:54Z

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

Nikhil-Vats · 2023-06-13T15:39:11Z

Proposal

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

Profile picture in tooltip is not updated in offline mode.

What is the root cause of that problem?

The root causes is -

The profile picture is queried using the wrong key in Onyx which is not updated in offline mode. The component uses PERSONAL_DETAILS_LIST to query the avatar but only PERSONAL_DETAILS is updated in offline mode.
The renderTooltipContent is used for rendering tooltip and it is wrapped within useCallback since the userDetails.avatar doesn't change so profile pic in tooltip is not updated.

App/src/components/UserDetailsTooltip/index.js

Lines 13 to 35 in 93957f0

    
           function UserDetailsTooltip(props) { 
        
               const userDetails = lodashGet(props.personalDetailsList, props.accountID, props.fallbackUserDetails); 
        
               const renderTooltipContent = useCallback( 
        
                   () => ( 
        
                       <View style={[styles.alignItemsCenter, styles.ph2, styles.pv2]}> 
        
                           <View style={styles.emptyAvatar}> 
        
                               <Avatar 
        
                                   containerStyles={[styles.actionAvatar]} 
        
                                   source={userDetails.avatar} 
        
                               /> 
        
                           </View> 
        
                           <Text style={[styles.mt2, styles.textMicroBold, styles.textReactionSenders, styles.textAlignCenter]}> 
        
                               {String(userDetails.displayName).trim() ? userDetails.displayName : ''} 
        
                           </Text> 
        
                           <Text style={[styles.textMicro, styles.fontColorReactionLabel]}> 
        
                               {String(userDetails.login).trim() && !_.isEqual(userDetails.login, userDetails.displayName) ? Str.removeSMSDomain(userDetails.login) : ''} 
        
                           </Text> 
        
                       </View> 
        
                   ), 
        
                   [userDetails.avatar, userDetails.displayName, userDetails.login], 
        
               );

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

Import withCurrentUserPersonalDetails because it contains the updated profile pic in offline mode.

App/src/components/withCurrentUserPersonalDetails.js

Lines 65 to 68 in 93957f0

    
           return withOnyx({ 
        
               personalDetails: { 
        
                   key: ONYXKEYS.PERSONAL_DETAILS, 
        
               },

All other details like login, displayName can also be used from the same personalDetails prop so we can remove the personalDetailsList key entirely from component.
Change useCallback dependency to use the new avatar variable as dependency. Since, it will be updated so the tooltip will have the updated profile pic.

What alternative solutions did you explore? (Optional)

Alternatively, we can also update the function below to change the avatar in PERSONAL_DETAILS_LIST as well. This would be great because it might be used at other places in future so I personally think this is the best way to go.

App/src/libs/actions/PersonalDetails.js

Lines 361 to 365 in 93957f0

    
           function updateAvatar(file) { 
        
               const optimisticData = [ 
        
                   { 
        
                       onyxMethod: Onyx.METHOD.MERGE, 
        
                       key: ONYXKEYS.PERSONAL_DETAILS,

Following changes will be needed -

Add another prop in updateAvatar for accountID or get it from personalDetails which is already a variable in the file.
Update optimisticData to update avatar in PERSONAL_DETAILS_LIST.
Update successData and failureData to set avatar in pendingFields to null in PERSONAL_DETAILS_LIST.

Result

Screen.Recording.2023-06-13.at.9.14.27.PM.mov

dummy-1111 · 2023-06-13T15:58:33Z

Proposal

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

Profile avatar in user details tooltip is not updated in offline mode.

What is the root cause of that problem?

UserDetailsTooltip component uses personal details list to show the avatar but this isn't updated in offline mode when profile avatar is updated as you can see here

App/src/libs/actions/PersonalDetails.js

Lines 361 to 412 in 93957f0

    
           function updateAvatar(file) { 
        
               const optimisticData = [ 
        
                   { 
        
                       onyxMethod: Onyx.METHOD.MERGE, 
        
                       key: ONYXKEYS.PERSONAL_DETAILS, 
        
                       value: { 
        
                           [currentUserEmail]: { 
        
                               avatar: file.uri, 
        
                               avatarThumbnail: file.uri, 
        
                               originalFileName: file.name, 
        
                               errorFields: { 
        
                                   avatar: null, 
        
                               }, 
        
                               pendingFields: { 
        
                                   avatar: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE, 
        
                                   originalFileName: null, 
        
                               }, 
        
                           }, 
        
                       }, 
        
                   }, 
        
               ]; 
        
               const successData = [ 
        
                   { 
        
                       onyxMethod: Onyx.METHOD.MERGE, 
        
                       key: ONYXKEYS.PERSONAL_DETAILS, 
        
                       value: { 
        
                           [currentUserEmail]: { 
        
                               pendingFields: { 
        
                                   avatar: null, 
        
                               }, 
        
                           }, 
        
                       }, 
        
                   }, 
        
               ]; 
        
               const failureData = [ 
        
                   { 
        
                       onyxMethod: Onyx.METHOD.MERGE, 
        
                       key: ONYXKEYS.PERSONAL_DETAILS, 
        
                       value: { 
        
                           [currentUserEmail]: { 
        
                               avatar: personalDetails[currentUserEmail].avatar, 
        
                               avatarThumbnail: personalDetails[currentUserEmail].avatarThumbnail || personalDetails[currentUserEmail].avatar, 
        
                               pendingFields: { 
        
                                   avatar: null, 
        
                               }, 
        
                           }, 
        
                       }, 
        
                   }, 
        
               ]; 
        
               API.write('UpdateUserAvatar', {file}, {optimisticData, successData, failureData}); 
        
           }

This is the root cause

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

Update the personal details list data when avatar is updated.

In the above updateAvatar function, add the following code to the optimisticData

        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: ONYXKEYS.PERSONAL_DETAILS_LIST,
            value: {
                [personalDetails[currentUserEmail].accountID]: {
                    avatar: file.uri,
                    errorFields: {
                        avatar: null,
                    },
                },
            },
        },

Result

20672_mac_chrome.mp4

What alternative solutions did you explore? (Optional)

pecanoro · 2023-06-13T16:46:29Z

It seems the problem is coming from this PR in which we introduced the feature. @cloudpresser can you please create a PR to fix this issue? Mostly because it's code you introduced and probably an oversight on the solution.

fedirjh · 2023-06-13T17:11:35Z

@pecanoro This issue should be fixed by #20473 , ONYXKEYS.PERSONAL_DETAILS is being deprecated and replaced by ONYXKEYS.PERSONAL_DETAILS_LIST , We just used the new key when we implemented the user details tooltip in #20276 (comment)

melvin-bot · 2023-06-14T21:03:30Z

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

melvin-bot · 2023-06-14T21:03:31Z

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

fix: avatar not changed in tooltip when updating avatar #20713

If no regressions arise, payment will be issued on 2023-06-21. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

Merged PR within 3 business days of assignment - 50% bonus
Merged PR more than 9 business days after assignment - 50% penalty

melvin-bot · 2023-06-14T21:03:33Z

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:

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

melvin-bot · 2023-06-16T18:20:22Z

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

fix: avatar not changed in tooltip when updating avatar #20713

If no regressions arise, payment will be issued on 2023-06-23. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

Merged PR within 3 business days of assignment - 50% bonus
Merged PR more than 9 business days after assignment - 50% penalty

melvin-bot · 2023-06-16T18:20:24Z

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:

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

slafortune · 2023-06-23T19:16:23Z

@gadhiyamanan, @allroundexperts , @s-alves10 offers are sent!

@allroundexperts can you complete the checklist?

gadhiyamanan · 2023-06-23T19:17:15Z

@slafortune accepted, thanks!

dummy-1111 · 2023-06-23T21:22:33Z

Offer accepted. Thank you

pecanoro · 2023-06-26T15:22:34Z

@allroundexperts Waiting for you to finish the checklist so we can finish the payment and close it!

dummy-1111 · 2023-06-28T20:46:16Z

@pecanoro

May I ask you if this is eligible for speed bonus?
#20672 (comment)

pecanoro · 2023-06-28T21:19:27Z

@s-alves10 Yes! It should! Wasn't the case?

dummy-1111 · 2023-06-28T21:21:36Z

I'm afraid not

slafortune · 2023-06-29T13:53:54Z

Yes! It should have - I missed that and can certainly add it! Bonus sent @s-alves10

allroundexperts · 2023-06-29T13:56:55Z

Yes! It should have - I missed that and can certainly add it! Bonus sent @s-alves10

Don't forget me @slafortune!

pecanoro · 2023-07-03T16:10:40Z

@slafortune Friendly bump to handle the remaining payment issues!

slafortune · 2023-07-05T15:06:47Z

You bet @allroundexperts - can you please complete the checklist? Is there a regression test?

allroundexperts · 2023-07-06T05:20:46Z

@slafortune It seems like the checklist is not needed here. As per #20672 (comment) we intentionally did not update PERSONAL_DETAILS key in the original PR. This got fixed in a subsequent PR but we just wanted to fix this temporarily here because it might have impacted user experience.

slafortune · 2023-07-06T12:02:17Z

Thanks @allroundexperts ! I think if the checklist isn't needed for any reason, having an explanation as to why it isn't needed is super helpful to make sure we are getting everything.

kavimuru added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 13, 2023

melvin-bot bot assigned slafortune Jun 13, 2023

github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Jun 13, 2023

OSBotify mentioned this issue Jun 13, 2023

Deploy Checklist: New Expensify 2023-06-12 #20640

Closed

71 tasks

melvin-bot bot assigned pecanoro Jun 13, 2023

melvin-bot bot added Daily KSv2 and removed Hourly KSv2 labels Jun 13, 2023

slafortune added the External Added to denote the issue can be worked on by a contributor label Jun 13, 2023

melvin-bot bot changed the title ~~profile pic does not update in tooltip in offline mode~~ [$1000] profile pic does not update in tooltip in offline mode Jun 13, 2023

melvin-bot bot assigned sobitneupane Jun 13, 2023

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

melvin-bot bot mentioned this issue Jun 13, 2023

[HOLD for payment 2023-06-21] [$1000] Update user tooltips to include avatar, display name, and handle #20086

Closed

melvin-bot bot mentioned this issue Jun 13, 2023

[HOLD for payment 2023-06-28] [Secure Logins] Replace login uses with accountIDs throughout App (when accessing Onyx) #19007

Closed

melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jun 16, 2023

melvin-bot bot changed the title ~~[HOLD for payment 2023-06-21] [$1000] profile pic does not update in tooltip in offline mode~~ [HOLD for payment 2023-06-23] [HOLD for payment 2023-06-21] [$1000] profile pic does not update in tooltip in offline mode Jun 16, 2023

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 21, 2023

melvin-bot bot added the Overdue label Jun 26, 2023

melvin-bot bot added Overdue and removed Overdue labels Jun 26, 2023

melvin-bot bot removed the Overdue label Jun 28, 2023

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

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

slafortune closed this as completed Jul 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2023-06-23] [HOLD for payment 2023-06-21] [$1000] profile pic does not update in tooltip in offline mode #20672

[HOLD for payment 2023-06-23] [HOLD for payment 2023-06-21] [$1000] profile pic does not update in tooltip in offline mode #20672

kavimuru commented Jun 13, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Jun 13, 2023

melvin-bot bot commented Jun 13, 2023 •

edited by slafortune

Loading

OSBotify commented Jun 13, 2023

melvin-bot bot commented Jun 13, 2023

mdneyazahmad commented Jun 13, 2023

melvin-bot bot commented Jun 13, 2023

melvin-bot bot commented Jun 13, 2023

melvin-bot bot commented Jun 13, 2023

melvin-bot bot commented Jun 13, 2023

Nikhil-Vats commented Jun 13, 2023 •

edited

Loading

dummy-1111 commented Jun 13, 2023

pecanoro commented Jun 13, 2023

fedirjh commented Jun 13, 2023

melvin-bot bot commented Jun 14, 2023

melvin-bot bot commented Jun 14, 2023

melvin-bot bot commented Jun 14, 2023

melvin-bot bot commented Jun 16, 2023 •

edited by slafortune

Loading

melvin-bot bot commented Jun 16, 2023

slafortune commented Jun 23, 2023

gadhiyamanan commented Jun 23, 2023

dummy-1111 commented Jun 23, 2023

pecanoro commented Jun 26, 2023

dummy-1111 commented Jun 28, 2023

pecanoro commented Jun 28, 2023

dummy-1111 commented Jun 28, 2023 •

edited

Loading

slafortune commented Jun 29, 2023

allroundexperts commented Jun 29, 2023

pecanoro commented Jul 3, 2023

slafortune commented Jul 5, 2023

allroundexperts commented Jul 6, 2023

slafortune commented Jul 6, 2023

[HOLD for payment 2023-06-23] [HOLD for payment 2023-06-21] [$1000] profile pic does not update in tooltip in offline mode #20672

[HOLD for payment 2023-06-23] [HOLD for payment 2023-06-21] [$1000] profile pic does not update in tooltip in offline mode #20672

Comments

kavimuru commented Jun 13, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Jun 13, 2023

melvin-bot bot commented Jun 13, 2023 • edited by slafortune Loading

Bug0 Triage Checklist (Main S/O)

OSBotify commented Jun 13, 2023

melvin-bot bot commented Jun 13, 2023

mdneyazahmad commented Jun 13, 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)

melvin-bot bot commented Jun 13, 2023

melvin-bot bot commented Jun 13, 2023

melvin-bot bot commented Jun 13, 2023

melvin-bot bot commented Jun 13, 2023

Nikhil-Vats commented Jun 13, 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?

What alternative solutions did you explore? (Optional)

dummy-1111 commented Jun 13, 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)

pecanoro commented Jun 13, 2023

fedirjh commented Jun 13, 2023

melvin-bot bot commented Jun 14, 2023

melvin-bot bot commented Jun 14, 2023

melvin-bot bot commented Jun 14, 2023

melvin-bot bot commented Jun 16, 2023 • edited by slafortune Loading

melvin-bot bot commented Jun 16, 2023

slafortune commented Jun 23, 2023

gadhiyamanan commented Jun 23, 2023

dummy-1111 commented Jun 23, 2023

pecanoro commented Jun 26, 2023

dummy-1111 commented Jun 28, 2023

pecanoro commented Jun 28, 2023

dummy-1111 commented Jun 28, 2023 • edited Loading

slafortune commented Jun 29, 2023

allroundexperts commented Jun 29, 2023

pecanoro commented Jul 3, 2023

slafortune commented Jul 5, 2023

allroundexperts commented Jul 6, 2023

slafortune commented Jul 6, 2023

kavimuru commented Jun 13, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Jun 13, 2023 •

edited by slafortune

Loading

Nikhil-Vats commented Jun 13, 2023 •

edited

Loading

melvin-bot bot commented Jun 16, 2023 •

edited by slafortune

Loading

dummy-1111 commented Jun 28, 2023 •

edited

Loading