feat: Handle the REJECTEDTOSUBMITTER#84053
Conversation
melvin-bot
bot
requested review from
linhvovan29546
and removed request for
a team
|
@linhvovan29546 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] |
melvin-bot
bot
requested review from
trjExpensify
and removed request for
a team
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
src/libs/OptionsListUtils/index.ts
Outdated
| lastMessageTextFromReport = translate('iou.forwarded'); | ||
| } | ||
| } else if (lastReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.REJECTED) { | ||
| } else if (lastReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.REJECTED || lastReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.REJECTED_TO_SUBMITTER) { |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The compound check actionName === CONST.REPORT.ACTIONS.TYPE.REJECTED || actionName === CONST.REPORT.ACTIONS.TYPE.REJECTED_TO_SUBMITTER is duplicated in 4 files (OptionsListUtils/index.ts, ReportNameUtils.ts, ContextMenuActions.tsx, PureReportActionItem.tsx). The codebase already has a well-established convention for this in ReportActionsUtils.ts with helpers like isApprovedAction, isForwardedAction, isRetractedAction, etc. A centralized isRejectedAction helper would follow that pattern and prevent future drift if more rejected-type actions are added.
Create an isRejectedAction helper in src/libs/ReportActionsUtils.ts and use it across all 4 call sites:
// In src/libs/ReportActionsUtils.ts
function isRejectedAction(
reportAction: OnyxInputOrEntry<ReportAction>,
): boolean {
return isActionOfType(reportAction, CONST.REPORT.ACTIONS.TYPE.REJECTED, CONST.REPORT.ACTIONS.TYPE.REJECTED_TO_SUBMITTER);
}Then replace all 4 occurrences with e.g.:
ReportActionsUtils.isRejectedAction(lastReportAction)Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
Applied! Created isRejectedAction helper in ReportActionsUtils.ts and replaced all 4 duplicated compound checks with it. See ac8a829.
Create a centralized isRejectedAction helper in ReportActionsUtils.ts that checks for both REJECTED and REJECTED_TO_SUBMITTER action types, replacing the duplicated compound checks across 4 files. Co-authored-by: truph01 <truph01@users.noreply.github.com>
trjExpensify
left a comment
trjExpensify
left a comment
There was a problem hiding this comment.
Why can't this just read "rejected" like others? (i.e "submitted")
I can't see "this report" in the doc mocks:
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-03-04.at.09.58.27.movAndroid: mWeb Chrome
iOS: HybridApp
iOS: mWeb SafariScreen.Recording.2026-03-04.at.09.30.06.movMacOS: Chrome / SafariScreen.Recording.2026-03-04.at.09.19.32.mov |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@MelvinBot Add unit tests for isRejectedAction and update ReportUtilsTest to include the new actionName REJECTED_TO_SUBMITTER. |
|
I reviewed the request to add unit tests for However, I can't push code changes to this PR because I didn't open it and I'm not listed as an assignee. Here's exactly what needs to be added: 1. Add Add this describe('isRejectedAction', () => {
it('should return true for REJECTED action type', () => {
// Given a report action with REJECTED action type
const action: ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.REJECTED> = {
...createRandomReportAction(0),
actionName: CONST.REPORT.ACTIONS.TYPE.REJECTED,
created: '2025-11-21',
reportActionID: '1',
originalMessage: undefined,
message: [],
previousMessage: [],
};
// When checking if the action is a rejected action
const result = ReportActionsUtils.isRejectedAction(action);
// Then it should return true
expect(result).toBe(true);
});
it('should return true for REJECTED_TO_SUBMITTER action type', () => {
// Given a report action with REJECTED_TO_SUBMITTER action type
const action: ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.REJECTED_TO_SUBMITTER> = {
...createRandomReportAction(0),
actionName: CONST.REPORT.ACTIONS.TYPE.REJECTED_TO_SUBMITTER,
created: '2025-11-21',
reportActionID: '1',
originalMessage: undefined,
message: [],
previousMessage: [],
};
// When checking if the action is a rejected action
const result = ReportActionsUtils.isRejectedAction(action);
// Then it should return true because REJECTED_TO_SUBMITTER is also a rejected action
expect(result).toBe(true);
});
it('should return false for non-rejected action type', () => {
// Given a report action with SUBMITTED action type (not rejected)
const action: ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.SUBMITTED> = {
...createRandomReportAction(0),
actionName: CONST.REPORT.ACTIONS.TYPE.SUBMITTED,
created: '2025-11-21',
reportActionID: '1',
originalMessage: {
amount: 10000,
currency: 'USD',
},
message: [],
previousMessage: [],
};
// When checking if the action is a rejected action
const result = ReportActionsUtils.isRejectedAction(action);
// Then it should return false
expect(result).toBe(false);
});
it('should return false for null action', () => {
// Given a null action
// When checking if the action is a rejected action
const result = ReportActionsUtils.isRejectedAction(null);
// Then it should return false because the action is null
expect(result).toBe(false);
});
});2. Add Add this test right after the existing test('should return rejected action name for REJECTED_TO_SUBMITTER', () => {
const rejectedToSubmitterAction: ReportAction = {
...baseParentReportAction,
actionName: CONST.REPORT.ACTIONS.TYPE.REJECTED_TO_SUBMITTER,
};
const threadReport: Report = {
...baseExpenseReport,
parentReportID: baseChatReport.reportID,
parentReportActionID: rejectedToSubmitterAction.reportActionID,
};
const reportActions = {
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${threadReport.parentReportID}`]: {[rejectedToSubmitterAction.reportActionID]: rejectedToSubmitterAction},
};
const reportName = computeReportName(threadReport, undefined, undefined, undefined, undefined, participantsPersonalDetails, reportActions);
expect(reportName).toBe('rejected');
}); |
|
@truph01 Could you please add the unit tests? |
lakchote
left a comment
lakchote
left a comment
There was a problem hiding this comment.
LGTM, @trjExpensify want to give it a final look?
trjExpensify
left a comment
trjExpensify
left a comment
There was a problem hiding this comment.
The sys message looks good now. 👍
|
@truph01 there's a merge conflict now. |
|
There are a lot of Eslint errors that aren't related to this PR. |
|
@truph01 I don't see that ESLint error on main. Do you know why it's only appearing on this PR? |
@linhvovan29546 I think it’s because the linter runs against all files changed in this PR, which is why the error is appearing here but not on |
|
I didn't see this ESLint error before the conflict appeared, so a recent PR merged to main probably triggered it. But I can't find which one |
|
That came from #83798 |
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.3.32-0 🚀
|
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.3.32-3 🚀
|
7 participants
Explanation of Change
Fixed Issues
$ #82793
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include ""[No QA].""
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-03-03.at.23.49.33.mov