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

timezone of completed tasks not taken into account #117

Closed
chrisgurney opened this issue Mar 10, 2024 · 7 comments
Closed

timezone of completed tasks not taken into account #117

chrisgurney opened this issue Mar 10, 2024 · 7 comments

Comments

@chrisgurney
Copy link
Contributor

To Reproduce
I was looking for all tasks completed on 2024-02-25:

kwargs['status'] = None
kwargs['stop_date'] = f'{DATE}'
tasks = things.tasks(**kwargs)

...but it returns some tasks on the day before, particularly those that were completed close to midnight:

Here's the stopDate for one of the returned tasks: 1708826542.13277

GMT: Sunday, February 25, 2024 2:02:22.132 AM
Your time zone: Saturday, February 24, 2024 9:02:22.132 PM GMT-05:00

Expected behavior
When I supply a stop_date I expect it to be reflective of my local timezone: i.e., get all tasks completed on that day for me.

@chrisgurney
Copy link
Contributor Author

chrisgurney commented Mar 10, 2024

@mikez per our discussion, adding your proposed solution here:

Currently:

  • in responses, stop_date, modified, and created are in "localtime";
  • in queries, stop_date expects GMT (aka UTC) dates.

The database itself stores these dates as UTC.

Suggested Changes

Make both responses and queries be in localtime. What do you think?

To do so, tweak these functions in database.py of things.py:

Want to give it a shot? Hopefully, these should be easy edits and not involve much more than adding "localtime" to the SQL-query, i.e., date(stopDate, 'unixepoch', 'localtime') instead of date(stopDate, 'unixepoch').

@mikez
Copy link
Contributor

mikez commented Mar 10, 2024

LGTM; if easy, maybe want to add some tests? https://github.com/thingsapi/things.py/blob/main/tests/test_things.py

/cc @AlexanderWillner in case you want to guide on tests or have thoughts on this.

@chrisgurney
Copy link
Contributor Author

chrisgurney commented Mar 10, 2024

@mikez @AlexanderWillner A few questions on this:

  1. How do I go about adding data to test against? I feel like directly updating the database is not the way to go. Is there a way to start up the Things app, pointed at this database? Or do I need to replace my own with this?

  2. How does this work with localtime? Won't the test(s) produce different results depending on who runs them? (Can we somehow hardcode the timezone the local user is in?)

  3. Is the test approach here something like this?

  • Add tasks to database around midnight UTC (within a hardcoded timezone offset).
  • things.tasks(stop_date=DATE) should only include tasks on DATE, in the user's local timezone
  • things.tasks(stop_date=DATE) should not return tasks for a DATE when it the date matches the UTC date, but is not in the user's local timezone.

@mikez
Copy link
Contributor

mikez commented Mar 10, 2024

@chrisgurney You can set the TZ environment variable for testing.

import os
os.environ['TZ'] = 'CET'  # or EST, JST, etc.

Updating the database is the way @AlexanderWillner has done tests in the past.

@AlexanderWillner
Copy link
Contributor

How do I go about adding data to test against? I feel like directly updating the database is not the way to go. Is there a way to start up the Things app, pointed at this database? Or do I need to replace my own with this?

I used the Makefile to copy the database file: https://github.com/thingsapi/things.py/blob/main/Makefile#L139
You should first backup the original database of course ;)

@mikez
Copy link
Contributor

mikez commented Mar 27, 2024

@chrisgurney Checking in where you're at with this and if you still have unanswered questions?

@mikez
Copy link
Contributor

mikez commented Apr 17, 2024

@chrisgurney Checking in again. Happy to help out here, let me know... :)

chrisgurney added a commit to chrisgurney/things.py that referenced this issue Jun 19, 2024
@mikez mikez closed this as completed in 2903a1f Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants