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-03-29] [Performance] Reduce Redundant Operations #37275

Closed
muttmuure opened this issue Feb 27, 2024 · 12 comments
Closed
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Monthly KSv2

Comments

@muttmuure
Copy link
Contributor

Problem

In our getOrderedReportIDs we have loops like forEach and sort. In there, we are invoking many utils functions and some of them have static translations in there. Which translates the same text on each loop iteration. For the lots of data, this becomes an issue for the application. Apart from this, we also have formatPhoneNumber which parses the passed number and this util function leverages awesome-phone-number library. With further investigation, it appeared that the number we are trying to parse, might not be a number at all. It can also be an email like concierge@expensify.com and it is not ideal to parse an email from a phone number library.

Solution

For translations, we created a separate file and there we are translating the text that was repeatedly reported in our audit. We also have Onyx connection to locale key, so if locale changes we have all the updated translations.
For phone number, we can early return if the passed number is a string or if it doesn’t contain sms domain.

#37200

@hurali97
Copy link
Contributor

Hi @muttmuure 👋 Please assign this to me 👍

@muttmuure
Copy link
Contributor Author

Done!

Copy link

melvin-bot bot commented Mar 20, 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.

@melvin-bot melvin-bot bot added Overdue Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Mar 20, 2024
@melvin-bot melvin-bot bot changed the title [Performance] Reduce Redundant Operations [HOLD for payment 2024-03-29] [Performance] Reduce Redundant Operations Mar 22, 2024
@melvin-bot melvin-bot bot removed the Overdue label Mar 22, 2024
Copy link

melvin-bot bot commented Mar 22, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.55-3 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-03-29. 🎊

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

  • @hurali97 does not require payment (Contractor)

@fedirjh
Copy link
Contributor

fedirjh commented Mar 26, 2024

cc @muttmuure Could you please assign this issue to me to track the payment? Thank you.

@fedirjh
Copy link
Contributor

fedirjh commented Apr 9, 2024

cc @muttmuure Friendly bump for the payment. Thank you.

@melvin-bot melvin-bot bot added Overdue and removed Weekly KSv2 labels Apr 9, 2024
Copy link

melvin-bot bot commented Apr 15, 2024

This issue has not been updated in over 15 days. @hurali97 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!

@melvin-bot melvin-bot bot added Monthly KSv2 and removed Overdue labels Apr 15, 2024
@muttmuure
Copy link
Contributor Author

@fedirjh invited you to this job: https://www.upwork.com/jobs/~010796b6f68d39c46b

@muttmuure muttmuure moved this from CRITICAL to Done in [#whatsnext] #quality May 2, 2024
@fedirjh
Copy link
Contributor

fedirjh commented May 2, 2024

@muttmuure Thank you. Accepted

@muttmuure
Copy link
Contributor Author

Offer sent

@fedirjh
Copy link
Contributor

fedirjh commented May 2, 2024

Accepted. Thank you

@muttmuure
Copy link
Contributor Author

Paid. Closing out

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 Monthly KSv2
Projects
Status: Done
Development

No branches or pull requests

3 participants