-
Notifications
You must be signed in to change notification settings - Fork 573
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 listing sources on customers #1064
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
using FluentAssertions; | ||
using Xunit; | ||
|
||
namespace Stripe.Tests.Xunit | ||
{ | ||
public class listing_sources_on_customer | ||
{ | ||
public StripeList<StripeSource> SourceListAll { get; } | ||
public StripeList<StripeSource> SourceListCard { get; } | ||
public StripeList<StripeSource> SourceListBitcoin { get; } | ||
|
||
public listing_sources_on_customer() | ||
{ | ||
var sourceService = new StripeSourceService(Cache.ApiKey); | ||
var customerService = new StripeCustomerService(Cache.ApiKey); | ||
|
||
// Create customer | ||
var CustomerCreateOptions = new StripeCustomerCreateOptions | ||
{ | ||
Email = "source-list@example.com", | ||
}; | ||
var Customer = customerService.Create(CustomerCreateOptions); | ||
|
||
// Create card source and attach it to customer | ||
var SourceCardCreateOptions = new StripeSourceCreateOptions | ||
{ | ||
Type = StripeSourceType.Card, | ||
Token = "tok_visa" | ||
}; | ||
var SourceCard = sourceService.Create(SourceCardCreateOptions); | ||
|
||
var SourceAttachOptions = new StripeSourceAttachOptions | ||
{ | ||
Source = SourceCard.Id | ||
}; | ||
SourceCard = sourceService.Attach(Customer.Id, SourceAttachOptions); | ||
|
||
// Create bitcoin source and attach it to customer | ||
var SourceBitcoinCreateOptions = new StripeSourceCreateOptions | ||
{ | ||
Type = StripeSourceType.Bitcoin, | ||
Amount = 1000, | ||
Currency = "usd", | ||
Owner = new StripeSourceOwner | ||
{ | ||
Email = "jenny.rosen+fill_now@example.com", | ||
}, | ||
}; | ||
var SourceBitcoin = sourceService.Create(SourceBitcoinCreateOptions); | ||
|
||
SourceAttachOptions.Source = SourceBitcoin.Id; | ||
SourceBitcoin = sourceService.Attach(Customer.Id, SourceAttachOptions); | ||
|
||
// List sources on customer | ||
SourceListAll = sourceService.List(Customer.Id); | ||
|
||
var SourceListOptions = new StripeSourceListOptions | ||
{ | ||
Type = StripeSourceType.Card | ||
}; | ||
SourceListCard = sourceService.List(Customer.Id, SourceListOptions); | ||
|
||
SourceListOptions.Type = StripeSourceType.Bitcoin; | ||
SourceListBitcoin = sourceService.List(Customer.Id, SourceListOptions); | ||
} | ||
|
||
[Fact] | ||
public void list_all_should_have_both_sources() | ||
{ | ||
SourceListAll.Data.Count.Should().Be(2); | ||
|
||
var SourceCard = SourceListAll.Data[0]; | ||
SourceCard.Type.Should().Be(StripeSourceType.Card); | ||
SourceCard.Card.Brand.Should().Be("Visa"); | ||
|
||
var SourceBitcoin = SourceListAll.Data[1]; | ||
SourceBitcoin.Type.Should().Be(StripeSourceType.Bitcoin); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't bitcoin be first? Aren't sources returned with most recent first? Or is it due to the default source always being first even if added before and this being undocumented? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the card source is attached first and becomes the default source. It's definitely not great practice to rely on that behavior in a test, and I should probably not have hardcoded the indices, but it's probably not worth putting in too much effort in those tests since we plan to 🔥 them and use stripe-mock Soon™️. |
||
SourceBitcoin.Bitcoin.Address.Should().NotBeNull(); | ||
} | ||
|
||
[Fact] | ||
public void list_card_should_have_card_source() | ||
{ | ||
SourceListCard.Data.Count.Should().Be(1); | ||
|
||
var SourceCard = SourceListCard.Data[0]; | ||
SourceCard.Type.Should().Be(StripeSourceType.Card); | ||
SourceCard.Card.Brand.Should().Be("Visa"); | ||
} | ||
|
||
[Fact] | ||
public void list_bitcoin_should_have_bitcoin_source() | ||
{ | ||
SourceListBitcoin.Data.Count.Should().Be(1); | ||
|
||
var SourceBitcoin = SourceListBitcoin.Data[0]; | ||
SourceBitcoin.Type.Should().Be(StripeSourceType.Bitcoin); | ||
SourceBitcoin.Bitcoin.Address.Should().NotBeNull(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
using Newtonsoft.Json; | ||
|
||
namespace Stripe | ||
{ | ||
public class StripeSourceListOptions : StripeListOptions | ||
{ | ||
[JsonProperty("object")] | ||
internal string Object => "source"; | ||
|
||
[JsonProperty("type")] | ||
public string Type { get; set; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,19 @@ public virtual StripeSource Detach(string customerId, string sourceId, StripeReq | |
); | ||
} | ||
|
||
public virtual StripeList<StripeSource> List(string customerId, StripeSourceListOptions listOptions = null, StripeRequestOptions requestOptions = null) | ||
{ | ||
var url = string.Format(Urls.CustomerSources, customerId); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it worth using a method to share the code? Not sure but that's what we do in other services (though the URL is used more than twice) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method is trivial enough that there's not much value in sharing it. That said, other API methods on |
||
|
||
if (listOptions == null) { | ||
listOptions = new StripeSourceListOptions(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need to do this? Is it to ensure that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, exactly. The |
||
|
||
return Mapper<StripeList<StripeSource>>.MapFromJson( | ||
Requestor.GetString(this.ApplyAllParameters(listOptions, url, true), | ||
SetupRequestOptions(requestOptions)) | ||
); | ||
} | ||
|
||
|
||
// Async | ||
|
@@ -73,5 +86,20 @@ await Requestor.DeleteAsync(url, | |
cancellationToken) | ||
); | ||
} | ||
|
||
public virtual async Task<StripeList<StripeSource>> ListAsync(string customerId, StripeSourceListOptions listOptions = null, StripeRequestOptions requestOptions = null, CancellationToken cancellationToken = default(CancellationToken)) | ||
{ | ||
var url = string.Format(Urls.CustomerSources, customerId); | ||
|
||
if (listOptions == null) { | ||
listOptions = new StripeSourceListOptions(); | ||
} | ||
|
||
return Mapper<StripeList<StripeSource>>.MapFromJson( | ||
await Requestor.GetStringAsync(this.ApplyAllParameters(listOptions, url, true), | ||
SetupRequestOptions(requestOptions), | ||
cancellationToken) | ||
); | ||
} | ||
} | ||
} |
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.
Minor but I like having separate options instances. Jayme used to compare the type on the object to the info on the options object itself but here since you override you can't do this.
It's definite more verbose my way but I found it cleaner when I started to work on the library. Feel free to ignore
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.
Oops, not sure why I missed this comment earlier.
Yeah, if you need the options object in your test it's better to instantiate it separately. In this case the options object are only instantiated within the test constructor though, and only the resulting list objects are persisted and used in test assertions.