[HOLD for payment 2023-06-05] [$2000] old picture appears in preview

[kavimuru]

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:

1. open https://staging.new.expensify.com/ for two users which have chat history
2. from userA , click on userB name to see details
3. from userB, change profile pic
4. from userA , click on userB’s updated profile pic to preview

Expected Result:

should show updated profile pic in preview

Actual Result:

old pic appears in preview

Workaround:

unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.2.97-2
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

Recording.192.mp4

Screen.Recording.2023-04-10.at.5.32.39.PM.mov

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01bad7defc2df74b70
• Upwork Job ID: 1654447071108284416
• Last Price Increase: 2023-05-17

[MelvinBot]

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

[MelvinBot]

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

[hungvu193]

Similar to this one:
#16909

[MelvinBot]

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

[muttmuure]

I've reproduced this

[muttmuure]

Asking about which issue to put the external label on here: https://expensify.slack.com/archives/C01GTK53T8Q/p1681477057328259

[muttmuure]

Looks like this is going to get picked up as part of #16528 (comment) which was reported 3 weeks ago.

[gadhiyamanan]

@muttmuure #16528 has been fixed but still able to reproduce this issue can we reopen the issue

[dukenv0307]

Proposal

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

Old picture appears in preview

What is the root cause of that problem?

The source state is only assigned with source props in constructor. So that, when source props is changed source state is still old value. That's why old picture appears in preview.

App/src/components/AttachmentModal.js

Line 85 in 2d0ef06

source: props.source,

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

We should add componentDidUpdate function to update source state when source props is changed.

What alternative solutions did you explore? (Optional)

NA

Result

Screencast.from.04-05-2023.14.53.11.webm

[melvin-bot]

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

[melvin-bot]

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

[muttmuure]

I can still reproduce this so yeah let's get it fixed

[melvin-bot]

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

[melvin-bot]

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

[DanutGavrus]

Proposal

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

The old picture appears in preview only if the Details Page is opened.

What is the root cause of that problem?

The DetailsPage has an AttachmentModal as a child, which has an AttachmentView as a child. If userB changes his photo when userA has the DetailsPage closed, everything works as normal, but if userB changes his photo when userA has the DetailsPage opened, the bug happens. That's because the source in state is passed to the AttachmentView, and is not updated later on. The picture's source inside props gets updated in the AttachmentModal, but not passed to the AttachmentView.

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

Inside the render method, we may do the following change:

AttachmentModal.js
248    render() {
249        const source = this.state.source; // this
249        const source = this.props.source || this.state.source; // with this

and unify the usages of source const, this.props.source and this.state.source to using only the source const because as of now, inside the render method, the usages are mixed between these.

In the DetailsPage(where this issue is reported), the AttachmentModal already gets re-rendered having the new value of source in props. In the case of attaching and previewing a file, source in props is undefined, and inside validateAndDisplayFileToUpload, the setState({..., source, ...}) method is called, which triggers a re-render with the new value of source in state. So, we just need to use the corresponding source. By implementing componentDidUpdate to set the state again, we may trigger unnecessary re-renders.

[chiragsalian]

The linked proposal looks good to me.
Feel free to create the PR @DanutGavrus.

[melvin-bot]

📣 @DanutGavrus You have been assigned to this job by @chiragsalian!
Please apply to this job in Upwork 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 📖

[DanutGavrus]

@fedirjh @chiragsalian
PR created: PR19107

It's draft right now as I have to upload screenshots from each platform.
Also, I've requested an invitation to the Slack Channels on 11 may in order to be able to request and test against a High-Traffic account. I didn't receive the invitation yet, I've sent another email now.

May I update from "Draft" to "Ready to review" while I'm taking the screenshots in parallel and waiting for the Slack invitations and High-Traffic account or should I wait until I'm able to check these marks in the PR too? Thanks!

[melvin-bot]

Upwork job price has been updated to $2000

[muttmuure]

Gonna increase the price to get this one over the line

[fedirjh]

@muttmuure No need to increase the price , a contributor already assigned to this issue.

[melvin-bot]

@chiragsalian, @fedirjh, @muttmuure, @DanutGavrus Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[fedirjh]

PR deployed to staging

[fedirjh]

PR deployed to production

[muttmuure]

The automation is broken here

[DanutGavrus]

@muttmuure PR was deployed to production on 25 May at 3:21 AM as seen here. Shouldn't the date be before 2023-06-05?

[DanutGavrus]

@muttmuure Hi, today is the 15th day after PR has been deployed to production with no regression, the payment was not issued and I'm not yet hired on the Upwork job. I've sent a mail to contributors@expensify.com 4 days ago, but I've got no response yet. I've sent 1 more this morning, should I also write on Slack, or do anything else? Thank you!

[muttmuure]

hey @DanutGavrus - I was going by Fedi's comment, sorry for the confusion.

Can you link your Upwork profile so I can help get you paid?

[DanutGavrus]

@muttmuure No problem!

Upwork profile: https://www.upwork.com/freelancers/~019ec730d28d49906e

[muttmuure]

Offers sent to everyone

[gadhiyamanan]

@muttmuure applied!

[DanutGavrus]

@muttmuure applied

[muttmuure]

Danut and Manan paid.

@fedirjh just you left!

[fedirjh]

@muttmuure Thanks. Accepted

[muttmuure]

Offer sent

[muttmuure]

Everyone has been paid. Thanks for the hustle everyone!