Skip to content

Additional functionality#43

Merged
confact merged 9 commits intoconfact:masterfrom
makisu:makisu-changes-20210919
Dec 16, 2021
Merged

Additional functionality#43
confact merged 9 commits intoconfact:masterfrom
makisu:makisu-changes-20210919

Conversation

@xaviablaza
Copy link
Contributor

Description

@rjmssevilla and I patched this shard quite a bit and wanted to match what we're using in the production to the upstream. The changes are as follows:

  • Added Stripe::Coupon.retrieve to retrieve a Stripe::Coupon given its id
  • Placed SignatureVerificationError around the namespace Stripe as we reference Stripe::SignatureVerificationError outside of the shard
  • Added a configuration attribute to Stripe::BillingPortal::Session.create_session to create billing portal sessions with different configurations
  • Added subscription_data, allow_promotion_codes, metadata, and discounts attributes when creating a Stripe::Checkout::Session
  • Added metadata to Stripe::Subscription to get the metadata created from the Stripe::Checkout::Session

@rmarronnier
Copy link
Contributor

Thanks for these additions !

I'm not a reviewer, but just a suggestion :

  • The Stripe::Coupon.retrieve method can be added to the Stripe::Coupon class using the add_retrieve_method. Example from the code base :
@[EventPayload]
class Stripe::TaxRate
  include JSON::Serializable
  include StripeMethods

  add_retrieve_method

  getter id : String
  getter inclusive : Bool
  getter object : String?
  getter customer : String?
  getter active : Bool

  @[JSON::Field(converter: Time::EpochConverter)]
  getter created : Time
  getter description : String?
  getter jurisdiction : String?
  getter metadata : Hash(String, String)?
  getter percentage : Float32?
end

@confact
Copy link
Owner

confact commented Oct 6, 2021

Like @rmarronnier (thanks!) - You can look through our help methods to use those to reduce repetitions and maybe remove some files. When that is done, this looks very good!

Thanks, @xaviablaza, and @rjmssevilla for the help! I appreciate it.

@xaviablaza xaviablaza force-pushed the makisu-changes-20210919 branch from 544db52 to b162b79 Compare December 12, 2021 04:39
@xaviablaza
Copy link
Contributor Author

@confact Ready for review, could you take a look?

@confact
Copy link
Owner

confact commented Dec 12, 2021

@xaviablaza, the test seems to fail on the SignatureVerificationError - can you check that? Otherwise, I can try to fix it as you worked so hard to fix this :)

@xaviablaza
Copy link
Contributor Author

@confact All right, I've namespaced the SignatureVerificationError under Stripe in the webhook specs and also ran the specs on my local machine. Should be good to go now, fingers crossed. 🤞 😄

@confact
Copy link
Owner

confact commented Dec 16, 2021

@xaviablaza, the tests passed! Great job! I will merge now.

@confact confact merged commit c984db4 into confact:master Dec 16, 2021
@xaviablaza xaviablaza deleted the makisu-changes-20210919 branch December 16, 2021 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants