Skip to content

[HOLD for payment 2025-02-04] [$250] Remove the getTripTransactions method #55077

Closed

Description

@tgolen

Coming from #54965 (comment)

Problem

When getTripTransactions() is called, it accesses data from Onyx that the view is not connected to. This breaks the reactivity of the component because when the transactions are updated in Onyx, the component won't receive the changes. This leads to stale data.

Solution

Remove getTripTransactions() and connect to the data using useOnyx() instead.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021877740338376092500
  • Upwork Job ID: 1877740338376092500
  • Last Price Increase: 2025-01-10
  • Automatic offers:
    • shubham1206agra | Contributor | 105641744
Issue OwnerCurrent Issue Owner: @shubham1206agra

Activity

  1. added
    Help WantedApply this label when an issue is open to proposals by contributors
    BugSomething is broken. Auto assigns a BugZero manager.
    on Jan 10, 2025
  2. self-assigned this
    on Jan 10, 2025
  3. melvin-bot commented on Jan 10, 2025

    @melvin-bot

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

  4. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Jan 10, 2025
  5. melvin-bot commented on Jan 10, 2025

    @melvin-bot
  6. changed the title [-]Remove the `getTripTransactions` method[/-] [+][$250] Remove the `getTripTransactions` method[/+] on Jan 10, 2025
  7. melvin-bot commented on Jan 10, 2025

    @melvin-bot

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

  8. shubham1206agra commented on Jan 10, 2025

    @shubham1206agra
    Contributor

    @tgolen You need to assign me for review as no one has trip rooms set up right now.

  9. mohit6789 commented on Jan 10, 2025

    @mohit6789
    Contributor

    Proposal

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

    Make changes in getTripTransactions() so it will use reports using withOnyx() instead of Onyx.connect

    What is the root cause of that problem?

    Improvement remove getTripTransactions and use withOnyx()

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

    1. Make changes in ReportsUtlis so it can access reports data using withOnyx().

    App/src/libs/ReportUtils.ts

    Lines 8175 to 8183 in 3541c89

    function getTripTransactions(tripRoomReportID: string | undefined, reportFieldToCompare: 'parentReportID' | 'reportID' = 'parentReportID'): Transaction[] {
    const tripTransactionReportIDs = Object.values(allReports ?? {})
    .filter((report) => report && report?.[reportFieldToCompare] === tripRoomReportID)
    .map((report) => report?.reportID);
    return tripTransactionReportIDs.flatMap((reportID) => getReportTransactions(reportID));
    }
    function getTripIDFromTransactionParentReportID(transactionParentReportID: string | undefined): string | undefined {
    return getReportOrDraftReport(transactionParentReportID)?.tripData?.tripID;

    function getTripTransactions(reports: OnyxCollection<Report>, tripRoomReportID: string | undefined, reportFieldToCompare: 'parentReportID' | 'reportID' = 'parentReportID'): Transaction[] {
        const tripTransactionReportIDs = Object.values(reports ?? {})
            .filter((report) => report && report?.[reportFieldToCompare] === tripRoomReportID)
            .map((report) => report?.reportID);
        return tripTransactionReportIDs.flatMap((reportID) => getReportTransactions(reportID));
    }
    
    1. pass reports data from TripDetailsView.tsx

    const tripTransactions = ReportUtils.getTripTransactions(tripRoomReportID);

    const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
    const tripTransactions = ReportUtils.getTripTransactions(reports, tripRoomReportID);
    
    1. pass reports data from TripRoomPreview.tsx
      const tripTransactions = ReportUtils.getTripTransactions(chatReport?.reportID);

    changes in both the components

    const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
    const tripTransactions = ReportUtils.getTripTransactions(reports, chatReport?.reportID);
    

    What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

    What alternative solutions did you explore? (Optional)

  10. github-actions commented on Jan 10, 2025

    @github-actions
    Contributor

    github123 Your proposal will be dismissed because you did not follow the proposal template.

  11. Shahidullah-Muffakir commented on Jan 10, 2025

    @Shahidullah-Muffakir
    Contributor

    Proposal

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

    When getTripTransactions() is called directly in the component, it accesses Onyx data that the component is not connected to. This breaks reactivity because the component won't re-render when the underlying transactions data changes in Onyx, leading to stale data being displayed to users.

    What is the root cause of that problem?

    The root cause is that we're accessing Onyx data through a utility function without establishing a proper subscription to that data.

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

    Instead of calling getTripTransactions() directly, we should connect to the data using useOnyx with a selector that utilizes the existing utility function. Like this:
    here:

    const tripTransactions = ReportUtils.getTripTransactions(tripRoomReportID);

    and here:
    const tripTransactions = ReportUtils.getTripTransactions(chatReport?.reportID);

    write it like this:

        const [tripTransactions] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {
            selector:(reports)=>  ReportUtils.getTripTransactions(reports, chatReport?.reportID),
        });

    and accept reports as param in the getTripTransactions

    What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

    What alternative solutions did you explore? (Optional)

  12. 25 remaining items

  13. melvin-bot commented on Jan 28, 2025

    @melvin-bot

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

  14. melvin-bot commented on Jan 28, 2025

    @melvin-bot

    The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.89-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 2025-02-04. 🎊

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

  15. melvin-bot commented on Jan 28, 2025

    @melvin-bot

    @shubham1206agra @isabelastisser @shubham1206agra The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

  16. cristipaval commented on Jan 29, 2025

    @cristipaval
    Contributor

    Sounds like a backend fix needs to be done for that, yeah. I'll cc @stitesExpensify about it so he is aware of the issue.

    I'm making this my top priority: #55754

  17. isabelastisser commented on Feb 4, 2025

    @isabelastisser
    Contributor

    @shubham1206agra @blazejkustra, any updates with the PR? Is this issue awaiting payment?

  18. shubham1206agra commented on Feb 4, 2025

    @shubham1206agra
    Contributor

    @isabelastisser PR was merged already
    This is waiting for payment
    And there are no tests to be added here

  19. isabelastisser commented on Feb 6, 2025

    @isabelastisser
    Contributor

    @shubham1206agra, please accept the offer in Upwork, and I will process the payment. Thanks!

  20. shubham1206agra commented on Feb 6, 2025

    @shubham1206agra
    Contributor

    @isabelastisser Offer accepted

  21. isabelastisser commented on Feb 19, 2025

    @isabelastisser
    Contributor

    @shubham1206agra I sent you a new offer in Upwork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions