Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.54k stars 2.89k forks source link

[HOLD for payment 2024-11-01] [$250] Tag rules - Tag rules are available for multi tags when the rules are not present on Old Dot #49543

Closed IuliiaHerets closed 5 days ago

IuliiaHerets commented 1 month ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.39-0 Reproducible in staging?: Y Reproducible in production?: N/A - new feature, doesn't exist in prod Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

Precondition:

  1. Go to staging.new.expensify.com
  2. Go to workspace settings > Tags.
  3. Click on any main tag.
  4. Click on any sub tag.

Expected Result:

Tag rules should not be available for multi tags (because the rules are not present on Old Dot).

Actual Result:

Tag rules are available for multi tags when the rules are not present on Old Dot.

Workaround:

Unknown

Platforms:

Screenshots/Videos

Bug66100821726848355301!Dependent-_Multi_Level_tags_NEW.csv

https://github.com/user-attachments/assets/33cf5eb8-a1a9-462a-a5dc-9bbe3f92a798

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021843317478267181555
  • Upwork Job ID: 1843317478267181555
  • Last Price Increase: 2024-10-14
  • Automatic offers:
    • situchan | Contributor | 104469276
Issue OwnerCurrent Issue Owner: @bfitzexpensify
melvin-bot[bot] commented 1 month ago

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

github-actions[bot] commented 1 month ago

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.
IuliiaHerets commented 1 month ago

We think that this bug might be related to #wave-control

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @deetergp (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

deetergp commented 1 month ago

This is behind a beta so it's not a blocker.

cc @marcaaron

marcaaron commented 1 month ago

I'm also slightly confused about why we don't allow approvers on multi-level tags. Maybe there's just not much overlap or it would be strange to potentially have multiple approvers? @JmillsExpensify what do you think about this one?

marcaaron commented 1 month ago

I'm not able to reproduce this on main fwiw. @yuwenmemon is working on tags related features for NewDot, maybe we can ask him to keep an eye on this.

marcaaron commented 1 month ago

2024-09-20_09-19-09

maybe this has something to do with multiple policy set up with different configurations?

IuliiaHerets commented 1 month ago

@marcaaron Policy should be Control, you can enable Rules in More features to upgrade it

marcaaron commented 1 month ago

Yes. Tested with a Control policy. The provided steps do not lead to a reproduction.

IuliiaHerets commented 1 month ago

Issue is still reproducible, tested just now

https://github.com/user-attachments/assets/40aece8b-8e6a-4bcb-aec5-592bb74de452

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

Current assignee @bfitzexpensify is eligible for the Bug assigner, not assigning anyone new.

strepanier03 commented 1 month ago

@bfitzexpensify - I am going OoO until the 3rd of October and I didn't get to this before I headed out. Thanks for taking over πŸ™Œ

deetergp commented 1 month ago

I was able to reproduce this.

@marcaaron Do you think this is something we can make external?

marcaaron commented 1 month ago

Maybe? I'd kinda still like to hear from @JmillsExpensify and @yuwenmemon on this. I'm not sure if this is something we should fix as a "bug" or scope into a future project. There's also the question "Tag rules" still being behind a beta. So, I'd wonder if this is a blocker or not for removing that beta.

deetergp commented 1 month ago

@JmillsExpensify @yuwenmemon Any thoughts to add here regarding Marc's comment?

melvin-bot[bot] commented 1 month ago

@deetergp, @bfitzexpensify Eep! 4 days overdue now. Issues have feelings too...

bfitzexpensify commented 1 month ago

Bump @JmillsExpensify @yuwenmemon re this comment

yuwenmemon commented 1 month ago

Seems like a bug to me. cc @dylanexpensify

I would say we should fix this independently because the multi-tags project won't touch this part of the tag editor (the individual view where rules would be shown) anyway.

melvin-bot[bot] commented 1 month ago

@deetergp @bfitzexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

deetergp commented 1 month ago

Okay, I am going to make this external and handle it here.

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

@deetergp, @parasharrajat, @bfitzexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] commented 3 weeks ago

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

parasharrajat commented 3 weeks ago

No proposals.

rezkiy37 commented 3 weeks ago

Hi, I am Michael (Mykhailo) from Callstack, an expert agency and I can work on this issue.

rezkiy37 commented 3 weeks ago

I can confirm that the problem is replicable. However, it requires a couple of betas: categoryAndTagApprovers and workspaceRules. I am preparing a proposal.

Details Screenshot 2024-10-16 at 16 16 44
rezkiy37 commented 3 weeks ago

Proposal

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

The tag rules settings are available for multi-tags when the rules are not present on Old Dot.

What is the root cause of that problem?

There are no conditions to restrict the tag rule settings for multi-tags now.

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

I am proposing to add one more condition to the tag settings page (TagSettingsPage.tsx) - !isMultiLevelTags (not multi-level tags).

https://github.com/Expensify/App/blob/2c211a03948ebc1dfb20ee2f836d9669051acf2f/src/pages/workspace/tags/TagSettingsPage.tsx#L157-L161

The file already has this variable here.

https://github.com/Expensify/App/blob/2c211a03948ebc1dfb20ee2f836d9669051acf2f/src/pages/workspace/tags/TagSettingsPage.tsx#L87

What alternative solutions did you explore? (Optional)

N/A

situchan commented 3 weeks ago

I can take this as C+ while @parasharrajat is OOO until Oct 25

rezkiy37 commented 3 weeks ago

I can take this as C+ while @parasharrajat is OOO until Oct 25

cc @yuwenmemon @marcaaron @deetergp

melvin-bot[bot] commented 3 weeks ago

πŸ“£ @situchan πŸŽ‰ 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 πŸ“–

deetergp commented 3 weeks ago

Swapped out @parasharrajat for @situchan. Thanks for stepping up, Situ!

situchan commented 3 weeks ago

restrict the tag rule settings for multi-tags for now

So this is what we landed!

The proposal looks good. I think we can continue PR. πŸŽ€πŸ‘€πŸŽ€ C+ reviewed

melvin-bot[bot] commented 3 weeks ago

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

deetergp commented 3 weeks ago

@rezkiy37 Proposal looks good. Adding them now.

rezkiy37 commented 2 weeks ago

I've opened a PR (https://github.com/Expensify/App/pull/50917) for review πŸ™‚

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.53-1 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-11-01. :confetti_ball:

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

melvin-bot[bot] commented 2 weeks ago

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

situchan commented 6 days ago

There was no regression.

Regression Test Proposal

Precondition: The rules are enabled. Dependent multi-tags are set up on OD.

  1. Go to workspace settings > Tags.
  2. Click on any main tag.
  3. Click on any sub tag.
  4. Verify that the tag rules should not be available for multi-tags.
bfitzexpensify commented 5 days ago

Payment complete, regression steps added. Closing this out, thanks everyone!