[No QA]: Move Track expense functions to a new file#85048
[No QA]: Move Track expense functions to a new file#85048DylanDylann wants to merge 1 commit intoExpensify:mainfrom
Conversation
melvin-bot
bot
requested review from
JmillsExpensify and
parasharrajat
and removed request for
a team
|
@parasharrajat 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -0,0 +1,2620 @@ | |||
| /* eslint-disable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-redundant-type-constituents, @typescript-eslint/prefer-nullish-coalescing, @typescript-eslint/no-unsafe-return */ | |||
There was a problem hiding this comment.
❌ CONSISTENCY-5 (docs)
This file-level eslint-disable comment suppresses 7 TypeScript rules without any accompanying justification comment explaining why these suppressions are necessary. Since this is a brand-new file, consider adding a brief explanation (e.g., -- Carried over from IOU/index.ts during refactor; tracked in #XXXXX) or, better yet, fixing the underlying type issues so the blanket disable isn't needed.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| import type {OnyxData} from '@src/types/onyx/Request'; | ||
| import type {Receipt, ReceiptSource, WaypointCollection} from '@src/types/onyx/Transaction'; | ||
| import {isEmptyObject} from '@src/types/utils/EmptyObject'; | ||
| // eslint-disable-next-line import/no-cycle |
There was a problem hiding this comment.
❌ CONSISTENCY-5 (docs)
The // eslint-disable-next-line import/no-cycle comment lacks a justification explaining why this circular dependency exists and is acceptable. Add a brief reason, e.g.:
// eslint-disable-next-line import/no-cycle -- TrackExpense and IOU/index have a mutual dependency: TrackExpense uses shared helpers while index re-exports TrackExpense typesPlease rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| RequestMoneyParticipantParams, | ||
| StartSplitBilActionParams, | ||
| } from './index'; | ||
| // eslint-disable-next-line import/no-cycle |
There was a problem hiding this comment.
❌ CONSISTENCY-5 (docs)
Same as above — this // eslint-disable-next-line import/no-cycle also needs a justification comment explaining the circular dependency.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| import type RecentlyUsedTags from '@src/types/onyx/RecentlyUsedTags'; | ||
| import type {ReportNextStep} from '@src/types/onyx/Report'; | ||
| import type ReportAction from '@src/types/onyx/ReportAction'; | ||
| import type {OnyxData} from '@src/types/onyx/Request'; | ||
| import type {Comment, Receipt, ReceiptSource, Routes, SplitShares, TransactionChanges, TransactionCustomUnit, WaypointCollection} from '@src/types/onyx/Transaction'; | ||
| import type {FileObject} from '@src/types/utils/Attachment'; | ||
| import {isEmptyObject} from '@src/types/utils/EmptyObject'; | ||
| // eslint-disable-next-line import/no-cycle |
There was a problem hiding this comment.
❌ CONSISTENCY-5 (docs)
The // eslint-disable-next-line import/no-cycle before the BaseTransactionParams import from ./PerDiem lacks a justification comment. Add a brief reason, e.g.:
// eslint-disable-next-line import/no-cycle -- PerDiem imports from IOU/index while index needs BaseTransactionParams from PerDiemPlease rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| import type RecentlyUsedTags from '@src/types/onyx/RecentlyUsedTags'; | ||
| import type {ReportNextStep} from '@src/types/onyx/Report'; | ||
| import type ReportAction from '@src/types/onyx/ReportAction'; | ||
| import type {OnyxData} from '@src/types/onyx/Request'; | ||
| import type {Comment, Receipt, ReceiptSource, Routes, SplitShares, TransactionChanges, TransactionCustomUnit, WaypointCollection} from '@src/types/onyx/Transaction'; | ||
| import type {FileObject} from '@src/types/utils/Attachment'; | ||
| import {isEmptyObject} from '@src/types/utils/EmptyObject'; | ||
| // eslint-disable-next-line import/no-cycle | ||
| import type {BaseTransactionParams} from './PerDiem'; | ||
| // eslint-disable-next-line import/no-cycle |
There was a problem hiding this comment.
❌ CONSISTENCY-5 (docs)
The // eslint-disable-next-line import/no-cycle before importing convertTrackedExpenseToRequest from ./TrackExpense lacks a justification comment.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| import type {BaseTransactionParams} from './PerDiem'; | ||
| // eslint-disable-next-line import/no-cycle | ||
| import {convertTrackedExpenseToRequest} from './TrackExpense'; | ||
| // eslint-disable-next-line import/no-cycle |
There was a problem hiding this comment.
❌ CONSISTENCY-5 (docs)
The // eslint-disable-next-line import/no-cycle before importing CreateTrackExpenseParams type from ./TrackExpense lacks a justification comment.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| @@ -13232,3 +10762,5 @@ export type { | |||
| BuildOnyxDataForMoneyRequestKeys, | |||
| MoneyRequestInformation, | |||
| }; | |||
| // eslint-disable-next-line import/no-cycle | |||
There was a problem hiding this comment.
❌ CONSISTENCY-5 (docs)
The // eslint-disable-next-line import/no-cycle before the re-export of TrackExpense types at the bottom of the file also lacks a justification comment.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1edcf8e617
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| submitReport, | ||
| trackExpense, | ||
| unapproveExpenseReport, |
There was a problem hiding this comment.
Removing trackExpense from @userActions/IOU breaks existing runtime callers that still use the namespace import, specifically src/libs/ReceiptUploadRetryHandler/handleFileRetry.ts which executes IOU.trackExpense(...) for TRACK_EXPENSE retries. In that flow, IOU.trackExpense is now undefined, so retrying a failed tracked-expense receipt upload will throw instead of retrying the API call.
Useful? React with 👍 / 👎.
1 participant
Explanation of Change
Fixed Issues
$ #72804
PROPOSAL:
Tests
Offline tests
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.