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

[$500] Chat - Attachment page keeps on loading infinitely. #35502

Closed
3 of 6 tasks
kbecciv opened this issue Jan 31, 2024 · 108 comments
Closed
3 of 6 tasks

[$500] Chat - Attachment page keeps on loading infinitely. #35502

kbecciv opened this issue Jan 31, 2024 · 108 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kbecciv
Copy link

kbecciv commented Jan 31, 2024

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: 1.4.34.0
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Go to https://staging.new.expensify.com/
  2. Tap on a report
  3. Tap plus icon near compose
  4. Tap Add attachment
  5. Take a picture using camera and send it
  6. Turn off mobile data
  7. Open the attachment

Expected Result:

Attachment page must show the image and must not load infinitely.

Actual Result:

Attachment page keeps on loading infinitely.

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

Bug6362671_1706724667975.full_video.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~019f92fc532944b2e5
  • Upwork Job ID: 1752760607717834752
  • Last Price Increase: 2024-03-14
  • Automatic offers:
    • situchan | Reviewer | 0
    • FitseTLT | Contributor | 0
@kbecciv kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 31, 2024
@melvin-bot melvin-bot bot changed the title Chat - Attachment page keeps on loading infinitely. [$500] Chat - Attachment page keeps on loading infinitely. Jan 31, 2024
Copy link

melvin-bot bot commented Jan 31, 2024

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

Copy link

melvin-bot bot commented Jan 31, 2024

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

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

melvin-bot bot commented Jan 31, 2024

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

@Yokomon
Copy link

Yokomon commented Jan 31, 2024

@kbecciv Please check recent changes. I can't send attachments to reproduce this issue.

@Amoralchik
Copy link
Contributor

Amoralchik commented Feb 1, 2024

Proposal

Hi there,

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

Clearly define the nature of "fallbackSource" in AttachmentView to address the current issue.

What is the root cause of that problem?

The problem arises from the undefined nature of "fallbackSource" in AttachmentView.

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

Implement a mechanism to display informative error messages to users when the "fallbackSource" is undefined. This enhancement aims to provide clarity in the event of an error, thus improving the overall user experience.

What alternative solutions did you explore? (Optional)

Uploading images to memory and subsequently hashing them after a successful upload. This approach ensures that the "fallbackSource" is always defined.


FYI example:

  1. Discord displays an Error
  2. Telegram display hires quality images from storage

Copy link

melvin-bot bot commented Feb 1, 2024

📣 @Amoralchik! 📣
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>

@Amoralchik
Copy link
Contributor

Amoralchik commented Feb 1, 2024

@kbecciv Please check recent changes. I can't send attachments to reproduce this issue.

I believe you can reproduce the issue on any device by following these steps

  1. Upload any image
  2. Immediately disconnect your internet connection after success
  3. open the image

This results in an infinite loading state.

FYI: I am using "version": "1.4.34-1".

@Amoralchik
Copy link
Contributor

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/vitaliip17

Copy link

melvin-bot bot commented Feb 1, 2024

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@melvin-bot melvin-bot bot added the Overdue label Feb 5, 2024
@lschurr
Copy link
Contributor

lschurr commented Feb 5, 2024

Could you review @situchan?

@melvin-bot melvin-bot bot removed the Overdue label Feb 5, 2024
@situchan
Copy link
Contributor

situchan commented Feb 5, 2024

@Amoralchik thanks for the proposal. The root cause doesn't make sense to me. It's incorrect.

@situchan
Copy link
Contributor

situchan commented Feb 5, 2024

We're waiting proposals

@Amoralchik
Copy link
Contributor

Amoralchik commented Feb 5, 2024

@situchan Let me try once again.

I propose adding a setBothSourceError function and incorporating it into the onError handler of AttachmentViewImage. This function will set bothSourceError to true if both source and fallbackSource return errors.

if (isImage || (file && Str.isImage(file.name))) {
return (
<AttachmentViewImage
url={imageError ? fallbackSource : source}
file={file}
isAuthTokenRequired={isAuthTokenRequired}
loadComplete={loadComplete}
isFocused={isFocused}
isUsedInCarousel={isUsedInCarousel}
isSingleCarouselItem={isSingleCarouselItem}
carouselItemIndex={carouselItemIndex}
carouselActiveItemIndex={carouselActiveItemIndex}
isImage={isImage}
onPress={onPress}
onError={() => {
setImageError(true);
}}
/>
);
}

       onError={() => {
            setImageError(true);
            if (imageError) setBothSourceError(true);
        }}

After this, we can add a check for allSourceError in the if (!bothSourceError || isImage || (file && Str.isImage(file.name))) condition, and then include:

    else if (bothSourceError) return (
        <Text style={[styles.textStrong, styles.flexShrink1, styles.breakAll, styles.flexWrap, styles.mw100]}>
            Unexpected error, please try again later
        </Text>
    );

This way, if both source and fallbackSource are absent in ONYX or return errors, we can display an error message instead of getting stuck in an infinite loading loop.

Copy link

melvin-bot bot commented Feb 7, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Feb 7, 2024
@lschurr
Copy link
Contributor

lschurr commented Feb 7, 2024

Could you review this again @situchan?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Feb 7, 2024
@situchan
Copy link
Contributor

situchan commented Feb 12, 2024

@Amoralchik sorry but your latest comment still doesn't explain the root cause correctly.
For proposals to be accepted, both root cause and solution should be correctly provided.

Still looking for proposals

@melvin-bot melvin-bot bot removed the Overdue label Feb 12, 2024
Copy link

melvin-bot bot commented Feb 14, 2024

@lschurr @situchan 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 melvin-bot bot removed the Overdue label Apr 8, 2024
@FitseTLT
Copy link
Contributor

FitseTLT commented Apr 8, 2024

It will be ready for review by EOD

@melvin-bot melvin-bot bot added the Overdue label Apr 10, 2024
@lschurr
Copy link
Contributor

lschurr commented Apr 10, 2024

Any change @FitseTLT?

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Overdue Daily KSv2 labels Apr 10, 2024
@FitseTLT
Copy link
Contributor

PR ready for review

@mallenexpensify
Copy link
Contributor

PR still being actively worked on #39290 (comment)

@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels May 7, 2024
Copy link

melvin-bot bot commented May 7, 2024

This issue has not been updated in over 15 days. @LLPeckham, @marcochavezf, @mollfpr, @lschurr, @FitseTLT eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@mallenexpensify
Copy link
Contributor

So close to merging...
#39290 (comment)

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@iwiznia
Copy link
Contributor

iwiznia commented May 23, 2024

FYI I think this PR caused a regression, see: #42316 (comment)

@FitseTLT
Copy link
Contributor

FYI I think this PR caused a regression, see: #42316 (comment)

It is not a regression but rather a missed edge case for a single/ios native platform.

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

@FitseTLT
Copy link
Contributor

FitseTLT commented Jun 6, 2024

@mvtglobally This issue is a fixed one; only payment is left to be processed. 👍

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Third week)

@lschurr
Copy link
Contributor

lschurr commented Jun 10, 2024

Looks like the automation failed on this one and the PR was merged on May 22nd.

I'll update and add a payment summary shortly.

@lschurr lschurr added Daily KSv2 and removed Reviewing Has a PR in review Monthly KSv2 labels Jun 10, 2024
@lschurr
Copy link
Contributor

lschurr commented Jun 10, 2024

Payment summary:

@lschurr
Copy link
Contributor

lschurr commented Jun 10, 2024

Closing :)

@lschurr lschurr closed this as completed Jun 10, 2024
@garrettmknight
Copy link
Contributor

garrettmknight commented Sep 11, 2024

$500 approved for @mollfpr

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 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
No open projects
Archived in project
Development

No branches or pull requests