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

[HOLD for payment 2024-08-26] [$125] Track expense - Preview image of the shared expense only loads if you open it and go back #43579

Closed
2 of 6 tasks
lanitochka17 opened this issue Jun 12, 2024 · 61 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@lanitochka17
Copy link

lanitochka17 commented Jun 12, 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.82-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: https://expensify.testrail.io/index.php?/tests/view/4624340&group_by=cases:section_id&group_id=309130&group_order=asc
Issue reported by: Applause - Internal Team

Action Performed:

  1. Navigate to https://staging.new.expensify.com/
  2. Log in with any account with a workspace
  3. Track a scan expense in the self DM
  4. "Share it with your accountant" on the actionable whisper
  5. Share it in any workspace
  6. Open the expense
  7. Navigate back

Expected Result:

Preview image should be available immediately

Actual Result:

Preview image of the shared expense only loads if you open the expense and go back

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

Bug6510808_1718197001166.SWZP9288.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~017e8abcd538b6fd27
  • Upwork Job ID: 1800905696348764772
  • Last Price Increase: 2024-08-26
  • Automatic offers:
    • dominictb | Contributor | 103173114
Issue OwnerCurrent Issue Owner: @sonialiap
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 12, 2024
Copy link

melvin-bot bot commented Jun 12, 2024

Triggered auto assignment to @sonialiap (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
Copy link
Author

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

@sonialiap sonialiap added the External Added to denote the issue can be worked on by a contributor label Jun 12, 2024
@melvin-bot melvin-bot bot changed the title Track expense - Preview image of the shared expense only loads if you open it and go back [$250] Track expense - Preview image of the shared expense only loads if you open it and go back Jun 12, 2024
Copy link

melvin-bot bot commented Jun 12, 2024

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

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

melvin-bot bot commented Jun 12, 2024

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

@TheGithubDev
Copy link

Proposal

Re-state the problem that we are trying to solve in this issue:

The preview image of a shared expense only loads if the user opens the expense and then navigates back. The preview image should be available immediately after sharing the expense.

Root cause of that problem?

The root cause of the issue is due to the image loading mechanism not being triggered correctly when the expense is initially shared. The image loader isn't fetching the image until the expense detail is opened and the view is refreshed.

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

To solve this problem, we need to ensure that the image loading mechanism is triggered immediately when the expense is shared.
To achieve this, we should -

  1. Trigger Image Load on Sharing:
  • Ensure that the image loader is called when the expense is shared, updating the preview image in the list without needing to open and go back.

      • Modify the sharing function to include a call to the image loader after the expense is shared.
    // Example logic to trigger image load on sharing
    function shareExpense(expenseId, workspaceId) {
        shareExpenseWithWorkspace(expenseId, workspaceId)
            .then(() => {
                loadImagePreview(expenseId); // Trigger image load
            })
            .catch(error => {
                console.error("Error sharing expense:", error);
            });
    }
    
    function loadImagePreview(expenseId) {
        // Logic to load and update image preview
        const previewImage = getExpensePreviewImage(expenseId);
        updatePreviewImageState(expenseId, previewImage);
    }
  1. Update State Management:
  • Make sure the state reflecting the preview image is updated promptly when the sharing action is completed.

      • Ensure that the state is updated correctly after the image is loaded.
    function updatePreviewImageState(expenseId, image) {
        // Logic to update state with the new image
        setState(prevState => ({
            ...prevState,
            expenses: prevState.expenses.map(expense => 
                expense.id === expenseId ? {...expense, previewImage: image} : expense
            )
        }));
    }

Any alternative solutions explored? (Optional)

Preload Images:

  • We can implement a mechanism to preload images for all expenses, reducing loading times when navigating through expenses.

@mollfpr
Copy link
Contributor

mollfpr commented Jun 13, 2024

The root cause of the issue is due to the image loading mechanism not being triggered correctly when the expense is initially shared.

@TheGithubDev Could you be more specific about where the issue is and where we need to apply the changes?

Copy link

melvin-bot bot commented Jun 18, 2024

@sonialiap, @mollfpr Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot melvin-bot bot added the Overdue label Jun 18, 2024
@mollfpr
Copy link
Contributor

mollfpr commented Jun 18, 2024

Not overdue. Waiting for the contributor and new proposals.

@melvin-bot melvin-bot bot removed the Overdue label Jun 18, 2024
@TheGithubDev
Copy link

@mollfpr Thanks for your feedback!

Specific Issue Location:
The issue lies within the image loading mechanism that handles the preview images of shared expenses. Currently, this mechanism is not triggered until the user opens the expense detail view and then navigates back. As a result, the preview image does not load immediately upon sharing the expense.

Specific Changes Required:

  1. Trigger Image Load on Sharing:
    Location: The function responsible for sharing the expense, located in the expense sharing service.
    Change: Modify this function to include a call to the image loading mechanism immediately after the expense is successfully shared.

    // Example logic to trigger image load on sharing
    function shareExpense(expenseId, workspaceId) {
        shareExpenseWithWorkspace(expenseId, workspaceId)
            .then(() => {
                loadImagePreview(expenseId); // Trigger image load
            })
            .catch(error => {
                console.error("Error sharing expense:", error);
            });
    }
    
    function loadImagePreview(expenseId) {
        // Logic to load and update image preview
        const previewImage = getExpensePreviewImage(expenseId);
        updatePreviewImageState(expenseId, previewImage);
    }
  2. Update State Management:
    Location: The component that manages the state of the expenses list, within the React state.
    Change: Ensure that the state is updated promptly with the new preview image once it has been loaded.

    function updatePreviewImageState(expenseId, image) {
        // Logic to update state with the new image
        setState(prevState => ({
            ...prevState,
            expenses: prevState.expenses.map(expense => 
                expense.id === expenseId ? {...expense, previewImage: image} : expense
            )
        }));
    }

Copy link

melvin-bot bot commented Jun 19, 2024

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

@mollfpr
Copy link
Contributor

mollfpr commented Jun 19, 2024

@TheGithubDev I understand what you're trying to achieve but I don't see the RCA pointing to the correct root cause. Could you be more specific where do we need to implement the changes?

Copy link

melvin-bot bot commented Jun 24, 2024

@sonialiap, @mollfpr Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot melvin-bot bot added the Overdue label Jun 24, 2024
@sonialiap
Copy link
Contributor

@TheGithubDev if you're still interested in working on this, would love to hear your thoughts on where we would need to implement this change? (bumping mollfpr's question #43579 (comment))

Copy link

melvin-bot bot commented Jun 26, 2024

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

Copy link

melvin-bot bot commented Jun 26, 2024

@sonialiap @mollfpr 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!

Copy link

melvin-bot bot commented Jun 26, 2024

@sonialiap, @mollfpr Still overdue 6 days?! Let's take care of this!

@mollfpr
Copy link
Contributor

mollfpr commented Jun 26, 2024

No new proposals.

@melvin-bot melvin-bot bot removed the Overdue label Jun 26, 2024
Copy link

melvin-bot bot commented Jul 2, 2024

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

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Aug 6, 2024
Copy link

melvin-bot bot commented Aug 6, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Aug 6, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.16-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-08-13. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Aug 6, 2024

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
  • [@mollfpr] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@mollfpr] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@mollfpr] Determine if we should create a regression test for this bug.
  • [@mollfpr] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@dominictb
Copy link
Contributor

@mollfpr I'll check later today.

@dominictb
Copy link
Contributor

@bondydaa @sonialiap @mollfpr bringing this up #43579 (comment)

I always got this error even on the latest staging and probably this is the same issue on the attached deploy blocker. I'm not sure whether there's something wrong with my account, or is there anything else missing here? This is blocking me from raising the PR to fix the regression.

@dominictb
Copy link
Contributor

image

Same error with my account on prod.

@dominictb
Copy link
Contributor

Ignore me. I'm able to add a scan track expense now. The PR is ready for review cc @mollfpr

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Aug 12, 2024
@sonialiap
Copy link
Contributor

@mollfpr was there a regression on this issue?

@mollfpr
Copy link
Contributor

mollfpr commented Aug 13, 2024

@sonialiap Yes. There's an issue with the submit track expense so we couldn't test it properly. The new PR seems to look good. I'll get it done today.

@sonialiap sonialiap added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Aug 16, 2024
Copy link

melvin-bot bot commented Aug 16, 2024

Triggered auto assignment to @CortneyOfstad (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.

@sonialiap
Copy link
Contributor

@CortneyOfstad I'm OOO Aug 19-30, adding leave buddy.
Status: waiting for #47216 to hit production before payment

@sonialiap
Copy link
Contributor

Payment summary:
$125 due to regression

@CortneyOfstad
Copy link
Contributor

Still on staging 👍

@CortneyOfstad
Copy link
Contributor

Wahoo! Hit production 2 days ago! 7-day period will be over Aug. 26, so adjusting title to reflect that 👍

@CortneyOfstad CortneyOfstad changed the title [HOLD for payment 2024-08-13] [$250] Track expense - Preview image of the shared expense only loads if you open it and go back [HOLD for payment 2024-08-26] [$250] Track expense - Preview image of the shared expense only loads if you open it and go back Aug 21, 2024
Copy link

melvin-bot bot commented Aug 23, 2024

@bondydaa, @CortneyOfstad, @sonialiap, @mollfpr, @dominictb Whoops! This issue is 2 days overdue. Let's get this updated quick!

@CortneyOfstad CortneyOfstad changed the title [HOLD for payment 2024-08-26] [$250] Track expense - Preview image of the shared expense only loads if you open it and go back [HOLD for payment 2024-08-26] [$125] Track expense - Preview image of the shared expense only loads if you open it and go back Aug 26, 2024
Copy link

melvin-bot bot commented Aug 26, 2024

Upwork job price has been updated to $125

@CortneyOfstad
Copy link
Contributor

Payment Summary

@mollfpr — to be paid $125 via NewDot
@dominictb — paid $125 via Upwork

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
No open projects
Status: No status
Development

No branches or pull requests

9 participants