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

CRITICAL: [Track expense] [$250] Remove beta flag for everyone #39309

Closed
2 tasks done
quinthar opened this issue Mar 30, 2024 · 22 comments
Closed
2 tasks done

CRITICAL: [Track expense] [$250] Remove beta flag for everyone #39309

quinthar opened this issue Mar 30, 2024 · 22 comments
Assignees
Labels
Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Reviewing Has a PR in review

Comments

@quinthar
Copy link
Contributor

quinthar commented Mar 30, 2024

Problem:

This feature is ready for everyone, let's ship it!

Solution:

Let's:

  1. Enable everyone for the beta
  2. Rip out all the beta code everywhere
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~014ddf8901aeb1c9cc
  • Upwork Job ID: 1787975091637518336
  • Last Price Increase: 2024-05-07
  • Automatic offers:
    • tienifr | Contributor | 0
Issue OwnerCurrent Issue Owner: @sobitneupane
@quinthar quinthar converted this from a draft issue Mar 30, 2024
@quinthar quinthar changed the title CRITICAL: [Track expense] Remove beta flag for everyone CRITICAL: [Track expense] [HOLD] Remove beta flag for everyone Apr 2, 2024
@melvin-bot melvin-bot bot added the Monthly KSv2 label Apr 5, 2024
@quinthar quinthar changed the title CRITICAL: [Track expense] [HOLD] Remove beta flag for everyone CRITICAL: [Track expense] Remove beta flag for everyone May 4, 2024
@quinthar
Copy link
Contributor Author

quinthar commented May 4, 2024

taking off hold!

@thienlnam thienlnam added the External Added to denote the issue can be worked on by a contributor label May 7, 2024
Copy link

melvin-bot bot commented May 7, 2024

Job added to Upwork: https://www.upwork.com/jobs/~014ddf8901aeb1c9cc

@melvin-bot melvin-bot bot changed the title CRITICAL: [Track expense] Remove beta flag for everyone [$250] CRITICAL: [Track expense] Remove beta flag for everyone May 7, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label May 7, 2024
Copy link

melvin-bot bot commented May 7, 2024

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Overdue Monthly KSv2 labels May 7, 2024
@thienlnam
Copy link
Contributor

Looking for someone to remove all the TrackExpense beta related code in the FE

@gijoe0295
Copy link
Contributor

gijoe0295 commented May 8, 2024

Proposal

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

Remove beta flag for track expense

What is the root cause of that problem?

This is a feature request

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

Check for canUseTrackExpense flags and remove them. This is quite a straight-forward work.

Most of them are tied with isSelfDM check so we can easily remove.

@tienifr
Copy link
Contributor

tienifr commented May 8, 2024

Proposal

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

We want to rip out all the beta code for track expense everywhere

What is the root cause of that problem?

This is feature request

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

The flag we need to remove is canUseTrackExpense, for places that are using canUseTrackExpense, we need to assume that canUseTrackExpense is always true, and refactor the code accordingly

There're 2 types of places we're using canUseTrackExpense:

  1. Using canUseTrackExpense with negated value, there's 1 instance of this here. When we assume that canUseTrackExpense will be true, !canUseTrackExpense will be false. If we simply remove canUseTrackExpense or !canUseTrackExpense here, it will cause a regression because the isSelfDM after it is still evaluated while it should not be, since !canUseTrackExpense should already be false.

In this case we need to remove the entire block where !canUseTrackExpense is used, which means removing:

|| (!canUseTrackExpense && isSelfDM(report))
  1. Using canUseTrackExpense value as is (not negated), the rest of the cases belong to this category, for example here, in this case removing the canUseTrackExpense itself will not impact the conditions (because canUseTrackExpense is true), so we're safe to remove only canUseTrackExpense without any additional refactoring.

For the methods that have canUseTrackExpense as param, we can remove the param too.

After removing usage of canUseTrackExpense, we can then remove the method itself here and then the beta const here

What alternative solutions did you explore? (Optional)

NA

@sobitneupane
Copy link
Contributor

Thanks for the proposal everyone.

Proposal from @tienifr looks good to me. They have explained the possible scenarios with the solution.

🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented May 8, 2024

Current assignee @thienlnam is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

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

melvin-bot bot commented May 8, 2024

📣 @tienifr 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@tienifr tienifr mentioned this issue May 9, 2024
50 tasks
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels May 9, 2024
@quinthar quinthar changed the title [$250] CRITICAL: [Track expense] Remove beta flag for everyone CRITICAL: [Track expense] [$250] Remove beta flag for everyone May 9, 2024
@quinthar
Copy link
Contributor Author

quinthar commented May 9, 2024

@tienifr what's your ETA on this?

@thienlnam
Copy link
Contributor

Just merged the App PR, now pending deploy

Copy link

melvin-bot bot commented May 15, 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 removed the Weekly KSv2 label Jun 3, 2024
Copy link

melvin-bot bot commented Jun 3, 2024

This issue has not been updated in over 15 days. @sobitneupane, @thienlnam, @tienifr 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 the Monthly KSv2 label Jun 3, 2024
@thienlnam
Copy link
Contributor

This is done

@thienlnam
Copy link
Contributor

Ah, re-opening so we can handle payment here

@thienlnam thienlnam reopened this Jun 6, 2024
@thienlnam thienlnam added the NewFeature Something to build that is a new item. label Jun 6, 2024
Copy link

melvin-bot bot commented Jun 6, 2024

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

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Monthly KSv2 labels Jun 6, 2024
@thienlnam thienlnam added Daily KSv2 and removed Weekly KSv2 labels Jun 6, 2024
@thienlnam
Copy link
Contributor

This is ready to be paid out cc @mallenexpensify - there were no regressions, the linked issue is from a different cause

@tienifr
Copy link
Contributor

tienifr commented Jun 6, 2024

@mallenexpensify According to this announcement, this issue is marked as CRITICAL and should have a base price of $500. Could you help to update the issue title and factor that into the payment summary?

Thanks!

While bugs/feature requests will now start at $250, any that are deemed Critical or High to our roadmap (noted via the label High Priority) will automatically be updated to $500. This is to ensure we get the necessary attention and urgency applied to these issues.

@mallenexpensify
Copy link
Contributor

@tienifr can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~014aedec8d9684b35c

For price, I commented on that last week

There have been questions about what constitutes a job being auto-increased to $500. These jobs are only the ones that have the High Priority label added to them (ie. issue with HIGH or CRITICAL in the title or project status/priority are not auto-increased to $500).

@tienifr
Copy link
Contributor

tienifr commented Jun 10, 2024

@mallenexpensify I accepted the offer, thanks!

@mallenexpensify
Copy link
Contributor

Contributor: @tienifr paid $250 via Upwork
Contributor+: @sobitneupane due $250 via NewDot

Def don't think we need a regression test here

@JmillsExpensify
Copy link

$250 approved for @sobitneupane

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Reviewing Has a PR in review
Projects
No open projects
Status: CRITICAL
Development

No branches or pull requests

7 participants