From ff84dd7143f0374cacced7215a276cdb1278ba6c Mon Sep 17 00:00:00 2001 From: sherrardb <32931314+sherrardb@users.noreply.github.com> Date: Wed, 8 Jan 2020 16:50:50 +0000 Subject: [PATCH] fix post_customer() arguments: * updated Kavorka signature to remove non-functional or illegitimate argument types * removed Net::Stripe::Card and disallowed card id for card, as neither form is valid conceptually * always create a Net::Stripe::Customer object before _post() to take advantage of argument coercion during objectification * include omitted arguments in object creation * clean up and centralize Net::Stripe:Token coercion code, since we always need the token id * added unit tests to exercise all allowed argument forms for customer creation and customer update * closes --- README.pod | 14 ++- lib/Net/Stripe.pm | 51 ++++++----- lib/Net/Stripe/Customer.pm | 5 +- lib/Net/Stripe/Resource.pm | 1 + t/live.t | 170 ++++++++++++++++++++++++++++++++++++- 5 files changed, 209 insertions(+), 32 deletions(-) diff --git a/README.pod b/README.pod index 3f7398d..b821400 100644 --- a/README.pod +++ b/README.pod @@ -252,11 +252,11 @@ L =over -=item * customer - L - existing customer to update, optional +=item * customer - L or StripeCustomerId - existing customer to update, optional =item * account_balance - Int, optional -=item * card - L, L, Str or HashRef, default card for the customer, optional +=item * card - L, StripeTokenId or HashRef, default card for the customer, optional =item * coupon - Str, optional @@ -1047,6 +1047,16 @@ paths and made the conditional structure more explicit, per discussion in . We also added unit tests for all calling forms, per . +=item fix post_customer() arguments + +Some argument types for `card` are not legitimate and have been removed from +the Kavorka method signature. We removed `Net::Stripe::Card` and updated the +string validation to disallow card id for `card`, as neither of these are +legitimate when creating or updating a customer L. +We have also updated the structure of the method so that we always create a +Net::Stripe::Customer object before posting L +and cleaned up and centralized Net::Stripe:Token coercion code. + =back =head1 SEE ALSO diff --git a/lib/Net/Stripe.pm b/lib/Net/Stripe.pm index 0deaea4..0dcd50d 100644 --- a/lib/Net/Stripe.pm +++ b/lib/Net/Stripe.pm @@ -127,6 +127,16 @@ paths and made the conditional structure more explicit, per discussion in . We also added unit tests for all calling forms, per . +=item fix post_customer() arguments + +Some argument types for `card` are not legitimate and have been removed from +the Kavorka method signature. We removed `Net::Stripe::Card` and updated the +string validation to disallow card id for `card`, as neither of these are +legitimate when creating or updating a customer L. +We have also updated the structure of the method so that we always create a +Net::Stripe::Customer object before posting L +and cleaned up and centralized Net::Stripe:Token coercion code. + =back =method new PARAMHASH @@ -392,11 +402,11 @@ L =over -=item * customer - L - existing customer to update, optional +=item * customer - L or StripeCustomerId - existing customer to update, optional =item * account_balance - Int, optional -=item * card - L, L, Str or HashRef, default card for the customer, optional +=item * card - L, StripeTokenId or HashRef, default card for the customer, optional =item * coupon - Str, optional @@ -499,9 +509,9 @@ Returns a L object containing L object =cut Customers: { - method post_customer(Net::Stripe::Customer|Str :$customer?, + method post_customer(Net::Stripe::Customer|StripeCustomerId :$customer?, Int :$account_balance?, - Net::Stripe::Card|Net::Stripe::Token|Str|HashRef :$card?, + Net::Stripe::Token|StripeTokenId|HashRef :$card?, Str :$coupon?, Str :$default_card?, Str :$description?, @@ -511,13 +521,10 @@ Customers: { Int :$quantity?, Int|Str :$trial_end?) { - if (defined($card) && ref($card) eq 'HASH') { - $card = Net::Stripe::Card->new($card); - } - - if (ref($customer) eq 'Net::Stripe::Customer') { - return $self->_post("customers/" . $customer->id, $customer); - } elsif (defined($customer)) { + my $customer_obj; + if ( ref( $customer ) ) { + $customer_obj = $customer; + } else { my %args = ( account_balance => $account_balance, card => $card, @@ -525,22 +532,20 @@ Customers: { default_card => $default_card, email => $email, metadata => $metadata, + plan => $plan, + quantity => $quantity, + trial_end => $trial_end, ); + $args{id} = $customer if defined( $customer ); - return $self->_post("customers/" . $customer, \%args); + $customer_obj = Net::Stripe::Customer->new( %args ); } - - $customer = Net::Stripe::Customer->new(account_balance => $account_balance, - card => $card, - coupon => $coupon, - description => $description, - email => $email, - metadata => $metadata, - plan => $plan, - quantity => $quantity, - trial_end => $trial_end); - return $self->_post('customers', $customer); + if ( my $customer_id = $customer_obj->id ) { + return $self->_post("customers/$customer_id", $customer_obj); + } else { + return $self->_post('customers', $customer_obj); + } } method list_subscriptions(Net::Stripe::Customer|Str :$customer, diff --git a/lib/Net/Stripe/Customer.pm b/lib/Net/Stripe/Customer.pm index 422e612..fb877d8 100644 --- a/lib/Net/Stripe/Customer.pm +++ b/lib/Net/Stripe/Customer.pm @@ -15,7 +15,7 @@ extends 'Net::Stripe::Resource'; has 'email' => (is => 'rw', isa => 'Maybe[Str]'); has 'description' => (is => 'rw', isa => 'Maybe[Str]'); has 'trial_end' => (is => 'rw', isa => 'Maybe[Int|Str]'); -has 'card' => (is => 'rw', isa => 'Maybe[Net::Stripe::Token|Net::Stripe::Card|Str]'); +has 'card' => (is => 'rw', isa => 'Maybe[Net::Stripe::Token|Net::Stripe::Card|StripeTokenId]'); has 'quantity' => (is => 'rw', isa => 'Maybe[Int]'); has 'plan' => (is => 'rw', isa => 'Maybe[Net::Stripe::Plan|Str]'); has 'coupon' => (is => 'rw', isa => 'Maybe[Net::Stripe::Coupon|Str]'); @@ -41,8 +41,7 @@ sub _build_subscription { method form_fields { return ( - (($self->card && ref($self->card) eq 'Net::Stripe::Token') ? - (card => $self->card->id) : $self->fields_for('card')), + $self->fields_for('card'), $self->fields_for('plan'), $self->fields_for('coupon'), $self->form_fields_for_metadata(), diff --git a/lib/Net/Stripe/Resource.pm b/lib/Net/Stripe/Resource.pm index e940c39..cf3099f 100644 --- a/lib/Net/Stripe/Resource.pm +++ b/lib/Net/Stripe/Resource.pm @@ -72,6 +72,7 @@ method fields_for($for) { return unless $self->can($for); my $thingy = $self->$for; return unless $thingy; + return ($for => $thingy->id) if $for eq 'card' && ref($thingy) eq 'Net::Stripe::Token'; return $thingy->form_fields if ref($thingy) =~ m/^Net::Stripe::/; return ($for => $thingy); } diff --git a/t/live.t b/t/live.t index 465e551..b52316b 100755 --- a/t/live.t +++ b/t/live.t @@ -629,7 +629,7 @@ Customers: { ok $customer->{deleted}, 'customer is now deleted'; } - Create_with_a_token: { + Create_with_a_token_id: { my $token = $stripe->post_token(card => $fake_card); my $customer = $stripe->post_customer( card => $token->id, @@ -637,10 +637,172 @@ Customers: { isa_ok $customer, 'Net::Stripe::Customer', 'got back a customer'; ok $customer->id, 'customer has an id'; my $card = $stripe->get_card( - customer=> $customer, - card_id=> $customer->default_card, + customer => $customer, + card_id => $customer->default_card, + ); + is $card->id, $token->card->id, 'token card id matches'; + } + + Create_with_a_token_object: { + my $token = $stripe->post_token(card => $fake_card); + my $customer = $stripe->post_customer( + card => $token, + ); + isa_ok $customer, 'Net::Stripe::Customer', 'got back a customer'; + ok $customer->id, 'customer has an id'; + my $card = $stripe->get_card( + customer => $customer, + card_id => $customer->default_card, + ); + is $card->id, $token->card->id, 'token card id matches'; + } + + Create_with_a_card_hashref: { + my $customer = $stripe->post_customer( + card => $fake_card, + ); + isa_ok $customer, 'Net::Stripe::Customer', 'got back a customer'; + ok $customer->id, 'customer has an id'; + my $card = $stripe->get_card( + customer => $customer, + card_id => $customer->default_card, + ); + is $card->last4, substr( $fake_card->{number}, -4 ), 'card last4 matches'; + } + + Update_card_for_customer_id_via_token_id: { + my $token = $stripe->post_token(card => $fake_card); + my $customer = $stripe->post_customer( + card => $token->id, + ); + isa_ok $customer, 'Net::Stripe::Customer', 'got back a customer'; + ok $customer->id, 'customer has an id'; + + my $cards = $stripe->get_cards(customer => $customer); + isa_ok $cards, "Net::Stripe::List"; + is scalar @{$cards->data}, 1, 'customer has one card'; + my $card = @{$cards->data}[0]; + isa_ok $card, "Net::Stripe::Card"; + is $card->id, $token->card->id, 'token card id matches'; + + my $new_token = $stripe->post_token(card => $fake_card); + $stripe->post_customer( + customer => $customer->id, + card => $new_token->id, ); - is $card->last4, '4242', 'card token ok'; + $cards = $stripe->get_cards(customer => $customer); + isa_ok $cards, "Net::Stripe::List"; + is scalar @{$cards->data}, 1, 'customer still has one card'; + my $new_card = @{$cards->data}[0]; + is $new_card->id, $new_token->card->id, 'new token card id matches'; + isnt $new_card->id, $card->id, 'new card has different card id'; + } + + Update_card_for_customer_object_via_token_id: { + my $token = $stripe->post_token(card => $fake_card); + my $customer = $stripe->post_customer( + card => $token->id, + ); + isa_ok $customer, 'Net::Stripe::Customer', 'got back a customer'; + ok $customer->id, 'customer has an id'; + + my $cards = $stripe->get_cards(customer => $customer); + isa_ok $cards, "Net::Stripe::List"; + is scalar @{$cards->data}, 1, 'customer has one card'; + my $card = @{$cards->data}[0]; + isa_ok $card, "Net::Stripe::Card"; + is $card->id, $token->card->id, 'token card id matches'; + + my $new_token = $stripe->post_token(card => $fake_card); + $customer->card($new_token->id); + $stripe->post_customer(customer => $customer); + $cards = $stripe->get_cards(customer => $customer); + isa_ok $cards, "Net::Stripe::List"; + is scalar @{$cards->data}, 1, 'customer still has one card'; + my $new_card = @{$cards->data}[0]; + is $new_card->id, $new_token->card->id, 'new token card id matches'; + isnt $new_card->id, $card->id, 'new card has different card id'; + } + + Update_card_for_customer_id_via_card_hashref: { + my $token = $stripe->post_token(card => $fake_card); + my $customer = $stripe->post_customer( + card => $token->id, + ); + isa_ok $customer, 'Net::Stripe::Customer', 'got back a customer'; + ok $customer->id, 'customer has an id'; + + my $cards = $stripe->get_cards(customer => $customer); + isa_ok $cards, "Net::Stripe::List"; + is scalar @{$cards->data}, 1, 'customer has one card'; + my $card = @{$cards->data}[0]; + isa_ok $card, "Net::Stripe::Card"; + is $card->id, $token->card->id, 'token card id matches'; + + $stripe->post_customer( + customer => $customer->id, + card => $fake_card, + ); + $cards = $stripe->get_cards(customer => $customer); + isa_ok $cards, "Net::Stripe::List"; + is scalar @{$cards->data}, 1, 'customer still has one card'; + my $new_card = @{$cards->data}[0]; + is $new_card->last4, substr( $fake_card->{number}, -4 ), 'new card last4 matches'; + isnt $new_card->id, $card->id, 'new card has different card id'; + } + + Update_card_for_customer_id_via_token_object: { + my $token = $stripe->post_token(card => $fake_card); + my $customer = $stripe->post_customer( + card => $token->id, + ); + isa_ok $customer, 'Net::Stripe::Customer', 'got back a customer'; + ok $customer->id, 'customer has an id'; + + my $cards = $stripe->get_cards(customer => $customer); + isa_ok $cards, "Net::Stripe::List"; + is scalar @{$cards->data}, 1, 'customer has one card'; + my $card = @{$cards->data}[0]; + isa_ok $card, "Net::Stripe::Card"; + is $card->id, $token->card->id, 'token card id matches'; + + my $new_token = $stripe->post_token(card => $fake_card); + $stripe->post_customer( + customer => $customer->id, + card => $new_token, + ); + $cards = $stripe->get_cards(customer => $customer); + isa_ok $cards, "Net::Stripe::List"; + is scalar @{$cards->data}, 1, 'customer still has one card'; + my $new_card = @{$cards->data}[0]; + is $new_card->id, $new_token->card->id, 'new token card id matches'; + isnt $new_card->id, $card->id, 'new card has different card id'; + } + + Update_card_for_customer_object_via_token_object: { + my $token = $stripe->post_token(card => $fake_card); + my $customer = $stripe->post_customer( + card => $token->id, + ); + isa_ok $customer, 'Net::Stripe::Customer', 'got back a customer'; + ok $customer->id, 'customer has an id'; + + my $cards = $stripe->get_cards(customer => $customer); + isa_ok $cards, "Net::Stripe::List"; + is scalar @{$cards->data}, 1, 'customer has one card'; + my $card = @{$cards->data}[0]; + isa_ok $card, "Net::Stripe::Card"; + is $card->id, $token->card->id, 'token card id matches'; + + my $new_token = $stripe->post_token(card => $fake_card); + $customer->card($new_token); + $stripe->post_customer(customer => $customer); + $cards = $stripe->get_cards(customer => $customer); + isa_ok $cards, "Net::Stripe::List"; + is scalar @{$cards->data}, 1, 'customer still has one card'; + my $new_card = @{$cards->data}[0]; + is $new_card->id, $new_token->card->id, 'new token card id matches'; + isnt $new_card->id, $card->id, 'new card has different card id'; } Add_card_for_customer_object_via_token_id: {