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

[HOLD for payment 2024-09-19] [$250] Workspace - Default avatar is different in full screen after uploading avatar and removing it #47535

Closed
6 tasks done
IuliiaHerets opened this issue Aug 16, 2024 · 30 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Aug 16, 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: v9.0.21-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Create a new workspace.
  3. Go to Profile.
  4. Upload a workspace avatar.
  5. Remove the workspace avatar.
  6. Go to any workspace room.
  7. Click on the room header.
  8. Click on the workspace avatar.

Expected Result:

The default workspace avatar when viewed in full screen should match.

Actual Result:

The default workspace avatar when viewed in full screen is different from the actual avatar after uploading a custom avatar and removing it.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6573345_1723757496095.20240816_052054.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01a4f7e2fd3a698d7c
  • Upwork Job ID: 1827400295018022101
  • Last Price Increase: 2024-08-31
  • Automatic offers:
    • FitseTLT | Contributor | 103826687
Issue OwnerCurrent Issue Owner: @MariaHCD
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 16, 2024
Copy link

melvin-bot bot commented Aug 16, 2024

Triggered auto assignment to @kevinksullivan (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.

@IuliiaHerets
Copy link
Author

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

@IuliiaHerets
Copy link
Author

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

@daledah
Copy link
Contributor

daledah commented Aug 16, 2024

Proposal

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

The default workspace avatar when viewed in full screen is different from the actual avatar after uploading a custom avatar and removing it.

What is the root cause of that problem?

After adding new workspace avatar and deleting it, we are not deleting originalFileName in Onyx
And in ReportAvatar we will reuse the expiration value of originalFileName

// In the case of default workspace avatar, originalFileName prop takes policyID as value to get the color of the avatar
originalFileName={policy?.originalFileName ?? policy?.id ?? report?.policyID}

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

When deleting workspace avatar, we should update originalFileName to null in here


   const optimisticData: OnyxUpdate[] = [
        {
                ...
                avatarURL: '',
                originalFileName: null,
            },
        },

What alternative solutions did you explore? (Optional)

NA

@FitseTLT
Copy link
Contributor

Proposal

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

Default avatar is different in full screen after uploading avatar and removing it

What is the root cause of that problem?

We forgot to clear originalFileName on deleting workspace avatar

const optimisticData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
value: {
pendingFields: {
avatarURL: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
},
errorFields: {
avatarURL: null,
},
avatarURL: '',
},
},
];

So the fill color will be calcuated based on the previous value of the originalFileName

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

We need to clear originalFileName optimistically and also not forget to set the previous value to the failureData (Note: we also forgot to revert avatarURL on failureData so we should fix that too)

    const policy = getPolicy(policyID);
    const optimisticData: OnyxUpdate[] = [
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
            value: {
                pendingFields: {
                    avatarURL: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
                },
                errorFields: {
                    avatarURL: null,
                },
                avatarURL: '',
                originalFileName: null,
            },
        },
    ];
    const failureData: OnyxUpdate[] = [
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
            value: {
                avatarURL:policy?.avatarURL,
                originalFileName:policy?.originalFileName,
                errorFields: {
                    avatarURL: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('avatarWithImagePicker.deleteWorkspaceError'),
                },
            },
        },
    ];

Similarly we might need to clearpolicy.avatar prop optimistically too.

What alternative solutions did you explore? (Optional)

@melvin-bot melvin-bot bot added the Overdue label Aug 19, 2024
Copy link

melvin-bot bot commented Aug 19, 2024

@kevinksullivan Whoops! This issue is 2 days overdue. Let's get this updated quick!

Copy link

melvin-bot bot commented Aug 21, 2024

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

Copy link

melvin-bot bot commented Aug 23, 2024

@kevinksullivan 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

@kevinksullivan
Copy link
Contributor

Polish collect issue

@melvin-bot melvin-bot bot removed the Overdue label Aug 24, 2024
@kevinksullivan kevinksullivan added External Added to denote the issue can be worked on by a contributor Overdue labels Aug 24, 2024
@melvin-bot melvin-bot bot changed the title Workspace - Default avatar is different in full screen after uploading avatar and removing it [$250] Workspace - Default avatar is different in full screen after uploading avatar and removing it Aug 24, 2024
Copy link

melvin-bot bot commented Aug 24, 2024

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

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

melvin-bot bot commented Aug 24, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Aug 24, 2024
Copy link

melvin-bot bot commented Aug 27, 2024

@Pujan92, @kevinksullivan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Aug 27, 2024
Copy link

melvin-bot bot commented Aug 29, 2024

@Pujan92, @kevinksullivan Eep! 4 days overdue now. Issues have feelings too...

@kevinksullivan
Copy link
Contributor

Hi @Pujan92 any update on the reviews?

Copy link

melvin-bot bot commented Aug 30, 2024

@Pujan92 @kevinksullivan 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!

@kevinksullivan
Copy link
Contributor

Friendly bump @Pujan92 . If you are unable to get back to this early next week I'll have to reassign.

Copy link

melvin-bot bot commented Aug 31, 2024

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

@Pujan92
Copy link
Contributor

Pujan92 commented Sep 1, 2024

Sorry @kevinksullivan for the delay.

@Pujan92
Copy link
Contributor

Pujan92 commented Sep 1, 2024

RCA from both the proposals is correct where originalFileName hasn't been updated in optimistic data while deleting the workspace avatar. Regarding the solution, I prefer @FitseTLT's proposal as it is also handling the failure case and can be considered as a complete solution.

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Sep 1, 2024

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

Copy link

melvin-bot bot commented Sep 4, 2024

@MariaHCD, @Pujan92, @kevinksullivan Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Sep 4, 2024
@MariaHCD
Copy link
Contributor

MariaHCD commented Sep 5, 2024

Reviewing this today

@melvin-bot melvin-bot bot removed the Overdue label Sep 5, 2024
@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 5, 2024
Copy link

melvin-bot bot commented Sep 5, 2024

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

@kevinksullivan
Copy link
Contributor

Looping in another BZ for payment as I'm going OOO

@kevinksullivan kevinksullivan removed their assignment Sep 12, 2024
@kevinksullivan kevinksullivan added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Sep 12, 2024
Copy link

melvin-bot bot commented Sep 12, 2024

Triggered auto assignment to @stephanieelliott (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.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Sep 12, 2024
@kevinksullivan kevinksullivan self-assigned this Sep 12, 2024
Copy link

melvin-bot bot commented Sep 20, 2024

@MariaHCD, @Pujan92, @kevinksullivan, @stephanieelliott, @FitseTLT Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@MariaHCD
Copy link
Contributor

Looks like the fix was deployed two weeks ago: #48791

@stephanieelliott
Copy link
Contributor

Oop yea looks like automation didn't work, this was ready to pay out on 9/19. Setting up for payment now.

@stephanieelliott stephanieelliott changed the title [$250] Workspace - Default avatar is different in full screen after uploading avatar and removing it [HOLD for payment 2024-09-19] [$250] Workspace - Default avatar is different in full screen after uploading avatar and removing it Sep 24, 2024
@stephanieelliott
Copy link
Contributor

Summarizing payment on this issue:

  • Contributor: @FitseTLT $250 via Upwork -- PAID
  • Contributor+: @Pujan92 $250 via ND, please request

Upwork job is here: https://www.upwork.com/jobs/~01a4f7e2fd3a698d7c

@garrettmknight
Copy link
Contributor

$250 approved for @Pujan92

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 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
No open projects
Status: Done
Development

No branches or pull requests

8 participants