Support updating an existing payment method

Created on 3 April 2024, about 1 year ago
Updated 1 May 2024, about 1 year ago

Problem/Motivation

To support two use cases:

1. Update a card's expiration date
2. Update the billing address of an existing card

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Feature request
Status

Fixed

Version

1.0

Component

Code

Created by

🇨🇳China skyredwang Shanghai

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @skyredwang
  • Status changed to Needs review about 1 year ago
  • Status changed to Needs work about 1 year ago
  • 🇵🇪Peru krystalcode

    Some feedback:

    $profile = $payment_method->getBillingProfile();
    // skip handling payment methods for anonymous users for now.
    if (!$profile || $profile->getOwner()->isAnonymous()) {
      throw new PaymentGatewayException('Updating payment method for anonymous users has not been implemented.');
    }
    

    Why are you fetching the profile to check ownership? The payment method has a direct owner since it implements `EntityOwnerInterface`, we should be checking for that.

      'expirationMonth' => $payment_method->card_exp_month,
      'expirationYear' => $payment_method->card_exp_year,
    

    Have you tested this? Without the `->value` at the end you'll be getting a `FieldItemListInterface` object.

  • Status changed to Needs review about 1 year ago
  • 🇨🇳China skyredwang Shanghai

    Why are you fetching the profile to check ownership?

    This is documented as in the comment

    // skip handling payment methods for anonymous users for now.

    The attached patch corrected the access of the fields.

  • Status changed to Needs work about 1 year ago
  • 🇵🇪Peru krystalcode

    Yes, the point I was making is that you should not fetch the profile to get the user, the payment method itself has an owner:

    if ($payment_method->getOwner()->isAnonymous()) {
      ...
    }
    

    Also check if the $user actually exists. That prevents errors when there are dead references.

  • Status changed to Needs review about 1 year ago
  • 🇨🇳China skyredwang Shanghai

    This version simplified the owner checking.

  • 🇵🇪Peru krystalcode

    - Removed the check for anonymous user; after review, since we have the vaulted shopper ID even if it's an anonymous user then we should be safe to update it here. It would be the responsibility of Drupal Commerce whether to expose editing payment methods for anonymous users or not. If that is done, then it can be updated in BlueSnap.
    - Made some coding standards improvements.

  • 🇵🇪Peru krystalcode

    The commit contains the following compared to the last patch.

    - Fixed bug that was sending the data in the wrong format.
    - Fixed bug that was deleting shopper names and billing info.

    This is now better tested and working as expected. A release will be made after running this and other pending issues on production for some time.

  • Status changed to Active about 1 year ago
  • Status changed to Fixed about 1 year ago
  • 🇵🇪Peru krystalcode

    Crediting everybody.

  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024