-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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-14] [MEDIUM] Ensure that we preserve order of tags for multiple levels of tag lists #37054
Comments
Triggered auto assignment to @CortneyOfstad ( |
I am not yet in agreement with the solution - https://expensify.slack.com/archives/C03TQ48KC/p1708604069214659?thread_ts=1707328858.869149&cid=C03TQ48KC |
@yuwenmemon so I tried reading through the linked comment above from @flodnv, and I am going to be honest — I have not a clue where we're at or what the next steps are. Do you have an update on where we're at with this so I can proceed from a BugZero perspective? Thanks! |
Unassigning @CortneyOfstad, we don't need BugZero here 😕 After discussing, I agree with the proposed solution |
Thanks @flodnv! I was wondering why I was here 🙃 😂 |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.48-0 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-14. 🎊 |
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:
|
Skipping the payment summary for this issue since all the assignees are employees or vendors. If this is incorrect, please manually add the payment summary SO. |
@yuwenmemon Eep! 4 days overdue now. Issues have feelings too... |
This is done, closing |
Context
We just recently merged #34983, which adds support for multiple levels of tags in the App.
Problem
However, one consideration we missed with the above project was that tags can be reordered, and that this order does not get preserved by the API when sending tags over as a JSON Object. In other words, the following tag list in our database:
Tags in DB Form
Would get translated to the following in Onyx, where the order is alphabetical for the keys:
Tags in Onyx Form
This causes the wrong tag level to be pulled from the code in NewDot that accesses tags by "index"
Solution
As discussed here (internal slack link), start sending an
orderWeight
with the JSON Object of tags sent to the App. This will be generated on the fly. Then use thatorderWeight
to retrieve the proper tag level ingetTagList
cc @flodnv @iwiznia @cead22 @rezkiy37 @amyevans @puneetlath
The text was updated successfully, but these errors were encountered: