Refactored code to address review comments

Author: MuralidharT03

tap_trello/streams/abstracts.py

@@ -11,6 +11,9 @@
 
 LOGGER = get_logger()
 
+# NB: We've observed that Trello will only return 50 actions, this is to sub-paginate
+MAX_API_RESPONSE_SIZE = 50
+
 
 class OrderChecker:
     """ Class with context manager to check ordering of values. """
@@ -56,25 +59,14 @@ def __exit__(self, *args):
         """ Required for context manager usage. """
 
 
-class Mixin:
-    """ Empty class to mark mixin classes as such. """
-
-
-class Unsortable(Mixin):
+class Unsortable:
     """
-    Mixin class to mark a Stream subclass as Unsortable
+    Marker class to identify Stream subclasses that are unsortable.
     NB: No current functionality, but we thought it was useful for higher-order declarative behavior.
-    e.g.:
-        class MyStream(Unsortable, Stream):
-            # Specify properties, implement unique things
     """
 
 
-# NB: We've observed that Trello will only return 50 actions, this is to sub-paginate
-MAX_API_RESPONSE_SIZE = 50
-
-
-class DateWindowPaginated(Mixin):
+class DateWindowPaginated:
     """
     Mixin class to provide date windowing on the `get_records` requests
     """
@@ -241,48 +233,6 @@ def sync(self):
             yield rec
 
 
-class AddCustomFields(Mixin):
-    def _get_dropdown_option_key(self, field_id, option_id):
-        return field_id + '_' + option_id
-
-    def build_custom_fields_maps(self, **kwargs):
-        custom_fields_map = {}
-        dropdown_options_map = {}
-        board_id_list = kwargs['parent_id_list']
-        # The custom fields are defined on the board level, so this function is called on a per-board basis
-        # Therefore, we assert that only one board is being passed in
-        assert len(board_id_list) == 1
-        custom_fields = self.client.get('/boards/{}/customFields'.format(board_id_list[0])) # pylint: disable=no-member
-        for custom_field in custom_fields:
-            custom_fields_map[custom_field['id']] = custom_field['name']
-            if custom_field['type'] == 'list':
-                for dropdown_option in custom_field['options']:
-                    dropdown_option_key = self._get_dropdown_option_key(dropdown_option['idCustomField'], dropdown_option['id'])
-                    dropdown_options_map[dropdown_option_key] = dropdown_option['value']['text']
-
-        return custom_fields_map, dropdown_options_map
-
-
-    def modify_record(self, record, **kwargs):
-        custom_fields_map = kwargs['custom_fields_map']
-        dropdown_options_map = kwargs['dropdown_options_map']
-        for custom_field in record['customFieldItems']:
-            custom_field['name'] = custom_fields_map[custom_field['idCustomField']]
-            if custom_field.get('idValue', None):
-                dropdown_option_key = self._get_dropdown_option_key(custom_field['idCustomField'], custom_field['idValue'])
-                custom_field['value'] = {'option': dropdown_options_map[dropdown_option_key]}
-
-        return record
-
-
-class AddBoardId(Mixin):
-    def modify_record(self, record, **kwargs):
-        board_id_list = kwargs['parent_id_list']
-        assert len(board_id_list) == 1
-        record["boardId"] = board_id_list[0]
-        return record
-
-
 class LegacyChildStream(LegacyStream):
     """
     Legacy base class for child streams (pre-refactor).

tap_trello/streams/actions.py

@@ -3,7 +3,6 @@
 
 
 class Actions(DateWindowPaginated, ChildStream):
-    # TODO: If an action is completed on a board, does the board's dateLastActivity get updated?
     stream_id = "actions"
     stream_name = "actions"
     endpoint = "/boards/{}/actions"

tap_trello/streams/board_memberships.py

@@ -1,8 +1,14 @@
-from tap_trello.streams.abstracts import AddBoardId, FullTableStream
+from tap_trello.streams.abstracts import FullTableStream
 
-class BoardMemberships(FullTableStream, AddBoardId):
+class BoardMemberships(FullTableStream):
     tap_stream_id = "board_memberships"
     key_properties = ["id", "boardId"]
     replication_method = "FULL_TABLE"
     path = "/boards/{id}/memberships"
     parent = "boards"
+
+    def modify_object(self, record, parent_record=None):
+        """Add boardId to board membership records."""
+        if parent_record and 'id' in parent_record:
+            record["boardId"] = parent_record['id']
+        return record

tap_trello/streams/boards.py

@@ -2,14 +2,11 @@
 
 
 class Boards(Unsortable, Stream):
-    # TODO: Should boards respect the start date? i.e., not emit records from before the configured start?
-    # TODO: Is this also limited to 50 records per response? If so, might need paginated...
     stream_id = "boards"
     stream_name = "boards"
     endpoint = "/members/{}/boards"
     key_properties = ["id"]
     replication_method = "FULL_TABLE"
 
-
     def get_format_values(self):
         return [self.client.member_id]

tap_trello/streams/cards.py

@@ -1,12 +1,12 @@
 import singer
 
-from tap_trello.streams.abstracts import AddCustomFields, ChildStream
+from tap_trello.streams.abstracts import ChildStream
 from tap_trello.streams.boards import Boards
 
 LOGGER = singer.get_logger()
 
 
-class Cards(AddCustomFields, ChildStream):
+class Cards(ChildStream):
     stream_id = "cards"
     stream_name = "cards"
     endpoint = "/boards/{}/cards/all"
@@ -15,6 +15,41 @@ class Cards(AddCustomFields, ChildStream):
     parent = Boards
     MAX_API_RESPONSE_SIZE = 1000
 
+    def _get_dropdown_option_key(self, field_id, option_id):
+        """Generate a unique key for dropdown options."""
+        return field_id + '_' + option_id
+
+    def build_custom_fields_maps(self, **kwargs):
+        """Build maps of custom field IDs to names and dropdown option IDs to values."""
+        custom_fields_map = {}
+        dropdown_options_map = {}
+        board_id_list = kwargs['parent_id_list']
+        # The custom fields are defined on the board level, so this function is called on a per-board basis
+        # Therefore, we validate that only one board is being passed in
+        if len(board_id_list) != 1:
+            raise ValueError(f"Expected exactly one board ID, got {len(board_id_list)}")
+        custom_fields = self.client.get('/boards/{}/customFields'.format(board_id_list[0]))
+        for custom_field in custom_fields:
+            custom_fields_map[custom_field['id']] = custom_field['name']
+            if custom_field['type'] == 'list':
+                for dropdown_option in custom_field['options']:
+                    dropdown_option_key = self._get_dropdown_option_key(dropdown_option['idCustomField'], dropdown_option['id'])
+                    dropdown_options_map[dropdown_option_key] = dropdown_option['value']['text']
+
+        return custom_fields_map, dropdown_options_map
+
+    def modify_record(self, record, **kwargs):
+        """Add custom field names and dropdown values to card records."""
+        custom_fields_map = kwargs['custom_fields_map']
+        dropdown_options_map = kwargs['dropdown_options_map']
+        for custom_field in record['customFieldItems']:
+            custom_field['name'] = custom_fields_map[custom_field['idCustomField']]
+            if custom_field.get('idValue', None):
+                dropdown_option_key = self._get_dropdown_option_key(custom_field['idCustomField'], custom_field['idValue'])
+                custom_field['value'] = {'option': dropdown_options_map[dropdown_option_key]}
+
+        return record
+
     def get_records(self, format_values, additional_params=None):
         # Get max_api_response_size from config and set to parameter
         cards_response_size = int(self.config.get('cards_response_size') or self.MAX_API_RESPONSE_SIZE)

tap_trello/streams/lists.py

@@ -3,7 +3,6 @@
 
 
 class Lists(Unsortable, ChildStream):
-    # TODO: If a list is added to a board, does the board's dateLastActivity get updated?
     stream_id = "lists"
     stream_name = "lists"
     endpoint = "/boards/{}/lists"

tap_trello/streams/users.py

@@ -1,13 +1,19 @@
-from tap_trello.streams.abstracts import Unsortable, AddBoardId, ChildStream
+from tap_trello.streams.abstracts import Unsortable, ChildStream
 from tap_trello.streams.boards import Boards
 
 
-class Users(Unsortable, AddBoardId, ChildStream):
-    # TODO: If a user is added to a board, does the board's dateLastActivity get updated?
-    # TODO: Should this assoc the board_id to the user records? Seems pretty useless without it
+class Users(Unsortable, ChildStream):
     stream_id = "users"
     stream_name = "users"
     endpoint = "/boards/{}/members"
     key_properties = ["id", "boardId"]
     replication_method = "FULL_TABLE"
     parent = Boards
+
+    def modify_record(self, record, **kwargs):
+        """Add boardId to user records."""
+        board_id_list = kwargs['parent_id_list']
+        if len(board_id_list) != 1:
+            raise ValueError(f"Expected exactly one board ID, got {len(board_id_list)}")
+        record["boardId"] = board_id_list[0]
+        return record