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

[Awaiting Payment 25th June] [QBO] Implement the Required toggle in the RHP with multiLevel tags. #41374

Closed
trjExpensify opened this issue May 1, 2024 · 22 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 NewFeature Something to build that is a new item.

Comments

@trjExpensify
Copy link
Contributor

Coming from here and a follow-up to this PR.

  1. We need to implement a new command to allow making an individual tag level required (i.e Customers required, Classes not required)
  2. Add the Required toggle in the RHP of each tag level when multiTags are enabled with QBO, like so:
image

CC: @luacmartins @s77rt @hayata-suenaga @aldo-expensify @zanyrenney

@trjExpensify trjExpensify added Daily KSv2 NewFeature Something to build that is a new item. labels May 1, 2024
Copy link

melvin-bot bot commented May 1, 2024

Current assignee @trjExpensify is eligible for the NewFeature assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels May 1, 2024
@melvin-bot melvin-bot bot added the Overdue label May 9, 2024
@trjExpensify
Copy link
Contributor Author

That PR is still in review.

@melvin-bot melvin-bot bot removed the Overdue label May 10, 2024
@trjExpensify
Copy link
Contributor Author

That PR has merged. @hayata-suenaga this is backend right to implement that command?

@hayata-suenaga
Copy link
Contributor

@trjExpensify, yes it involves the backend change.

Currently, we have SetPolicyRequiresTag.

That code is just enabling all tags in the tags list if there is a tags list (I assume this means that the parent level tags or multi level tags).

We either need to create a new Web-E command or modify the existing command to accomodate the multi level tag situation.

I'll try to work on this but might need some help from @aldo-expensify. I'll open a draft PR today and assign @aldo-expensify so that we can work together.

@trjExpensify
Copy link
Contributor Author

Cool, I was looking at this on OldDot early to see what we call when we set one of the levels as required:

image

.. and equally, enable/disable one of the imported tag list's tag values:

image

Not sure if you looked at that, or if it's helpful here.

@hayata-suenaga
Copy link
Contributor

hayata-suenaga commented May 15, 2024

Memo for myself

Policy_Taglist_Set_Required

tagListIndex -> index of the tag list? each tag list doesn't have an ID?
requireTagList -> boolean indicating if each tag list is required

PolicyAPI::setTaglistRequired

  • Should I create a new command or should I conditionally send the onyx update?

@hayata-suenaga
Copy link
Contributor

hayata-suenaga commented May 15, 2024

memo for myself: Is there something that needs additionally done for the command meant for New Expensify?

@hayata-suenaga
Copy link
Contributor

Reopened an App PR here for this issue (the previous one has issues with merge conflicts) 🙇

@aldo-expensify
Copy link
Contributor

Fixed lint issue with PR #42972

@hayata-suenaga hayata-suenaga removed the Reviewing Has a PR in review label Jun 14, 2024
@melvin-bot melvin-bot bot added the Overdue label Jun 14, 2024
@hayata-suenaga
Copy link
Contributor

With Aldo's final help, the PR has been deployed. removing the reviewing label

@hayata-suenaga hayata-suenaga added the Awaiting Payment Auto-added when associated PR is deployed to production label Jun 14, 2024
@hayata-suenaga
Copy link
Contributor

Payment for @s77rt for the reviewer role after the regression period.

Copy link

melvin-bot bot commented Jun 14, 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.

@hayata-suenaga
Copy link
Contributor

Checked the above deploy blocker and commented there

@trjExpensify
Copy link
Contributor Author

Looks like we're working on a follow-up here.

@melvin-bot melvin-bot bot removed the Overdue label Jun 18, 2024
@hayata-suenaga
Copy link
Contributor

Yes, Rodrigo is working on a regression fix in this PR

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 20, 2024
@hayata-suenaga
Copy link
Contributor

The fix for the regression was deployed to production.

The payment is handled in this issue. Closing this issue now. Thank you everyone!

@melvin-bot melvin-bot bot added the Overdue label Jun 24, 2024
@trjExpensify
Copy link
Contributor Author

Alright, but doesn't that mean @s77rt is required payment of $125 for the initial PR?

@melvin-bot melvin-bot bot removed the Overdue label Jun 24, 2024
@hayata-suenaga
Copy link
Contributor

that's right! Good thing I didn't close this issue 😓

@hayata-suenaga
Copy link
Contributor

@trjExpensify, if you could handle that payment, I'd appreciate it 🙇

@trjExpensify
Copy link
Contributor Author

Cool, @s77rt sent you an offer! Payment summary as follows:

  • $125 for the C+ review of the PR

Thanks!

@trjExpensify trjExpensify changed the title [QBO] Implement the Required toggle in the RHP with multiLevel tags. [Awaiting Payment 25th June] [QBO] Implement the Required toggle in the RHP with multiLevel tags. Jun 25, 2024
@s77rt
Copy link
Contributor

s77rt commented Jun 25, 2024

@trjExpensify Accepted! Thanks!

@trjExpensify
Copy link
Contributor Author

Paid!

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 NewFeature Something to build that is a new item.
Projects
No open projects
Archived in project
Development

No branches or pull requests

4 participants