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

TDL 17738 sync conversations stream correctly #54

Open
wants to merge 8 commits into
base: TDL-20356-update-function-based-to-class-based
Choose a base branch
from
13 changes: 13 additions & 0 deletions tap_freshdesk/streams.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ def build_url(self, base_url, *args):
"""
return base_url + '/api/v2/' + self.path.format(*args)

def add_fields_at_1st_level(self, record):
"""Adding nested fields at first level."""

def write_records(self, catalog, state, selected_streams, start_date, data, max_bookmark,
client, streams_to_sync, child_max_bookmarks, predefined_filter=None):
"""
Expand All @@ -112,6 +115,7 @@ def write_records(self, catalog, state, selected_streams, start_date, data, max_
extraction_time = singer.utils.now()
stream_metadata = singer.metadata.to_map(stream_catalog['metadata'])
for row in data:
self.add_fields_at_1st_level(row)
if self.tap_stream_id in selected_streams and row[self.replication_keys[0]] >= bookmark:
# Custom fields are expected to be strings, but sometimes the API sends
# booleans. We cast those to strings to match the schema.
Expand Down Expand Up @@ -312,6 +316,15 @@ class Conversations(ChildStream):
path = 'tickets/{}/conversations'
parent = 'tickets'

def add_fields_at_1st_level(self, record):
"""
Overwrite updated__at value.
"""
# For edited conversations `last_edited_at` value gets update instead of `updated_at`
# Hence `updated_at` will be overwritten if `last_edited_at` > `updated_at`
if record.get("last_edited_at"):
record["updated_at"] = max(record["updated_at"], record["last_edited_at"])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add comments here regarding why the above changes are needed, and we can't use updated_at directly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added code comments to explain above if condition.



class SatisfactionRatings(ChildStream):
"""
Expand Down
56 changes: 56 additions & 0 deletions tests/unittests/test_conversations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import unittest
from unittest import mock
from parameterized import parameterized
from tap_freshdesk.streams import Tickets


class TestConversationSync(unittest.TestCase):
"""
Test `sync_obj` method of conversation stream.
If `last_edited_at` field is not null then `updated_at` will be overwritten by `last_edited_at`.
"""
start_date = "2019-06-01T00:00:00Z"

responses_1 = [
[{"id": "33", "updated_at": "2020-03-01T00:00:00Z"}], # Tickets Response
[{"id": "44", "updated_at": "2020-04-01T00:00:00Z",
"last_edited_at": "2020-05-01T00:00:00Z"}], # conversations Response
[], []
]

responses_2 = [
[{"id": "33", "updated_at": "2020-03-01T00:00:00Z"}], # Tickets Response
[{"id": "44", "updated_at": "2020-04-01T00:00:00Z",
"last_edited_at": None}], # conversations Response
[], []
]

@parameterized.expand([
# ["test_name", "responses", "expected_updated_at"]
["with last_edited_at value", responses_1, "2020-05-01T00:00:00Z"],
["with null last_edited_at", responses_2, "2020-04-01T00:00:00Z"],
])
@mock.patch("singer.write_record")
@mock.patch("singer.write_bookmark")
def test_stream(self, test_name, responses, expected_updated_at, mock_write_bookmark, mock_write_record):
"""
Test that the stream is writing the expected record.
"""
stream = Tickets()
state = {}
client = mock.Mock()
client.page_size = 100
client.base_url = ""
client.request.side_effect = responses

# Record with expected `updated_at` value
expected_record = {**responses[1][0], "updated_at": expected_updated_at}
catalog = [
{"schema": {}, "tap_stream_id": "tickets", "metadata": []},
{"schema": {}, "tap_stream_id": "conversations", "metadata": []}
]

stream.sync_obj(state, self.start_date, client, catalog, ["conversations"], ["tickets"])

# Verify that the expected record is written
mock_write_record.assert_called_with(mock.ANY, expected_record, time_extracted=mock.ANY)
2 changes: 1 addition & 1 deletion tests/unittests/test_streams.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def test_parent_child_both_selected(self, test_name, state, expected_state, writ
client.request.side_effect = [
[{"id": i, "updated_at": f"2020-03-{i}T00:00:00Z"} for i in [11, 15, 12]], # Tickets Response
[{"id": 10+i, "updated_at": f"2020-03-{i}T00:00:00Z"} for i in [13, 24]], # conversations Response
[{"id": 13, "updated_at": "2020-03-01T00:00:00Z"}], # conversations Response
[{"id": 13, "updated_at": "2020-03-01T00:00:00Z", "last_edited_at": "2020-03-02T00:00:00Z"}], # conversations Response
[{"id": 95, "updated_at": "2020-04-01T00:00:00Z"}], # conversations Response
[{"id": 73, "updated_at": "2020-05-01T00:00:00Z"}], # Deleted tickets response
[{"id": 30+i, "updated_at": f"2020-03-{i}T00:00:00Z"}for i in [22, 10]], # conversations response
Expand Down