Fixed TimeEntryAPI class (#65)

* Fixed TimeEntryAPI class which makes list_time_entries work without ticket id.
Also removed unrelated get_role method.

* Using .format for URL to be consistent

* Renamed time_entry API property to time_entries to be consistent

* Updated time entry model to be in sync with comments

* Added tests for time entries

Co-authored-by: Csaba Major <csaba.major@commsignia.com>

freshdesk/v2/api.py

@@ -438,18 +438,28 @@ class TimeEntryAPI(object):
     def __init__(self, api):
         self._api = api
 
-    def list_time_entries(self, ticket_id=None):
-        url = "tickets/time_entries"
+    def list_time_entries(self, ticket_id=None, **kwargs):
+        url = "time_entries?"
+
         if ticket_id is not None:
-            url = "tickets/%d/time_entries" % ticket_id
+            url = "tickets/%d/time_entries?" % ticket_id
+
+        page = kwargs.get("page", 1)
+        per_page = kwargs.get("per_page", 100)
+
         time_entries = []
-        for r in self._api._get(url):
-            time_entries.append(TimeEntry(**r))
-        return time_entries
 
-    def get_role(self, role_id):
-        url = "roles/%s" % role_id
-        return Role(**self._api._get(url))
+        # Skip pagination by looping over each page and adding tickets if 'page' key is not in kwargs.
+        # else return the requested page and break the loop
+        while True:
+            this_page = self._api._get(url + 'page={}&per_page={}'.format(page, per_page), kwargs)
+            time_entries += this_page
+            if len(this_page) < per_page or "page" in kwargs:
+                break
+
+            page += 1
+
+        return [TimeEntry(**c) for c in time_entries]
 
 
 class TicketFieldAPI(object):
@@ -556,7 +566,7 @@ def __init__(self, domain, api_key, verify=True, proxies=None):
         self.agents = AgentAPI(self)
         self.roles = RoleAPI(self)
         self.ticket_fields = TicketFieldAPI(self)
-        self.time_entry = TimeEntryAPI(self)
+        self.time_entries = TimeEntryAPI(self)
 
         if domain.find("freshdesk.com") < 0:
             raise AttributeError("Freshdesk v2 API works only via Freshdesk" "domains and not via custom CNAMEs")

freshdesk/v2/models.py

@@ -99,10 +99,10 @@ def __repr__(self):
 
 class TimeEntry(FreshdeskModel):
     def __str__(self):
-        return str(self.id)
+        return "{} ({})".format(self.note, self.time_spent)
 
     def __repr__(self):
-        return "<Timesheet Entry {}>".format(self.id)
+        return "<Timesheet entry for Ticket #{}>".format(self.ticket_id)
 
 
 class Customer(FreshdeskModel):

freshdesk/v2/tests/conftest.py

@@ -25,6 +25,8 @@ def __init__(self, *args):
                 re.compile(r"tickets/1$"): self.read_test_file("ticket_1.json"),
                 re.compile(r"tickets/1?include=stats,requester$"): self.read_test_file("ticket_1.json"),
                 re.compile(r"tickets/1/conversations"): self.read_test_file("conversations.json"),
+                re.compile(r"tickets/1/time_entries"): self.read_test_file("time_entry_1.json"),
+                re.compile(r"time_entries"): self.read_test_file("time_entries.json"),
                 re.compile(r"companies\?page=1&per_page=100$"): self.read_test_file("companies.json"),
                 re.compile(r"companies/1$"): self.read_test_file("company.json"),
                 re.compile(r"contacts\?page=1&per_page=100$"): self.read_test_file("contacts.json"),

freshdesk/v2/tests/sample_json_data/time_entries.json

@@ -0,0 +1,44 @@
+[
+  {
+    "billable": true,
+    "note": "This is the first entry",
+    "id": 1,
+    "timer_running": false,
+    "agent_id": 1,
+    "ticket_id": 1,
+    "company_id": 2,
+    "time_spent": "00:15",
+    "executed_at": "2020-09-27T22:00:00Z",
+    "start_time": "2020-09-28T12:14:00Z",
+    "created_at": "2020-09-28T12:14:00Z",
+    "updated_at": "2020-09-28T12:14:00Z"
+  },
+  {
+    "billable": true,
+    "note": "This is the second entry",
+    "id": 2,
+    "timer_running": false,
+    "agent_id": 1,
+    "ticket_id": 2,
+    "company_id": 2,
+    "time_spent": "00:10",
+    "executed_at": "2020-09-24T22:00:00Z",
+    "start_time": "2020-09-25T07:44:32Z",
+    "created_at": "2020-09-25T07:44:32Z",
+    "updated_at": "2020-09-25T07:44:32Z"
+  },
+  {
+    "billable": true,
+    "note": "",
+    "id": 3,
+    "timer_running": false,
+    "agent_id": 1,
+    "ticket_id": 3,
+    "company_id": 2,
+    "time_spent": "00:15",
+    "executed_at": "2020-09-23T22:00:00Z",
+    "start_time": "2020-09-24T10:25:05Z",
+    "created_at": "2020-09-24T10:25:05Z",
+    "updated_at": "2020-09-24T10:25:05Z"
+  }
+]

freshdesk/v2/tests/sample_json_data/time_entry_1.json

@@ -0,0 +1,44 @@
+[
+  {
+    "billable": true,
+    "note": "This is test time entry",
+    "id": 1,
+    "timer_running": false,
+    "agent_id": 2,
+    "ticket_id": 521,
+    "company_id": 3,
+    "time_spent": "00:10",
+    "executed_at": "2020-09-27T22:00:00Z",
+    "start_time": "2020-09-28T12:14:00Z",
+    "created_at": "2020-09-28T12:14:00Z",
+    "updated_at": "2020-09-28T12:14:00Z"
+  },
+  {
+    "billable": true,
+    "note": "",
+    "id": 2,
+    "timer_running": false,
+    "agent_id": 2,
+    "ticket_id": 521,
+    "company_id": 3,
+    "time_spent": "00:10",
+    "executed_at": "2020-09-24T22:00:00Z",
+    "start_time": "2020-09-25T07:44:32Z",
+    "created_at": "2020-09-25T07:44:32Z",
+    "updated_at": "2020-09-25T07:44:32Z"
+  },
+  {
+    "billable": true,
+    "note": "",
+    "id": 3,
+    "timer_running": false,
+    "agent_id": 2,
+    "ticket_id": 521,
+    "company_id": 3,
+    "time_spent": "00:15",
+    "executed_at": "2020-09-23T22:00:00Z",
+    "start_time": "2020-09-24T10:25:05Z",
+    "created_at": "2020-09-24T10:25:05Z",
+    "updated_at": "2020-09-24T10:25:05Z"
+  }
+]

freshdesk/v2/tests/test_time_entry.py

@@ -0,0 +1,41 @@
+import pytest
+
+from freshdesk.v2.models import TimeEntry
+
+
+@pytest.fixture
+def time_entries_1(api):
+    return api.time_entries.list_time_entries(1)
+
+@pytest.fixture
+def time_entries_all(api):
+    return api.time_entries.list_time_entries()
+
+
+def test_time_entries_1_list(time_entries_1):
+    assert isinstance(time_entries_1, list)
+    assert len(time_entries_1) == 3
+    assert isinstance(time_entries_1[0], TimeEntry)
+
+
+def test_time_entries_all_list(time_entries_all):
+    assert isinstance(time_entries_all, list)
+    assert len(time_entries_all) == 3
+    assert isinstance(time_entries_all[0], TimeEntry)
+
+
+def test_time_entries_1_str(time_entries_1):
+    assert str(time_entries_1[0]) == "This is test time entry (00:10)"
+
+
+def test_time_entries_all_str(time_entries_all):
+    assert str(time_entries_all[0]) == "This is the first entry (00:15)"
+
+
+def test_time_entries_1_repr(time_entries_1):
+    assert repr(time_entries_1[0]) == "<Timesheet entry for Ticket #521>"
+
+
+def test_time_entries_all_repr(time_entries_all):
+    assert repr(time_entries_all[0]) == "<Timesheet entry for Ticket #1>"
+