Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[No QA][CRITICAL] [Advanced Approval Workflows] Implement utils that transform data between backend and frontend #46168
[No QA][CRITICAL] [Advanced Approval Workflows] Implement utils that transform data between backend and frontend #46168
Changes from all commits
37e8cc5
8a316a5
ed067de
b47115f
a98914d
8d63e99
8467eee
52ad6c2
0584034
142a12d
f90748c
e0341b4
de18619
a23e4fe
7b43485
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the result of this method is in the wrong format, but it's close! There are two formats for policy employees:
I believe this method needs to produce the second format because that is the format that the backend API will be expecting. You can see this being done in the API parameters for updating a workspace members role (this endpoint is also a direct alias for
Policy_Employees_Merge
command for OldDot.Shouldn't be a big deal to change this and then update the tests to match for format. I don't think we need to support converting workflows into the first employeeList format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see in your second PR that you will need to use the first format for the optimistic data here: https://github.com/Expensify/App/pull/46189/files#diff-0322d176e073c6db8f9e2a5203bc59580b8e4cda517da2ccddf6d53318789ff3R45-R47
So it needs to support both formats.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or... I could also change the backend to accept the employeeList in the same format that the frontend is using 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think converting to an array it's not a big deal (one call to
Object.values()
). Therefore, I don't think it's necessary to change the format on the backend.