-
Notifications
You must be signed in to change notification settings - Fork 426
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
Add support for v1/issuer_fraud_records endpoint #425
Conversation
self.assertIsInstance(resource, stripe.IssuerFraudRecord) | ||
|
||
def test_is_retrievable_by_charge(self): | ||
resource = stripe.IssuerFraudRecord.retrieve(TEST_CHARGE_ID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably doesn't take the exact same arguments as retrieving by issuer fraud record ID right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, that's not right. Thanks for catching!
|
||
|
||
class IssuerFraudRecord(ListableAPIResource): | ||
OBJECT_NAME = 'issuer_fraud_record' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you might need something like this here:
@classmethod
def class_name(cls):
return 'issuer_fraud_record'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Looks good! Can you try updating ptal @fay-stripe |
@brandur-stripe I don't think I can update |
Yeah, you'll have to leave this for now. |
@fay-stripe Can you rebase and take another look at this one? I just shipped a new version of |
ptal @brandur-stripe |
'get', | ||
'/v1/issuer_fraud_records/%s' % TEST_RESOURCE_ID | ||
) | ||
self.assertIsInstance(resource, stripe.IssuerFraudRecord) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the test case around CHARGE_ID since that should come as part of list's functionality, just like in the other libraries
|
||
|
||
TEST_RESOURCE_ID = 'issfr_123' | ||
TEST_CHARGE_ID = 'ch_123' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we get rid of TEST_CHARGE_ID
? I don't think it's used for anything anymore.
ptal @fay-stripe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup saw it right after I assigned you :)
ptal @brandur-stripe |
Excellent. Thanks Fay! |
Released as 1.81.0. |
Need to update stripe mock first