Move usePaymentActivityData() to PaymentActivity because filters will need it
#8751
+107
−65
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: +34 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
…hub.com:Automattic/woocommerce-payments into update/8733-move-payment-activity-data-fetching
…ndefined (usually when server returns an error).
Jinksi
reviewed
May 2, 2024
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
|
|
||
| import EmptyStateAsset from 'assets/images/payment-activity-empty-state.svg?asset'; | ||
| import PaymentActivityData from './payment-activity-data'; | ||
| import PaymentActivityDataComponent from './payment-activity-data'; |
There was a problem hiding this comment.
Kudos for clarifying this component name and fixing the naming conflict with the TS interface.
@shendy-a8c I think there is an opportunity to separate out the Empty State component to prevent this network call and clarify the logic. Example: const PaymentActivityWrapper: React.FC = () => {
const { lifetimeTPV } = wcpaySettings;
const hasAtLeastOnePayment = lifetimeTPV > 0;
if ( ! hasAtLeastOnePayment ) {
// This component can just do empty state.
return <PaymentActivityEmptyState />;
}
// This component can call the hook to request API, etc.
return <PaymentActivity />;
};I've created a PR targeting this branch in #8756 to see if it works or not. Update – merged into this PR. |
…ogic and prevent erroneous network call (#8756)
…tching # Conflicts: # client/components/payment-activity/payment-activity-data.tsx # client/components/payment-activity/test/__snapshots__/index.test.tsx.snap
|
✓ Resolved conflicts and merged base branch to PR branch. Ready for another round of review. |
…tching # Conflicts: # client/components/payment-activity/payment-activity-data.tsx
…tching # Conflicts: # client/components/payment-activity/test/__snapshots__/index.test.tsx.snap
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 #8733
Changes proposed in this Pull Request
usePaymentActivityData()call to<PaymentActivity>from<PaymentActivityData>.Because we are going to show filters in the Payment Activity widget, the payment activity data fetch (the call to
usePaymentActivityData()) needs to happen in the same component of the filter, in<PaymentActivity>.This is needed because while
isLoadingtrue, I like to either disable or hide the filters and it's hard to do if theisLoadingis within the child element of<PaymentActivity>.note: I realize that with this change, Payments > Overview page will try to fetch payment activity data even though lifetime TPV is 0 where with base branch, the data fetch will only happen only if lifetime TPV > 0.
Rename component
<PaymentActivityData>to<PaymentActivityDataComponent>so it does not conflict with thePaymentActivityDatatype.Use the date start and end from payment activity data returned by the backend to generate the report links.
Testing instructions
_wcpay_feature_payment_overview_widget.Testing error handling when server does not return proper response.
includes/admin/class-wc-rest-payments-reporting-controller.phplocally so it returns an error to frontend.On this line:
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