[Simplified Collect][Workflows] Select workspace approver #37391

rushatgabhane · 2024-02-28T04:20:39Z

Details

Fixed Issues

$ #37195
PROPOSAL:

Tests

Verify that you can set approver

Go to workspace settings
Select a control policy workspace that you're an admin of
Enable add approvals
Click approver
Select a user
Verify that the approver is changed and persisted.
Open the approver screen again, verify that the approver selected in step 5 has a green check next to it.

Verify that filtering (searching) works

Go to approver page
Search for a member, verify that the members are filtered.
Verify that you can select a searched member and it's set as an approver

Verify that no errors appear in the JS console

Offline tests

QA Steps

Verify that no errors appear in the JS console

PR Author Checklist

I linked the correct issue in the ### Fixed Issues section above
I wrote clear testing steps that cover the changes made in this PR
- I added steps for local testing in the Tests section
- I added steps for the expected offline behavior in the Offline steps section
- I added steps for Staging and/or Production testing in the QA steps section
- I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
- I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
I included screenshots or videos for tests on all platforms
I ran the tests on all platforms & verified they passed on:
- Android: Native
- Android: mWeb Chrome
- iOS: Native
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
I followed proper code patterns (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
- I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
  - If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I followed the guidelines as stated in the Review Guidelines
I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
I verified that if a function's arguments changed that all usages have also been updated correctly
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label so the design team can review the changes.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

Screen.Recording.2024-02-28.at.12.55.59.mov

MacOS: Desktop

allroundexperts · 2024-03-01T17:24:43Z

Have you tried cleaning everything and installing again?

rushatgabhane · 2024-03-01T17:25:28Z

hahaha

rushatgabhane · 2024-03-01T18:18:49Z

@allroundexperts all typechecks and lint errors fixed. I didn't fix errors that are also present on main branch.
Requesting your review again 🙇

allroundexperts · 2024-03-01T18:20:29Z

@rushatgabhane All checks are passing on main.

rushatgabhane · 2024-03-01T18:22:30Z

@allroundexperts we didn't make any changes in withPolicy. But there's still an error

allroundexperts · 2024-03-01T18:25:13Z

@allroundexperts we didn't make any changes in withPolicy. But there's still an error

Hm... Something must have changed. Because this is passing on main.

allroundexperts · 2024-03-01T18:30:56Z

TS passed 🎉

allroundexperts

Looks good. Great work @rushatgabhane!

luacmartins

LGTM

OSBotify · 2024-03-01T19:12:13Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

luacmartins · 2024-03-01T19:30:02Z

Created internal issue to send approverAccountID so we can get rid of getPersonalDetailByEmail - https://github.com/Expensify/Expensify/issues/374749

rushatgabhane · 2024-03-11T23:01:21Z

src/libs/Navigation/types.ts

+    [SCREENS.WORKSPACE.WORKFLOWS_APPROVER]: {
+        policyID: string;
+    };


could you please link the TS check. i can't find failing check

sorry that was mistake 🙇‍♂️

OSBotify · 2024-03-04T21:04:35Z

🚀 Deployed to staging by https://github.com/luacmartins in version: 1.4.47-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	failure ❌
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2024-03-06T21:53:37Z

🚀 Deployed to production by https://github.com/roryabraham in version: 1.4.47-10 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

eh2077 · 2024-06-14T17:23:27Z

src/pages/workspace/workflows/WorkspaceWorkflowsApproverPage.tsx

+        if (searchTerm !== '') {
+            const filteredOptions = [...formattedApprover, ...formattedPolicyMembers].filter((option) => {
+                const searchValue = OptionsListUtils.getSearchValueForPhoneOrEmail(searchTerm);
+                return !!option.text?.toLowerCase().includes(searchValue) || !!option.login?.toLowerCase().includes(searchValue);


This line caused a regression here Custom name user searched with email id shows no results found

rushatgabhane added 30 commits February 27, 2024 15:43

add spend route

646b689

add spend route

08b1ed2

add approver to stack nav

52d78fa

open workflow page on click

e30eb65

add dummy approver page

b45c106

fix linking

c335676

fix tpo

7028951

rename

15704d4

rename: remove spend

90beac6

add api type

e254f56

link write api params

bcd89b7

fix policy id route

c814c51

rm console

d4e4de1

use policy members

84d4d35

add settings to routes as per doc

f215f03

get personal details for policy members

353d350

get personal details for policy members

c65636b

add check for approver

3f524ba

add all section

1d325d1

add all section

1e4e5cb

add all section

4e0d42b

add all section 0 offset

97dd3a2

add all section 0 offset

54f846e

add all section 0 offset

f5a0592

add approver section

76d03fa

add approver section

06399aa

add approver section

eeaae97

fix index offset for all

8a058da

make api call on approver select

4c629b6

add set approver action

a9fc405

rushatgabhane added 3 commits March 1, 2024 20:37

fix lint

5c6423c

fix deps cycle

1c98660

memoize deletedpolicy member

25a5b0f

rushatgabhane requested a review from allroundexperts March 1, 2024 18:19

Merge branch 'main' of github.com:rushatgabhane/exfy into approval-page

e2495bf

fix lint

47738f6

rushatgabhane added 3 commits March 1, 2024 21:34

ignore deps rule

e60a960

ignore deps rule

8b0d417

ignore deps rule

45585e8

allroundexperts approved these changes Mar 1, 2024

View reviewed changes

luacmartins approved these changes Mar 1, 2024

View reviewed changes

luacmartins merged commit 902f2b8 into Expensify:main Mar 1, 2024
15 checks passed

ishpaul777 reviewed Mar 1, 2024

View reviewed changes

github-actions bot mentioned this pull request Mar 4, 2024

Deploy Checklist: New Expensify 2024-03-04 #37698

Closed

97 tasks

izarutskaya mentioned this pull request Mar 5, 2024

[$500] Workflows - Offline indicator is not at the bottom of the page #37721

Closed

6 tasks

melvin-bot bot mentioned this pull request Mar 6, 2024

[HOLD for payment 2024-03-13] [Wave8] [Simplified Collect] [Workflows] Create Approval page #37195

Closed

luacmartins mentioned this pull request Apr 2, 2024

[HOLD for payment 2024-04-15] Return the approverAccountID and reimburserAccountID to Onyx #39423

Closed

eh2077 reviewed Jun 14, 2024

View reviewed changes

eh2077 mentioned this pull request Jun 14, 2024

[HOLD for payment 2024-06-20] [$250] Workflow - Custom name user searched with email id shows no results found #42576

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Simplified Collect][Workflows] Select workspace approver #37391

[Simplified Collect][Workflows] Select workspace approver #37391

rushatgabhane commented Feb 28, 2024 •

edited

Loading

allroundexperts commented Mar 1, 2024

rushatgabhane commented Mar 1, 2024 •

edited

Loading

rushatgabhane commented Mar 1, 2024 •

edited

Loading

allroundexperts commented Mar 1, 2024

rushatgabhane commented Mar 1, 2024 •

edited

Loading

allroundexperts commented Mar 1, 2024

allroundexperts commented Mar 1, 2024

allroundexperts left a comment

luacmartins left a comment

OSBotify commented Mar 1, 2024

luacmartins commented Mar 1, 2024

This comment was marked as duplicate.

rushatgabhane Mar 11, 2024 •

edited

Loading

ishpaul777 Mar 11, 2024 •

edited

Loading

OSBotify commented Mar 4, 2024

OSBotify commented Mar 6, 2024

eh2077 Jun 14, 2024

[Simplified Collect][Workflows] Select workspace approver #37391

[Simplified Collect][Workflows] Select workspace approver #37391

Conversation

rushatgabhane commented Feb 28, 2024 • edited Loading

Details

Fixed Issues

Tests

Verify that you can set approver

Verify that filtering (searching) works

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

allroundexperts commented Mar 1, 2024

rushatgabhane commented Mar 1, 2024 • edited Loading

rushatgabhane commented Mar 1, 2024 • edited Loading

allroundexperts commented Mar 1, 2024

rushatgabhane commented Mar 1, 2024 • edited Loading

allroundexperts commented Mar 1, 2024

allroundexperts commented Mar 1, 2024

allroundexperts left a comment

Choose a reason for hiding this comment

luacmartins left a comment

Choose a reason for hiding this comment

OSBotify commented Mar 1, 2024

luacmartins commented Mar 1, 2024

This comment was marked as duplicate.

rushatgabhane Mar 11, 2024 • edited Loading

Choose a reason for hiding this comment

ishpaul777 Mar 11, 2024 • edited Loading

Choose a reason for hiding this comment

OSBotify commented Mar 4, 2024

OSBotify commented Mar 6, 2024

eh2077 Jun 14, 2024

Choose a reason for hiding this comment

rushatgabhane commented Feb 28, 2024 •

edited

Loading

rushatgabhane commented Mar 1, 2024 •

edited

Loading

rushatgabhane commented Mar 1, 2024 •

edited

Loading

rushatgabhane commented Mar 1, 2024 •

edited

Loading

rushatgabhane Mar 11, 2024 •

edited

Loading

ishpaul777 Mar 11, 2024 •

edited

Loading