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-07-10] [$250] [Wave Collect] Disable all but the default distance rate when feature is disabled same for categories and tags #39463

Closed
mountiny opened this issue Apr 2, 2024 · 65 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

@mountiny
Copy link
Contributor

mountiny commented Apr 2, 2024

Problem

When the Distance Rates feature is disabled in NewDot, the distance rates remain enabled. Similarly to other features in the App, we should disable the rates when the feature is disabled.

However, this is also not yet implemented optimistically for tags and categories. This is an edge case that helps in cases of rare offline usage, and as such, it was not noted as regression.

When we disable the categories and tags feature in the backend, we also disable any enabled categories and tags. We need to do the same optimistically in the client (failure data should revert the state to the one before toggling the feature off).

Solution

When the distance rates feature is turned off in NewDot, go through all the distance rates on the policy and disable them except the Default Rate if available. If it's not available, keep the first rate that you go over as enabled and disable all the rest. We need to keep one enabled.

For tags and categories, make sure to turn off the requiresCategory and requiresTag optimistically and then all the tags, even multi-level.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~010ba691024d0e69b3
  • Upwork Job ID: 1775299514448031744
  • Last Price Increase: 2024-06-07
  • Automatic offers:
    • getusha | Reviewer | 102703385
    • dominictb | Contributor | 102703388
Issue OwnerCurrent Issue Owner: @
Issue OwnerCurrent Issue Owner: @kadiealexander
@mountiny mountiny added Daily KSv2 Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. labels Apr 2, 2024
@mountiny mountiny self-assigned this Apr 2, 2024
Copy link

melvin-bot bot commented Apr 2, 2024

Job added to Upwork: https://www.upwork.com/jobs/~010ba691024d0e69b3

Copy link

melvin-bot bot commented Apr 2, 2024

Triggered auto assignment to Contributor Plus for review of internal employee PR - @getusha (Internal)

Copy link

melvin-bot bot commented Apr 2, 2024

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Apr 2, 2024
@mountiny
Copy link
Contributor Author

mountiny commented Apr 4, 2024

Will try to look into this tomorrow

@trjExpensify
Copy link
Contributor

Similarly to other features in the App, we should disable the rates when the feature is disabled (we disable all categories, tags and taxes when the feature is disabled)

Apart from the default or whatever that is, right?

@trjExpensify trjExpensify changed the title [Wave Collect] [Distance] Disable all but the default distance rate when feature is disabled [Wave Collect] [Distance rates] Disable all but the default distance rate when feature is disabled Apr 5, 2024
@mountiny
Copy link
Contributor Author

mountiny commented Apr 5, 2024

Correct, one rate will always stay enabled

@mountiny
Copy link
Contributor Author

mountiny commented Apr 8, 2024

working on this one today

@mountiny mountiny added Daily KSv2 and removed Weekly KSv2 labels Apr 8, 2024
@mountiny
Copy link
Contributor Author

mountiny commented Apr 8, 2024

adding tests

@mountiny mountiny added the Reviewing Has a PR in review label Apr 8, 2024
@mountiny
Copy link
Contributor Author

mountiny commented Apr 8, 2024

PR is ready for a review

@mountiny
Copy link
Contributor Author

mountiny commented Apr 9, 2024

Also: lets make sure the App handles this optimistically

Copy link

melvin-bot bot commented Apr 17, 2024

@mountiny, @kadiealexander, @getusha Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Copy link

melvin-bot bot commented Apr 19, 2024

@mountiny, @kadiealexander, @getusha Huh... This is 4 days overdue. Who can take care of this?

@mountiny
Copy link
Contributor Author

Will have to handle this later this week, focusing on more urgent tasks

Copy link

melvin-bot bot commented Apr 30, 2024

@mountiny, @kadiealexander, @getusha Eep! 4 days overdue now. Issues have feelings too...

Copy link

melvin-bot bot commented May 2, 2024

@mountiny, @kadiealexander, @getusha Still overdue 6 days?! Let's take care of this!

Copy link

melvin-bot bot commented May 7, 2024

@mountiny, @kadiealexander, @getusha 12 days overdue now... This issue's end is nigh!

1 similar comment
Copy link

melvin-bot bot commented May 8, 2024

@mountiny, @kadiealexander, @getusha 12 days overdue now... This issue's end is nigh!

@mountiny
Copy link
Contributor Author

mountiny commented May 8, 2024

Will look into this soon

@dominictb
Copy link
Contributor

PR should be ready for review by tomorrow at most.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Jun 14, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jul 3, 2024
@melvin-bot melvin-bot bot changed the title [$250] [Wave Collect] Disable all but the default distance rate when feature is disabled same for categories and tags [HOLD for payment 2024-07-10] [$250] [Wave Collect] Disable all but the default distance rate when feature is disabled same for categories and tags Jul 3, 2024
Copy link

melvin-bot bot commented Jul 3, 2024

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

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jul 3, 2024
Copy link

melvin-bot bot commented Jul 3, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.3-7 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-07-10. 🎊

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

Copy link

melvin-bot bot commented Jul 3, 2024

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@getusha] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
  • [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@mountiny
Copy link
Contributor Author

mountiny commented Jul 5, 2024

this should be ready for payment when the time comes

$250 to @dominictb and to @getusha

@kadiealexander
Copy link
Contributor

@getusha @mountiny do we need a regession test here?

@melvin-bot melvin-bot bot removed the Overdue label Jul 14, 2024
@kadiealexander
Copy link
Contributor

kadiealexander commented Jul 15, 2024

The contracts expired so I've resent them here. Payouts due:

Upwork job is here.

@dominictb
Copy link
Contributor

TL;DR: effective today, Thursday, April 4th, 2024 the initial job price for bugs/feature requests will start at $250.

@kadiealexander Hi, this issue was created on Apr 3, so according to the official post here, this one's initial price should remain at $500

@kadiealexander
Copy link
Contributor

Thanks @dominictb, I've updated the offers.

@dominictb
Copy link
Contributor

@kadiealexander I accepted it, thank you

@kadiealexander
Copy link
Contributor

@getusha please share the regression test steps.

@getusha
Copy link
Contributor

getusha commented Jul 17, 2024

[@getusha] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.

Regression Test Proposal

  1. Go offline
  2. Go to workspaces > Select a workspace
  3. Go to categories
  4. Make sure to have one category enabled
  5. Go to more features
  6. Disable & Enable Categories feature
  7. Go to categories
  8. Verify that every category is disabled
  9. Go to distance rates
  10. Make sure to have at least two rates enabled
  11. Go to more features page
  12. Disable & Enable distance rates feature
  13. Go to Distance rates page
  14. Verify that only one distance rate remains enabled
  15. Go to Tags page
  16. Make sure to have at least one tag enabled
  17. Go to more features page
  18. Disable & Enable Tags feature
  19. Go to Tags page
  20. Verify that every options are disabled

Do we agree 👍 or 👎
cc @mountiny @dominictb

@getusha
Copy link
Contributor

getusha commented Jul 18, 2024

Accepted the offer @kadiealexander

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
Archived in project
Development

No branches or pull requests

6 participants