-
Notifications
You must be signed in to change notification settings - Fork 3k
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-04-30] [$500] Deprecate POLICY_MEMBERS #39376
Comments
Triggered auto assignment to @MitchExpensify ( |
Job added to Upwork: https://www.upwork.com/jobs/~0113641b8717113c5d |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Deprecate POLICY_MEMBERS What is the root cause of that problem?New feature What changes do you think we should make in order to solve the problem?1. Delete old values 2. Refactor code In this part we need to check for the presence of policies or policiesDraft in the components(utils) and then use Also we need update all places that retrieve the 3. Update tests and PolicyMembersUtils I noticed that we have some tests where we use What alternative solutions did you explore? (Optional)NA |
ProposalPlease re-state the problem that we are trying to solve in this issue.Need to deprecate What is the root cause of that problem?Currently we have
What changes do you think we should make in order to solve the problem?We need to update
Some examples are
What alternative solutions did you explore? (Optional)N/A |
ProposalPlease re-state the problem that we are trying to solve in this issue.Deprecate What is the root cause of that problem?No RCA What changes do you think we should make in order to solve the problem?
Line 312 in a4d01a8
Line 310 in a4d01a8
What alternative solutions did you explore? (Optional)NA |
Thanks everyone for the proposals. Since this a pretty straight forward task, I'll go with the first propposal that we got. Assigning @ZhenjaHorbach to this issue! |
📣 @ZhenjaHorbach 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
PR will be ready today or tomorrow |
📣 @hungvu193 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
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. |
PR was deployed into production on April 22 |
Payment summary C $500 @ZhenjaHorbach Upwork |
Paid and contracts ended |
We're updating how we store Onyx data for certain policy related keys and we decided to deprecate
POLICY_MEMBERS
in favor of policy.employeeList. That means that we have to:policyMembers_
key to usepolicy_
key and retrieve the data fromemployeeList
accountID
to useemail
instead, sinceemployeeList
is keyed by emails, not accountIDI got started in this PR, but I'll have to prioritize other tasks and won't be able to finish it any time soon.
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @MitchExpensifyThe text was updated successfully, but these errors were encountered: