Skip to content
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

Earn: add one time payment option to payment plans #43417

Merged
merged 3 commits into from
Jun 24, 2020
Merged

Conversation

gwwar
Copy link
Contributor

@gwwar gwwar commented Jun 18, 2020

Changes proposed in this Pull Request

Exposes the one time payment option. Copy changes are handled in #42848 under the same feature flag

This also includes a fix for one time payments being used to back premium content.

Screen Shot 2020-06-17 at 5 01 00 PM

Screen Shot 2020-06-17 at 5 01 12 PM

Payments button testing instructions

  • Set up a recurring payment plan on a site using the testing store. More info in PCYsg-lW4-p2 #sandbox-method.
  • Checkout this branch locally, and verify that a new one-time payment option is available in development
  • Verify no changes in production or add ?flags=-earn/rename-payment-blocks and refresh
  • Try adding recurring payments button

Premium Content Testing Instructions

  • (Requires a sandbox) Checkout this branch and either apply the linked diff, or run cd apps/full-site-editing && yarn dev --sync
  • Set up a recurring payment plan on a site using the testing store. More info in PCYsg-lW4-p2 #sandbox-method.
  • Insert a premium content block and select a one-time payment block
  • With another user, pay for the premium content
  • Verify that we can see this content without errors

Fixes #42858

@gwwar gwwar added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Earn labels Jun 18, 2020
@gwwar gwwar requested review from a team June 18, 2020 00:06
@gwwar gwwar self-assigned this Jun 18, 2020
@matticbot
Copy link
Contributor

@matticbot
Copy link
Contributor

matticbot commented Jun 18, 2020

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~24 bytes added 📈 [gzipped])

name  parsed_size           gzip_size
earn       +110 B  (+0.0%)      +24 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

Copy link
Member

@mmtr mmtr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests well on a Recurring Payments block, but there is a fatal error after subscribing using a Premium Content block:

Screen Shot 2020-06-18 at 13 11 01

Likely the end_date checked here is unset on one-time plans:

if ( in_array( $product_id, $product_ids, true ) && strtotime( $token_subscription->end_date ) > time() ) {

@kwight
Copy link
Contributor

kwight commented Jun 18, 2020

Verified that I could create a new product with the one-time option, and that the option is not available in non-dev environments.

Once I created the one-time product though, I started getting fatals related to products here being null: https://github.com/Automattic/wp-calypso/blob/master/client/my-sites/earn/memberships/products.jsx#L78

Adding a check before it fixes it, but I'm not sure I understand why it would suddenly be an issue after creating a one-time product.

I also had the same experience as @mmtr with the strtotime error on the front-end.

@gwwar
Copy link
Contributor Author

gwwar commented Jun 18, 2020

Oh hmm the format of end_date is different if it's a one time vs recurring:

Recurring:

(
    [end_date] => 2020-07-18 23:09:52
)

One Time

(
    [end_date] => 1624057639
)

Is this expected @Automattic/earn ?

@gwwar gwwar requested a review from a team as a code owner June 19, 2020 00:05
@gwwar gwwar removed the request for review from a team June 19, 2020 00:13
@gwwar
Copy link
Contributor Author

gwwar commented Jun 19, 2020

@Automattic/cylon are there updated instructions for installing and running phpcs? I ended up having to commit with --no-verify since it was running into ERROR: Referenced sniff "PHPCompatibility" does not exist

@matticbot
Copy link
Contributor

Caution: This PR affects files in the FSE Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D45169-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

@noahtallen
Copy link
Contributor

noahtallen commented Jun 19, 2020

@gwwar great question I also keep seeing that error locally 🤔. running phpcs earlier today, I did the following (from wp-calypso root) and it worked:

composer install
./vendor/bin/phpcs --standard=apps/phpcs.xml apps/full-site-editing

@noahtallen
Copy link
Contributor

we are also introducing phpcs in CI here: #43458

@gwwar
Copy link
Contributor Author

gwwar commented Jun 19, 2020

Thanks @noahtallen I think that did the trick. There's a lot of warnings to tidy though 🙈

@noahtallen
Copy link
Contributor

There's a lot of warnings to tidy though 🙈

we briefly talked about spending a few hours trying to fix all phpcs errors in the FSE plugin today 😅 I may start doing that. It sounds relaxing compared to what i've been working on this week

@kwight
Copy link
Contributor

kwight commented Jun 22, 2020

Fantastico, all works as expected 👍

@n3f
Copy link
Contributor

n3f commented Jun 22, 2020

Is this expected @Automattic/earn ?

Apparently. Looking in MembershipsBillingTest::test_memberships_paymentintent_subscribe_valid_jwt_token there is a test that validates the end_date should be in the '2020-07-22 21:01:10' format. While in MembershipsBillingTest::test_memberships_paymentintent_onetime it checks for the end_date to be greater or equal to time() + 360 * 24 * 3600.

As discussed in slack, we might want to use some constant here and/or have @artpi determine if longreads depends on using the int value.

@gwwar
Copy link
Contributor Author

gwwar commented Jun 22, 2020

Thanks for digging into that @n3f We can also return now + 1 year, but in the same ISO string format as well. Mainly it's rather unexpected to have different types here for a product, so this may be a good chance to tidy this before we have more records.

@gwwar
Copy link
Contributor Author

gwwar commented Jun 24, 2020

🤔 Since this is behind a feature flag, I'll land this one first, then follow up with a more general fix for types.

@gwwar
Copy link
Contributor Author

gwwar commented Jun 24, 2020

Thanks for the reviews folks!

@gwwar gwwar merged commit 7e7865d into master Jun 24, 2020
@gwwar gwwar deleted the add/one-time-payments branch June 24, 2020 16:32
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 24, 2020
@a8ci18n
Copy link

a8ci18n commented Jun 24, 2020

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/3925227

Thank you @gwwar for including a screenshot in the description! This is really helpful for our translators.

@a8ci18n
Copy link

a8ci18n commented Jul 5, 2020

Translation for this Pull Request has now been finished.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Settings: Update Recurring Payments plan options to offer one time payments
7 participants