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

fix(ras-sync): account for missing creation data and first/last name #3677

Open
wants to merge 1 commit into
base: release
Choose a base branch
from

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented Jan 14, 2025

All Submissions:

Changes proposed in this Pull Request:

Avoids a fatal and accounts for some missing user meta when building contact data for migrated Woo customers. This seems to be an edge case with migrated/imported users as "naturally created" WP users should always have a user_registered value, but we've seen that in rare cases imported users can lack this value.

How to test the changes in this Pull Request:

Since it's impossible to delete the user_registered value from a user once created, we need to use wp shell to simulate the conditions for the error.

  1. On release, use wp shell to delete the date_created value and the First and Last Name meta fields from the user (replace 56 with your test user's ID):
> $customer = new WC_Customer( 56 );
= WC_Customer {#16805}

> $customer->set_date_created( null );
= null

> $customer->save();
= 56

> delete_user_meta( 56, 'first_name' );
= true

> delete_user_meta( 56, 'last_name' );
= true
  1. While in the same wp shell session, get the contact data and observe a fatal error:
> Newspack\Reader_Activation\Sync\WooCommerce::get_contact_from_customer( $customer );

   Error  Call to a member function date() on null.
  1. Check out this branch, start a new wp shell session, and repeat. This time confirm that the contact data gets built, but without a registration_date value:
> $customer = new WC_Customer( 56 );
= WC_Customer {#16805}

> $customer->set_date_created( null );
= null

> $customer->save();
= 56

> delete_user_meta( 56, 'first_name' );
= true

> delete_user_meta( 56, 'last_name' );
= true

> Newspack\Reader_Activation\Sync\WooCommerce::get_contact_from_customer( $customer );
= [
    "email" => "user@test.co",
    "metadata" => [
      "payment_page" => "https://site.url/test"
      "membership_status" => "active",
      "sub_start_date" => "2025-01-14 20:53:22",
      "sub_end_date" => "",
      "billing_cycle" => "month",
      "recurring_payment" => "50.00",
      "last_payment_amount" => "50.00",
      "last_payment_date" => "2025-01-14 20:53:22",
      "next_payment_date" => "2025-02-13 20:53:22",
      "product_name" => "Premium Subscription",
      "payment_page_utm_source" => "",
      "payment_page_utm_medium" => "",
      "payment_page_utm_campaign" => "",
      "payment_page_utm_term" => "",
      "payment_page_utm_content" => "",
      "cancellation_reason" => "",
      "total_paid" => "50.00",
      "account" => 56,
      "registration_date" => "",
    ],
    "name" => "Test User",
  ]
  1. Confirm that after calling get_contact_from_customer the billing first/last names are copied to the first_name and last_name user meta fields (this happens automatically when a new user is created during the checkout flow). You can check this in the User admin page for this user, or via wp user meta list <user ID> in WP CLI.
Screenshot 2025-01-14 at 2 38 09 PM

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@dkoo dkoo added the [Status] Needs Review The issue or pull request needs to be reviewed label Jan 14, 2025
@dkoo dkoo self-assigned this Jan 14, 2025
@dkoo dkoo requested a review from a team as a code owner January 14, 2025 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Needs Review The issue or pull request needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant