handle detection list removal errors (#153)

* handle detectionlist removal errors

* also handle alert-rules removal error

* style

* add tests

* style

Author: timabrmsn

src/code42cli/cmds/alert_rules.py

@@ -2,6 +2,7 @@
 
 import click
 from click import echo
+from py42.exceptions import Py42BadRequestError
 from py42.util import format_json
 
 from code42cli import PRODUCT_NAME
@@ -68,7 +69,12 @@ def add_user(state, rule_id, username):
 @sdk_options()
 def remove_user(state, rule_id, username):
     """Remove a user from an alert rule."""
-    _remove_user(state.sdk, rule_id, username)
+    try:
+        _remove_user(state.sdk, rule_id, username)
+    except Py42BadRequestError:
+        raise Code42CLIError(
+            "User {} is not currently assigned to rule-id {}.".format(username, rule_id)
+        )
 
 
 @alert_rules.command("list")

src/code42cli/cmds/departing_employee.py

@@ -1,4 +1,5 @@
 import click
+from py42.exceptions import Py42NotFoundError
 
 from code42cli.bulk import generate_template_cmd_factory
 from code42cli.bulk import run_bulk_process
@@ -46,7 +47,14 @@ def add(state, username, cloud_alias, departure_date, notes):
 @sdk_options()
 def remove(state, username):
     """Remove a user from the departing-employee detection list."""
-    _remove_departing_employee(state.sdk, username)
+    try:
+        _remove_departing_employee(state.sdk, username)
+    except Py42NotFoundError:
+        raise Code42CLIError(
+            "User {} is not currently on the departing-employee detection list.".format(
+                username
+            )
+        )
 
 
 @departing_employee.group(cls=OrderedGroup)

src/code42cli/cmds/high_risk_employee.py

@@ -1,5 +1,6 @@
 import click
 from py42.clients.detectionlists import RiskTags
+from py42.exceptions import Py42NotFoundError
 
 from code42cli.bulk import generate_template_cmd_factory
 from code42cli.bulk import run_bulk_process
@@ -11,6 +12,7 @@
 from code42cli.cmds.detectionlists.options import notes_option
 from code42cli.cmds.detectionlists.options import username_arg
 from code42cli.cmds.shared import get_user_id
+from code42cli.errors import Code42CLIError
 from code42cli.file_readers import read_csv_arg
 from code42cli.file_readers import read_flat_file_arg
 from code42cli.options import OrderedGroup
@@ -48,7 +50,14 @@ def add(state, username, cloud_alias, risk_tag, notes):
 @sdk_options()
 def remove(state, username):
     """Remove a user from the high risk employees detection list."""
-    _remove_high_risk_employee(state.sdk, username)
+    try:
+        _remove_high_risk_employee(state.sdk, username)
+    except Py42NotFoundError:
+        raise Code42CLIError(
+            "User {} is not currently on the high-risk-employee detection list.".format(
+                username
+            )
+        )
 
 
 @high_risk_employee.command()

tests/cmds/test_alert_rules.py

@@ -2,6 +2,7 @@
 import logging
 
 import pytest
+from py42.exceptions import Py42BadRequestError
 from py42.exceptions import Py42InternalServerError
 from py42.exceptions import Py42InvalidRuleOperationError
 from py42.response import Py42Response
@@ -55,6 +56,17 @@ def side_effect(*args, **kwargs):
     return side_effect
 
 
+def get_user_not_on_alert_rule_side_effect(mocker):
+    def side_effect(*args, **kwargs):
+        err = mocker.MagicMock(spec=HTTPError)
+        resp = mocker.MagicMock(spec=Response)
+        resp.text = "TEST_ERR"
+        err.response = resp
+        raise Py42BadRequestError(err)
+
+    return side_effect
+
+
 def create_invalid_rule_type_side_effect(mocker):
     def side_effect(*args, **kwargs):
         err = mocker.MagicMock(spec=HTTPError)
@@ -312,3 +324,22 @@ def test_list_cmd_formats_to_csv_when_format_is_passed(runner, cli_state):
     assert "ruleSource" in result.output
     assert "name" in result.output
     assert "severity" in result.output
+
+
+def test_remove_when_user_not_on_rule_raises_expected_error(runner, cli_state, mocker):
+    cli_state.sdk.alerts.rules.remove_user.side_effect = get_user_not_on_alert_rule_side_effect(
+        mocker
+    )
+    test_username = "test@example.com"
+    test_rule_id = "101010"
+    result = runner.invoke(
+        cli,
+        ["alert-rules", "remove-user", "-u", test_username, "--rule-id", test_rule_id],
+        obj=cli_state,
+    )
+    assert (
+        "User {} is not currently assigned to rule-id {}.".format(
+            test_username, test_rule_id
+        )
+        in result.output
+    )

tests/cmds/test_departing_employee.py

@@ -1,10 +1,26 @@
+from py42.exceptions import Py42NotFoundError
+from requests import HTTPError
+from requests import Request
+from requests import Response
 from tests.cmds.conftest import thread_safe_side_effect
 from tests.conftest import TEST_ID
 
 from .conftest import TEST_EMPLOYEE
 from code42cli.main import cli
 
 
+def get_user_not_on_departing_employee_list_side_effect(mocker):
+    def side_effect(*args, **kwargs):
+        err = mocker.MagicMock(spec=HTTPError)
+        resp = mocker.MagicMock(spec=Response)
+        resp.text = "TEST_ERR"
+        err.response = resp
+        err.response.request = mocker.MagicMock(spec=Request)
+        raise Py42NotFoundError(err)
+
+    return side_effect
+
+
 def test_add_departing_employee_when_given_cloud_alias_adds_alias(
     runner, cli_state_with_user
 ):
@@ -192,3 +208,21 @@ def test_add_departing_employee_when_invalid_date_format_validation_raises_error
     assert (
         "Invalid value for '--departure-date': invalid datetime format" in result.output
     )
+
+
+def test_remove_departing_employee_when_user_not_on_list_prints_expected_error(
+    mocker, runner, cli_state
+):
+    cli_state.sdk.detectionlists.departing_employee.remove.side_effect = get_user_not_on_departing_employee_list_side_effect(
+        mocker
+    )
+    test_username = "test@example.com"
+    result = runner.invoke(
+        cli, ["departing-employee", "remove", test_username], obj=cli_state
+    )
+    assert (
+        "User {} is not currently on the departing-employee detection list.".format(
+            test_username
+        )
+        in result.output
+    )

tests/cmds/test_high_risk_employee.py

@@ -1,3 +1,7 @@
+from py42.exceptions import Py42NotFoundError
+from requests import HTTPError
+from requests import Request
+from requests import Response
 from tests.cmds.conftest import TEST_EMPLOYEE
 from tests.cmds.conftest import thread_safe_side_effect
 from tests.conftest import TEST_ID
@@ -7,6 +11,18 @@
 _NAMESPACE = "code42cli.cmds.high_risk_employee"
 
 
+def get_user_not_on_high_risk_employee_list_side_effect(mocker):
+    def side_effect(*args, **kwargs):
+        err = mocker.MagicMock(spec=HTTPError)
+        resp = mocker.MagicMock(spec=Response)
+        resp.text = "TEST_ERR"
+        err.response = resp
+        err.response.request = mocker.MagicMock(spec=Request)
+        raise Py42NotFoundError(err)
+
+    return side_effect
+
+
 def test_add_high_risk_employee_adds(runner, cli_state_with_user):
     runner.invoke(
         cli, ["high-risk-employee", "add", TEST_EMPLOYEE], obj=cli_state_with_user
@@ -219,3 +235,21 @@ def test_bulk_remove_risk_tags_uses_expected_arguments(runner, cli_state, mocker
             {"username": "test@example.com", "tag": "tag1"},
             {"username": "test2@example.com", "tag": "tag2"},
         ]
+
+
+def test_remove_high_risk_employee_when_user_not_on_list_prints_expected_error(
+    mocker, runner, cli_state
+):
+    cli_state.sdk.detectionlists.high_risk_employee.remove.side_effect = get_user_not_on_high_risk_employee_list_side_effect(
+        mocker
+    )
+    test_username = "test@example.com"
+    result = runner.invoke(
+        cli, ["high-risk-employee", "remove", test_username], obj=cli_state
+    )
+    assert (
+        "User {} is not currently on the high-risk-employee detection list.".format(
+            test_username
+        )
+        in result.output
+    )