anthonyreilly / NetCoreForce

Salesforce REST API toolkit for .NET Standard and .NET Core
MIT License
110 stars 63 forks source link

Salesforce Date to DateTime convertion leads to addition of timezone. #65

Open sergey-bulavskiy opened 1 year ago

sergey-bulavskiy commented 1 year ago

Hello Anthony,

First of all i want to thank you for your incredible library, which drastically improved my experience with Salesforce API, it is trully remarkable that you did it all on your own.

Previously i've already created an issue with DateTime UTC convertion, and figured my way around this problem, but currently i've faced it again and i'd like to propose a solution to solve it once and for all.

As i've read in the source code (https://github.com/anthonyreilly/NetCoreForce/blob/54f5296e758109e132546c8a56d5176aa5123ae2/src/NetCoreForce.ModelGenerator/SfTypeConverter.cs#L30) you're converting Salesforce Date to DateTime type, which, unfortunately leads to addition of the timezone, and therefore if my timezone is already in the next day and server's one is in the previous one, scenarios like this happens:

Info: Server time - yesterday, Local timezone - today

  1. Get any entity with Date field (e.g. Date Of Birth) set to today, let's say 25.08.2023, it is automatically converted to local DateTime - 25.08.2023 00:00:00
  2. Send it back without changing it at all, and while serialization happening - DateTime will automatically be converted to UTC, therefore date will go back (or forward) depending on a local timezone, which leads to dangerous sideeffects.

My proposal would be to convert Salesforce Date fields to DateOnly type, and will prevent any further issues like this one.

Potential breaking change: it would require ModelGenerator to be migrated to at least .NET 6. But i suppose it could be solved with preprocessor directives, for old .NET Core it could be left as is, for newer ones - as DateOnly. Something like this

        case FieldType.date_sf:
            #if NET6_0_OR_GREATER
            type = typeof(DateOnly);
            #else
            type = typeof(DateTime);
            #endif
            break;

I can create a pull request, to avoid taking your time, maybe there is some other drawbacks that i'm missing.

Thank you once again for your time and work!

anthonyreilly commented 1 year ago

the Date type wasn't an option originally, but it looks like a good idea now. I'd like to maintain backwards compatibility a bit more, but I think that's manageable with the preprocessor directive and maybe a config flag or two. I'll dig into it, since that date issue is something that has annoyed me in my own production apps.

senj commented 2 months ago

Is there an update to this, as we also have problems with this issue.