Repository: SebRut/pygrocy (python)

Issue:
Grocy API doesn't use offset timezone info (chore execution gets the wrong time)
Forgive me if I've made an error, my python is rusty, but I've been 🕵️ [an issue with the Home Assistant custom component](https://github.com/custom-components/grocy/issues/252) which led me to changes in #250 which seem to result in the time zone offset being included in the API calls to Grocy (at least for `/chores/000/execute` which I was focusing on). According to [this issue](https://github.com/grocy/grocy/issues/1827) in the Grocy repo, the API calls will not parse time zone information so we shouldn't send it (the database [stores UTC](https://github.com/grocy/grocy/issues/214)).

**TL/DR**: I thought the changes in #250 broke the recording of the time when a chore is executed, but it seems it's been broken (in a different way) for a while now. The fix seems to be that `tracked time` *needs to be* in the format `YYYY-MM-DD hh:mm:ss` or Grocy's API will insert the current time (in GMT) instead, resulting in an offset time value (unless you're in GMT 😉).

### Observed behavior (debug level logging in Home Assistant):

With pygrocy **1.5.0** (and integration 4.7.1):
```
2022-09-23 11:19:44.032 DEBUG (SyncWorker_13) [pygrocy.grocy_api_client] --> POST /chores/25/execute
2022-09-23 11:19:44.033 DEBUG (SyncWorker_13) [pygrocy.grocy_api_client] {'tracked_time': '2022-09-23T11:17:35.185320-05:00', 'skipped': False, 'done_by': 3}
2022-09-23 11:19:44.033 DEBUG (SyncWorker_13) [pygrocy.grocy_api_client] <-- 200 for /chores/25/execute
2022-09-23 11:19:44.033 DEBUG (SyncWorker_13) [pygrocy.grocy_api_client] b'{"id":"90","chore_id":"25","tracked_time":"2022-09-23 16:19:44","done_by_user_id":"3","row_created_timestamp":"2022-09-23 11:19:44","undone":"0","undone_timestamp":null,"skipped":"0"}'
```

With pygrocy **1.4.1** (and integration 4.7.1):
```
2022-09-23 12:11:10.801 DEBUG (SyncWorker_10) [pygrocy.grocy_api_client] --> POST /chores/25/execute
2022-09-23 12:11:10.802 DEBUG (SyncWorker_10) [pygrocy.grocy_api_client] {'tracked_time': '2022-09-23T17:06:58.892222+00:00', 'skipped': False, 'done_by': 3}
2022-09-23 12:11:10.802 DEBUG (SyncWorker_10) [pygrocy.grocy_api_client] <-- 200 for /chores/25/execute
2022-09-23 12:11:10.802 DEBUG (SyncWorker_10) [pygrocy.grocy_api_client] b'{"id":"99","chore_id":"25","tracked_time":"2022-09-23 17:11:10","done_by_user_id":"3","row_created_timestamp":"2022-09-23 12:11:10","undone":"0","undone_timestamp":null,"skipped":"0"}'
```

### Observed behavior using the REST API browser within Grocy

Inspecting how the Grocy UI (web page) makes the call, I captured this post:
```
{ "tracked_time": "2022-09-23 14:18:16", "skipped": false }
```
To which the API then responds:
```
{"id":"105","chore_id":"26","tracked_time":"2022-09-23 14:18:16","done_by_user_id":"8","row_created_timestamp":"2022-09-23 14:18:15","undone":"0","undone_timestamp":null,"skipped":"0"}
```
This appears correct in the UI and the next chore execution appears to be correct as well. 

If I send values for `tracked_time` that are not in `YYYY-MM-DD hh:mm:ss` format (for example using ISO8601-`YYYY-MM-DDThh:mm:ss.sss` or with any time zone offset, like `2022-09-23T20:23:16.033Z` or `2022-09-23T20:23:16.033+05:00`) Grocy will **ignore** the value and use the current time (GMT) for the `tracked_time` (same as if the `tracked_time` is omitted). This causes executions to appear at the incorrect time (and next due to be miscalculated) if you're not also in GMT.

We should be able to fix this by sending the *local time* in `YYYY-MM-DD hh:mm:ss` format to the API (at least for chore execution, not sure if it would apply to all API calls?).

cc: @marcelvriend (author of the PR)

Pull request:
Fix tracking time for chores, tasks and batteries
## Description

Fix incorrect tracking time for chores, tasks and batteries.

- The Grocy API expects a string datetime for `tracked_time` in the format 2022-09-24 18:00:00. It ignores other time formats (and replace it with Grocy `local_time`).

- Default function parameters are evaluated only once. So the `datetime.now()` is not being updated for successive calls. Change the default value to None and set `datetime.now()` in the function body.
 
**Related issue (if applicable):** fixes #254 

## Checklist
  - [x] The code change is tested and works locally.
  - [x] tests pass. **Your PR won't be merged unless tests pass**
  - [x] There is no commented out code in this PR


Patch:
diff --git a/pygrocy/grocy.py b/pygrocy/grocy.py
index b305b51..a7d9e6e 100644
--- a/pygrocy/grocy.py
+++ b/pygrocy/grocy.py
@@ -125,7 +125,7 @@ class Grocy(object):
         self,
         chore_id: int,
         done_by: int = None,
-        tracked_time: datetime = datetime.now(),
+        tracked_time: datetime = None,
         skipped: bool = False,
     ):
         return self._api_client.execute_chore(chore_id, done_by, tracked_time, skipped)
@@ -326,7 +326,7 @@ class Grocy(object):
         resp = self._api_client.get_task(task_id)
         return Task(resp)
 
-    def complete_task(self, task_id, done_time: datetime = datetime.now()):
+    def complete_task(self, task_id, done_time: datetime = None):
         return self._api_client.complete_task(task_id, done_time)
 
     def meal_plan(
@@ -361,7 +361,7 @@ class Grocy(object):
         if battery:
             return Battery(battery)
 
-    def charge_battery(self, battery_id: int, tracked_time: datetime = datetime.now()):
+    def charge_battery(self, battery_id: int, tracked_time: datetime = None):
         return self._api_client.charge_battery(battery_id, tracked_time)
 
     def add_generic(self, entity_type: EntityType, data):
diff --git a/pygrocy/grocy_api_client.py b/pygrocy/grocy_api_client.py
index 6f140b3..3bbf4f6 100644
--- a/pygrocy/grocy_api_client.py
+++ b/pygrocy/grocy_api_client.py
@@ -11,7 +11,7 @@ from pydantic import BaseModel, Extra, Field, root_validator, validator
 from pydantic.schema import date
 
 from pygrocy import EntityType
-from pygrocy.utils import localize_datetime, parse_date
+from pygrocy.utils import grocy_datetime_str, localize_datetime, parse_date
 
 from .errors import GrocyError
 
@@ -453,13 +453,16 @@ class GrocyApiClient(object):
         self,
         chore_id: int,
         done_by: int = None,
-        tracked_time: datetime = datetime.now(),
+        tracked_time: datetime = None,
         skipped: bool = False,
     ):
+        if tracked_time is None:
+            tracked_time = datetime.now()
+
         localized_tracked_time = localize_datetime(tracked_time)
 
         data = {
-            "tracked_time": localized_tracked_time.isoformat(),
+            "tracked_time": grocy_datetime_str(localized_tracked_time),
             "skipped": skipped,
         }
 
@@ -733,12 +736,15 @@ class GrocyApiClient(object):
         parsed_json = self._do_get_request(url)
         return TaskResponse(**parsed_json)
 
-    def complete_task(self, task_id: int, done_time: datetime = datetime.now()):
+    def complete_task(self, task_id: int, done_time: datetime = None):
         url = f"tasks/{task_id}/complete"
 
+        if done_time is None:
+            done_time = datetime.now()
+
         localized_done_time = localize_datetime(done_time)
 
-        data = {"done_time": localized_done_time.isoformat()}
+        data = {"done_time": grocy_datetime_str(localized_done_time)}
         self._do_post_request(url, data)
 
     def get_meal_plan(self, query_filters: List[str] = None) -> List[MealPlanResponse]:
@@ -765,9 +771,12 @@ class GrocyApiClient(object):
         if parsed_json:
             return BatteryDetailsResponse(**parsed_json)
 
-    def charge_battery(self, battery_id: int, tracked_time: datetime = datetime.now()):
+    def charge_battery(self, battery_id: int, tracked_time: datetime = None):
+        if tracked_time is None:
+            tracked_time = datetime.now()
+
         localized_tracked_time = localize_datetime(tracked_time)
-        data = {"tracked_time": localized_tracked_time.isoformat()}
+        data = {"tracked_time": grocy_datetime_str(localized_tracked_time)}
 
         return self._do_post_request(f"batteries/{battery_id}/charge", data)
 
diff --git a/pygrocy/utils.py b/pygrocy/utils.py
index e139f5f..5fc3299 100644
--- a/pygrocy/utils.py
+++ b/pygrocy/utils.py
@@ -37,3 +37,9 @@ def parse_bool_int(input_value):
 
 def localize_datetime(timestamp: datetime) -> datetime:
     return timestamp.astimezone()
+
+
+def grocy_datetime_str(timestamp: datetime) -> str:
+    if timestamp is None:
+        return ""
+    return timestamp.strftime("%Y-%m-%d %H:%M:%S")


Test patch:
diff --git a/test/test_utils.py b/test/test_utils.py
index 06fb838..e8f8f80 100644
--- a/test/test_utils.py
+++ b/test/test_utils.py
@@ -97,3 +97,9 @@ class TestUtils:
             633809,
             tzinfo=zoneinfo.ZoneInfo("America/Los_Angeles"),
         )
+
+    def test_grocy_datetime_str(self):
+        date = datetime(2022, 7, 10, 21, 17, 34, 633809, tzinfo=None)
+        date_str = utils.grocy_datetime_str(date)
+
+        assert date_str == "2022-07-10 21:17:34"