Skip to content
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

Inconsistency in conversion between the DueDate.StringDate and DueDate.Date properties #36

Closed
AhmedZaki99 opened this issue Sep 30, 2023 · 1 comment · Fixed by #37 or #41
Closed

Comments

@AhmedZaki99
Copy link
Contributor

The following integration tests will fail when they are run on a machine with a GMT+2 time zone, they pretty much explain the problem:

  1. When we get a recently added task, or any task in general, the DueDate.Date property works as expected but the DueDate.StringDate property shifts time by the local time zone offset.
[Fact]
public async Task CreatedTask_ShouldReturnMatchingDueDate_WhenWeGetItById()
{
    // Arrange
    var client = new TodoistClient(TodoistAccessToken);
    var item = new Item("Task with time")
    {
        DueDate = new DueDate("22 Dec 2021 at 9:15", language: Language.English)
    };

    // Act
    await client.Items.AddAsync(item);
    var itemInfo = await client.Items.GetAsync(item.Id);

    try
    {
        // Assert
        Assert.Equal(new DateTime(2021, 12, 22, 9, 15, 0), itemInfo.Item.DueDate.Date); // Succeeds
        Assert.Equal("2021-12-22T09:15:00", itemInfo.Item.DueDate.StringDate); // Fails: StringDate returns 2021-12-22T07:15:00
    }
    finally
    {
        // Cleanup
        await client.Items.DeleteAsync(item.Id);
    }
}
  1. When we get a task and then update it with no changes at all, the due date will change the same way that DueDate.StringDate did in the last test, and again, DueDate.StringDate will shift time by additional two hours.
[Fact]
public async Task UpdatedTask_ShouldReturnMatchingDueDate_WhenNoChangesAreMade()
{
    // Arrange
    var client = new TodoistClient(TodoistAccessToken);
    var item = new Item("Task with time")
    {
        DueDate = new DueDate("22 Dec 2021 at 9:15", language: Language.English)
    };

    // Act
    await client.Items.AddAsync(item);
    var itemInfo = await client.Items.GetAsync(item.Id);

    await client.Items.UpdateAsync(itemInfo.Item);
    itemInfo = await client.Items.GetAsync(item.Id);

    try
    {
        // Assert
        Assert.Equal(new DateTime(2021, 12, 22, 9, 15, 0), itemInfo.Item.DueDate.Date); // Fails: Date returns 22/12/2021 07:15:00
        Assert.Equal("2021-12-22T09:15:00", itemInfo.Item.DueDate.StringDate); // Fails: StringDate returns 2021-12-22T05:15:00
    }
    finally
    {
        // Cleanup
        await client.Items.DeleteAsync(item.Id);
    }
}

This problem occurs in general when the task contains a floating due date, the date is treated like a local time when being converted from DateTime to string in the DueDate.StringDate property getter, hence, its string value changes from what it was given by the API.

In the previous tests, the value "2021-12-22T09:15:00" fetched from the API was converted to a DateTime of kind DateTimeKind.Unspecified, and then converted back to "2021-12-22T07:15:00" when sent back to the API for update.
That results in an unwanted update on the task without even changing any field.

@AhmedZaki99
Copy link
Contributor Author

The problem turns out to be in that line within the DueDate.StringDate setter:

var date = Date.Value.ToUniversalTime();

Which is not problematic on its own, but combined with the getter's DateTime.Parse, the date given by the API is converted to date with kind DateTimeKind.Unspecified in case it's a floating due date, which is then converted to Utc date in the setter.

I will now open a pull request for the fix I've just made on my fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants