Adds notice that deposits are paused to Deposits Listing screen #8347
Conversation
…account has an error.
- Adds notice that deposits are paused when a payout account is in `errored` state
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: +418 B (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
|
I think the banner notice is fine to use for this, however we should hide the timeline notice to help merchants focus on the more urgent message. Can we hide this one when the bank account issue is present? Side note, there was some design exploration around integrating the timeline notice into the table a bit better. I think @rogermattic shared a solution for this. Is that still planned? |
|
Made changes to hide the timeline notice when the deposit failure notice is present. |
there's an issue with the bank account
…posits-list-failure-notice
details, instead of link to documentation.
…b.com/Automattic/woocommerce-payments into add/8332-deposits-list-failure-notice
I think this was the exploration you're talking about: 8026 |
| @@ -38,10 +41,24 @@ const useNextDepositNoticeState = () => { | |||
| }; | |||
| }; | |||
|
|
|||
| const NextDepositNotice: React.FC = () => { | |||
| const useAccountStatus = () => { | |||
There was a problem hiding this comment.
Changes in this PR can use a behavior test, wdyt?
There was a problem hiding this comment.
The Deposit Listing screen has no test file, will create a issue to add tests for this file, including this functionality
There was a problem hiding this comment.
Created issue here #8417
There was a problem hiding this comment.
Looks good, works well 🙌!
I've haven't been able to test it with currency other than USD, @nagpai would you like to help with testing it if you have it setup.
client/deposits/index.tsx
Outdated
| import { TestModeNotice } from 'components/test-mode-notice'; | ||
| import BannerNotice from 'components/banner-notice'; | ||
| import DepositSchedule from 'components/deposits-overview/deposit-schedule'; | ||
| import { useAllDepositsOverviews } from 'data'; | ||
| import { useSettings } from 'wcpay/data'; | ||
| import DepositsList from './list'; | ||
| import { hasAutomaticScheduledDeposits } from 'wcpay/deposits/utils'; | ||
| import { ExternalLink } from '@wordpress/components'; |
There was a problem hiding this comment.
Nitpick: Let's keep external imports together.
Why?
It makes it easier to identify and manage dependencies in a project. By grouping all external imports together, you can quickly see which packages your code is dependent on. This can be especially useful when you need to update or remove a package, as you can easily identify all the parts of your code that rely on it.
There was a problem hiding this comment.
Moved to External imports section.
Tried this with a France account having a working USD and a failing EUR account. The failure banner showed up as expected. USD payments continued to get paid out successfully due to correct bank account in order. Screenshots :
|
|
|
||
| /** | ||
| * Internal dependencies. | ||
| */ | ||
| import Page from 'components/page'; | ||
| import interpolateComponents from '@automattic/interpolate-components'; | ||
| import { __ } from '@wordpress/i18n'; |
There was a problem hiding this comment.
Nit: Can we club the wordpress imports together?
There was a problem hiding this comment.
Reason: #8347 (comment)
| components: { | ||
| updateLink: ( | ||
| <ExternalLink | ||
| href={ wcpaySettings.accountStatus.accountLink } |
There was a problem hiding this comment.
Nitpick: Maybe we can move this outside the jsx block.
Why?
Deconstructing objects outside the JS block can improve the readability and maintainability of your code. By doing so, you can avoid cluttering the JSX block with lengthy code, making it easier to understand. Additionally, it allows you to reuse the deconstructed values throughout your code, reducing the likelihood of making errors or typos. By keeping your JSX block clean and organized, you can make sure that your code is easier to maintain and update over time.
const { accountLink } = wcpaySetting.accountStatus;There was a problem hiding this comment.
Since we are not using any other field from wcpaySettings.accountStatus, I feel accessing wcpaySettings.accountStatus.accountLink just where it's used makes it easier to read. Would reconsider if we had multiple fields being used
Co-authored-by: Jessy <[email protected]> Co-authored-by: Naman Malhotra <[email protected]> Co-authored-by: Nagesh Pai <[email protected]>
Fixes #8332
Changes proposed in this Pull Request
DepositFailureNoticeindicating that deposits are paused when any of the customers payout bank accounts are inerroredstate.overview-allAPI changes from https://github.com/Automattic/woocommerce-payments-server/pull/4862Testing 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