fix: use TRACK instead of SUBMIT for Teachers Unite expenses - #97858
fix: use TRACK instead of SUBMIT for Teachers Unite expenses#97858huult wants to merge 11 commits into
Conversation
|
@QichenZhu 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] |
|
@ChavdaSachin @rlinoz One of you needs to 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] |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6fe293d85
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@MelvinBot review |
ReviewThe fix is correct and minimal. Deriving the IOU type from the target report's own policy is the right approach, and it lines up with what A few non-blocking things worth a look: 1. A third "Add expense" entry point still hardcodes 2. No unit test in the diff. The author checklist has "I added unit tests" checked, but only the two source files changed. A tiny test for 3. PR description nit. The description says Runtime verificationI attempted to verify the fix end-to-end on web (create a Teachers Unite report → Add expense → Create expense / Track distance → confirm the RHP opens instead of the Not Found page). The test was blocked before reaching the report: the "I am a teacher" flow gates on validating a school-domain email as the default contact method via a magic code, which the test account can't receive. This is an environment/account-provisioning limitation, unrelated to the PR — the fix itself could not be exercised in the browser. The code path was verified by inspection instead. Overall this looks good to merge once the above are considered — none are blockers. |
|
@QichenZhu can you review this one please? |
|
@rlinoz sure thing! |
|
@huult, what's the agreed solution? This PR doesn't seem to align with your comment in the main thread.
|
@QichenZhu Initially, after a quick investigation, I thought hiding it was a reasonable solution. However, after implementing the PR, I found a better approach that matches the expected behavior described in the ticket, so I updated the implementation accordingly. |
@huult, is the restriction on the frontend or the backend? Is it intentional or a bug? |
|
I think this is intentional. I noticed that we already hide the Create Report option for isTeachersUniteReport. |
If the restriction is intentional, then your initial approach of hiding the options is probably the right solution. @rlinoz @heyjennahay, could you clarify the expected behavior? |
|
Yeah, coming from this internal issue https://github.com/Expensify/Expensify/issues/500617 and this other one #64271, I think we should hide create expense/create report. cc: @heyjennahay |
|
I’ll update it after I get confirmation from @heyjennahay. |
|
@heyjennahay what do you think? |
…e-not-here-rhp-add-expense
…Teachers Unite report
|
@QichenZhu @rlinoz I’ve updated the dropdown so it only shows “Add existing expense”; “Create expense” and “Track distance” are now hidden. |
Bug: "Create report" is hidden from the + button next to the composer, but it's still available from the FAB. "Create expense" is available from both. Screen.Recording.2026-08-13.at.7.20.43.PM.mov |
@QichenZhu Yeah, I noticed this while testing the PR as well. However, everything works as expected, and the backend also allows it, so I don’t think this is a bug. |
Based on this comment, I don't think it works as expected. The implementation should match the product requirement. |
|
@rlinoz what do you think? |
|
asked internally exactly how this should work https://expensify.slack.com/archives/C03U7DCU4/p1786994105546789 |
|
Ok what I think we need
|
|
Sounds good, thanks! |
Screen.Recording.2026-08-18.at.11.50.16.mov@QichenZhu @rlinoz I’ve updated it. Could you review it? |
This comment was marked as resolved.
This comment was marked as resolved.
… directly instead of through ReportUtils
Explanation of Change
Teachers Unite reports don’t support SUBMIT, but the “Add expense” entry points were still hardcoded to use SUBMIT, causing NotFoundPage to show instead of the expense form. The fix hides “Create expense” and “Track distance” for these reports, leaving only “Add existing expense.”
Fixed Issues
$ #97646
PROPOSAL:
Tests
Same QA step
Offline tests
QA Steps
Precondition
Create two Expensifail accounts:
Principal – "I know a teacher" flow
Teacher – "I am a teacher" flow
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso 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
Screen.Recording.2026-08-13.at.09.45.42.mov
Android: mWeb Chrome
Screen.Recording.2026-08-13.at.09.43.46.mov
iOS: Native
Screen.Recording.2026-08-13.at.09.51.23.mov
iOS: mWeb Safari
Screen.Recording.2026-08-13.at.09.48.31.mov
MacOS: Chrome / Safari
Screen.Recording.2026-08-13.at.09.39.38.mov