[HOLD FOR 45043] [$250] Report - "Uploading attachment..." message disappears after restarting the app in offline

[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.6-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: https://expensify.testrail.io/index.php?/tests/view/4695551
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

1. Open New Expensify app
2. Navigate to any conversation
3. Turn off the Internet
4. Send an image to the conversation
5. Reload the application(⇧-⌘-R)
6. Return to the conversation with the sent attachment
7. Turn on the Internet

Expected Result:

The message "Uploading attachment..." should be visible in the conversation after reloading the app in offline mode

Actual Result:

"Uploading attachment..." message disappears in the conversation after reloading the application in offline mode

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6533139_1720100548631.Recording__192.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c0c3582f7d243cde
• Upwork Job ID: 1812449609428388373
• Last Price Increase: 2024-07-21

Issue Owner

Current Issue Owner: @rushatgabhane

[melvin-bot]

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

@bfitzexpensify 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 #vip-vsp

[melvin-bot]

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

[melvin-bot]

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

[nyomanjyotisa]

Proposal

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

"Uploading attachment..." message disappears after restarting the app in offline

What is the root cause of that problem?

Uploading attachment will give us an error message in console, both for offline and online. Here is the console error:

This happens because we directly assigning the file object (which contains non-clonable properties) to attachmentInfo which we use on optimisticData

This cause an error because we cannot store non-cloneable properties on IndexedDB

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

App/src/libs/ReportUtils.ts

Line 3702 in 4ae133d

const attachmentInfo = file ?? {};

Change this code to const attachmentInfo = file ? {...file} : {}; by this changes we create a shallow copy of the file object. This shallow copy contains only the properties of the file object, excluding any non-clonable properties

RESULT

New-Expensify.12.mp4

What alternative solutions did you explore? (Optional)

[nyomanjyotisa]

Proposal updated

[tsa321]

Proposal

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

"Uploading attachment..." message disappears after restarting the app in offline

What is the root cause of that problem?

The problem arises because when users add attachments while offline, the optimistic report action fails to be saved in Onyx. This issue comes from a recent update in react-native-onyx, specifically in utils.js (In this PR), where the mergeObject function was modified. The change replaced the usage of Object.keys with a for (key in object) loop when merging objects. This changes causes the attachmentInfo object, which undergoes merging, to include additional fields, as shown in the screenshots below:

• Using for (key in...) loop:
  

• Using Object.keys:
  

The attachmentInfo includes arrayBuffer, slice function, stream function, etc, which will make Onyx to fail when merging the optimistic report action, resulting in an error in the ss below:

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

To resolve this issue, we can revert the mergeObject of utils.js to use Object.keys to retrieve the keys of the source and target objects during merging. Below are the reverted code snippets for both the target and source:

// Reverted code for the target:
const targetKeys = Object.keys(targetObject);
for (let i = 0; i < targetKeys.length; ++i) {
    const key = targetKeys[i];
    ...
}

// Reverted code for the source:
const sourceKeys = Object.keys(source);
for (let i = 0; i < sourceKeys.length; ++i) {
    const key = sourceKeys[i];
    const sourceValue = source?.[key];
    ...
}

[nkdengineer]

This issue can be fixed here.

[rushatgabhane]

@nkdengineer cool, we will hold on your PR

[melvin-bot]

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

[melvin-bot]

@rushatgabhane, @bfitzexpensify Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

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

[rushatgabhane]

same on hold

[melvin-bot]

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

[melvin-bot]

@rushatgabhane, @bfitzexpensify Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@rushatgabhane, @bfitzexpensify Still overdue 6 days?! Let's take care of this!

[melvin-bot]

@rushatgabhane, @bfitzexpensify 10 days overdue. I'm getting more depressed than Marvin.

[melvin-bot]

@rushatgabhane, @bfitzexpensify 12 days overdue. Walking. Toward. The. Light...

[melvin-bot]

This issue has not been updated in over 14 days. @rushatgabhane, @bfitzexpensify eroding to Weekly issue.

[kavimuru]

Not reproducible

Recording.74.3.mp4

[bfitzexpensify]

Awesome, let's close this out.