Repository navigation
Conversation
Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
🤖 Web test steps verification results: ❌ fail The test browser could not sign in, so I couldn't run any duplicate-expense steps on web. The sign-in "Continue" button and other links on that page did nothing when clicked, and no sign-in request reached the server.
|
|
@MelvinBot Please merge main and resolve conflicts |
…seStaysOnSourceReport Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com> # Conflicts: # src/components/MoneyRequestHeaderSecondaryActions.tsx
|
Merged
|
|
Please add these test cases to PR description in Test and QA Test part. Also replace all |
|
Updated the PR description:
|
|
@MelvinBot Please fix the ESLint check and then mark PR as ready for review |
|
The failing ESLint check is unrelated to this PR. Analysis: ESLint reports 3 fix: follow-up #100567 merged today. It changed Evidence:
Next step: The fix belongs on |
|
@MelvinBot Mark PR as ready for review |
|
The ESLint check still fails because of
Next step: Fix |
|
@MelvinBot Other dev will fix it later, let's just open this PR for review, we will merge main later |
|
@thelullabyy 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] |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Marked this PR ready for review. I didn't change any code: the ESLint failure comes from The check reports 3
Why it fails on main
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari103072.mov |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #101327 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
This PR is already open for review. I marked it ready for review at 20:32 UTC, and I made no other changes. GitHub currently reports no merge conflicts with |
|
Going OOO so don't block on me for this PR> |
| return false; | ||
| } | ||
|
|
||
| return !shouldCreateNewMoneyRequestReport(sourceReport, chatReport, false, isASAPSubmitBetaEnabled, rules, CONST.IOU.ACTION.CREATE, true); |
There was a problem hiding this comment.
Won't this allow adding the duplicated expenses to a submitted report before it gets any approval? The PR description says that submitted reports fall back to default routing and are not reopened. NAB though, and we should keep it that way to be consistent.
|
@MelvinBot please pull |
|
I didn't pull You're right that the ESLint failure doesn't come from this PR. The failing run reports three
To get the merge, ask the PR assignee ( |
|
@thelullabyy can you ask Melvin to pull main please? I would do it, but then I can't merge this without another reviewer. |



Explanation of Change
"Duplicate expense" always targeted the default workspace chat, so the copy landed on whatever report that chat's
iouReportIDpointed at, or on a brand-new report. People duplicate an expense to get a second copy next to the original, so the copy should stay on the report they're looking at.This PR adds
canDuplicateExpenseIntoSourceReporttoReportUtils. It returns true when the source is an expense report that is not archived, its chat report is loaded and not archived, its workspace is accessible, andshouldCreateNewMoneyRequestReportsays the report can take the expense. ReusingshouldCreateNewMoneyRequestReport(which wrapscanAddTransaction) keeps the UI choice in sync with the report the money request builder picks, so there is no second definition of "addable".Both single-expense duplicate entry points (
useExpenseActionsfor the report header,MoneyRequestHeaderSecondaryActionsfor the expense thread header) now use that check to choose the target:requestMoney,createDistanceRequest, andsubmitPerDiemExpensealready add to a specific report when given an expense report, the same way "Add expense" inside a report works.Duplicate.tsneeds no changes.Everything that depends on the target follows it: categories, tags,
policyTagList, participants, the billing restriction check, and the per diem cross-workspace block. InuseExpenseActions, the menu now closes when the copy lands on the viewed report, matching the existingactivePolicyExpenseChat?.iouReportID === moneyRequestReport?.reportIDcase.Cases from the issue:
trackExpense.Bulk duplicate from the search selection toolbar (
useBulkDuplicateAction) is out of scope. ItsiouReportIDhandling inDuplicate.tsassumes the target is a chat, so it is better handled as a follow-up.AI Tests (run locally by MelvinBot):
npm run typecheck: passednpm run lint-changed: passedoxfmt(npm run fmt): no changes outside the edited filesnpm run spell-changed: passednpm run react-compiler-compliance-check checkon the two changed hook/component files: passed.check-changedcould not resolve a base ref in this environment.ReportUtilsTest,DuplicateTest,MoneyRequestHeaderSecondaryActionsTest,useExpenseActionsInvoiceDeleteTest,MoneyReportHeaderMoreContentVisibilityTest,MoneyReportHeaderActionsPlacementTest, andMoneyRequestReportViewTestall passed (1532 tests). New tests cover the helper (open, approved, archived, inaccessible workspace, non-expense-report source) andduplicateExpenseTransactionsendingREQUEST_MONEYto the open expense report passed astargetReport.Fixed Issues
$ #101327
PROPOSAL: #101327 (comment)
Tests
Case 1: two drafts in the same workspace
iouReportIDpoints at it.useExpenseActionsentry point too.Case 2: report in a non-default workspace
Case 3: locked source report (should behave as before)
Case 4: unreported / self-DM expense
Offline tests
QA Steps
Case 1: two drafts in the same workspace
iouReportIDpoints at it.useExpenseActionsentry point too.Case 2: report in a non-default workspace
Case 3: locked source report (should behave as before)
Case 4: unreported / self-DM expense
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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