braintree / braintree_dotnet

Braintree .NET library
https://developer.paypal.com/braintree/docs/start/overview
MIT License
136 stars 73 forks source link

Timezone confusion in documentation #94

Closed leo-labs closed 4 years ago

leo-labs commented 4 years ago

General information

The documentation regarding how to create a subscription states at https://developers.braintreepayments.com/reference/request/subscription/create/dotnet#first_billing_date for the field FirstBillingDate that

The day the subscription starts billing. The field can accept date objects and must be a future date. This will override default plan details. Dates will be interpreted as UTC dates.

This does not seem to be correct. My sandbox is in Central Time; When passing UTC DateTime objects in this field, the API seems to interpret the value of the DateTime object as Central Time.

Further, for all DateTime fields of subscription, e.g. BillingPeriodStartDate, DateTime.Kind is UTC when it is actually CST.

hollabaq86 commented 4 years ago

👋 @leo-labs, the behavior you're seeing definitely sounds weird since we're setting UTC in our requests.

To help us investigate further, please contact Support with some examples that you're seeing so we can look into our logs for more insight.

benmccallum commented 2 years ago

I'm seeing the same behaviour as @leo-labs; here's some more details @hollabaq86 .

If I send the following request at current time of:

being super explicit that I'm passing a UTC DateTime at the start of the 24th, which 6 hours earlier in CT is on the 23rd...

the braintree sandbox dashboard says for the sub:

Billing Period 02/24/2022 - 03/23/2022

Unless the dashboard is using UTC times, I can't see how the docs can be correct in two ways:

  1. It appears to interpret the date in same TZ as the Braintree account
  2. It can actually start a subscription on the same date (doc says must be future date)

Interestingly if I passed new DateTime(2022, 2, 23, 59, 59, 0, DateTimeKind.Utc) (1 sec before the other time I passed), I got rejected with "First Billing Date cannot be in the past.". So IMO it looks like you guys are just taking the YYYY, MM, DD as given (and also sent back), as if it was like a NodaTime LocalDate, i.e. void of any TZ and local to the Braintree account.

var client = _sp.GetRequiredService<IBraintreeGateway>();

var request = new SubscriptionRequest()
{
    PlanId = "some-plan-id",
    FirstBillingDate = new DateTime(2022, 2, 24, 0, 0, 0, DateTimeKind.Utc),

    // https://sandbox.braintreegateway.com/merchants/b3c422qjkhvjqt7t/payment_methods/any/4kmjgq
    PaymentMethodToken = "some-payment-method-token"
};

var result = await client.Subscription.CreateAsync(request);

Then, just like Leonard mentioned, if I find the subscription... var findResult = await client.Subscription.FindAsync("57tw8g");

I get the following in the immediate window. Which matches what's shown in the dashboard in terms of the date, but if these dates a subscription runs are apparently in the tz of the braintree account (as per another part of the docs here), then Kind shouldn't be UTC I'd imagine.

findResult.FirstBillingDate
{24/02/2022 12:00:00 AM}
    Date: {24/02/2022 12:00:00 AM}
    Day: 24
    DayOfWeek: Thursday
    DayOfYear: 55
    Hour: 0
    Kind: Utc
    Millisecond: 0
    Minute: 0
    Month: 2
    Second: 0
    Ticks: 637812576000000000
    TimeOfDay: {00:00:00}
    Year: 2022
findResult.BillingPeriodStartDate
{24/02/2022 12:00:00 AM}
    Date: {24/02/2022 12:00:00 AM}
    Day: 24
    DayOfWeek: Thursday
    DayOfYear: 55
    Hour: 0
    Kind: Utc
    Millisecond: 0
    Minute: 0
    Month: 2
    Second: 0
    Ticks: 637812576000000000
    TimeOfDay: {00:00:00}
    Year: 2022
findResult.BillingPeriodEndDate
{23/03/2022 12:00:00 AM}
    Date: {23/03/2022 12:00:00 AM}
    Day: 23
    DayOfWeek: Wednesday
    DayOfYear: 82
    Hour: 0
    Kind: Utc
    Millisecond: 0
    Minute: 0
    Month: 3
    Second: 0
    Ticks: 637835904000000000
    TimeOfDay: {00:00:00}
    Year: 2022

I guess this behaviour is fine, but it'd be so much better if this was NodaTime's LocalDate type, or perhaps .NET 6's new DateOnly so it was clear this is a local date.

It feels like the docs could do with some corrections though.