Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking change with removing deleted value for invoices #1474

Closed
driesvints opened this issue Apr 4, 2023 · 2 comments · Fixed by #1473
Closed

Breaking change with removing deleted value for invoices #1474

driesvints opened this issue Apr 4, 2023 · 2 comments · Fixed by #1473
Labels

Comments

@driesvints
Copy link

driesvints commented Apr 4, 2023

Describe the bug

The deleted value was removed for invoices in https://github.com/stripe/stripe-php/releases/tag/v10.12.0

While it indeed may be that the value wasn't used anymore, the constant related to it was real and was used in code. And thus this change broke Laravel Cashier Stripe: laravel/cashier-stripe#1521

Imho, the ideal path for this would be to deprecate the constant and remove it in v11 of the SDK.

To Reproduce

Use Invoice::STATUS_DELETED with a version before v10.12.0, upgrade to v10.12.0 and see the code break.

Expected behavior

Invoice::STATUS_DELETED should remain in minor and patch versions of this library as removing any public API is a breaking change.

Code snippets

No response

OS

macOS

PHP version

PHP 8.2

Library version

v10.12.0

API version

2022-08-01

Additional context

No response

@richardm-stripe
Copy link
Contributor

Hello @driesvints, thanks for the report. I've restored the deleted enum value in #1473 and released the fix in https://github.com/stripe/stripe-php/releases/tag/v10.12.1. We're sorry for breaking you/your users.

For a little background, I do think "you can't break what's already broken" is legitimate for removing very broken interfaces under a minor in some circumstances, but in retrospect it was the wrong call here. Here, a user who relied on STATUS_DELETED would likely have had some dead code, not a severe problem with their integration.

@driesvints
Copy link
Author

Thanks a lot @richardm-stripe! : )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants