refactor: move getPolicyTagsData to IOU and Transaction files#72678
Conversation
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.
|
|
Hello @tgolen , can you create subtasks for IOU, Transaction and TransactionUtils for removing |
|
OK, I created them! |
b9c2934 to
5978bbf
Compare
…nnect/getPolicyTagsData-src-libs-actions-Policy-Tag.ts
|
@shubham1206agra 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] |
| // `allPolicyTags` was moved here temporarily from `src/libs/actions/Policy/Tag.ts` during the `Deprecate Onyx.connect` refactor. | ||
| // All uses of this variable should be replaced with `useOnyx`. | ||
| let allPolicyTags: OnyxCollection<OnyxTypes.PolicyTagLists> = {}; | ||
| Onyx.connect({ |
There was a problem hiding this comment.
@shubham1206agra, bump. Can you review it again so we can get this one merged? :D
…nnect/getPolicyTagsData-src-libs-actions-Policy-Tag.ts
…nnect/getPolicyTagsData-src-libs-actions-Policy-Tag.ts
|
@shubham1206agra @tgolen bump |
|
@Skalakid Can you give better test steps as current steps are too generic? |
|
@shubham1206agra I updated test steps and uploaded a video 🫡 This PR affects many functions and scenarios, but it only moves the Onyx.connect from the Transaction file to IOU.ts, so it shouldn't make any difference in the behaviour |
…nnect/getPolicyTagsData-src-libs-actions-Policy-Tag.ts # Conflicts: # src/libs/TransactionUtils/index.ts # src/libs/actions/IOU.ts # src/libs/actions/Policy/Tag.ts
…/libs/actions/Policy/Tag.ts
|
There are some checks failing, probably because someone used |
|
Hi @tgolen , can we please get another issue created, this time for |
|
OK, here you go: #80401 |
…-fork into @Skalakid/refactor/useonyx-deprecate-onyx-connect/getPolicyTagsData-src-libs-actions-Policy-Tag.ts
…licyTagsData-src-libs-actions-Policy-Tag.ts' of github.com:software-mansion-labs/expensify-app-fork into @Skalakid/refactor/useonyx-deprecate-onyx-connect/getPolicyTagsData-src-libs-actions-Policy-Tag.ts
…-fork into @Skalakid/refactor/useonyx-deprecate-onyx-connect/getPolicyTagsData-src-libs-actions-Policy-Tag.ts
…-fork into @Skalakid/refactor/useonyx-deprecate-onyx-connect/getPolicyTagsData-src-libs-actions-Policy-Tag.ts
|
Hi, I went with the getter pattern and refactored this PR - we migrated Onyx.connect only to IOU/index.ts and from then we are using getPolicyTags(); This way it will be easier to migrate and track, I'll delete both the connect and getter function while all the usages are removed ✅ In the meantime, @shubham1206agra are you able to review this PR? If you need some help, I can ask @tgolen for assigning someone else |
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-01-26.at.7.35.08.PM.mov |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #69022 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
@youssef-lr looks like this one's ready, let's get it merged 🚀 |
…-fork into @Skalakid/refactor/useonyx-deprecate-onyx-connect/getPolicyTagsData-src-libs-actions-Policy-Tag.ts
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/youssef-lr in version: 9.3.10-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.10-6 🚀
|
Explanation of Change
This PR is part of a refactor to remove Onyx.connect from the
src/libs/actions/Policy/Tag.tsfile and replace it with useOnyx.It isolates the
getPolicyTagsDatafunction from the Onyx.connect data by moving Onyx.connect temporarily to the files that were using it. Refactoring it right now would require a lot of complex changes, which would be hard to test in the future. Thanks to separating it toIOU.ts,Transaction.ts, andTransactionUtils/index.ts, we will be able to refactor everything in smaller PRs in a more organized and testable way.I've marked
getPolicyTagsDataas deprecated and added the comment // TODO: Replace getPolicyTagsData with useOnyx hook above each use.Fixed Issues
$ #69022
PROPOSAL:
Tests
Offline tests
Same as Tests
QA Steps
Same as Tests
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
web.mov