Feature Request: Ability to access the underlying model on creation.

[joynerd]

Hi,

This gem does a lot of the boiler plate needed for accepting payments but one thing that would make this a real winner for me is the ability to extend the models. For example, consider the following:

You have the models User and Item.

class User
  has_many :items
end

class Item
  belongs_to :user
  has_one :pay_subscription
end

To clarify the above, each Item would have its own subscription that the user of the items is responsible for.

The current docs about extending models works well enough to add the relation to Pay::Subscription docs

The problem arises in

@user.payment_processor.subscribe

in that you are unable to pass attributes in to the subscription object that will eventually be created.

It appears that the subscription is created in the processor, then sync is called to create the record based on a few parameters and data returned from the API.

My suggestion, and I would happily create a PR for this, would be to allow a param on .subscribe, perhaps record_attributes or attributes that accepts a set of params that would be passed to the underlying model on sync.

For my specific example that would be some changes on around

pay/lib/pay/stripe/billable.rb

Line 104 in 6a7917f

def subscribe(name: Pay.default_product_name, plan: Pay.default_plan_name, **options)

I haven't looked, but I would assume something like this would be very similar on the other Pay models as well.

What are your thoughts on this?

[excid3]

What do your proposed changes look like?

[joynerd]

Something along the lines of:

def subscribe(name: Pay.default_product_name, plan: Pay.default_plan_name, **options)
        quantity = options.delete(:quantity)
        # CHANGE HERE. Plucking the record attributes out of optitons.
        record_attributes = options.delete(:record_attributes) # PROPOSED CHANGE
        opts = {
          expand: ["pending_setup_intent", "latest_invoice.payment_intent", "latest_invoice.charge"],
          items: [plan: plan, quantity: quantity]
        }.merge(options)

        # Load the Stripe customer to verify it exists and update payment method if needed
        opts[:customer] = customer.id

        # Create subscription on Stripe
        stripe_sub = ::Stripe::Subscription.create(opts.merge(Pay::Stripe::Subscription.expand_options), stripe_options)

        # Save Pay::Subscription
       # CHANGE HERE passing record_attributes to sync
        subscription = Pay::Stripe::Subscription.sync(stripe_sub.id, object: stripe_sub, name: name, record_attributes: record_attributes)

        # No trial, payment method requires SCA
        if options[:payment_behavior].to_s != "default_incomplete" && subscription.incomplete?
          Pay::Payment.new(stripe_sub.latest_invoice.payment_intent).validate
        end

        subscription
      rescue ::Stripe::StripeError => e
        raise Pay::Stripe::Error, e
      end

def self.sync(subscription_id, object: nil, name: nil, stripe_account: nil, try: 0, retries: 1, record_attributes: {})
        # Skip loading the latest subscription details from the API if we already have it
        object ||= ::Stripe::Subscription.retrieve({id: subscription_id}.merge(expand_options), {stripe_account: stripe_account}.compact)

        pay_customer = Pay::Customer.find_by(processor: :stripe, processor_id: object.customer)
        return unless pay_customer

        attributes = {
          application_fee_percent: object.application_fee_percent,
          processor_plan: object.items.first.price.id,
          quantity: object.items.first.try(:quantity) || 0,
          status: object.status,
          stripe_account: pay_customer.stripe_account,
          metadata: object.metadata,
          subscription_items: [],
          metered: false,
          pause_behavior: object.pause_collection&.behavior,
          pause_resumes_at: (object.pause_collection&.resumes_at ? Time.at(object.pause_collection&.resumes_at) : nil)
        }

        attributes.merge!(record_attributes) #PROPOSED CHANGE, merging record attributes into defined attributes

        # Subscriptions that have ended should have their trial ended at the same time
        if object.trial_end
          attributes[:trial_ends_at] = Time.at(object.ended_at || object.trial_end)
        end
      ...........
      end

So basically allowing the param of record_attributes as a hash, then merging them with the existing attributes. This example is just for subscriptions as I haven't really reviewed Charges or anything else yet.

This is just what I could think of to allow the convenience method of .subscribe to also allow manipulation of the underlying model attributes.

[excid3]

The only record that Subscribe would create is a Pay::Subscription and the intention for those is to always be a mirror of what's in the payment processor API.

Are you suggesting that record_attributes would create an Item record in your example or something?

[joynerd]

I understand that and I believe that is a good design. To only have Pay::Subscription reflect information about the subscription's state with the processor.

No, in my specific use case, I'm not looking for any knock on effects. I mean that would be possible with this implementation but that is not the goal.

My specific use case is a customer can have multiple subscriptions where each subscription has a 1:1 relationship with an Item/Product. So really, what I want to add is references to Pay::Subscription.

In reality, I guess I could also just do the inverse and store the FK on the item.

I know I could use the name field on the subscription to store the ID of the item, it just doesn't quite feel right.

[excid3]

That's how I've done it previously, adding pay_subscription_id to my models.

One downside of that is if a subscription is canceled and you want to create a new one, you'd lose the association between them.

Stripe provides the metadata hash for adding things like item_id so you can associate records. The downside is that it's not easily to associate in Rails.

I could see adding a polymorphic relationship to the Subscription model to make that easier. That would mean you could have Item has_many :subscriptions, as: :record or something.

The downside is that subscriptions created in Stripe directly wouldn't be associated with a record, so you'd have to make sure they were created in Ruby.

Any better name than record that you can think of?

[joynerd]

I agree on all points.

I evaluated metadata as an option and then parsing it, the issue being that the record is created before metadata is set.

What do you mean better name than record? For the gem, I would try and keep it more generic so things like record, relation, related_record, etc.

On your point about Ruby and Stripe being out of sync, I think a good safety would be to pass the extra fields in metadata as well. This way on the return, you could validate the metadata against the actual attributes.

[joynerd]

What do you think of something along these lines for using the metadata route to set the subscription without having to touch the Pay model.

module SubscriptionExtensions
  extend ActiveSupport::Concern

  included do
    before_validation :associate_item
  end

  def associate_item
    item = Item.find(metadata['item_id'])
    item.update_column(:subscription_id, id) if !item.nil?
  end

end

class Item
  belongs_to :subscription, optional: true

  before_validation :associate_subscription

  def associate_subscription
    pay_subscription = Pay::Subscription.where('metadata @> ?', "[{\"item_id\": \"#{self.id}\"}]").first
    self.subscription = pay_subscription if !pay_subscription.nil?
  end
end

[excid3]

That definitely works, so long as you're using a database with json columns. Sadly @> doesn't work consistently across SQLite, MySQL and Postgres, so it's a pain to do in a library if we wanted to add it as a feature to Pay.

Here's how we had to do it in Noticed
https://github.com/excid3/noticed/blob/master/lib/noticed/has_notifications.rb#L17-L31

In the case of Noticed, that can be associated with many different records, but Pay only needs one, so I think we can go with a record:belongs_to{polymorphic} association added to Pay::Subscription and Pay::Charge

We can also support a pay_record metadata entry like we do with pay_name. If the value starts with gid:// we could look it up as a GlobalID otherwise, try to parse it as a Signed GlobalID and set the record association on sync automatically.

I am going to start a branch and see how that might look.

[excid3]

If a PaymentIntent fails for SCA, we wouldn't have a charge to sync immediately, so the metadata: { pay_record: "gid://" } would be required to pass that down to the Charge.