Repository: nylas/nylas-python

Issue:
Filtering on booleans and datetimes fails
When trying to filter on boolean fields, like `starred` or `unread`, the SDK converts the boolean object to a string, so `True` becomes "True" and `False` becomes "False". Both of these strings are treated as true by the backend. As a result, it's not possible to filter on boolean False for any boolean fields.

```python
>>> client.threads.where(unread=True).all()
# fetches `https://api.nylas.com/threads?unread=True&offset=0&limit=50`
>>> client.threads.where(unread=False).all()
# fetches `https://api.nylas.com/threads?unread=False&offset=0&limit=50`
# same results either way
```

In addition, in Python it's common to use the [`datetime`](https://docs.python.org/3/library/datetime.html) module to refer to dates and times. There are several APIs that return datetimes, or accept datetimes as parameters (like `starts_before` when filtering events). The SDK _only_ accepts timestamps, and does not work with datetime objects at all. In fact, trying to pass a datetime object to the SDK results in an internal exception, like this:

```python
>>> now
datetime.datetime(2017, 8, 7, 16, 56, 36, 56763)
>>> client.events.where(starts_before=now).all()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/singingwolfboy/clones/nylas-python/nylas/client/restful_model_collection.py", line 45, in all
    return self._range(self.filters['offset'], limit)
  File "/Users/singingwolfboy/clones/nylas-python/nylas/client/restful_model_collection.py", line 118, in _range
    to_fetch)
  File "/Users/singingwolfboy/clones/nylas-python/nylas/client/restful_model_collection.py", line 107, in _get_model_collection
    **filters)
  File "/Users/singingwolfboy/clones/nylas-python/nylas/client/client.py", line 76, in caught
    return func(*args, **kwargs)
  File "/Users/singingwolfboy/clones/nylas-python/nylas/client/client.py", line 259, in _get_resources
    url = str(URLObject(url).add_query_params(filters.items()))
  File "/Users/singingwolfboy/.virtualenvs/nylas-python/lib/python3.6/site-packages/urlobject/urlobject.py", line 464, in add_query_params
    return self.with_query(self.query.add_params(*args, **kwargs))
  File "/Users/singingwolfboy/.virtualenvs/nylas-python/lib/python3.6/site-packages/urlobject/query_string.py", line 67, in add_params
    new = new.add_param(name, value)
  File "/Users/singingwolfboy/.virtualenvs/nylas-python/lib/python3.6/site-packages/urlobject/query_string.py", line 58, in add_param
    parameter = qs_encode(name) + '=' + qs_encode(value)
  File "/Users/singingwolfboy/.virtualenvs/nylas-python/lib/python3.6/site-packages/urlobject/query_string.py", line 138, in _qs_encode_py3
    return urlparse.quote_plus(s)
  File "/usr/local/Cellar/python3/3.6.1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/urllib/parse.py", line 791, in quote_plus
    string = quote(string, safe + space, encoding, errors)
  File "/usr/local/Cellar/python3/3.6.1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/urllib/parse.py", line 775, in quote
    return quote_from_bytes(string, safe)
  File "/usr/local/Cellar/python3/3.6.1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/urllib/parse.py", line 800, in quote_from_bytes
    raise TypeError("quote_from_bytes() expected bytes")
TypeError: quote_from_bytes() expected bytes
```

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/nylas/nylas-python/pull/88?src=pr&el=h1) Report
> Merging [#88](https://codecov.io/gh/nylas/nylas-python/pull/88?src=pr&el=desc) into [master](https://codecov.io/gh/nylas/nylas-python/commit/b6c4558e0652433e77696803090d0e111ec87b40?src=pr&el=desc) will **increase** coverage by `0.1%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/nylas/nylas-python/pull/88/graphs/tree.svg?width=650&height=150&src=pr&token=GFzivPuPDA)](https://codecov.io/gh/nylas/nylas-python/pull/88?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##           master      #88     +/-   ##
=========================================
+ Coverage   96.11%   96.22%   +0.1%     
=========================================
  Files           6        7      +1     
  Lines         644      662     +18     
=========================================
+ Hits          619      637     +18     
  Misses         25       25
```


| [Impacted Files](https://codecov.io/gh/nylas/nylas-python/pull/88?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [nylas/utils.py](https://codecov.io/gh/nylas/nylas-python/pull/88?src=pr&el=tree#diff-bnlsYXMvdXRpbHMucHk=) | `100% <100%> (ø)` | |
| [nylas/client/restful\_models.py](https://codecov.io/gh/nylas/nylas-python/pull/88?src=pr&el=tree#diff-bnlsYXMvY2xpZW50L3Jlc3RmdWxfbW9kZWxzLnB5) | `97.2% <100%> (+0.11%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/nylas/nylas-python/pull/88?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/nylas/nylas-python/pull/88?src=pr&el=footer). Last update [b6c4558...c350c0b](https://codecov.io/gh/nylas/nylas-python/pull/88?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/examples/webhooks/server.py b/examples/webhooks/server.py
index 9d4b77d..4ccc8b3 100755
--- a/examples/webhooks/server.py
+++ b/examples/webhooks/server.py
@@ -129,7 +129,7 @@ def process_delta(delta):
     """
     kwargs = {
         "type": delta["type"],
-        "date": datetime.datetime.fromtimestamp(delta["date"]),
+        "date": datetime.datetime.utcfromtimestamp(delta["date"]),
         "object_id": delta["object_data"]["id"],
     }
     print(" * {type} at {date} with ID {object_id}".format(**kwargs))
diff --git a/nylas/client/client.py b/nylas/client/client.py
index aaf92d8..5ab5833 100644
--- a/nylas/client/client.py
+++ b/nylas/client/client.py
@@ -1,8 +1,13 @@
 from __future__ import print_function
 import sys
-import json
 from os import environ
 from base64 import b64encode
+import json
+try:
+    from json import JSONDecodeError
+except ImportError:
+    JSONDecodeError = ValueError
+
 import requests
 from urlobject import URLObject
 from six.moves.urllib.parse import urlencode
@@ -14,10 +19,7 @@ from nylas.client.restful_models import (
     Label, Draft
 )
 from nylas.client.errors import APIClientError, ConnectionError, STATUS_MAP
-try:
-    from json import JSONDecodeError
-except ImportError:
-    JSONDecodeError = ValueError
+from nylas.utils import convert_datetimes_to_timestamps
 
 DEBUG = environ.get('NYLAS_CLIENT_DEBUG')
 API_SERVER = "https://api.nylas.com"
@@ -256,7 +258,10 @@ class APIClient(json.JSONEncoder):
                 postfix
             )
 
-        url = str(URLObject(url).add_query_params(filters.items()))
+        converted_filters = convert_datetimes_to_timestamps(
+            filters, cls.datetime_filter_attrs,
+        )
+        url = str(URLObject(url).add_query_params(converted_filters.items()))
         response = self._get_http_session(cls.api_root).get(url)
         results = _validate(response).json()
         return [
@@ -280,7 +285,10 @@ class APIClient(json.JSONEncoder):
             url = "{}/a/{}/{}/{}{}".format(self.api_server, self.app_id,
                                            cls.collection_name, id, postfix)
 
-        url = str(URLObject(url).add_query_params(filters.items()))
+        converted_filters = convert_datetimes_to_timestamps(
+            filters, cls.datetime_filter_attrs,
+        )
+        url = str(URLObject(url).add_query_params(converted_filters.items()))
 
         response = self._get_http_session(cls.api_root).get(url, headers=headers)
         return _validate(response)
@@ -311,10 +319,10 @@ class APIClient(json.JSONEncoder):
         if cls == File:
             response = session.post(url, files=data)
         else:
-            data = json.dumps(data)
+            converted_data = convert_datetimes_to_timestamps(data, cls.datetime_attrs)
             headers = {'Content-Type': 'application/json'}
             headers.update(self.session.headers)
-            response = session.post(url, data=data, headers=headers)
+            response = session.post(url, json=converted_data, headers=headers)
 
         result = _validate(response).json()
         if cls.collection_name == 'send':
@@ -332,10 +340,13 @@ class APIClient(json.JSONEncoder):
         if cls == File:
             response = session.post(url, files=data)
         else:
-            data = json.dumps(data)
+            converted_data = [
+                convert_datetimes_to_timestamps(datum, cls.datetime_attrs)
+                for datum in data
+            ]
             headers = {'Content-Type': 'application/json'}
             headers.update(self.session.headers)
-            response = session.post(url, data=data, headers=headers)
+            response = session.post(url, json=converted_data, headers=headers)
 
         results = _validate(response).json()
         return [cls.create(self, **x) for x in results]
@@ -363,7 +374,8 @@ class APIClient(json.JSONEncoder):
 
         session = self._get_http_session(cls.api_root)
 
-        response = session.put(url, json=data)
+        converted_data = convert_datetimes_to_timestamps(data, cls.datetime_attrs)
+        response = session.put(url, json=converted_data)
 
         result = _validate(response).json()
         return cls.create(self, **result)
@@ -390,9 +402,10 @@ class APIClient(json.JSONEncoder):
             URLObject(self.api_server)
             .with_path(url_path)
         )
+        converted_data = convert_datetimes_to_timestamps(data, cls.datetime_attrs)
 
         session = self._get_http_session(cls.api_root)
-        response = session.post(url, json=data)
+        response = session.post(url, json=converted_data)
 
         result = _validate(response).json()
         return cls.create(self, **result)
diff --git a/nylas/client/restful_models.py b/nylas/client/restful_models.py
index e73c100..1cb3bb9 100644
--- a/nylas/client/restful_models.py
+++ b/nylas/client/restful_models.py
@@ -1,5 +1,8 @@
+from datetime import datetime
+
 from nylas.client.restful_model_collection import RestfulModelCollection
 from nylas.client.errors import FileUploadError
+from nylas.utils import timestamp_from_dt
 from six import StringIO
 
 # pylint: disable=attribute-defined-outside-init
@@ -7,6 +10,8 @@ from six import StringIO
 
 class NylasAPIObject(dict):
     attrs = []
+    datetime_attrs = {}
+    datetime_filter_attrs = {}
     # The Nylas API holds most objects for an account directly under '/',
     # but some of them are under '/a' (mostly the account-management
     # and billing code). api_root is a tiny metaprogramming hack to let
@@ -44,6 +49,9 @@ class NylasAPIObject(dict):
                 attr = attr_name[1:]
             if attr in kwargs:
                 obj[attr_name] = kwargs[attr]
+        for dt_attr, ts_attr in cls.datetime_attrs.items():
+            if obj.get(ts_attr):
+                obj[dt_attr] = datetime.utcfromtimestamp(obj[ts_attr])
         if 'id' not in kwargs:
             obj['id'] = None
 
@@ -54,6 +62,9 @@ class NylasAPIObject(dict):
         for attr in self.cls.attrs:
             if hasattr(self, attr):
                 dct[attr] = getattr(self, attr)
+        for dt_attr, ts_attr in self.cls.datetime_attrs.items():
+            if self.get(dt_attr):
+                dct[ts_attr] = timestamp_from_dt(self[dt_attr])
         return dct
 
     def child_collection(self, cls, **filters):
@@ -83,6 +94,13 @@ class Message(NylasAPIObject):
              "account_id", "object", "snippet", "starred", "subject",
              "thread_id", "to", "unread", "starred", "_folder", "_labels",
              "headers"]
+    datetime_attrs = {
+        "received_at": "date",
+    }
+    datetime_filter_attrs = {
+        "received_before": "received_before",
+        "received_after": "received_after",
+    }
     collection_name = 'messages'
 
     def __init__(self, api):
@@ -206,8 +224,21 @@ class Thread(NylasAPIObject):
     attrs = ["draft_ids", "id", "message_ids", "account_id", "object",
              "participants", "snippet", "subject", "subject_date",
              "last_message_timestamp", "first_message_timestamp",
+             "last_message_received_timestamp", "last_message_sent_timestamp",
              "unread", "starred", "version", "_folders", "_labels",
              "received_recent_date"]
+    datetime_attrs = {
+        "first_message_at": "first_message_timestamp",
+        "last_message_at": "last_message_timestamp",
+        "last_message_received_at": "last_message_received_timestamp",
+        "last_message_sent_at": "last_message_sent_timestamp",
+    }
+    datetime_filter_attrs = {
+        "last_message_before": "last_message_before",
+        "last_message_after": "last_message_after",
+        "started_before": "started_before",
+        "started_after": "started_after",
+    }
     collection_name = 'threads'
 
     def __init__(self, api):
@@ -313,6 +344,9 @@ class Draft(Message):
              "account_id", "object", "subject", "thread_id", "to",
              "unread", "version", "file_ids", "reply_to_message_id",
              "reply_to", "starred", "snippet", "tracking"]
+    datetime_attrs = {
+        "last_modified_at": "date",
+    }
     collection_name = 'drafts'
 
     def __init__(self, api, thread_id=None):  # pylint: disable=unused-argument
@@ -407,6 +441,9 @@ class Event(NylasAPIObject):
              "read_only", "when", "busy", "participants", "calendar_id",
              "recurrence", "status", "master_event_id", "owner",
              "original_start_time", "object", "message_id"]
+    datetime_attrs = {
+        "original_start_at": "original_start_time",
+    }
     collection_name = 'events'
 
     def __init__(self, api):
diff --git a/nylas/utils.py b/nylas/utils.py
new file mode 100644
index 0000000..5c9b389
--- /dev/null
+++ b/nylas/utils.py
@@ -0,0 +1,32 @@
+from __future__ import division
+from datetime import datetime
+
+
+def timestamp_from_dt(dt, epoch=datetime(1970, 1, 1)):
+    """
+    Convert a datetime to a timestamp.
+    https://stackoverflow.com/a/8778548/141395
+    """
+    delta = dt - epoch
+    # return delta.total_seconds()
+    return delta.seconds + delta.days * 86400
+
+
+def convert_datetimes_to_timestamps(data, datetime_attrs):
+    """
+    Given a dictionary of data, and a dictionary of datetime attributes,
+    return a new dictionary that converts any datetime attributes that may
+    be present to their timestamped equivalent.
+    """
+    if not data:
+        return data
+
+    new_data = {}
+    for key, value in data.items():
+        if key in datetime_attrs and isinstance(value, datetime):
+            new_key = datetime_attrs[key]
+            new_data[new_key] = timestamp_from_dt(value)
+        else:
+            new_data[key] = value
+
+    return new_data
diff --git a/pylintrc b/pylintrc
index 20eed34..1f4681c 100644
--- a/pylintrc
+++ b/pylintrc
@@ -156,7 +156,7 @@ function-name-hint=(([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$
 function-rgx=(([a-z][a-z0-9_]{2,50})|(_[a-z0-9_]*))$
 
 # Good variable names which should always be accepted, separated by a comma
-good-names=i,j,k,ex,Run,_,id
+good-names=i,j,k,ex,Run,_,id,dt,db
 
 # Include a hint for the correct naming format with invalid-name
 include-naming-hint=no


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index e8b3efc..6a51745 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -264,7 +264,8 @@ def mock_messages(mocked_responses, api_url, account_id):
                 }
             ],
             "starred": False,
-            "unread": True
+            "unread": True,
+            "date": 1265077342,
         }, {
             "id": "1238",
             "subject": "Test Message 2",
@@ -278,7 +279,8 @@ def mock_messages(mocked_responses, api_url, account_id):
                 }
             ],
             "starred": False,
-            "unread": True
+            "unread": True,
+            "date": 1265085342,
         }, {
             "id": "12",
             "subject": "Test Message 3",
@@ -292,7 +294,8 @@ def mock_messages(mocked_responses, api_url, account_id):
                 }
             ],
             "starred": False,
-            "unread": False
+            "unread": False,
+            "date": 1265093842,
         }
     ])
     endpoint = re.compile(api_url + '/messages')
@@ -369,7 +372,11 @@ def mock_threads(mocked_responses, api_url, account_id):
                 "id": "abcd"
             }],
             "starred": True,
-            "unread": False
+            "unread": False,
+            "first_message_timestamp": 1451703845,
+            "last_message_timestamp": 1483326245,
+            "last_message_received_timestamp": 1483326245,
+            "last_message_sent_timestamp": 1483232461,
         }
     ])
     endpoint = re.compile(api_url + '/threads')
@@ -395,7 +402,11 @@ def mock_thread(mocked_responses, api_url, account_id):
             "id": "abcd"
         }],
         "starred": True,
-        "unread": False
+        "unread": False,
+        "first_message_timestamp": 1451703845,
+        "last_message_timestamp": 1483326245,
+        "last_message_received_timestamp": 1483326245,
+        "last_message_sent_timestamp": 1483232461,
     }
     response_body = json.dumps(base_thrd)
 
@@ -451,7 +462,11 @@ def mock_labelled_thread(mocked_responses, api_url, account_id):
                 "account_id": account_id,
                 "object": "label"
             }
-        ]
+        ],
+        "first_message_timestamp": 1451703845,
+        "last_message_timestamp": 1483326245,
+        "last_message_received_timestamp": 1483326245,
+        "last_message_sent_timestamp": 1483232461,
     }
     response_body = json.dumps(base_thread)
 
@@ -881,10 +896,17 @@ def mock_events(mocked_responses, api_url):
             "title": "Pool party",
             "location": "Local Community Pool",
             "participants": [
-                "Alice",
-                "Bob",
-                "Claire",
-                "Dot",
+                {
+                    "comment": None,
+                    "email": "kelly@nylas.com",
+                    "name": "Kelly Nylanaut",
+                    "status": "noreply",
+                }, {
+                    "comment": None,
+                    "email": "sarah@nylas.com",
+                    "name": "Sarah Nylanaut",
+                    "status": "no",
+                },
             ]
         }
     ])
diff --git a/tests/test_drafts.py b/tests/test_drafts.py
index bfd84d7..7827254 100644
--- a/tests/test_drafts.py
+++ b/tests/test_drafts.py
@@ -1,9 +1,20 @@
+from datetime import datetime
+
 import pytest
 from nylas.client.errors import InvalidRequestError
+from nylas.utils import timestamp_from_dt
 
 # pylint: disable=len-as-condition
 
 
+@pytest.mark.usefixtures("mock_drafts")
+def test_draft_attrs(api_client):
+    draft = api_client.drafts.first()
+    expected_modified = datetime(2015, 8, 4, 10, 34, 46)
+    assert draft.last_modified_at == expected_modified
+    assert draft.date == timestamp_from_dt(expected_modified)
+
+
 @pytest.mark.usefixtures(
     "mock_draft_saved_response", "mock_draft_sent_response"
 )
diff --git a/tests/test_messages.py b/tests/test_messages.py
index 8a76136..6e76bf1 100644
--- a/tests/test_messages.py
+++ b/tests/test_messages.py
@@ -1,8 +1,11 @@
+from datetime import datetime
 import json
+
 import six
 import pytest
 from urlobject import URLObject
 from nylas.client.restful_models import Message
+from nylas.utils import timestamp_from_dt
 
 
 @pytest.mark.usefixtures("mock_messages")
@@ -15,6 +18,14 @@ def test_messages(api_client):
     assert not message.starred
 
 
+@pytest.mark.usefixtures("mock_messages")
+def test_message_attrs(api_client):
+    message = api_client.messages.first()
+    expected_received = datetime(2010, 2, 2, 2, 22, 22)
+    assert message.received_at == expected_received
+    assert message.date == timestamp_from_dt(expected_received)
+
+
 @pytest.mark.usefixtures("mock_account", "mock_messages", "mock_message")
 def test_message_stars(api_client):
     message = api_client.messages.first()
@@ -89,3 +100,21 @@ def test_slice_messages(api_client):
     messages = api_client.messages[0:2]
     assert len(messages) == 3
     assert all(isinstance(message, Message) for message in messages)
+
+
+@pytest.mark.usefixtures("mock_messages")
+def test_filter_messages_dt(mocked_responses, api_client):
+    api_client.messages.where(received_before=datetime(2010, 6, 1)).all()
+    assert len(mocked_responses.calls) == 1
+    request = mocked_responses.calls[0].request
+    url = URLObject(request.url)
+    assert url.query_dict["received_before"] == "1275350400"
+
+
+@pytest.mark.usefixtures("mock_messages")
+def test_filter_messages_ts(mocked_responses, api_client):
+    api_client.messages.where(received_before=1275350400).all()
+    assert len(mocked_responses.calls) == 1
+    request = mocked_responses.calls[0].request
+    url = URLObject(request.url)
+    assert url.query_dict["received_before"] == "1275350400"
diff --git a/tests/test_threads.py b/tests/test_threads.py
index 2abf27f..d59758d 100644
--- a/tests/test_threads.py
+++ b/tests/test_threads.py
@@ -1,5 +1,40 @@
+from datetime import datetime
+
 import pytest
+from urlobject import URLObject
 from nylas.client.restful_models import Message, Draft, Label
+from nylas.utils import timestamp_from_dt
+
+
+@pytest.mark.usefixtures("mock_threads")
+def test_thread_attrs(api_client):
+    thread = api_client.threads.first()
+    expected_first = datetime(2016, 1, 2, 3, 4, 5)
+    expected_last = datetime(2017, 1, 2, 3, 4, 5)
+    expected_last_received = datetime(2017, 1, 2, 3, 4, 5)
+    expected_last_sent = datetime(2017, 1, 1, 1, 1, 1)
+
+    assert thread.first_message_timestamp == timestamp_from_dt(expected_first)
+    assert thread.first_message_at == expected_first
+    assert thread.last_message_timestamp == timestamp_from_dt(expected_last)
+    assert thread.last_message_at == expected_last
+    assert thread.last_message_received_timestamp == timestamp_from_dt(expected_last_received)
+    assert thread.last_message_received_at == expected_last_received
+    assert thread.last_message_sent_timestamp == timestamp_from_dt(expected_last_sent)
+    assert thread.last_message_sent_at == expected_last_sent
+
+
+def test_update_thread_attrs(api_client):
+    thread = api_client.threads.create()
+    first = datetime(2017, 2, 3, 10, 0, 0)
+    second = datetime(2016, 10, 5, 14, 30, 0)
+    # timestamps and datetimes are handled totally separately
+    thread.last_message_at = first
+    thread.last_message_timestamp = timestamp_from_dt(second)
+    assert thread.last_message_at == first
+    assert thread.last_message_timestamp == timestamp_from_dt(second)
+    # but datetimes overwrite timestamps when serializing to JSON
+    assert thread.as_json()['last_message_timestamp'] == timestamp_from_dt(first)
 
 
 @pytest.mark.usefixtures("mock_threads")
@@ -98,3 +133,21 @@ def test_thread_reply(api_client):
     assert isinstance(draft, Draft)
     assert draft.thread_id == thread.id
     assert draft.subject == thread.subject
+
+
+@pytest.mark.usefixtures("mock_threads")
+def test_filter_threads_dt(mocked_responses, api_client):
+    api_client.threads.where(started_before=datetime(2010, 6, 1)).all()
+    assert len(mocked_responses.calls) == 1
+    request = mocked_responses.calls[0].request
+    url = URLObject(request.url)
+    assert url.query_dict["started_before"] == "1275350400"
+
+
+@pytest.mark.usefixtures("mock_threads")
+def test_filter_threads_ts(mocked_responses, api_client):
+    api_client.threads.where(started_before=1275350400).all()
+    assert len(mocked_responses.calls) == 1
+    request = mocked_responses.calls[0].request
+    url = URLObject(request.url)
+    assert url.query_dict["started_before"] == "1275350400"