App icon indicating copy to clipboard operation
App copied to clipboard

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

Open IuliiaHerets opened this issue 1 year ago • 9 comments

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.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:

  • [ ] Android: Native
  • [ ] Android: mWeb Chrome
  • [ ] iOS: Native
  • [ ] iOS: mWeb Safari
  • [x] MacOS: Chrome / Safari
  • [ ] MacOS: Desktop

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-06
Issue OwnerCurrent Issue Owner: @getusha

IuliiaHerets avatar Sep 06 '24 07:09 IuliiaHerets

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.

melvin-bot[bot] avatar Sep 06 '24 07:09 melvin-bot[bot]

@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

IuliiaHerets avatar Sep 06 '24 07:09 IuliiaHerets

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 😡

jliexpensify avatar Sep 06 '24 09:09 jliexpensify

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

melvin-bot[bot] avatar Sep 06 '24 09:09 melvin-bot[bot]

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

melvin-bot[bot] avatar Sep 06 '24 09:09 melvin-bot[bot]

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

abzokhattab avatar Sep 06 '24 09:09 abzokhattab

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

Here is a file I uploaded:

logs.txt

jliexpensify avatar Sep 06 '24 10:09 jliexpensify

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

hoangzinh avatar Sep 06 '24 15:09 hoangzinh

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);

bernhardoj avatar Sep 07 '24 09:09 bernhardoj

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

huult avatar Sep 08 '24 16:09 huult

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

melvin-bot[bot] avatar Sep 09 '24 18:09 melvin-bot[bot]

Bumping @getusha to test and look at proposals!

jliexpensify avatar Sep 09 '24 20:09 jliexpensify

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

getusha avatar Sep 10 '24 10:09 getusha

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.

shawnborton avatar Sep 10 '24 10:09 shawnborton

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

huult avatar Sep 10 '24 10:09 huult

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

getusha avatar Sep 10 '24 10:09 getusha

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

huult avatar Sep 10 '24 11:09 huult

📣 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] avatar Sep 13 '24 16:09 melvin-bot[bot]

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

melvin-bot[bot] avatar Sep 13 '24 18:09 melvin-bot[bot]

@getusha bumping this one!

jliexpensify avatar Sep 15 '24 22:09 jliexpensify

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

getusha avatar Sep 16 '24 06:09 getusha

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

huult avatar Sep 16 '24 09:09 huult

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

melvin-bot[bot] avatar Sep 19 '24 18:09 melvin-bot[bot]

Bump @getusha for a review of the above comment, thanks!

jliexpensify avatar Sep 19 '24 22:09 jliexpensify

📣 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] avatar Sep 20 '24 16:09 melvin-bot[bot]

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

melvin-bot[bot] avatar Sep 20 '24 17:09 melvin-bot[bot]

Bumping @getusha again, and also on Slack

jliexpensify avatar Sep 23 '24 00:09 jliexpensify

@jliexpensify Is this resolved? I am interested in contributing to more projects. I took a look at this, seems it requires testing on multiple platforms, involves many different files without clear direction as to what those are so the bounty does seem low for this if that is true.

llvee avatar Sep 23 '24 00:09 llvee

📣 @llvee! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

melvin-bot[bot] avatar Sep 23 '24 00:09 melvin-bot[bot]

@MelvinBot Is that response completely automated? Jw for appriopriate followup responses.

llvee avatar Sep 23 '24 00:09 llvee