laravel / cashier-stripe

Laravel Cashier provides an expressive, fluent interface to Stripe's subscription billing services.
https://laravel.com/docs/billing
MIT License
2.39k stars 680 forks source link

Add support for new Event Meters #1698

Closed Bark-fa closed 3 months ago

Bark-fa commented 3 months ago

Hello! This PR adds support for the new event meters by stripe for tracking usage in usage-based subscriptions, this is my first time contributing to cashier, and I've made every effort to maintain the style and naming conventions, and everything is fully typed, please let me know if I missed something either in the code itself or the tests.

Thank you so much for your time and effort.

Bark-fa commented 3 months ago

I see the CI has failed but for some reason, I cannot see why, it shows a generic "blocked" message, is it some geo-restriction?

Bark-fa commented 3 months ago

I managed to access it with a VPN, all style issues have been fixed

driesvints commented 3 months ago

Can you remove all of your idea settings from this PR? Thanks!

Bark-fa commented 3 months ago

Ah yes, sorry about that I thought they'd be in the .gitignore!

Bark-fa commented 3 months ago

done @driesvints

Bark-fa commented 3 months ago

@driesvints Do you need me to delete them from the previous commit history too?

driesvints commented 3 months ago

@Bark-fa no that's okay. This PR will get squashed anyway.

Bark-fa commented 3 months ago

Hello @driesvints, I'm reaching out because I noticed the PR is still in draft, I know you must be very busy, so if the PR needs any more work or if I missed anything in the docs please let me know and I'll gladly take it off your hands, I've got plenty of time this week.

Thanks!

driesvints commented 3 months ago

@Bark-fa sorry, needed a bit to have a thorough look here. I completely refactored all of this to be on a new trait on the billable. It's not needed for usage based billing to live closely to a subscription or subscription item. I've revised the API's and simplified things.

The downside of this is that the written tests aren't applicable anymore. Could you rewrite those into a new UsageBasedBillingTest.php file? Then I believe this PR will be ready for review.

Bark-fa commented 3 months ago

Done, I refactored the new tests into UsageBasedBillingTest.php and removed them from the old file.

Let me know if anything else comes up, thank you so much!

Bark-fa commented 3 months ago

And regarding the new trait, I agree it's a better approach, I didn't allow myself the freedom to create new files though so I just wrote the functions where the old (now deprecated) code was 😅

driesvints commented 3 months ago

Done. Thanks a bunch for this one @Bark-fa. Would also highly appreciate a PR to the docs!

Bark-fa commented 3 months ago

Hello @driesvints, you're very much welcome, I just submitted a PR for the docs

driesvints commented 3 months ago

@Bark-fa did you try these tests? Because they're failing atm.

Bark-fa commented 3 months ago

Hello @driesvints, what tests specifically? I tried all the tests I wrote, and the old ones as well, and all ran with success, which ones are failing?

driesvints commented 3 months ago

These: https://github.com/laravel/cashier-stripe/actions/runs/10444767851/job/28919710065

Bark-fa commented 3 months ago

I just checked, I think the problem was caused by this line: $startTime = $options['start_time'] ?? $this->created_at->timestamp;

This used to be in the Subscription class, so the logic was that if the user did not provide a start time, we'd set it to the date the subscription was created, but when it was moved to a trait, $this references the User model, this should fix it: $startTime = $options['start_time'] ?? 1;

I pushed it to my fork

driesvints commented 3 months ago

Can you send in a PR? Thanks!

Bark-fa commented 3 months ago

Of course, on it