Misc devices command bugfixes (#204)

* fix nightly test runs

* remove dupe command

* stop testing python3.5

* revert

* in `devices show`, don't echo "backupUsage" again when --format is JSON/RAW, since it's already contained in the first JSON output

* drop CSV --format option from `devices show` since it doesn't really make sense given the data

* clarify --days-since-last-connected help text a bit

* add --inactive option to make it possible to select only deactivated devices

* add new date option placeholders

* refactor magic date parsing into a click type, and make search options more flexible

* remove redundant import

* use MagicDate in date options

* fix option ordering

* rename options

* make magic time period case-insensitive

* allow case-insensitive periods in regex

* inadvertent commit

* set UTC timezone explicitly in MagicDate result

* use correct method to set UTC (not convert to UTC)

* add negative tests for MagicDate, adjust timestamp regex

* use .timestamp() for converting from dt > timestamp

* add note to changelog

* add connection/creation date filter logic, remove 'drop_most_recent' option and logic, clean up some help strings, update tests

* fix JSON/RAW-JSON dataframe output format bug

* fix recursion error trying to convert backup set data to json

* style and dupe changelog entry

* fix output format tests

* remove unused imports

* "the" provided value

* improve error handling in _deactivate_device()

* fix read_csv()

* style

* fix 3.8+ error about iteration over dict while modifying it

* add docs and fix doc error with trailing underscore

* put back --drop-most-recent as --exclude-most-recently-connected

* add list-backup-sets to changelog

* typo

* standardize triple-quoted strings to single-quoted

* wrap long help strings

* simplify read_csv a bit

* add more read_csv tests

* style

* add test for invalid guid check

* use elif

Author: timabrmsn

CHANGELOG.md

@@ -16,6 +16,7 @@ how a consumer would use the library (e.g. adding unit tests, updating documenta
     - `devices deactivate` to deactivate a single computer.
     - `devices show` to retrieve detailed information about a computer.
     - `devices list` to retrieve info about many devices, including device settings.
+    - `devices list-backup-sets` to retrieve detailed info about device backup sets.
     - `devices bulk deactivate` to deactivate a list of devices.
 
 - `code42 departing-employee list` command.
@@ -32,7 +33,7 @@ how a consumer would use the library (e.g. adding unit tests, updating documenta
 - `code42 cases file-events` commands:
     - `add` to add an event to a case.
     - `remove` to remove an event from the case.
-    - `list` to view all events assocaited to the case.
+    - `list` to view all events associated to the case.
 
 ### Changed
 

docs/commands.md

@@ -6,6 +6,7 @@
 * [Alerts](commands/alerts.rst)
 * [Alert Rules](commands/alertrules.rst)
 * [Departing Employee](commands/departingemployee.rst)
+* [Devices](commands/devices.rst)
 * [High Risk Employee](commands/highriskemployee.rst)
 * [Legal Hold](commands/legalhold.rst)
 * [Cases](commands/cases.rst)

docs/commands/devices.rst

@@ -0,0 +1,3 @@
+.. click:: code42cli.cmds.devices:devices
+  :prog: devices
+  :show-nested:

src/code42cli/cmds/devices.py

@@ -1,16 +1,18 @@
 from datetime import date
-from datetime import datetime
 
 import click
 from pandas import concat
 from pandas import DataFrame
 from pandas import to_datetime
-from pandas import to_timedelta
 from py42 import exceptions
 from py42.exceptions import Py42NotFoundError
 
 from code42cli.bulk import run_bulk_process
 from code42cli.click_ext.groups import OrderedGroup
+from code42cli.click_ext.options import incompatible_with
+from code42cli.click_ext.types import MagicDate
+from code42cli.date_helper import round_datetime_to_day_end
+from code42cli.date_helper import round_datetime_to_day_start
 from code42cli.errors import Code42CLIError
 from code42cli.file_readers import read_csv_arg
 from code42cli.options import format_option
@@ -33,8 +35,7 @@ def devices(state):
     required=False,
     is_flag=True,
     default=False,
-    help="""Prepend "deactivated_" and today's date to the name of any
-    deactivated devices.""",
+    help="Prepend 'deactivated_<current_date>' to the name of any deactivated devices.",
 )
 
 DATE_FORMAT = "%Y-%m-%d"
@@ -43,8 +44,8 @@ def devices(state):
     required=False,
     type=click.DateTime(formats=[DATE_FORMAT]),
     default=None,
-    help="""The date on which the archive should be purged from cold storage in yyyy-MM-dd format.
-    If not provided, the date will be set according to the appropriate org settings.""",
+    help="The date on which the archive should be purged from cold storage in yyyy-MM-dd format. "
+    "If not provided, the date will be set according to the appropriate org settings.",
 )
 
 
@@ -59,8 +60,12 @@ def deactivate(state, device_guid, change_device_name, purge_date):
 
 
 def _deactivate_device(sdk, device_guid, change_device_name, purge_date):
-    device = sdk.devices.get_by_guid(device_guid)
     try:
+        int(device_guid)
+    except ValueError:
+        raise Code42CLIError("Not a valid guid.")
+    try:
+        device = sdk.devices.get_by_guid(device_guid)
         sdk.devices.deactivate(device.data["computerId"])
     except exceptions.Py42BadRequestError:
         raise Code42CLIError("The device {} is in legal hold.".format(device_guid))
@@ -103,10 +108,9 @@ def _change_device_name(sdk, guid, name):
 
 @devices.command()
 @device_guid_argument
-@format_option
 @sdk_options()
 def show(state, device_guid, format=None):
-    """Print device info. Requires device GUID."""
+    """Print individual device info. Requires device GUID."""
 
     formatter = OutputFormatter(format, _device_info_keys_map())
     backup_set_formatter = OutputFormatter(format, _backup_set_keys_map())
@@ -145,20 +149,23 @@ def _get_device_info(sdk, device_guid):
 
 active_option = click.option(
     "--active",
-    required=False,
-    type=bool,
     is_flag=True,
+    help="Limits results to only active devices.",
     default=None,
-    help="Get only active or deactivated devices. Defaults to getting all devices.",
 )
-
+inactive_option = click.option(
+    "--inactive",
+    is_flag=True,
+    help="Limits results to only deactivated devices.",
+    cls=incompatible_with("active"),
+)
 org_uid_option = click.option(
     "--org-uid",
     required=False,
     type=str,
     default=None,
-    help="""Limit devices to only the ones in the org you specify.
-    Note that child orgs will be included.""",
+    help="Limit devices to only the ones in the org you specify. "
+    "Note that child orgs will be included.",
 )
 
 include_usernames_option = click.option(
@@ -171,31 +178,18 @@ def _get_device_info(sdk, device_guid):
 )
 
 
-@devices.command(name="list", help="Get information about many devices")
+@devices.command(name="list")
 @active_option
-@click.option(
-    "--days-since-last-connected",
-    required=False,
-    type=int,
-    help="Return only devices that have not connected in the number of days specified.",
-)
+@inactive_option
 @org_uid_option
-@click.option(
-    "--drop-most-recent",
-    required=False,
-    type=int,
-    help="""Will drop the X most recently connected devices for each user from the
-    result list where X is the number you provide as this argument. Can be used to
-    avoid passing the most recently connected device for a user to the deactivate command.""",
-)
 @click.option(
     "--include-backup-usage",
     required=False,
     type=bool,
     default=False,
     is_flag=True,
-    help="""Return backup usage information for each device
-    (may significantly lengthen the size of the return).""",
+    help="Return backup usage information for each device (may significantly lengthen the size "
+    "of the return).",
 )
 @include_usernames_option
 @click.option(
@@ -204,53 +198,99 @@ def _get_device_info(sdk, device_guid):
     type=bool,
     default=False,
     is_flag=True,
-    help="""Include device settings in output.""",
+    help="Include device settings in output.",
+)
+@click.option(
+    "--exclude-most-recently-connected",
+    type=int,
+    help="Filter out the N most recently connected devices per user. "
+    "Useful for identifying duplicate and/or replaced devices that are no longer needed across "
+    "an environment. If a user has 2 devices and N=1, the one device with the most recent "
+    "'lastConnected' date will not show up in the result list.",
+)
+@click.option(
+    "--last-connected-before",
+    type=MagicDate(rounding_func=round_datetime_to_day_start),
+    help=f"Include devices only when the 'lastConnected' field is after the provided value. {MagicDate.HELP_TEXT}",
+)
+@click.option(
+    "--last-connected-after",
+    type=MagicDate(rounding_func=round_datetime_to_day_end),
+    help="Include devices only when 'lastConnected' field is after the provided value. "
+    "Argument format options are the same as --last-connected-before.",
+)
+@click.option(
+    "--created-before",
+    type=MagicDate(rounding_func=round_datetime_to_day_start),
+    help="Include devices only when 'creationDate' field is less than the provided value. "
+    "Argument format options are the same as --last-connected-before.",
+)
+@click.option(
+    "--created-after",
+    type=MagicDate(rounding_func=round_datetime_to_day_end),
+    help="Include devices only when 'creationDate' field is greater than the provided value. "
+    "Argument format options are the same as --last-connected-before.",
 )
 @format_option
 @sdk_options()
 def list_devices(
     state,
     active,
-    days_since_last_connected,
-    drop_most_recent,
+    inactive,
     org_uid,
     include_backup_usage,
     include_usernames,
     include_settings,
+    exclude_most_recently_connected,
+    last_connected_after,
+    last_connected_before,
+    created_after,
+    created_before,
     format,
 ):
-    """Outputs a list of all devices."""
+    """Get information about many devices."""
+    if inactive:
+        active = False
     columns = [
         "computerId",
         "guid",
         "name",
         "osHostname",
         "status",
         "lastConnected",
+        "creationDate",
         "productVersion",
         "osName",
         "osVersion",
         "userUid",
     ]
-    devices_dataframe = _get_device_dataframe(
+    df = _get_device_dataframe(
         state.sdk, columns, active, org_uid, include_backup_usage
     )
-    if drop_most_recent:
-        devices_dataframe = _drop_n_devices_per_user(
-            devices_dataframe, drop_most_recent
-        )
-    if days_since_last_connected:
-        devices_dataframe = _drop_devices_which_have_not_connected_in_some_number_of_days(
-            devices_dataframe, days_since_last_connected
+    if last_connected_after:
+        df = df.loc[to_datetime(df.lastConnected) > last_connected_after]
+    if last_connected_before:
+        df = df.loc[to_datetime(df.lastConnected) < last_connected_before]
+    if created_after:
+        df = df.loc[to_datetime(df.creationDate) > created_after]
+    if created_before:
+        df = df.loc[to_datetime(df.creationDate) < created_before]
+    if exclude_most_recently_connected:
+        most_recent = (
+            df.sort_values(["userUid", "lastConnected"], ascending=False)
+            .groupby("userUid")
+            .head(exclude_most_recently_connected)
         )
+        df = df.drop(most_recent.index)
     if include_settings:
-        devices_dataframe = _add_settings_to_dataframe(state.sdk, devices_dataframe)
+        df = _add_settings_to_dataframe(state.sdk, df)
     if include_usernames:
-        devices_dataframe = _add_usernames_to_device_dataframe(
-            state.sdk, devices_dataframe
-        )
-    formatter = DataFrameOutputFormatter(format)
-    formatter.echo_formatted_dataframe(devices_dataframe)
+        df = _add_usernames_to_device_dataframe(state.sdk, df)
+    if df.empty:
+        click.echo("No results found.")
+    else:
+        formatter = DataFrameOutputFormatter(format)
+        formatter.echo_formatted_dataframe(df)
 
 
 def _get_device_dataframe(
@@ -297,35 +337,6 @@ def handle_row(guid):
         return device_dataframe
 
 
-def _drop_devices_which_have_not_connected_in_some_number_of_days(
-    devices_dataframe, days_since_last_connected
-):
-    utc_now = to_datetime(datetime.utcnow(), utc=True)
-    devices_last_connected_dates = to_datetime(
-        devices_dataframe["lastConnected"], utc=True
-    )
-    days_since_last_connected_delta = to_timedelta(
-        days_since_last_connected, unit="days"
-    )
-    return devices_dataframe.loc[
-        utc_now - devices_last_connected_dates > days_since_last_connected_delta, :,
-    ]
-
-
-def _drop_n_devices_per_user(
-    device_dataframe,
-    number_to_drop,
-    sort_field="lastConnected",
-    sort_ascending=False,
-    group_field="userUid",
-):
-    return (
-        device_dataframe.sort_values(by=sort_field, ascending=sort_ascending)
-        .drop(device_dataframe.groupby(group_field).head(number_to_drop).index)
-        .reset_index(drop=True)
-    )
-
-
 def _add_usernames_to_device_dataframe(sdk, device_dataframe):
     users_generator = sdk.users.get_all()
     users_list = []
@@ -337,30 +348,29 @@ def _add_usernames_to_device_dataframe(sdk, device_dataframe):
     return device_dataframe.merge(users_dataframe, how="left", on="userUid")
 
 
-@devices.command(
-    name="list-backup-sets",
-    help="Get information about many devices and their backup sets",
-)
+@devices.command()
 @active_option
+@inactive_option
 @org_uid_option
 @include_usernames_option
 @format_option
 @sdk_options()
 def list_backup_sets(
-    state, active, org_uid, include_usernames, format,
+    state, active, inactive, org_uid, include_usernames, format,
 ):
-    """Outputs a list of all devices."""
+    """Get information about many devices and their backup sets."""
+    if inactive:
+        active = False
     columns = ["guid", "userUid"]
-    devices_dataframe = _get_device_dataframe(state.sdk, columns, active, org_uid)
+    df = _get_device_dataframe(state.sdk, columns, active, org_uid)
     if include_usernames:
-        devices_dataframe = _add_usernames_to_device_dataframe(
-            state.sdk, devices_dataframe
-        )
-    devices_dataframe = _add_backup_set_settings_to_dataframe(
-        state.sdk, devices_dataframe
-    )
-    formatter = DataFrameOutputFormatter(format)
-    formatter.echo_formatted_dataframe(devices_dataframe)
+        df = _add_usernames_to_device_dataframe(state.sdk, df)
+    df = _add_backup_set_settings_to_dataframe(state.sdk, df)
+    if df.empty:
+        click.echo("No results found.")
+    else:
+        formatter = DataFrameOutputFormatter(format)
+        formatter.echo_formatted_dataframe(df)
 
 
 def _add_backup_set_settings_to_dataframe(sdk, devices_dataframe):
@@ -414,17 +424,14 @@ def bulk(state):
     pass
 
 
-@bulk.command(
-    name="deactivate",
-    help="""Deactivate all devices on the given list.
-            Takes as input a CSV with a 'guid' column.""",
-)
+@bulk.command(name="deactivate")
 @read_csv_arg(headers=["guid"])
 @change_device_name_option
 @purge_date_option
 @format_option
 @sdk_options()
 def bulk_deactivate(state, csv_rows, change_device_name, purge_date, format):
+    """Deactivate all devices from the provided CSV containing a 'guid' column."""
     sdk = state.sdk
     csv_rows[0]["deactivated"] = False
     formatter = OutputFormatter(format, {key: key for key in csv_rows[0].keys()})