Show deposit schedule message if account's deposits are unrestricted #7875
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +29 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
Jinksi
changed the title
Jinksi
changed the title
Jinksi
changed the title
|
I have a couple of live accounts. I enabled this PR via Jetpack Beta and tested it in a couple of scenarios; it LGTM so far. Sharing these here in case they help. Live mode with a pending balance, no available balance, and in the new account waiting period: Test mode with an available balance: Live mode with no transactions: Activating this with the WCPay dev tools did produce a fatal error on one site, but it was because I was running an outdated build of the dev tools — doesn't seem to be an issue with the PR. |
This was referenced Dec 12, 2023
Jinksi
commented
Dec 12, 2023
6 tasks
This comment was marked as outdated.
This comment was marked as outdated.
shendy-a8c
reviewed
Dec 12, 2023
shendy-a8c
reviewed
Dec 12, 2023
| @@ -35,6 +35,9 @@ const DepositsOverview: React.FC = () => { | |||
| overview, | |||
| isLoading: isLoadingOverview, | |||
| } = useSelectedCurrencyOverview(); | |||
| const isDepositsUnrestricted = | |||
| wcpaySettings.accountStatus.deposits?.restrictions === | |||
There was a problem hiding this comment.
Why do you check for wcpaySettings.accountStatus.deposits.restrictions instead of account.deposits_blocked?
If I understand it correctly, account.deposits_blocked takes account new account waiting period and wcpaySettings.accountStatus.deposits.restrictions does not.
Just curious.
Current code would work because the deposits card won't show if available balance is 0 and there is no deposits anyway. When in waiting period, merchant won't have deposits.
There was a problem hiding this comment.
AFAICT account.deposits_blocked only checks for deposits blocked, and doesn't take into consideration schedule restrictions.
In this case, we want to check for both schedule restrictions and blocked deposits, which wcpaySettings.accountStatus.deposits.restrictions === 'deposits_unrestricted' covers.
I could include a check for wcpaySettings.accountStatus.deposits.completed_waiting_period === true, that might be more explicit. What do you think?
There was a problem hiding this comment.
Thank you for explaining. I don't think it's necessary to check waiting period.
shendy-a8c
approved these changes
Dec 12, 2023
shendy-a8c
requested changes
Dec 12, 2023
Jinksi
force-pushed
the
fix/7874-show-deposit-schedule-when-zero-available-balance
branch
from
34215f7 to
9cec48f
Compare
Co-authored-by: Shendy <[email protected]>
Jinksi
force-pushed
the
fix/7874-show-deposit-schedule-when-zero-available-balance
branch
from
f6135dd to
ed4a651
Compare
…-zero-available-balance
|
Rebased to |
shendy-a8c
approved these changes
Dec 13, 2023
There was a problem hiding this comment.
Looks good. Retested to make sure and everything looks good.
Also, I made sure the rebase went OK by comparing with the PR branch before rebased, https://github.com/Automattic/woocommerce-payments/pull/7896/files. I still had it on my local minus your latest commit.
Jinksi
deleted the
fix/7874-show-deposit-schedule-when-zero-available-balance
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #7874
Changes proposed in this Pull Request
This helps communicate to merchants that they have an expected upcoming deposit, even if their available balance <= 0.
Testing instructions
npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge