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-04-23] [$500] [Performance] Decouple draft state computation from report #37880

Closed
muttmuure opened this issue Mar 7, 2024 · 19 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@muttmuure
Copy link
Contributor

muttmuure commented Mar 7, 2024

Problem

Whenever a user starts to type a new message in Composer, the hasDraft state inside current Report object needs to be updated. The state is later used in LHN to determine if a draft indicator (pencil icon) should be displayed.

Calculations performed by the app to acquire updated hasDraft state are costly. It is caused by the fact that the hasDraft state is a property of a Report object which is then part of the entire list of reports user has access to (14k on testing account). Change of a single report causes list to be invalidated, making costly calculations of getOrderedReportIDs inside SidebarLinksData (component that resides on top of LHN) to run again.

❗ The getOrderedReportIDs calculations might take a few hundred ms, up to a few seconds depending on device and number of reports passed to the function. It is a severe execution overhead, considering how often user starts typing new message during regular conversation.

Solution

The hasDraft state can be decoupled from Report object and stored as an individual entry (or list of entries for many reports) under separate Onyx key. Such a solution should help us avoid invalidating the entire list of reports on every new message being typed, thus not triggering costly re-calculation of getOrderedReportIDs.

In Onyx, the REPORT_DRAFT_COMMENT key exists and serves similar purpose as hasDraft. Considering how hasDraft is used (to display draft indicator) it should not be an issue to re-use REPORT_DRAFT_COMMENT key for such functionality. It is mostly an implementation detail, but this way we are reducing app complexity and state redundancy.

From : @kacper-mikolajczak here

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~012397339f4d656420
  • Upwork Job ID: 1777386978362355712
  • Last Price Increase: 2024-04-20
@mountiny
Copy link
Contributor

@kacper-mikolajczak can you comment here?

@kacper-mikolajczak
Copy link
Contributor

kacper-mikolajczak commented Mar 11, 2024

Yessir! @mountiny

@melvin-bot melvin-bot bot added the Monthly KSv2 label Mar 11, 2024
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Monthly KSv2 labels Mar 25, 2024
Copy link

melvin-bot bot commented Apr 4, 2024

Triggered auto assignment to @AndrewGable, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@shubham1206agra
Copy link
Contributor

Please assign me here as C+

@muttmuure muttmuure added the External Added to denote the issue can be worked on by a contributor label Apr 8, 2024
@melvin-bot melvin-bot bot changed the title [Performance] Decouple draft state computation from report [$250] [Performance] Decouple draft state computation from report Apr 8, 2024
Copy link

melvin-bot bot commented Apr 8, 2024

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

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

melvin-bot bot commented Apr 8, 2024

Current assignee @shubham1206agra is eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Apr 8, 2024
@muttmuure muttmuure removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 8, 2024
Copy link

melvin-bot bot commented Apr 8, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

Copy link

melvin-bot bot commented Apr 15, 2024

@AndrewGable, @shubham1206agra, @kacper-mikolajczak Whoops! This issue is 2 days overdue. Let's get this updated quick!

@kacper-mikolajczak
Copy link
Contributor

Melvin, PR was merged already

@shubham1206agra
Copy link
Contributor

@mountiny Can you bump the price of this issue to $500, as this issue was created before the change?
And can you get this issue ready for payment?

@mountiny mountiny changed the title [$250] [Performance] Decouple draft state computation from report [HOLD for payment 2024-04-23] [$500] [Performance] Decouple draft state computation from report Apr 20, 2024
Copy link

melvin-bot bot commented Apr 20, 2024

Upwork job price has been updated to $500

@mountiny mountiny added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review Daily KSv2 labels Apr 20, 2024
@mountiny mountiny added the NewFeature Something to build that is a new item. label Apr 20, 2024
Copy link

melvin-bot bot commented Apr 20, 2024

Triggered auto assignment to @dylanexpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

@mountiny
Copy link
Contributor

I am fine with the price as it was complex change and it was made before the price change too.

$500 to @shubham1206agra once ready @dylanexpensify

Copy link

melvin-bot bot commented Apr 23, 2024

Payment Summary

Upwork Job

BugZero Checklist (@dylanexpensify)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1777386978362355712/hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@dylanexpensify
Copy link
Contributor

@shubham1206agra sent invite to apply!

@shubham1206agra
Copy link
Contributor

@dylanexpensify Accepted

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Apr 27, 2024
@dylanexpensify
Copy link
Contributor

@shubham1206agra offer sent!

@shubham1206agra
Copy link
Contributor

@dylanexpensify Offer accepted

@dylanexpensify
Copy link
Contributor

Done!

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 Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
Development

No branches or pull requests

6 participants