[No QA][CRITICAL] [Advanced Approval Workflows] Implement utils that transform data between backend and frontend #46168
Conversation
|
@allgandalf Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
Does this need a C+ review? |
| }; | ||
|
|
||
| /** Convert an approval workflow to a list of policy employees */ | ||
| function convertApprovalWorkflowToPolicyEmployees({approvalWorkflow, employeeList, removeWorkflow = false}: ConvertApprovalWorkflowToPolicyEmployeesParams): PolicyEmployeeList { |
There was a problem hiding this comment.
I think the result of this method is in the wrong format, but it's close! There are two formats for policy employees:
- What is stored in Onyx only used on the frontend (an object keyed by email):
{
'1@example.com': {email: '1@example.com'},
}
- What is stored in the database and the format that the backend users (an array of objects):
[
{email: '1@example.com'},
]
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.
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.
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.
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.
Details
Transforming backend data
Backend data (
policy.employeeList) was structured so that it is easy to display in a table, separately for every user. In NewDot we decided to group users to improve the UX, that’s why before displaying<ApprovalWorkflowSection />(will be implemented here) we need to first transform the data to make it more accessible.The logic will be built into a function called
convertPolicyEmployeesToApprovalWorkflows. It will construct an object for every approval workflow based on the data in the policy (empoyeeList and policy.approver).Converting the data back to backend-friendly shape
In order to call the backend endpoint we need a function that converts the
ApprovalWorkflowback to backend compatible shape (it basically inverts the function above). In summary it generates a list of employees with updatedsubmitsToandforwardsTofor each person in the workflow.Fixed Issues
$ #45957
PROPOSAL: N/A
Tests
N/A
Offline tests
N/A
QA Steps
N/A
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel so the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
N/A
Android: mWeb Chrome
N/A
iOS: Native
N/A
iOS: mWeb Safari
N/A
MacOS: Chrome / Safari
N/A
MacOS: Desktop
N/A