fix: remove onyx connect ONYXKEYS.NVP_PRIVATE_OWNER_BILLING_GRACE_PERIOD_END (part 5)#84867
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc15f15b09
ℹ️ 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".
| isSelfTourViewed, | ||
| userBillingGraceEndPeriods, | ||
| amountOwed, | ||
| ownerBillingGraceEndPeriod, |
There was a problem hiding this comment.
Include owner grace period in payment callback deps
confirmPayment is memoized but its dependency list was not updated after adding ownerBillingGraceEndPeriod to the payMoneyRequest payload. If the owner grace-period timestamp updates while this screen stays mounted (for example after a billing status refresh), the callback keeps sending the stale captured value, so shouldRestrictUserBillableActions() can make pay/approve decisions using outdated billing state.
Useful? React with 👍 / 👎.
| isSelfTourViewed, | ||
| userBillingGraceEndPeriods, | ||
| amountOwed, | ||
| ownerBillingGraceEndPeriod, |
There was a problem hiding this comment.
Include owner grace period in preview payment deps
confirmPayment in this component now passes ownerBillingGraceEndPeriod, but the useCallback dependency array still omits it. When Onyx pushes a newer grace-period value, the handler can continue using an older timestamp and route users through the wrong restricted/unrestricted payment path until another listed dependency changes.
Useful? React with 👍 / 👎.
| activePolicy, | ||
| userBillingGraceEndPeriodCollection, | ||
| amountOwed, | ||
| ownerBillingGraceEndPeriod, |
There was a problem hiding this comment.
Recompute track-expense menu when grace period changes
menuItems is memoized and now captures ownerBillingGraceEndPeriod inside the Track Expense actions, but that value is not in the useMemo dependency list. If the grace-period timestamp changes after initial render, these actions keep using the stale captured value and can navigate to restricted/unrestricted flows incorrectly.
Useful? React with 👍 / 👎.
| activePolicy, | ||
| userBillingGraceEndPeriodCollection, | ||
| amountOwed, | ||
| ownerBillingGraceEndPeriod, |
There was a problem hiding this comment.
Refresh actionable buttons when grace period updates
actionableItemButtons now forwards ownerBillingGraceEndPeriod to draft-transaction navigation, but its useMemo dependencies were not updated. That leaves these actionable whisper handlers using stale billing-grace data after Onyx updates, which can produce incorrect restriction checks for submit/categorize/share actions.
Useful? React with 👍 / 👎.
JmillsExpensify
left a comment
There was a problem hiding this comment.
No product review required on this one.
|
@truph01 Fix conflicts please |
|
@shubham1206agra I fixed conflicts |
|
@shubham1206agra I fixed conflicts |
|
The
isn't related to this PR |
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-03-20.at.10.24.38.AM.mov |
|
@tgolen Please review this PR |
|
@truph01 Please merge main |
tgolen
left a comment
There was a problem hiding this comment.
PR looks fine, but has conflicts. I mentioned renaming the variable, but that doesn't need to happen in this PR. If you can just update that in future PRs, that should be fine, so I'm going to approve for now.
| isSelfTourViewed, | ||
| userBillingGraceEndPeriods, | ||
| amountOwed, | ||
| ownerBillingGraceEndPeriod, |
There was a problem hiding this comment.
Same as the other PR, but please rename all these to ownerBillingGracePeriodEnd
There was a problem hiding this comment.
@tgolen I think it would be better to create a separate PR that focuses solely on the rename, since ownerBillingGraceEndPeriod is used throughout the app—not just in this PR.
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @tgolen has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.3.43-3 🚀
|
Explanation of Change
This PR is part of an effort to remove
Onyx.connect()usage for the keyONYXKEYS.NVP_PRIVATE_OWNER_BILLING_GRACE_PERIOD_ENDinsrc/libs/SubscriptionUtils.ts.Since this refactor affects multiple areas, it is split into a few small PRs. This PR is the 5th step and focuses on the following refactor:
Fixed Issues
$ #66449
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include ""[No QA].""
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
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-03-11.at.18.46.13.mov