-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Donations: Fall back to default products if no products are already defined. #43665
Conversation
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
Caution: This PR affects files in the FSE Plugin on WordPress.com D45454-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing in the FSE Plugin for more info: PCYsg-ly5-p2 |
apps/full-site-editing/full-site-editing-plugin/donations/fetch-default-products.js
Outdated
Show resolved
Hide resolved
apps/full-site-editing/full-site-editing-plugin/donations/fetch-status.js
Show resolved
Hide resolved
|
||
setIsLoading( false ); | ||
}; | ||
if ( result.products.length ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd need to check there are products for the current currency as well. Right now if you remove all the donation-type plans (AFAIK can be done only with wp shell
) and create a few for a currency other than USD, no default plans for USD will be created.
I also think we need to check that returned products contains all intervals and run fetchDefaultProducts
if any is missing (the API will create only the missing ones, duplicates prevention is handled in D45327-code).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK can be done only with wp shell
Ah, found this today: WP.com API v1.1 POST /sites/:site-id/memberships/product/:product-id/delete
(got it from the Earn UI when deleting from there). And it has to be the site ID – the domain doesn't work, for some reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a dev flag for managing this in the Calypso UI? (eg simply return all products in the earn management section) It might be helpful for general debugging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f613589 should take care of the additional checks. I did notice though, that all three defaults were generated even with existing valid intervals, so I'm not sure if that's a bug with what I've done, or the generation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that all three defaults were generated even with existing valid intervals
Note that the defaults endpoint will return any valid existing plan. It'll create only missing plans. That means the endpoint will return always the 3 plans, but it doesn't mean all of them were generated during the request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the defaults endpoint will return any valid existing plan. It'll create only missing plans. That means the endpoint will return always the 3 plans, but it doesn't mean all of them were generated during the request.
I confirmed this by deleting a valid plan using the delete endpoint mentioned above and inserting the block. Only one new plan was created and the endpoint keeps returning the existing ones.
apps/full-site-editing/full-site-editing-plugin/donations/edit.js
Outdated
Show resolved
Hide resolved
4420b95
to
6434ec2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The Donations block will rely on membership products existing to give the site visitor options for donating. If the user inserts a Donations block but does not already have some USD subscription products set up, the block will call the new
POST /memberships/products
endpoint (Automattic/jetpack#16242) to generate some default products.Part of #42856 .
Testing Instructions
cd apps/full-site-editing; yarn dev --sync;
fse-donations-block
blog sticker applied (if not, it can be done from the site's Blog RC)./earn/payments-plans/:site
, delete any existing test products./memberships/products?type=donation¤cy=USD
.