Chore/user already added error (#55)

Author: Juliya Smith

src/code42cli/cmds/detectionlists/__init__.py

@@ -9,6 +9,25 @@
 )
 
 
+class UserAlreadyAddedError(Exception):
+    def __init__(self, username, list_name):
+        msg = u"'{}' is already on the {} list.".format(username, list_name)
+        super(UserAlreadyAddedError, self).__init__(msg)
+
+
+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
+    return False
+
+
+def _error_is_user_already_added(bad_request_error_text):
+    return u"User already on list" in bad_request_error_text
+
+
 class UserDoesNotExistError(Exception):
     """An error to represent a username that is not in our system. The CLI shows this error when 
     the user tries to add or remove a user that does not exist. This error is not shown during 

src/code42cli/cmds/detectionlists/departing_employee.py

@@ -4,7 +4,11 @@
     load_user_descriptions,
     get_user_id,
     update_user,
+    handle_bad_request_during_add,
 )
+from code42cli.cmds.detectionlists.enums import DetectionLists
+
+from py42.exceptions import Py42BadRequestError
 
 
 def load_subcommands():
@@ -33,8 +37,13 @@ def add_departing_employee(
         notes: (str): Notes about the employee.
     """
     user_id = get_user_id(sdk, username)
-    sdk.detectionlists.departing_employee.add(user_id, departure_date)
-    update_user(sdk, user_id, cloud_alias, notes=notes)
+
+    try:
+        sdk.detectionlists.departing_employee.add(user_id, departure_date)
+        update_user(sdk, user_id, cloud_alias, notes=notes)
+    except Py42BadRequestError as err:
+        if not handle_bad_request_during_add(err, username, DetectionLists.DEPARTING_EMPLOYEE):
+            raise
 
 
 def remove_departing_employee(sdk, profile, username):

src/code42cli/cmds/detectionlists/high_risk_employee.py

@@ -5,10 +5,14 @@
     load_username_description,
     get_user_id,
     update_user,
+    handle_bad_request_during_add,
 )
+from code42cli.cmds.detectionlists.enums import DetectionLists
 from code42cli.cmds.detectionlists.enums import DetectionListUserKeys
 from code42cli.commands import Command
 
+from py42.exceptions import Py42BadRequestError
+
 
 def load_subcommands():
 
@@ -65,8 +69,13 @@ def add_high_risk_employee(sdk, profile, username, cloud_alias=None, risk_tag=No
     """
     risk_tag = _handle_list_args(risk_tag)
     user_id = get_user_id(sdk, username)
-    sdk.detectionlists.high_risk_employee.add(user_id)
-    update_user(sdk, user_id, cloud_alias, risk_tag, notes)
+
+    try:
+        sdk.detectionlists.high_risk_employee.add(user_id)
+        update_user(sdk, user_id, cloud_alias, risk_tag, notes)
+    except Py42BadRequestError as err:
+        if not handle_bad_request_during_add(err, username, DetectionLists.HIGH_RISK_EMPLOYEE):
+            raise
 
 
 def remove_high_risk_employee(sdk, profile, username):

tests/cmds/detectionlists/conftest.py

@@ -1,4 +1,7 @@
 import pytest
+from requests import Response, HTTPError
+
+from py42.exceptions import Py42BadRequestError
 
 
 TEST_ID = "TEST_ID"
@@ -14,3 +17,23 @@ def sdk_with_user(sdk):
 def sdk_without_user(sdk):
     sdk.users.get_by_username.return_value = {"users": []}
     return sdk
+
+
+@pytest.fixture
+def bad_request_for_user_already_added(mocker):
+    resp = mocker.MagicMock(spec=Response)
+    resp.text = "User already on list"
+    return _create_bad_request_mock(resp)
+
+
+@pytest.fixture
+def bad_request_for_other_reasons(mocker):
+    resp = mocker.MagicMock(spec=Response)
+    resp.text = "Some other, non-supported reason for a bad request"
+    return _create_bad_request_mock(resp)
+
+
+def _create_bad_request_mock(resp):
+    base_err = HTTPError()
+    base_err.response = resp
+    return Py42BadRequestError(base_err)

tests/cmds/detectionlists/test_departing_employee.py

@@ -6,8 +6,11 @@
     add_departing_employee,
     remove_departing_employee,
 )
+
 from .conftest import TEST_ID
 
+from py42.exceptions import Py42BadRequestError
+
 
 _EMPLOYEE = "departing employee"
 
@@ -46,6 +49,27 @@ 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
+):
+    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
+
+
+def test_add_departing_employee_when_bad_request_but_not_user_already_added_raises_Py42BadRequestError(
+    sdk_with_user, profile, bad_request_for_other_reasons, caplog
+):
+    sdk_with_user.detectionlists.departing_employee.add.side_effect = bad_request_for_other_reasons
+    with pytest.raises(Py42BadRequestError):
+        add_departing_employee(sdk_with_user, profile, _EMPLOYEE)
+
+
 def test_remove_departing_employee_calls_remove(sdk_with_user, profile):
     remove_departing_employee(sdk_with_user, profile, _EMPLOYEE)
     sdk_with_user.detectionlists.departing_employee.remove.assert_called_once_with(TEST_ID)

tests/cmds/detectionlists/test_high_risk_employee.py

@@ -10,6 +10,8 @@
 )
 from .conftest import TEST_ID
 
+from py42.exceptions import Py42BadRequestError
+
 
 _EMPLOYEE = "risky employee"
 
@@ -59,6 +61,27 @@ 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
+):
+    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
+
+
+def test_add_high_risk_employee_when_bad_request_but_not_user_already_added_raises_Py42BadRequestError(
+    sdk_with_user, profile, bad_request_for_other_reasons, caplog
+):
+    sdk_with_user.detectionlists.high_risk_employee.add.side_effect = bad_request_for_other_reasons
+    with pytest.raises(Py42BadRequestError):
+        add_high_risk_employee(sdk_with_user, profile, _EMPLOYEE)
+
+
 def test_remove_high_risk_employee_calls_remove(sdk_with_user, profile):
     remove_high_risk_employee(sdk_with_user, profile, _EMPLOYEE)
     sdk_with_user.detectionlists.high_risk_employee.remove.assert_called_once_with(TEST_ID)