Preserve custom report name when duplicating a report and editing expenses - #98387
Preserve custom report name when duplicating a report and editing expenses#98387MelvinBot wants to merge 3 commits into
Conversation
Co-authored-by: huult <huult@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. |
|
There's nothing to fix in code here — the two failing checks aren't code failures, they're process gates that only a human can clear:
All actual code checks are green — So no push is needed from me. To get these two checks green, |
|
@MelvinBot Check all items on the author checklist, I will verify them later. |
|
Done, Since these are self-attestation boxes and you said you'd verify, here's the punch-list of things the checked boxes now claim but that aren't actually substantiated in the PR body yet — worth completing before you mark it "Ready for review":
I also tried to run the duplicate-report → add-expense → toggle Reimbursable flow on web to confirm the fix at runtime, but the test session wasn't authenticated (landed on the logged-out page), so I couldn't verify the behavior live — please confirm that step manually. The code change itself looks correct for the approved proposal: it skips seeding the FORMULA title field when a custom |
|
@MelvinBot ready for review |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-08-12.at.13.57.06.movAndroid: mWeb ChromeScreen.Recording.2026-08-12.at.13.58.47.moviOS: HybridAppScreen.Recording.2026-08-12.at.14.01.15.moviOS: mWeb SafariScreen.Recording.2026-08-12.at.14.01.51.movMacOS: Chrome / SafariScreen.Recording.2026-08-12.at.13.52.39.mov |
|
Done, One heads-up so it doesn't stall in review: the PR body still has the |
|
@MelvinBot please update QA step |
|
Done,
The |
|
The only remaining failing check is Every code check is green ( No fix from me is needed here — the PR just needs a reviewer's approval. |
| optimisticData.push(...updateTitleFieldToMatchPolicy(reportID, policy)); | ||
| // When a custom report name is provided (e.g. "Copy of …" from duplicating a report), don't seed the FORMULA | ||
| // title field. Otherwise a later expense edit would recompute the name from the policy title formula and | ||
| // overwrite the custom name. See https://github.com/Expensify/App/issues/97804. |
There was a problem hiding this comment.
@MelvinBot, please make this comment shorter, emphasizing why the next condition is needed. Don't add links to GH issues.
There was a problem hiding this comment.
Here's a shorter version that leads with why the guard is needed and drops the issue link:
// Skip seeding the FORMULA title field when a custom report name is set — otherwise a later
// expense edit would recompute the name from the policy title formula and overwrite it.
if (!reportName) {I can only make code changes when directed by the PR author, an assignee, or an existing co-author, so I can't push this on cristipaval's request. huult, if you'd like me to apply it, reply here tagging me and I'll commit it — or feel free to make the edit directly.
There was a problem hiding this comment.
Done — applied in 53dc324. The comment now leads with why the guard is needed and drops the issue link:
// Skip seeding the FORMULA title field when a custom report name is set — otherwise a later
// expense edit would recompute the name from the policy title formula and overwrite it.
if (!reportName) {There was a problem hiding this comment.
@MelvinBot I don’t think we need a comment here since it’s already clear. Please remove it.
There was a problem hiding this comment.
Done — removed the comment in 2632bc5. The if (!reportName) guard stands on its own now.
|
The only remaining failing check is The So no code fix is needed — this PR just needs a reviewer's approval to go green. |
|
Nothing to fix — every code and CI check is now green (the PR Author Checklist, The only remaining failing check is |
|
No code fix is needed — the only failing check is Everything else is green now, including the So there's nothing for me to push here — just get a reviewer's approval and all checks will be green. |
|
All code checks are now green — the checklist is complete, and The only remaining failing check is |
Co-authored-by: huult <huult@users.noreply.github.com>
|
@huult @cristipaval 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] |
|
Nothing to fix in code — all code checks are green now. The only failing check is The So no push is needed. Just get a reviewer to approve to clear the remaining check. |
|
🤖 The only failing check is Check independent approval (step "Verify peer review") — this isn't a code or test failure, so there's nothing for me to push a fix for. That check just verifies the PR has an approval from an independent reviewer, and it stays red until a qualified reviewer approves. Every actual CI check is green — tests, typecheck, ESLint, Oxfmt, Bun tests, Storybook, and all builds pass. Next step: get an independent peer review/approval on this PR and the check will clear on its own. |
Co-authored-by: huult <huult@users.noreply.github.com>
|
🤖 Still nothing to fix here. The only failing check remains Check independent approval (step "Verify peer review") — a non-code check that stays red until an independent reviewer approves the PR. The Next step: get an independent peer review/approval on this PR and the check will clear on its own. There's no code change I can push to resolve it. |
3 participants
Explanation of Change
When a report is duplicated, the "Copy of …" name is stored on
report.reportName, butbuildNewReportOptimisticDataalso seeded the new report'sexpensify_text_titlefield as a FORMULA field (viaupdateTitleFieldToMatchPolicy). Later, editing an expense in that report — adding it, or toggling Reimbursable — runsmaybeUpdateReportNameForFormulaTitle, which, seeing a FORMULA title field, recomputes the name from the policy title formula and overwritesreport.reportName, dropping the "Copy of …" prefix and switching the header/row back to the default "Expense Report …" name.The fix skips seeding the FORMULA title field when an explicit custom
reportNameis passed tobuildNewReportOptimisticData(the duplicate "Copy of …" case). With no FORMULA field present, the guard inmaybeUpdateReportNameForFormulaTitleshort-circuits during later expense edits, so the custom name is preserved.This implements the approved proposal.
Fixed Issues
$ #97804
PROPOSAL: #97804 (comment)
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review". Please describe what tests you performed that validate your change worked.
Offline tests
QA Steps
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