beam-community / stripity-stripe

An Elixir Library for Stripe
Other
963 stars 344 forks source link

Possibly missing `id` (for SubscriptionItem) on `items` type signature for `Subscription`. #823

Closed crova closed 5 months ago

crova commented 7 months ago

Problem Statement

Hello everyone, first of all, thanks for the great work on this library.

Am I missing something or the type signature for items is missing id?

To update a subscription, Stripe demands the subscription item id (ref):

curl https://api.stripe.com/v1/subscriptions/sub_xxxxxxxxx \
  -u "sk_test_Ho24N7La5CVDtbmpjc377lJI
:" \
  -d "items[0][id]"={{SUB_ITEM_ID}} \
  -d "items[0][price]"={{NEW_PRICE_ID}}

However, if I pass something like below (which works as far as updating a subscription within Stripe):

Stripe.Subscription.update(subscription_stripe_id, %{items: [%{id: subscription_item_stripe_id, price: selected_plan.stripe_id}]})

Dialyzer will complain:

The function call will not succeed.

Stripe.Subscription.update(_subscription_stripe_id :: any(), %{:items => [%{:id => _, :price => _}, ...]})

will never return since the 2nd arguments differ  
from the success typing arguments:

(binary(), %{         
  :add_invoice_items => [
    %{                                                                                                          
      :price => binary(),
      :price_data => map(),                                                                                     
      :quantity => integer(),
      :tax_rates => binary() | [any()]
    }                                 
  ],                                  
  :application_fee_percent => number(),
  :automatic_tax => %{:enabled => boolean()},
  :billing_cycle_anchor => :now | :unchanged,
  :billing_thresholds =>                  
    binary() | %{:amount_gte => integer(), :reset_billing_cycle_anchor => boolean()},
  :cancel_at => binary() | integer(),            
  :cancel_at_period_end => boolean(),                                                                           
  :cancellation_details => %{
    :comment => binary(),          
    :feedback =>                                                                                                
      :customer_service       
      | :low_quality                                                                                            
      | :missing_features      
      | :other                                                                                                  
      | :switched_service        
      | :too_complex            
      | :too_expensive 
      | :unused                                                                                                 
  },
  :collection_method => :charge_automatically | :send_invoice,
  :coupon => binary(),
  :days_until_due => integer(),                                                                                 
  :default_payment_method => binary(),
  :default_source => binary(),
  :default_tax_rates => binary() | [binary()],                                                                                                                                                                                   
  :description => binary(),
  :expand => [binary()],
  :items => [
    %{
      :billing_thresholds => binary() | map(),
      :metadata => map(),
      :plan => binary(),
      :price => binary(),
      :price_data => map(),
      :quantity => integer(),
      :tax_rates => binary() | [any()]
    }
  ],
  :metadata => binary() | %{binary() => binary()},
  :off_session => boolean(),
  :on_behalf_of => binary(),
  :pause_collection =>
    binary()
    | %{:behavior => :keep_as_draft | :mark_uncollectible | :void, :resumes_at => integer()},
  :payment_behavior =>
    :allow_incomplete | :default_incomplete | :error_if_incomplete | :pending_if_incomplete,
  :payment_settings => %{
    :payment_method_options => %{
      :acss_debit => binary() | map(),
      :bancontact => binary() | map(),
      :card => binary() | map(),
      :customer_balance => binary() | map(),
      :konbini => binary() | map(),
      :us_bank_account => binary() | map()
    },
    :payment_method_types => binary() | [atom()],
    :save_default_payment_method => :off | :on_subscription
  },
  :pending_invoice_item_interval =>
    binary() | %{:interval => :day | :month | :week | :year, :interval_count => integer()},
  :promotion_code => binary(),
  :proration_behavior => :always_invoice | :create_prorations | :none,
  :proration_date => integer(),
  :transfer_data => binary() | %{:amount_percent => number(), :destination => binary()},
  :trial_end => :now | integer(),
  :trial_from_plan => boolean(),
  :trial_settings => %{
    :end_behavior => %{:missing_payment_method => :cancel | :create_invoice | :pause}
  }
})

Removing id from items will make diayzer happy, but won't work to update the Subscription with Stripe:

All prices on a subscription must have the same `recurring.interval` and `recurring.interval_count`. If you meant to update an existing subscription item instead of creating a new one, make sure to include the subscription item ID in your request. See examples at https://stripe.com/docs/billing/subscriptions/upgrade-downgrade#changing.

So, am I missing something here or we actually should have id within items type which is not there?

Cheers.

yordis commented 7 months ago

Is this related to https://github.com/beam-community/stripity-stripe/pull/786?

yordis commented 7 months ago

Also, please follow the Issue Template, it help us to help you!

crova commented 7 months ago

Thanks for getting back to me @yordis . I don't think this is related to #786. This is about a missing field from one of the type specs, items should have id as we need to pass the subscription_item.id when updating a subscription (say, upgrading from monthly to annual).

Also, sorry, but I don't quite understand the Issue Template comment. I used the Feature Request template as it seemed more appropriate then a Bug Report, but if it's the wrong one, please let me know and I can change this.

github-actions[bot] commented 6 months ago

This issue has been automatically marked as "stale:discard". If this issue still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment.

kradydal commented 6 months ago

I had the same issue with the dialyzer as @crova. It looks like subscription_item is missing id as an optional parameter on the subscription update.

Version: 3.1.1

Steps to reproduce:

  1. Create a recurring product with two prices (monthly and yearly)

  2. Create a subscription with a monthly price and activate

  3. Update the subscription price to yearly with:

    Stripe.Subscription.update(stripe_subscription_id, %{
      items: [%{id: item.id, price: price_id}]
    })

    where item_id is the item_id of subscription created in the step 1 and price_id is the yearly price id.

  4. Run dialyzer, it returns error, because there is no id in the items

    
    The function call will not succeed.

Stripe.Subscription.update(any(), %{:items => [%{:id => , :price => }, ...]})

will never return since the 2nd arguments differ from the success typing arguments:

(binary(), %{ :add_invoice_items => [ %{ :price => binary(), :price_data => map(), :quantity => integer(), :tax_rates => binary() | [any()] } ], :application_fee_percent => number(), :automatic_tax => %{:enabled => boolean()}, :billing_cycle_anchor => :now | :unchanged, :billing_thresholds => binary() | %{:amount_gte => integer(), :reset_billing_cycle_anchor => boolean()}, :cancel_at => binary() | integer(), :cancel_at_period_end => boolean(), :cancellation_details => %{ :comment => binary(), :feedback => :customer_service | :low_quality | :missing_features | :other | :switched_service | :too_complex | :too_expensive | :unused }, :collection_method => :charge_automatically | :send_invoice, :coupon => binary(), :days_until_due => integer(), :default_payment_method => binary(), :default_source => binary(), :default_tax_rates => binary() | [binary()], :description => binary(), :expand => [binary()], :items => [ %{ :billing_thresholds => binary() | map(), :metadata => map(), :plan => binary(), :price => binary(), :price_data => map(), :quantity => integer(), :tax_rates => binary() | [any()] } ], :metadata => binary() | %{binary() => binary()}, :off_session => boolean(), :on_behalf_of => binary(), :pause_collection => binary() | %{:behavior => :keep_as_draft | :mark_uncollectible | :void, :resumes_at => integer()}, :payment_behavior => :allow_incomplete | :default_incomplete | :error_if_incomplete | :pending_if_incomplete, :payment_settings => %{ :payment_method_options => %{ :acss_debit => binary() | map(), :bancontact => binary() | map(), :card => binary() | map(), :customer_balance => binary() | map(), :konbini => binary() | map(), :us_bank_account => binary() | map() }, :payment_method_types => binary() | [atom()], :save_default_payment_method => :off | :on_subscription }, :pending_invoice_item_interval => binary() | %{:interval => :day | :month | :week | :year, :interval_count => integer()}, :promotion_code => binary(), :proration_behavior => :always_invoice | :create_prorations | :none, :proration_date => integer(), :transfer_data => binary() | %{:amount_percent => number(), :destination => binary()}, :trial_end => :now | integer(), :trial_from_plan => boolean(), :trial_settings => %{ :end_behavior => %{:missing_payment_method => :cancel | :create_invoice | :pause} } })

github-actions[bot] commented 5 months ago

This issue has been automatically marked as "stale:discard". If this issue still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment.

github-actions[bot] commented 5 months ago

Closing this issue after a prolonged period of inactivity. If this issue is still relevant, feel free to re-open the issue. Thank you!

Maryna-Serhiichuk commented 2 months ago

I had the same. Recurring must be the same, in one subscription with several items image