search

PaySimple / PaySimpleSDK

.NET SDK for PaySimple API v4
Other
4 stars 1 forks source link

Update 'must start in the future checks' to only check against days in the past, not times #15

Open WillBDaniels opened 7 years ago

WillBDaniels commented 7 years ago

Currently, for all of our recurring payments, we validate the recurring payment input parameters as such: 

            RuleFor(m => m.StartDate).NotNull().WithMessage("StartDate is required").Must(m => m >= DateTime.Now).WithMessage("StartDate must be in the future");

This is suboptimal, as if someone wants a payment to start on the day it is made using DateTime.Now, this will fail, as the initial call to DateTime.Now will have been evaluated some number of milliseconds+ before this check, causing it to fail. Instead, anywhere we do this sort of check, we should be checking that the start date is not an entire day in the past. This also brings up UTC considerations that should be investigated... do we have the user set a global TimeZone across the SDK, which is then used in all date math, or do we force UTC across the board? I'm inclined to force UTC to avoid bugs, but I could be swayed.

scottlance commented 7 years ago

Using UTC is a good call as long as the PaySimple API uses UTC, if not then conversion would have to be done before the final request is submitted to the API. Then again, if you are only doing things on a date basis I'm not sure UTC would really help the matter.

WillBDaniels commented 7 years ago

The Paysimple API definitely uses UTC under the hood. I can only think of situations where we would compare days with the currently exposed functionality, so the only issue with it currently (if we switched everything to checking days like it should) would be in the following scenario:

I submit a new recurring payment for DateTime.Now, which happens to be, say, 11:00 P.M. MST Under the hood, the system does a comparison to DateTime.UTCNow, which would then say 'oh, it's actually tomorrow', which would then cause the check for '>= today' to fail, and the validation would fail out. This would create an effective 'dead zone' of DateTime.Now for 5-8 hours before midnight where recurring payments that are set to start on the current day would fail.

In my mind, there are 3 solutions:

  1. change everything in the sdk to use UTC, modify the documentation to match, and expect that anyone programming against it can do the necessary time conversions in their programs. (not a bad choice)
  2. modify everything in the sdk to use UTC, then add the ability for the user to configure a global timezone which would override all of our validation checks, use the global timezone instead, and then convert into UTC at the last second before sending the request. << This allows for a lot of 'user control' but it could create some unexpected behavior for the user
  3. Leave it alone and have everything just be the local timezone of the user. << worst solution, as it causes confusion at best.

my vote for ease of implementation would just be #1, but I could be convinced other ways also