Update attributes for Account and LegalEntity #580
Conversation
ob-stripe
force-pushed
the
ob-update-account-attrs
branch
from
9247836 to
1bc2b3b
Compare
| @@ -17,6 +17,10 @@ | |||
| public class Account extends ApiResource implements HasId, MetadataStore<Account> { | |||
| @Getter(onMethod = @__({@Override})) String id; | |||
| String object; | |||
| String applicationIcon; | |||
There was a problem hiding this comment.
This is not in the docs for me. None of the application* are and it's not something we surface in the other bindings. We should remove
There was a problem hiding this comment.
The application_* attributes are in the OpenAPI spec. I can remove them as they don't seem all that useful today, but on the other hand it doesn't cost us anything to keep them and they'll be available if someone ever needs to use them.
There was a problem hiding this comment.
I'm in favour of removing. I have never heard of those properties before. They seem more for the dashboard than for the API to be honest. If we added them here we'd have to add them to all the other ones and keep maintaining them.
There was a problem hiding this comment.
Actually, these attributes can be useful for Connect platforms that use destination charges and their users. Retrieving the py_ charge object on the destination account returns these attributes in the source attribute:
"id": "py_123",
"object": "charge",
...
"source": {
"id": "acct_123",
"object": "account",
"application_name": "Planet Express",
"application_url": "http://localhost:3000"
},
...
Unlike most other attributes, they're only returned if they have non-null values, which is why application_icon and application_logo are missing in the example above.
Are you still in favor of removing them?
There was a problem hiding this comment.
huh why did we do something like this, that does not make sense. The charge already has application as a hash for that. This information should live there in that case. I still think we should at least discuss this internally and do this in a separate PR
| String document; | ||
| String documentBack; | ||
| String status; | ||
| public static class JapanAddress extends StripeObject { |
There was a problem hiding this comment.
why not extend Address and just defined town as extra?
There was a problem hiding this comment.
Internally and in the OpenAPI spec, they're defined as two different schemas.
Also, at the moment there is no concrete resource class that derives from another concrete class, and I'd rather keep that invariant in place. If we had JapanAddress extends Address, and ever needed to add a field to Address, chances are whoever makes the change will not notice that the field would also be added to JapanAddress.
There was a problem hiding this comment.
Gotcha, I'm fine with that in that case.
ob-stripe
force-pushed
the
ob-update-account-attrs
branch
from
1bc2b3b to
4db16e9
Compare
|
ptal @remi-stripe |
|
Released as 6.9.0. |
r? @brandur-stripe @remi-stripe
cc @stripe/api-libraries @scherr-stripe
Updates attributes for the
AccountandLegalEntityclasses. A lot of attributes were missing, particularly the Japan-specific ones.