feat: add reimbursable toggle to create expense flow #59209

daledah · 2025-03-27T09:52:18Z

Explanation of Change

Fixed Issues

$ #58588
PROPOSAL: #58588 (comment)

Tests

Click FAB
Click Create Expense
Enter amount and next
Click show more button
Verify that: There's a working Reimbursable toggle
Submit the expense
Verify that: There are no error
Submit another expense with Reimbursable toggle off
Go to OD
View the submitted expense in step 8
Verify that: Reimbursable checkbox is checked off

Cross-platform consistency test:

In OD, go to Workspace -> Expenses
Change the field Cash expenses are to different values
Verify the changes in ND matches the condition:
If Default reimbursable is set via OldDot, show the toggle enabled by default on expense creation.
If Default non-reimbursable is set via OldDot, show the toggle disabled by default on expense creation.
If Always non-reimbursable is set via OldDot, don't show the toggle at all and create the cash expense as non-reimbursable.
If Always reimbursable is set via OldDot, don't show the toggle at all and create the cash expense as reimbursable.

Verify that no errors appear in the JS console

Offline tests

QA Steps

Click FAB
Click Create Expense
Enter amount and next
Click show more button
Verify that: There's a working Reimbursable toggle
Submit the expense
Verify that: There are no error
Submit another expense with Reimbursable toggle off
Go to OD
View the submitted expense in step 8
Verify that: Reimbursable checkbox is checked off

Cross-platform consistency test:

In OD, go to Workspace -> Expenses
Change the field Cash expenses are to different values
Verify the changes in ND matches the condition:
If Default reimbursable is set via OldDot, show the toggle enabled by default on expense creation.
If Default non-reimbursable is set via OldDot, show the toggle disabled by default on expense creation.
If Always non-reimbursable is set via OldDot, don't show the toggle at all and create the cash expense as non-reimbursable.
If Always reimbursable is set via OldDot, don't show the toggle at all and create the cash expense as reimbursable.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

Screen.Recording.2025-03-27.at.18.28.10.mov

iOS: Native

Screen.Recording.2025-03-27.at.18.29.58.mov

iOS: mWeb Safari

Screen.Recording.2025-03-27.at.18.30.53.mov

MacOS: Chrome / Safari

Screen.Recording.2025-03-27.at.18.35.59.mp4

Screen.Recording.2025-03-27.at.18.39.10.mp4

MacOS: Desktop

Screen.Recording.2025-03-27.at.18.31.20.mp4

melvin-bot · 2025-03-27T11:46:24Z

@getusha 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]

daledah · 2025-03-27T11:46:41Z

Currently facing some troubles building Android, I'll upload the screenshot later.

shawnborton · 2025-03-27T12:11:53Z

cc @Expensify/design for review too

shawnborton · 2025-03-27T12:12:59Z

Is there a reason why we don't show the toggle in the expense details view after the expense is created?

dannymcclain · 2025-03-27T13:36:27Z

The toggle in the create expense flow is looking good to me, but I'm also curious about why it's not in the details view after the expense has been created.

trjExpensify · 2025-03-27T15:42:09Z

100% should be there when applicable.

Not on card transactions
Not on expenses on workspaces that have Always non-reimbursable or Always reimbursable set.
Not on expenses on IOUreports

Then there's some rules around when it can be edited on the expense...

The toggle should abide by the same permission logic as the date and amount fields when it comes to editing, which I believe is as follows:

Submitter: can't edit the toggle on processing reports after first level approval.
Non-admin manager: can't edit the toggle after they've approved (i.e no longer the report's managerID)
Admins: can't edit the toggle if it's on a report in the approved, reimbursed or closed state.

github-actions · 2025-03-27T15:43:59Z

🚧 @trjExpensify has triggered a test app build. You can view the workflow run here.

dannymcclain · 2025-03-27T16:01:16Z

Then there's some rules around when it can be edited on the expense...

@trjExpensify @Expensify/design in those situations where it should be there but it's not editable, should we still show it, but show it with the lock? I feel like that's how I would expect it to show up (since we also still show the other non-editable fields in those views).

shawnborton · 2025-03-27T16:02:03Z

Yeah, I think that makes sense to me too

trjExpensify · 2025-03-27T16:07:23Z

Yeah, I think we can show it with a lock if we want. Equally, we could just have it disabled from editing displaying as on/off depending on what it was before moving into a state where you can't edit it anymore.

github-actions · 2025-03-27T16:23:38Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/59209/index.html	https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/59209/index.html

Android Hybrid 🤖🔄	iOS Hybrid 🍎🔄
❌ FAILED ❌	❌ FAILED ❌
The QR code can't be generated, because the Android Hybrid build failed	The QR code can't be generated, because the iOS Hybrid build failed
Desktop 💻	Web 🕸️
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/59209/NewExpensify.dmg	https://59209.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

dubielzyk-expensify · 2025-03-28T01:16:26Z

Oh yeah, the lock makes sense to me in those instances

daledah · 2025-03-28T03:01:37Z

@trjExpensify Should I implement the toggle in the report details now? In the issue we agreed that this is out of scope so I didn't make any changes.

If we decide to implement the toggle, as in my proposal:

Note

To make the toggle work, we need to make a new API endpoint similar to UpdateMoneyRequestBillable, which isn't available right now, so this part is open for discussions

So we're gonna need some BE changes

trjExpensify · 2025-03-28T09:06:36Z

@trjExpensify Should I implement the toggle in the report details now? In the issue we agreed that this is out of scope so I didn't make any changes.

Oh, must have misunderstood that comment. When you said inside the "expense report" I thought you were talking about some new feature at the expense report level. We for sure need to add the toggle at the "expense" level.

flaviadefaria · 2025-03-28T09:37:59Z

I also misunderstood your comment the @daledah. I like @trjExpensify suggestion of showing the toggle disabled from editing displaying as on/off depending on what it was before moving into a state where you can't edit it anymore.

daledah · 2025-03-28T10:52:55Z

@trjExpensify @flaviadefaria So the next step here should be:

HOLD this PR for BE changes
Implement reimbursable button on expense details page
The button will not show on card/IOU transactions
The button will always show on other transactions, but some will be locked (always reimbursable for example).

Does that look correct to you? Also, should the point 4 above be applied for create expense flow as well? Or should we keep current behavior?

flaviadefaria · 2025-03-28T12:07:33Z

Looks good to me @daledah!

Regarding:

Also, should the point 4 above be applied for create expense flow as well? Or should we keep current behavior?

I believe we should keep the current behavior. A user should be able to toggle off reimbursable expenses unless there is a special setting enabled (such as force non-reimbursable for cards or always reimbursable/always non-reimbursable). Does this make sense? Does this sound correct @trjExpensify?

trjExpensify · 2025-03-28T17:54:55Z

On OldDot we don't show the toggle (checkbox) at all if the setting is Always Reimbursable or Always non-reimbursable.

I think the design team suggestion was to use the lock for the cases where the selection can no longer be changed. I.e a submitter after their report is approved by at least one person.

flaviadefaria · 2025-04-01T15:12:41Z

Just to confirm, are we blocked here on the backend changes?

trjExpensify · 2025-04-01T16:51:54Z

Yea, we'll need backend changes to implement this. I've reached out to Issa and pointed to my comment here: #58588 (comment)

flaviadefaria · 2025-04-02T09:04:50Z

Cool, I also tagged them on the weekly updated in Slack but saw they were OoO. Hopefully they'll respond today when they're back.

inimaga · 2025-04-02T20:24:59Z

Hiya,
@flaviadefaria I was OOO when we realised we need the backend change. I am back today and have now picked this up and working on it. The PRs for the change will be undergoing review shortly, and you can track these here

Will provide an update once it makes it to staging, so daledah can keep working on it.

daledah · 2025-04-14T06:14:53Z

Hi @inimaga, just checking in - has BE changes made it to production yet? I'm ready to continue my FE work once it's live.

getusha · 2025-05-14T18:49:08Z

Will review 👍

getusha · 2025-05-14T18:50:58Z

seems like we've a conflict @daledah

flaviadefaria · 2025-05-15T14:56:00Z

@daledah think you can handle this today? I'd love to get this merged by tomorrow.

daledah · 2025-05-15T15:29:54Z

@flaviadefaria @getusha Sorry for the delay, conflicts resolved. Note that getTransaction deprecation warning is being handled here

getusha · 2025-05-16T16:05:29Z

If Default reimbursable is set via OldDot, show the toggle enabled by default on expense creation.

The Reimbursable toggle should be enabled by default if no credit cards are connected (more details below)

@daledah I think, the toggle should be enabled by default, regardless of whether it's explicitly set to 'Default Reimbursable' from oldDot.

cc @flaviadefaria

flaviadefaria · 2025-05-19T12:34:50Z

@daledah I think, the toggle should be enabled by default, regardless of whether it's explicitly set to 'Default Reimbursable' from oldDot.

These are the options we have in NewDot and how they should map in NewDot:
Reimbursable by default - expense set to reimbursable by default
Non-reimbursable by default - expense set to non-reimbursable by default
Always Reimbursable - expense will be reimbursable by default (no toggle displayed)
Always Non-Reimbursable - expense will be non-reimbursable by default (no toggle displayed)

flaviadefaria · 2025-05-19T12:36:19Z

If all looks correct now, then it would be great to get this merged today.

daledah · 2025-05-20T03:19:13Z

I think we're well-aligned with design now

flaviadefaria · 2025-05-20T09:32:42Z

@getusha any other concerns here?

inimaga · 2025-05-20T15:33:11Z

@daledah can you resolve the ESLint issues and complete the author checklist so this can be merged ASAP.

daledah · 2025-05-20T16:13:13Z

@inimaga As I mentioned, the deprecation lint error is being handled here, and I'm not sure why the Checklist test failed as I checked off every boxes.

inimaga · 2025-05-20T16:35:47Z

Okay noted. Re-ran the check and all good. Proceeding with merge.

inimaga · 2025-05-20T16:36:54Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible 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 checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
I verified tests pass on all platforms & I tested again on:
- Android: HybridApp
- Android: mWeb Chrome
- iOS: HybridApp
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified there are no new alerts related to the canBeMissing param for useOnyx
I verified proper code patterns were followed (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 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
- 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 either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- 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 verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
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.ts or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
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 UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design 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.
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
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.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

Android: mWeb Chrome

iOS: HybridApp

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

melvin-bot · 2025-05-20T16:43:35Z

@inimaga looks like this was merged without a test passing. Please add a note explaining why this was done and remove the Emergency label if this is not an emergency.

getusha · 2025-05-20T16:45:41Z

@getusha any other concerns here?

Yes, maybe i misunderstood but even for new workspaces Reimbursable isn't enabled by default.

OSBotify · 2025-05-20T16:51:48Z

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

flaviadefaria · 2025-05-20T17:23:44Z

Yes, maybe i misunderstood but even for new workspaces Reimbursable isn't enabled by default.

Oh that's a good catch, I agree that reimbursable should be enabled by default for a new workspace.

inimaga · 2025-05-20T17:25:53Z

Regarding failing test from @MelvinBot, failing as per comment here: #59209 (comment), failing due to getTransaction deprecation is already being handled here #61910

inimaga · 2025-05-20T17:28:03Z

Yes, maybe i misunderstood but even for new workspaces Reimbursable isn't enabled by default.

Oh that's a good catch, I agree that reimbursable should be enabled by default for a new workspace.

@daledah we'll need to handle this in a separate PR as this one is now merged.

daledah · 2025-05-20T17:38:37Z

@inimaga I've handled this in this commit - policy created has defaultReimbursable as true now.

getusha · 2025-05-20T17:50:21Z

@daledah could you invite a member and test from the non-admin side?

daledah · 2025-05-20T18:05:24Z

could you invite a member and test from the non-admin side?

In this case I think we should make BE changes to send correct defaultReimbursable value instead of always defaulting to true.

daledah added 2 commits March 27, 2025 16:35

feat: add reimbursable toggle to create expense flow

d1e63ea

fix: prettier

47d13ec

daledah marked this pull request as ready for review March 27, 2025 11:46

daledah requested a review from a team as a code owner March 27, 2025 11:46

melvin-bot bot requested review from getusha and removed request for a team March 27, 2025 11:46

daledah mentioned this pull request Mar 27, 2025

[$250] Add a reimbursable toggle on the expense form in New Expensify #58588

Open

Merge branch 'main' into fix/58588

7882ff9

Merge branch 'main' into fix/58588

6362c64

inimaga self-requested a review May 20, 2025 16:36

inimaga approved these changes May 20, 2025

View reviewed changes

Expensify deleted a comment from getusha May 20, 2025

inimaga merged commit 6943615 into Expensify:main May 20, 2025
15 of 16 checks passed

melvin-bot bot added the Emergency label May 20, 2025

inimaga removed the Emergency label May 20, 2025

feat: add reimbursable toggle to create expense flow #59209

feat: add reimbursable toggle to create expense flow #59209

Conversation

daledah commented Mar 27, 2025 • edited Loading

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Mar 27, 2025

daledah commented Mar 27, 2025

shawnborton commented Mar 27, 2025

shawnborton commented Mar 27, 2025

dannymcclain commented Mar 27, 2025

trjExpensify commented Mar 27, 2025 • edited Loading

github-actions bot commented Mar 27, 2025

dannymcclain commented Mar 27, 2025

shawnborton commented Mar 27, 2025

trjExpensify commented Mar 27, 2025

github-actions bot commented Mar 27, 2025

dubielzyk-expensify commented Mar 28, 2025

daledah commented Mar 28, 2025

trjExpensify commented Mar 28, 2025

flaviadefaria commented Mar 28, 2025

daledah commented Mar 28, 2025 • edited Loading

flaviadefaria commented Mar 28, 2025

trjExpensify commented Mar 28, 2025

flaviadefaria commented Apr 1, 2025

trjExpensify commented Apr 1, 2025

flaviadefaria commented Apr 2, 2025

inimaga commented Apr 2, 2025

daledah commented Apr 14, 2025

getusha commented May 14, 2025

getusha commented May 14, 2025

flaviadefaria commented May 15, 2025

daledah commented May 15, 2025

getusha commented May 16, 2025

flaviadefaria commented May 19, 2025

flaviadefaria commented May 19, 2025

daledah commented May 20, 2025

flaviadefaria commented May 20, 2025

inimaga commented May 20, 2025

daledah commented May 20, 2025

inimaga commented May 20, 2025

inimaga commented May 20, 2025 • edited Loading

Reviewer Checklist

Screenshots/Videos

melvin-bot bot commented May 20, 2025

getusha commented May 20, 2025

OSBotify commented May 20, 2025

flaviadefaria commented May 20, 2025

inimaga commented May 20, 2025 • edited Loading

inimaga commented May 20, 2025

daledah commented May 20, 2025

getusha commented May 20, 2025

daledah commented May 20, 2025

daledah commented Mar 27, 2025 •

edited

Loading

trjExpensify commented Mar 27, 2025 •

edited

Loading

daledah commented Mar 28, 2025 •

edited

Loading

inimaga commented May 20, 2025 •

edited

Loading

inimaga commented May 20, 2025 •

edited

Loading