[HOLD for payment 2024-05-03] [Backwards Compatibility] [$250] Remove usages of policy.submitsTo in favor of using a new method PolicyUtils.getSubmitTo

[flodnv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Problem

policy.submitsTo is a field calculated on the fly by the API (and the OldDot JS code). This field is sent to Onyx by the API in the policy_ collection, which breaks these fairly newly established Onyx guidelines:

We should NEVER send slightly different versions of an update to each of the users. I think we stopped doing this, but I am calling it out anyway. So if for example we are returning a report, all users of that report should have the same data in onyx instead of sending slightly different updates to the users (eg: visibleParticipants that do not include yourself or sending the policy submitsTo to who the each submits to)

we should keep the data key and values 1:1 with the database. For instance:

• NVP Onyx keys should always follow the same naming pattern in Onyx as in the DB (although they have the prefix nvp_, we did this for easier backwards compatibility).
• Policy attributes should be stored at the same path as in the database

Solution

Update App code to stop accessing policy.submitsTo and instead implement PolicyUtils.getSubmitTo, which should match the PHP server implementation:

    /**
     * Gets the submit to of a given employee in this policy.
     *
     * @param string $employeeEmail The email address of the employee
     *
     * @return string The submit to email address, can be empty
     */
    public function getSubmitTo($employeeEmail): string
    {
        // For policy using the optional or basic workflow, the manager is the policy default approver.
        if (in_array($this->getApprovalWorkflow(), [self::APPROVAL_MODE_OPTIONAL, self::APPROVAL_MODE_BASIC], true)) {
            return strtolower($this->getDefaultApprover());
        }

        // Handle Advance approval mode
        $employee = $this->getEmployeeByEmail($employeeEmail);
        if ($employee === null) {
            Log::info('Employee not on the policy', [
                'policyID' => $this->getID(),
                'employeeEmail' => $employeeEmail,
            ]);

            return '';
        }

        $submitTo = $employee->getSubmitsTo();

        // On some old policies, employees don't have a submitTo, so we fallback on the default approver.
        if (!$submitTo) {
            $submitTo = $this->getDefaultApprover();
        }

        return strtolower($submitTo);
    }

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @getusha (External)

[shahinyan11]

Dibs

[flodnv]

Thanks @shahinyan11, it'd be great if you could have the PR up today 🙇

[melvin-bot]

⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

[melvin-bot]

Triggered auto assignment to @sonialiap (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

[melvin-bot]

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

[melvin-bot]

Triggered auto assignment to Design team member for new feature review - @dannymcclain (NewFeature)

[trjExpensify]

@trjExpensify @flodnv re-assigning to other C+, sorry, i thought i would be able to get this today 🙇! @ikevin127 will review it.,

Thanks for the heads up! Assigned you, Kevin! 👍

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.66-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Replace policy.submitsTo with PolicyUtils.getSubmitToAccountID #40532

If no regressions arise, payment will be issued on 2024-05-03. 🎊

For reference, here are some details about the assignees on this issue:

• @bernhardoj requires payment (Needs manual offer from BZ)
• @ikevin127 requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@ikevin127] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@trjExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[ikevin127]

• [@ikevin127] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.

We could use the same steps detailed in the PR's description #40532 (comment):

Test Case 1 - Prevent Self-Approval

1. Go to collect workspace setting in OldDot.
2. Enable prevent self-approval settings.
3. In Workflow tab, either disable Add approval or enable it and set it to yourself.
4. Go to collect workspace chat in NewDot.
5. Request money.
6. Verify the Submit button is disabled.

Note: Remember to disable prevent self-approval in OldDot after the test is completed.

Test Case 2 - Approval optional

1. Go to collect workspace setting.
2. Open the Workflow tab.
3. Disable the Add approvals.
4. Go to collect workspace chat.
5. Request money.
6. Submit the request.
7. Verify the submit request is successful and the pay button is shown.

Test Case 3 - Self-approval

1. Go to collect workspace setting.
2. Open the Workflow tab.
3. Enable Add approval and set the approver as yourself.
4. Go to collect workspace chat.
5. Request money.
6. Open the expense report.
7. Submit the request.
8. Verify the next step below the header is "Waiting for you to review these expenses".
9. Verify there is an Approve button.

Test Case 4 - Other approver

1. Go to collect workspace setting.
2. Open the Workflow tab.
3. Enable Add approval and set the approver to another user.
4. Go to collect workspace chat.
5. Request money.
6. Open the expense report.
7. Submit the request.
8. Verify the next step below the header is "Waiting for {another user name} to approve these expenses".
9. Verify that the approve button doesn't show for the current user which is not the approver.
10. As the approver user, go to the workspace chat and approve the request.
11. Verify that the approval was successful.

Do we agree 👍 or 👎.

[melvin-bot]

Payment Summary

Upwork Job

• ROLE: @bernhardoj paid $(AMOUNT) via Upwork (LINK)
• ROLE: @ikevin127 paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@trjExpensify)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[ikevin127]

@trjExpensify Friendly bump, payment overdue!

[trjExpensify]

Yeah, sorry @ikevin127. I was out yesterday for a national holiday here in the UK. Confirming payments as follows:

• $250 to @bernhardoj for the fix
• $250 to @ikevin127 for the C+ review

I've sent offers in Upwork.

As for the regression tests. We're shortly going to make self-approval a control plan feature only, so I'm hesitant to add them for collect at this time. The others we have for approvals, so no need for new ones in that regard.

[trjExpensify]

@bernhardoj - paid!

[ikevin127]

@trjExpensify No worries, offer accepted. Cheers!

[trjExpensify]

Paid!