-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Hide paying invoices via Expensify behind a beta #82046
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@youssef-lr 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] |
Code Review SummaryI've completed a comprehensive review of PR #82046. The changes successfully implement hiding invoice payment via Expensify behind a beta flag. Key FindingsCode Duplication (CONSISTENCY-3)
Each file independently implements:
Recommendation: Consider extracting this shared logic into a utility function or shared hook to reduce duplication and make future maintenance easier. Overall AssessmentThe implementation is functionally correct and follows the existing patterns in the codebase. The beta flag approach is appropriate for this use case. While there's code duplication, it's consistent with the current architectural patterns where each component/hook manages its own payment options independently. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c797b8a56d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
04baf28 to
5d9bf9d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d9bf9d1f6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Explanation of Change
This PR hides the options to pay invoices with bank accounts or cards via Expensify. Even if invoices are not promoted in NewDot yet, we realised that some users use them, and we have an issue that prevents the senders from getting the funds in their bank account. Here is another Slack thread where you can see a user having the same problem. We decided not to prioritize fixing this right now, because this would require product changes, and it might not be worth it since #billpay is on the roadmap for sometime later this year.
Fixed Issues
#67218
PROPOSAL:
Tests
Test that the invoices can only be marked as paid from Reports/Search and from the invoice rooms.
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Test that invoices can only be marked as paid; they can't be paid via Expensify.
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-02-11.at.13.36.03.mov