[HOLD] Move deleteReport to a separate file and use Onyx.connectWithoutView#82481
[HOLD] Move deleteReport to a separate file and use Onyx.connectWithoutView#82481bernhardoj wants to merge 9 commits intoExpensify:mainfrom
Conversation
|
@alitoshmatov 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] |
|
@brunovjk @MarioExpensify 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] |
|
I believe @eVoloshchak will review this |
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.
|
tests/unit/DeleteReportTest.ts
Outdated
| import deleteReport from '@libs/actions/Report/DeleteReport'; | ||
| import CONST from '@src/CONST'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import {ReportAction} from '@src/types/onyx'; |
There was a problem hiding this comment.
| import {ReportAction} from '@src/types/onyx'; | |
| import type {ReportAction} from '@src/types/onyx'; |
MarioExpensify
left a comment
There was a problem hiding this comment.
Awesome, thank you!
|
Woops, @eVoloshchak can you proceed with reviewer checklist please? |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeScreen.Recording.2026-02-18.at.12.53.08.moviOS: HybridAppiOS: mWeb SafariScreen.Recording.2026-02-18.at.12.50.00.movMacOS: Chrome / SafariScreen.Recording.2026-02-18.at.12.45.24.mov |
@bernhardoj, this doesn't work on native (both iOS and Android), you're not redirected to Concierge. Screen.Recording.2026-02-18.at.12.56.01.movScreen.Recording.2026-02-18.at.12.48.41.mov |
|
@bernhardoj please address @eVoloshchak's comments, also, please don't forget to add proper screenshots/videos for each platform. |
Hmm, this is because inlineRequires, which imports the file/module when the function is called for the first time. So, when it's called for the first time, the report collection is empty. |
|
Confirmed this is also the case on the current staging/main, doesn't seem related to this PR. |
Hmm, I get redirected to concierge though in |
|
Asked in Slack, but no response yet. |
When building the native app? That's not the case for me, both when building locally or using a staging build (I verified using the Android build from #81778 (comment)) |
|
I haven't tested the Android, but on iOS, it's not redirected ios.mp4 |
Same for me, no redirection on both native platforms (either production build or latest main in dev, same behavior). Which means it was not caused by this PR. I don't have access to the slack discussion, waiting on the decision |
|
Oh wait, I forgot whether I test it on main or on this branch 🤦. Let me retest again. |
heyjennahay
left a comment
There was a problem hiding this comment.
Product review not required
|
@eVoloshchak you're right. It's not being redirected either on |
|
Hmm, looks like the |
|
@eVoloshchak I found the bug. The App/src/libs/actions/Report/index.ts Lines 4931 to 4936 in 6ff8eea App/src/libs/actions/Report/index.ts Lines 1202 to 1210 in 6ff8eea I've reported this in Slack. Should we continue with the PR or wait until the bug is fixed? |
I think the most reliable way would be to put this on hold until it's fixed in |
Explanation of Change
Fixed Issues
$ #73651
PROPOSAL:
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
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
android.mweb.mp4
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
web.mp4