Fix bulk (#56)

Author: Juliya Smith

src/code42cli/bulk.py

@@ -94,7 +94,10 @@ def _process_csv_row(self, row):
         # Removes problems from including extra comments. Error messages from out of order args
         # are more indicative this way too.
         row.pop(None, None)
-        self.__worker.do_async(lambda *args, **kwargs: self._row_handler(*args, **kwargs), **row)
+        row_values = {key: val if val != u"" else None for key, val in row.items()}
+        self.__worker.do_async(
+            lambda *args, **kwargs: self._row_handler(*args, **kwargs), **row_values
+        )
 
     def _process_flat_file_row(self, row):
         if row:

src/code42cli/cmds/detectionlists/__init__.py

@@ -17,10 +17,7 @@ def __init__(self, username, list_name):
 
 def handle_bad_request_during_add(bad_request_err, username_tried_adding, list_name):
     if _error_is_user_already_added(bad_request_err.response.text):
-        logger = get_main_cli_logger()
-        new_err = UserAlreadyAddedError(username_tried_adding, list_name)
-        logger.print_and_log_error(new_err)
-        return True
+        raise UserAlreadyAddedError(username_tried_adding, list_name)
     return False
 
 

src/code42cli/logger.py

@@ -83,6 +83,7 @@ def _get_user_error_logger():
 class RedStderrHandler(logging.StreamHandler):
     """Logging handler for logging error messages to stderr using red scary text prefixed by the 
     word `ERROR`. For logging info to stderr, it will not add the scary red text."""
+
     def __init__(self):
         super(RedStderrHandler, self).__init__(sys.stderr)
 

tests/cmds/detectionlists/test_departing_employee.py

@@ -1,7 +1,7 @@
 import pytest
 import logging
 
-from code42cli.cmds.detectionlists import UserDoesNotExistError
+from code42cli.cmds.detectionlists import UserDoesNotExistError, UserAlreadyAddedError
 from code42cli.cmds.detectionlists.departing_employee import (
     add_departing_employee,
     remove_departing_employee,
@@ -49,17 +49,14 @@ def test_add_departing_employee_when_user_does_not_exist_prints_error(
             assert str(UserDoesNotExistError(_EMPLOYEE)) in caplog.text
 
 
-def test_add_departing_employee_when_user_already_added_prints_error(
-    sdk_with_user, profile, bad_request_for_user_already_added, caplog
+def test_add_departing_employee_when_user_already_added_raises_UserAlreadyAddedError(
+    sdk_with_user, profile, bad_request_for_user_already_added
 ):
     sdk_with_user.detectionlists.departing_employee.add.side_effect = (
         bad_request_for_user_already_added
     )
-    add_departing_employee(sdk_with_user, profile, _EMPLOYEE)
-    with caplog.at_level(logging.ERROR):
-        assert _EMPLOYEE in caplog.text
-        assert "already on the" in caplog.text
-        assert "departing-employee" in caplog.text
+    with pytest.raises(UserAlreadyAddedError):
+        add_departing_employee(sdk_with_user, profile, _EMPLOYEE)
 
 
 def test_add_departing_employee_when_bad_request_but_not_user_already_added_raises_Py42BadRequestError(

tests/cmds/detectionlists/test_high_risk_employee.py

@@ -1,7 +1,7 @@
 import pytest
 import logging
 
-from code42cli.cmds.detectionlists import UserDoesNotExistError
+from code42cli.cmds.detectionlists import UserDoesNotExistError, UserAlreadyAddedError
 from code42cli.cmds.detectionlists.high_risk_employee import (
     add_high_risk_employee,
     remove_high_risk_employee,
@@ -61,17 +61,14 @@ def test_add_high_risk_employee_when_user_does_not_exist_prints_error(
             assert str(UserDoesNotExistError(_EMPLOYEE)) in caplog.text
 
 
-def test_add_high_risk_employee_when_user_already_added_prints_error(
-    sdk_with_user, profile, bad_request_for_user_already_added, caplog
+def test_add_high_risk_employee_when_user_already_added_raises_UserAlreadyAddedError(
+    sdk_with_user, profile, bad_request_for_user_already_added
 ):
     sdk_with_user.detectionlists.high_risk_employee.add.side_effect = (
         bad_request_for_user_already_added
     )
-    add_high_risk_employee(sdk_with_user, profile, _EMPLOYEE)
-    with caplog.at_level(logging.ERROR):
-        assert _EMPLOYEE in caplog.text
-        assert "already on the" in caplog.text
-        assert "high-risk-employee" in caplog.text
+    with pytest.raises(UserAlreadyAddedError):
+        add_high_risk_employee(sdk_with_user, profile, _EMPLOYEE)
 
 
 def test_add_high_risk_employee_when_bad_request_but_not_user_already_added_raises_Py42BadRequestError(

tests/test_bulk.py

@@ -218,7 +218,7 @@ def __call__(self, *args, **kwargs):
             processor.run()
             assert get_view_exceptions_location_message() not in caplog.text
 
-    def test_run_when_row_is_endline_does_not_process_row(self, mock_open, caplog):
+    def test_run_when_row_is_endline_does_not_process_row(self, mock_open):
         processed_rows = []
 
         def func_for_bulk(test):
@@ -229,9 +229,25 @@ def __call__(self, *args, **kwargs):
                 return ["row1", "row2", "\n"]
 
         processor = BulkProcessor("some/path", func_for_bulk, MockRowReader())
-        with caplog.at_level(logging.INFO):
-            processor.run()
+        processor.run()
 
         assert "row1" in processed_rows
         assert "row2" in processed_rows
         assert "row3" not in processed_rows
+
+    def test_run_when_reader_returns_dict_rows_containing_empty_strs_converts_them_to_none(
+        self, mock_open
+    ):
+        processed_rows = []
+
+        def func_for_bulk(test1, test2):
+            processed_rows.append((test1, test2))
+
+        class MockDictReader(object):
+            def __call__(self, *args, **kwargs):
+                return [{"test1": "", "test2": "foo"}, {"test1": "bar", "test2": u""}]
+
+        processor = BulkProcessor("some/path", func_for_bulk, MockDictReader())
+        processor.run()
+        assert (None, "foo") in processed_rows
+        assert ("bar", None) in processed_rows