Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.34k stars 2.77k forks source link

[$250] Web - Attachment - Unable to download a txt attachment until the browser is refreshed #48679

Open IuliiaHerets opened 1 week ago

IuliiaHerets commented 1 week ago

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: 9.0.30-7 Reproducible in staging?: Y Reproducible in production?: Y Issue reported by: Applause Internal Team

Action Performed:

  1. Log in with a new Gmail account
  2. Navigate to FAB - Start chat
  3. Open a chat with any Gmail account
  4. Send any txt file
  5. Click on the txt file
  6. Refresh the browser
  7. Click on the txt file

Expected Result:

I should be able to download the attachment.

Actual Result:

Unable to download a txt attachment until the browser is refreshed. The download icon and the function itself becomes available again.

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/eba88040-eed4-4685-b08c-3baebe5dc007

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021831987490764045084
  • Upwork Job ID: 1831987490764045084
  • Last Price Increase: 2024-09-13
Issue OwnerCurrent Issue Owner: @getusha
melvin-bot[bot] commented 1 week ago

Triggered auto assignment to @jliexpensify (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 commented 1 week ago

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

jliexpensify commented 1 week ago

I can reproduce, but I'm only on v9.0.29-12 and I can't actually download the .txt file, even after refreshing multiple times 😡

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

abzokhattab commented 1 week ago

not able to reproduce it on the latest main. can you please share the txt file that you use? @jliexpensify

jliexpensify commented 1 week ago

@abzokhattab just to confirm, are you using a Gmail account, and chatting to another Gmail account?

Here is a file I uploaded:

logs.txt

hoangzinh commented 1 week ago

Proposal

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

The real issue here is when an attachment is uploading, it does nothing when the user clicks on the attachment, although it's showing as clickable.

What is the root cause of that problem?

When an attachment is uploading, it returns null here (because source is still a local URL) https://github.com/Expensify/App/blob/cadd5a85f0d9508bcb1063480f1a76259735487d/src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.tsx#L35

Therefore, it early exists when user clicks on the attachment => Unable to download https://github.com/Expensify/App/blob/cadd5a85f0d9508bcb1063480f1a76259735487d/src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.tsx#L47-L53

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

The simplest solution is we also need to not show attachments as clickable when sourceID is null here

https://github.com/Expensify/App/blob/cadd5a85f0d9508bcb1063480f1a76259735487d/src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.tsx#L46

Then we indicate that this attachment is not clickable when it's not uploaded yet.

Demo:

https://github.com/user-attachments/assets/058a3c90-5638-4612-b84c-a03a9cec4a2b

What alternative solutions did you explore? (Optional)

We can show an additional "uploading" status when attachments is still uploading

bernhardoj commented 1 week ago

Proposal

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

Can't download a text file while uploading.

What is the root cause of that problem?

When we upload the attachment, the source is still a local file (blob://...), so, the sourceID here is empty. https://github.com/Expensify/App/blob/4af620b439636f714ff72a26320ed57f9d2faac3/src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.tsx#L35

The sourceID checks for the report action ID from the uploaded attachment source (/chat-attachments/actionID/...). Because the id is empty, the download icon won't be shown and pressing the attachment won't do anything. https://github.com/Expensify/App/blob/4af620b439636f714ff72a26320ed57f9d2faac3/src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.tsx#L66 https://github.com/Expensify/App/blob/4af620b439636f714ff72a26320ed57f9d2faac3/src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.tsx#L47-L50

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

Just want to provide an alternative if we want to allow the user to download the text attachment while still uploading. To do that, instead of finding the action ID from the attachment source, we can find it from the report action itself. https://github.com/Expensify/App/blob/4af620b439636f714ff72a26320ed57f9d2faac3/src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.tsx#L43-L44

We also need to convert the withOnyx with useOnyx.

const { anchor, report, reportNameValuePairs, action, checkIfContextMenuActive } = useContext(ShowContextMenuContext);
const sourceID = action?.reportActionID;

const [download] = useOnyx(`${ONYXKEYS.COLLECTION.DOWNLOAD}${sourceID}`);

Last, we need to update the source so it only append encryptedAuthToken if it's a remote file.

const sourceURLWithAuth = (source.startsWith('blob:') || source.startsWith('file:')) ? source : addEncryptedAuthTokenToURL(source);
huult commented 1 week ago

Proposal

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

Unable to download a txt attachment until the browser is refreshed

What is the root cause of that problem?

The reportAction is not updated in IndexedDB as shown in the image below, even after addAttachment completes successfully.

Screenshot 2024-09-08 at 23 19 06

The condition check for sourceID returns undefined, which prevents the attachment from being downloaded.

https://github.com/Expensify/App/blob/9affb1384815fa8155fe23c523127c1e72b1120d/src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.tsx#L35-L54

The sourceID returns undefined due to const sourceID = (source.match(CONST.REGEX.ATTACHMENT_ID) ?? [])[1];. This happens because the source value is blob:https://dev.new.expensify.com:8082/7f479bf0-ef65-49f7-8f89-9e1cb1bf2d2f, which is stored in IndexedDB but not updated in reportAction even after addAttachment completes successfully.

Screenshot 2024-09-08 at 23 26 55

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

We should update the reportAction data after addAttachment completes successfully. Something like this:

// src/libs/Middleware/SaveResponseInOnyx.ts#L36

+        onyxUpdates.forEach((onyxUpdatesItem) => {
+            const onyxUpdatesKet = onyxUpdatesItem.key;
+            if (onyxUpdatesKet.startsWith(ONYXKEYS.COLLECTION.REPORT_ACTIONS)) {
+                const reportActionId = onyxUpdatesKet.replace(ONYXKEYS.COLLECTION.REPORT_ACTIONS, '');
+
+                if (onyxUpdatesItem.value) {
+                    Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportActionId}`, onyxUpdatesItem.value);
+                }
+            }
+        });
POC https://github.com/user-attachments/assets/0e73fa8f-ef1a-4b0e-8019-521503fdc8c2
melvin-bot[bot] commented 1 week ago

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

jliexpensify commented 1 week ago

Bumping @getusha to test and look at proposals!

getusha commented 1 week ago

I am inclined to @hoangzinh's solution, adding an indicator that the file is still being uploaded sounds good as well. Probably something like this? (same as when we download after it is uploaded)

Screenshot 2024-09-10 at 1 07 18 in the afternoon

cc @Expensify/design

shawnborton commented 1 week ago

I'm not sure that we need a tooltip on the loading spinner (it's pretty obvious what it means IMO), but I do like the idea of including it until the attachment is ready. That being said, it might make sense to replace the attachment icon with the spinner so that the shape of the attachment doesn't grow/shrink when it loads.

huult commented 1 week ago

Hi @getusha Can you check again? The issue with this ticket is not uploading the file, but that the user cannot download the file. The problem is that the user cannot download the file even though it has been successfully uploaded.

https://github.com/user-attachments/assets/92cce693-d7f2-4e2e-8612-58d1b2e55089

getusha commented 1 week ago

Hi @huult I cannot reproduce the issue you mentioned, i tried multiple times and i can download the file after it has been successfully uploaded.

https://github.com/user-attachments/assets/c19ca1b6-fc6f-4ef7-94cc-ef1ff80694b2

huult commented 1 week ago

Hi @getusha I had the same problem as you when I first reproduced it. Watch my video below to see how to reproduce the issue (using any email and sending an attachment to any email).

https://github.com/user-attachments/assets/2875b92d-13ef-4ce0-b21c-5256aeacc1a8

melvin-bot[bot] commented 5 days ago

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

melvin-bot[bot] commented 5 days ago

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

jliexpensify commented 3 days ago

@getusha bumping this one!

getusha commented 2 days ago

@huult any reason why this is happening on newly created chats?

huult commented 2 days ago

Hi @getusha , The first time we send the attachment, we make an API request, and Onyx stores the reportAction in local storage. The log numbers 1 and 2 illustrate what I mentioned.

After successfully calling the API, we save the response to Onyx. In this case, we do not have the updated IDs, so we call the saveUpdateInformation function. In this function, we skip updating the value of reportAction because we are using the set function.

So, the reportAction values that we stored in local storage before (Make API request) are not updated, and this is where the issue occurs.

Screenshot 2024-09-16 at 16 17 13