Remove call to getReportNameValuePairs() in method getIcons from ReportUtils.ts.#67178
Remove call to getReportNameValuePairs() in method getIcons from ReportUtils.ts.#67178tgolen merged 21 commits intoExpensify:mainfrom
getReportNameValuePairs() in method getIcons from ReportUtils.ts.#67178Conversation
…rtUtils.ts. Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
src/libs/SearchUIUtils.ts
Outdated
| // eslint-disable-next-line deprecation/deprecation | ||
| const policy = getPolicy(parentReport.policyID); | ||
| const parentReportName = getReportName(parentReport, policy, undefined, undefined); | ||
| // No need to pass `isReportArchived`, archived tasks are not shown on reports page |
There was a problem hiding this comment.
@DylanDylann, could you also confirm this, please?
There was a problem hiding this comment.
isReportArchived is a check for the first parameter of getIcons function, in this case it is parentReport. So I think we still need to pass isReportArchived
|
@DylanDylann 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] |
|
@Krishna2323 Ops, it seems you haven't got an official assignment on the issue. As mentioned in guideline, we need to wait for the official assignment before going to the PR phase (please note in the next time) |
src/libs/SearchUIUtils.ts
Outdated
| // eslint-disable-next-line deprecation/deprecation | ||
| const policy = getPolicy(parentReport.policyID); | ||
| const parentReportName = getReportName(parentReport, policy, undefined, undefined); | ||
| // No need to pass `isReportArchived`, archived tasks are not shown on reports page |
There was a problem hiding this comment.
isReportArchived is a check for the first parameter of getIcons function, in this case it is parentReport. So I think we still need to pass isReportArchived
|
A missing place: getQuickActionDetails But I notice this function isn't used anywhere in our codebase. So I think we can remove it |
src/pages/home/HeaderView.tsx
Outdated
| const shouldShowSubscript = shouldReportShowSubscript(report, isArchived); | ||
| const defaultSubscriptSize = isExpenseRequest(report) ? CONST.AVATAR_SIZE.SMALL_NORMAL : CONST.AVATAR_SIZE.DEFAULT; | ||
| const icons = getIcons(reportHeaderData, personalDetails, null, '', -1, policy, invoiceReceiverPolicy); | ||
| const icons = getIcons(reportHeaderData, personalDetails, null, '', -1, policy, invoiceReceiverPolicy, isArchived); |
There was a problem hiding this comment.
In this file, please call isArchivedReport function once
I’m really sorry about that. I knew I hadn’t been assigned yet, but I was confused because I got the go-ahead from you, and you had already mentioned that we needed to raise the PR immediately. Sorry again! |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
src/libs/SearchUIUtils.ts
Outdated
| const policy = getPolicy(parentReport.policyID); | ||
| const parentReportName = getReportName(parentReport, policy, undefined, undefined); | ||
| const icons = getIcons(parentReport, personalDetails, null, '', -1, policy); | ||
| const parentReportNameValuePairs = getReportNameValuePairsFromKey(data, parentReport); |
There was a problem hiding this comment.
You are changing the source of data. Is it safe to use ReportNameValuePairs from searchData?
There was a problem hiding this comment.
You're right about that—I thought the report was also coming from searchData. I believe we need to pass reportNameValuePairs from the search page. I’ve updated it accordingly, please check.
There was a problem hiding this comment.
I believe we need to pass reportNameValuePairs from the search page.
@Krishna2323 Thanks for the update. We're currently passing a large object as a parameter, but many of its fields aren't actually used. I suggest either integrating the isArchivedReport field directly into the existing data object, or creating a mapper that filters and returns only the necessary fields. This mapper could take a list of keys and return just the corresponding values.
There was a problem hiding this comment.
This mapper will be useful in other tasks, so I suggest we implement it here and get this PR merged as soon as possible. That way, others can reuse it for their tasks.
There was a problem hiding this comment.
I'm not sure what the best approach is—whether to use the Onyx selector or integrate the data into the search data object. Either way, we’d still be subscribing to ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS.
I think it's fine as it is. We're already subscribing to the entire ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS collection in some components.
App/src/pages/iou/request/step/IOURequestEditReportCommon.tsx
Lines 51 to 72 in c1f5357
There was a problem hiding this comment.
@Krishna2323 I think if we're only using ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS to check whether a report is archived, we should add a selector in useOnyx to only get the necessary fields
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@Krishna2323 Also please merge the latest main |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Reviewer Checklist
Screenshots/VideosScreen.Recording.2025-07-30.at.11.37.29.movAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@DylanDylann do you want to take a look at the recent changes? They look fine to me, but I wanted you to see them too. |
src/components/Search/index.tsx
Outdated
| if (!all) { | ||
| return new Set(); | ||
| } | ||
|
|
||
| const ids = new Set<string>(); |
There was a problem hiding this comment.
| if (!all) { | |
| return new Set(); | |
| } | |
| const ids = new Set<string>(); | |
| const ids = new Set<string>(); | |
| if (!all) { | |
| return ids | |
| } | |
| for (const [key, value] of Object.entries(all)) { | ||
| if (isArchivedReport(value)) { | ||
| ids.add(key.slice(ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS.length)); | ||
| } | ||
| } | ||
| return ids; |
There was a problem hiding this comment.
| for (const [key, value] of Object.entries(all)) { | |
| if (isArchivedReport(value)) { | |
| ids.add(key.slice(ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS.length)); | |
| } | |
| } | |
| return ids; | |
| const prefixLength = ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS.length; | |
| for (const [key, value] of Object.entries(all)) { | |
| if (isArchivedReport(value)) { | |
| const reportID = key.slice(prefixLength); | |
| ids.add(reportID); | |
| } | |
| } | |
| return ids; |
|
@Krishna2323 Minor suggestion. The rest looks fine |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
Conflicts |
|
@tgolen conflicts resolved. |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
😭 more conflicts. If you can fix them quickly, I will try to merge as soon as I see it! |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
|
||
| const canSendInvoice = useMemo(() => canSendInvoicePolicyUtils(allPolicies as OnyxCollection<OnyxTypes.Policy>, session?.email), [allPolicies, session?.email]); | ||
| const isValidReport = !(isEmptyObject(quickActionReport) || isReportArchived); | ||
| const isArchivedReport = useReportIsArchived(quickActionReport?.reportID); |
There was a problem hiding this comment.
@DylanDylann @tgolen This variable is defined twice due to. 2 parallel PRs https://github.com/Expensify/App/pull/67600/files
|
🚀 Deployed to staging by https://github.com/tgolen in version: 9.1.89-1 🚀
|
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.1.89-21 🚀
|

Explanation of Change
Fixed Issues
$ #67092
PROPOSAL: #67092 (comment)
Tests
Submit your expenses below:Deletefrom the more menu on top right > ConfirmOffline tests
Submit your expenses below:Deletefrom the more menu on top right > ConfirmQA Steps
Submit your expenses below:Deletefrom the more menu on top right > ConfirmPR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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_native.mp4
Android: mWeb Chrome
android_chrome.mp4
iOS: Native
ios_native.mp4
iOS: mWeb Safari
ios_safari.mp4
MacOS: Chrome / Safari
web_chrome.mp4
MacOS: Desktop
desktop_app.mp4