[WIP] Codegen more enum values #961
Conversation
There was a problem hiding this comment.
Didn't read through the whole PR for now as I think there are real problems with the namings and which constants we should put in stripe-php
I believe we need to have a high level conversation around those constants and their risks, especially around API versions and breaking changes.
| @@ -42,24 +42,119 @@ class Account extends ApiResource | |||
| use ApiOperations\NestedResource; | |||
| use ApiOperations\Update; | |||
|
|
|||
| const AU_BECS_DEBIT_PAYMENTS_ACTIVE = 'active'; | |||
There was a problem hiding this comment.
I don't think we should do those this way since they are just identical. If I had done this right originally, it should be a list of STATUS_* constants inside the Capability class instead right?
| const CARD_PAYMENTS_INACTIVE = 'inactive'; | ||
| const CARD_PAYMENTS_PENDING = 'pending'; | ||
|
|
||
| const CODE_INVALID_ADDRESS_CITY_STATE_POSTAL_CODE = 'invalid_address_city_state_postal_code'; |
There was a problem hiding this comment.
I really don't think we should add something like this to the codebase, but I'm obviously biased against all those constants in the first place.
IMO those only make sense the day we start supporting real enums and API versions. Because right now, any value disappearing would be a breaking change or require an override
| const CODE_VERIFICATION_FAILED_NAME_MATCH = 'verification_failed_name_match'; | ||
| const CODE_VERIFICATION_FAILED_OTHER = 'verification_failed_other'; | ||
|
|
||
| const INTERVAL_DAILY = 'daily'; |
There was a problem hiding this comment.
I don't think this name is right. It should be something like SETTINGS_PAYOUTS_SCHEDULE_INTERVAL_DAILY instead? Back to "let's not add any of those" in my mind
| const LEGACY_PAYMENTS_INACTIVE = 'inactive'; | ||
| const LEGACY_PAYMENTS_PENDING = 'pending'; | ||
|
|
||
| const STRUCTURE_GOVERNMENT_INSTRUMENTALITY = 'government_instrumentality'; |
There was a problem hiding this comment.
same as above, it should be prefixed by the full path to avoid naming conflicts no?
| const STRUCTURE_UNINCORPORATED_ASSOCIATION = 'unincorporated_association'; | ||
| const STRUCTURE_UNINCORPORATED_NON_PROFIT = 'unincorporated_non_profit'; | ||
|
|
||
| const TAX_REPORTING_US_1099_K_ACTIVE = 'active'; |
There was a problem hiding this comment.
Minor but why the different color?
| @@ -46,6 +46,9 @@ class Card extends ApiResource | |||
| use ApiOperations\Delete; | |||
| use ApiOperations\Update; | |||
|
|
|||
| const ACCOUNT_HOLDER_TYPE_COMPANY = 'company'; | |||
| const ACCOUNT_HOLDER_TYPE_INDIVIDUAL = 'individual'; | |||
There was a problem hiding this comment.
This is incorrect, there's no such thing on Card
|
^ was interesting as a proof of concept, will re-visit at a later time when the codegen infrastructure is more mature |
Up until now, we generate constants for enums that appear at the top-level of stripe responses and put them at the top of the corresponding resource class. We do not generate constants corresponding to
account.capabilities.card_issuingis anenum("active", "inactive", "pending"), which is not generated according to the current scheme.Subscription#create.payment_behavior(as reported in Add pending update constants #941).This PR adds those enums.
Q. What about cases when the values of the request enum differs from the response enum, or other request enums on the resource?
A. The enum values represent the superset of all enums sharing that name.
TODO: This doesn't yet include enums "nullable" or "emptyStringable" enums, or arrays of enums (if those exist)