Skip to content

Remove Onyx.connect() for the key: ${ONYXKEYS.COLLECTION.REPORT}${reportID} in src/libs/actions/Report.ts #73667

Closed

Description

@tgolen

This is part of deprecating Onyx.connect. Parent Issue

Details

  • Module: src/libs/actions/Report.ts
  • Onyx Key: ${ONYXKEYS.COLLECTION.REPORT}${reportID}
  • Reference: Line 3597: ${ONYXKEYS.COLLECTION.REPORT}${reportID}

Be sure to refactor methods using test driven development:

  1. Create a unit test for methods before refactoring
  2. Refactor the methods
  3. Ensure the unit test still runs after refactoring
  4. Reinforce the unit testing with a functional test and a QA test
  5. Once you are done, decrease the number of allowable ESLint warnings here to equal the number of Onyx.connect references remaining in the code
Issue OwnerCurrent Issue Owner: @parasharrajat

Activity

  1. self-assigned this
    on Oct 28, 2025
  2. tgolen commented on Oct 28, 2025

    @tgolen
    ContributorAuthor

    Warning

    DO NOT POST PROPOSALS ON THIS ISSUE!!

    Bug-Zero Instructions

    • This issue is CLOSED for proposals
    • It is a special issue and part of a bigger project
    • A contributor will be manually assigned to work on this issue and they do not need to submit a proposal
    • The bug-zero person only needs to take care of payments to the contributor for the PR and the C+ for reviews
    • There do not need to be any regression tests added or updated since this is only refactoring and shouldn't change any existing functionality
  3. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Oct 28, 2025
  4. melvin-bot commented on Oct 28, 2025

    @melvin-bot

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

  5. samranahm commented on Oct 28, 2025

    @samranahm
    Contributor

    Are we accepting External contributors on it, I’m ready to work on this one.

  6. tgolen commented on Oct 28, 2025

    @tgolen
    ContributorAuthor

    @samranahm historically, this project has been for a closed group of contributors. However, I would be open to seeing what your proposal would be for this one. If it works out and you are solid and know what you're doing with few instuctions, then perhaps you can work on more of these for us. Can you post a quick proposal for this one, please?

  7. samranahm commented on Oct 28, 2025

    @samranahm
    Contributor

    Proposal

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

    Remove Onyx.connect() for the key

    What is the root cause of that problem?

    Part of deprecating Onyx.connect

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

    Currently we use this key ${ONYXKEYS.COLLECTION.REPORT}${reportID} to get the associated report using Onyx.connect

    App/src/libs/actions/Report.ts

    Lines 3597 to 3607 in f2b8cc1

    const reportConnection = Onyx.connect({
    key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
    // eslint-disable-next-line rulesdir/prefer-early-return
    callback: (report) => {
    // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
    if (report?.errorFields?.notFound || report?.reportID) {
    Onyx.disconnect(reportConnection);
    navigateHandler(report);
    }
    },
    });

    • We should move the openReportFromDeepLink function to libs/actions/Link.ts from Report.ts
    • Second we should change the Onyx.connect to Onyx.connectWithoutView

    here

    let isNetworkOffline = false;
    // Use connectWithoutView since this is to open an external link and doesn't affect any UI
    Onyx.connectWithoutView({
    key: ONYXKEYS.NETWORK,
    callback: (value) => (isNetworkOffline = value?.isOffline ?? false),
    });

    We also need to add unit test and verify current test work as expected after the refactoring and decrease the max-warnings.

    "lint": "NODE_OPTIONS=--max_old_space_size=8192 eslint . --max-warnings=154 --cache --cache-location=node_modules/.cache/eslint",

    What alternative solutions did you explore? (Optional)

    Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

  8. 13 remaining items

  9. parasharrajat commented on Nov 14, 2025

    @parasharrajat
    Member

    @twisterdotcom No checklist is needed here. Its ready for payment.

  10. twisterdotcom commented on Nov 14, 2025

    @twisterdotcom
    Contributor

    @tgolen what is the payment here?

  11. samranahm commented on Nov 19, 2025

    @samranahm
    Contributor

    @tgolen Gentle bump.

  12. tgolen commented on Nov 19, 2025

    @tgolen
    ContributorAuthor

    Let's set this one to $750 to cover all three issues.

  13. tgolen commented on Nov 19, 2025

    @tgolen
    ContributorAuthor

    I spoke with Rajat and we actually thought that $375 was a more accurate price, considering the scope of the PRs. Let's go with $375 @twisterdotcom.

  14. twisterdotcom commented on Nov 19, 2025

    @twisterdotcom
    Contributor

    When is this due for payment? Is it only Rajat due for $375? What about samranahm?

  15. samranahm commented on Nov 20, 2025

    @samranahm
    Contributor

    @twisterdotcom PR #74237 hit production two weeks ago. We’re both due $375 each.

  16. twisterdotcom commented on Nov 20, 2025

    @twisterdotcom
    Contributor

    Payment Summary:

  17. samranahm commented on Nov 20, 2025

    @samranahm
    Contributor

    Thanks @twisterdotcom, I've accepted the offer.

  18. parasharrajat commented on Dec 19, 2025

    @parasharrajat
    Member

    Payment requested as per #73667 (comment)

  19. twisterdotcom commented on Dec 19, 2025

    @twisterdotcom
    Contributor

    @parasharrajat has submitted payment for this report on ND here.

    SO: https://stackoverflowteams.com/c/expensify/questions/7582

    Zapier Logs Run ID: 00040eee-106c-a7e1-b7be-8f423a4bcad4
  20. trjExpensify commented on Dec 19, 2025

    @trjExpensify
    Contributor

    Confirming payment summary here is correct.

  21. added
    Skip PaymentSkips all the payment process steps for an issue. Use it for fixed-price projects.
    on Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

BugSomething is broken. Auto assigns a BugZero manager.EngineeringImprovementItem broken or needs improvement.ReviewingHas a PR in reviewSkip PaymentSkips all the payment process steps for an issue. Use it for fixed-price projects.WeeklyKSv2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions