Feat: refactor getReportName and move report name related functions#75357
Conversation
|
@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] |
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.
|
This comment was marked as resolved.
This comment was marked as resolved.
|
Closing this as explained here |
|
Ok never mind - we can re-open this because @shubham1206agra pointed out that it's helping us to remove dependencies on Onyx.connect variables. |
|
🚀 Deployed to staging by https://github.com/neil-marcellini in version: 9.2.67-0 🚀
|
|
🚀 Deployed to staging by https://github.com/neil-marcellini in version: 9.2.70-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.2.70-0 🚀
|
| } | ||
|
|
||
| if (isActionOfType(parentReportAction, CONST.REPORT.ACTIONS.TYPE.UNREPORTED_TRANSACTION)) { | ||
| return getUnreportedTransactionMessage(); |
There was a problem hiding this comment.
We need to parse html to text here to avoid issue #78840
| if (parentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_REPORT_FIELD) { | ||
| return getWorkspaceReportFieldAddMessage(parentReportAction); | ||
| } | ||
| if (parentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.UPDATE_REPORT_FIELD) { |
There was a problem hiding this comment.
Along with the handling of all POLICY_CHANGE_LOG actions, we missed handling UPDATE_PROHIBITED_EXPENSES. We handled this in #81605
| return payerPaidAmountMessage; | ||
| } | ||
|
|
||
| function computeReportNameBasedOnReportAction(parentReportAction?: ReportAction, report?: Report, reportPolicy?: Policy, parentReport?: Report): string | undefined { |
There was a problem hiding this comment.
I'm not sure if this is related to this PR
But we need to cover HOLD/UNHOLD actions too
More context here
Details
This PR refactors our report naming logic by extracting it from
ReportUtilsinto a new dedicated moduleReportNameUtils:computeReportNamehelper that encapsulates the full “smart” name generation logic for reports (DMs, group chats, policy rooms, tasks, threads, invoice/money request reports, etc).ReportNameUtils.getReportName(report, reportAttributes)that only reads the name from derived values orreport.reportName, and deprecates the old heavygetReportNameinReportUtils.ReportUtilsintoReportNameUtilsand leaves deprecated wrappers.Performance profile:
BEFORE:
AFTER:
Fixed Issues
$ #72613
$ #73839
PROPOSAL: N/A
Tests
DM and group chat names still correct
floki@vikings.net).Policy room and policy expense chat names
#admins).#adminswhen active.Money request / invoice names and previews
All of the above were tested on Web (macOS Chrome), and the changes are platform‑independent, so behavior should be the same across platforms.
Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)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
Details