chore(formatting,documentation): Improved cli documentation; formatti…

…ng cleanup in service and cli

invenio_remote_user_data/cli.py

@@ -1,15 +1,13 @@
 import click
 from flask.cli import with_appcontext
 from invenio_access.permissions import system_identity
-from invenio_accounts.models import User
 from invenio_accounts.proxies import current_datastore
 from invenio_oauthclient.models import UserIdentity
 from invenio_users_resources.proxies import current_users_service
 from pprint import pprint
 import re
 from .proxies import (
     current_remote_user_data_service as user_data_service,
-    current_remote_group_data_service as group_data_service,
 )
 
 
@@ -25,11 +23,40 @@ def cli():
     "--groups",
     is_flag=True,
     default=False,
-    help=("If true, update groups rather than users."),
+    help=(
+        "If true, update groups rather than users. The provided "
+        "IDs should be group IDs. (Not yet implemented)"
+    ),
+)
+@click.option(
+    "-s",
+    "--source",
+    default="knowledgeCommons",
+    help=(
+        "Remote source name. Should be the same as the saml IDP listed in "
+        "the UserIdentity table."
+    ),
+)
+@click.option(
+    "-e",
+    "--by-email",
+    is_flag=True,
+    default=False,
+    help=(
+        "Update by email address. If true, the provided ID(s) should be "
+        "email addresses."
+    ),
+)
+@click.option(
+    "-n",
+    "--by-username",
+    is_flag=True,
+    default=False,
+    help=(
+        "Update by username. If true, the provided ID(s) should be "
+        "usernames from the remote service."
+    ),
 )
-@click.option("-s", "--source", default="knowledgeCommons")
-@click.option("-e", "--by-email", is_flag=True, default=False)
-@click.option("-n", "--by-username", is_flag=True, default=False)
 @with_appcontext
 def update_user_data(
     ids: list,
@@ -44,9 +71,29 @@ def update_user_data(
     If IDS are not specified, all records (either users or groups)
     will be updated from the specified remote service.
 
+    IDS can be a list of user or group IDs, or a range of IDs
+    separated by a hyphen, e.g. 1-10.
+
+    Parameters:
+
+    ids (list): List of user or group IDs, or ranges of IDs.
+    groups (bool): Flag to indicate if groups should be updated.
+    source (str): The source of the remote data service. This should
+        match the SAML IDP listed in the UserIdentity table.
+    by_email (bool): Flag to update by email. If true, the ID(s) should
+        be one or more email addresses.
+    by_username (bool): Flag to update by remote username. If true,
+        the ID(s) should be one or more usernames from the remote
+        service.
+
+    Returns:
+
+    None
     """
     print(
-        f"Updating {'all ' if len(ids) == 0 else ''}{'users' if not groups else 'groups'} {','.join(ids)}"
+        f"Updating {'all ' if len(ids) == 0 else ''}"
+        f"{'users' if not groups else 'groups'} "
+        f"{','.join(ids)}"
     )
     counter = 0
     successes = []
@@ -146,7 +193,8 @@ def update_user_data(
         print(f"Timeouts occurred for {len(timed_out)} records: {timed_out}")
     if len(invalid_responses):
         print(
-            f"Invalid responses returned for {len(invalid_responses)} records: "
+            f"Invalid responses returned for "
+            f"{len(invalid_responses)} records: "
             f"{invalid_responses}"
         )
     if len(failures):

invenio_remote_user_data/service.py

@@ -12,16 +12,13 @@
 
 # frm pprint import pformat
 from invenio_access.permissions import system_identity
-from invenio_accounts.models import User, UserIdentity, Role
+from invenio_accounts.models import User, UserIdentity
 from invenio_accounts.proxies import current_accounts
-from invenio_communities.communities.services.results import CommunityItem
-from invenio_group_collections.utils import make_base_group_slug  # noqa
 from invenio_group_collections.proxies import (
     current_group_collections_service,
 )  # noqa
 from invenio_queues.proxies import current_queues
 from invenio_records_resources.services import Service
-from invenio_users_resources.proxies import current_groups_service
 import json
 import os
 
@@ -136,13 +133,15 @@ def update_group_from_remote(
         """Update group data from remote server.
 
         If Invenio group roles for the remote group do not exist,
-        they will not be created. If the group's collection exists, its metadata
-        will be updated. If the group's collection does not exist, it will NOT
-        be created. Creation of group collections is handled by the
-        `invenio_group_collections` service.
+        they will not be created. If the group's collection exists,
+        its metadata will be updated. If the group's collection
+        does not exist, it will NOT be created. Creation of group
+        collections is handled by the `invenio_group_collections` service.
 
         If the update uncovers deleted group collections, the method will
-        not update them. Instead, it will return a value of "deleted" for the "metadata_updated" key for that collection's slug in the return dictionary.
+        not update them. Instead, it will return a value of "deleted" for
+        the "metadata_updated" key for that collection's slug in the
+        return dictionary.
 
         This method is triggered by the
         :class:`invenio_remote_user_data.views.RemoteUserDataUpdateWebhook`
@@ -155,7 +154,12 @@ def update_group_from_remote(
             **kwargs: Additional keyword arguments to pass to the method.
 
         Returns:
-            dict: A dictionary of the updated group data. The keys are the slugs of the updated group collections. The values are dictionaries with the key "metadata_updated" and a value of "deleted" if the group collection was deleted, or the result of the update operation if the group collection was updated.
+            dict: A dictionary of the updated group data. The keys are
+            the slugs of the updated group collections. The values are
+            dictionaries with the key "metadata_updated" and a value of
+            "deleted" if the group collection was deleted, or the
+            result of the update operation if the group collection was
+            updated.
         """
         self.require_permission(identity, "trigger_update")
         results_dict = {}
@@ -404,17 +408,13 @@ def update_user_from_remote(
             remote_data = self.fetch_from_remote_api(
                 user, idp, remote_id, **kwargs
             )
-            self.logger.debug("Remote data: " + pformat(remote_data))
             new_data, user_changes, groups_changes = [{}, {}, {}]
             if "users" in remote_data.keys():
                 new_data, user_changes, groups_changes = (
                     self.compare_remote_with_local(
                         user, remote_data, idp, **kwargs
                     )
                 )
-                self.logger.debug("New data: " + pformat(new_data))
-                self.logger.debug("User changes: " + pformat(user_changes))
-                self.logger.debug("Group changes: " + pformat(groups_changes))
             elif "error" in remote_data.keys():
                 if remote_data["error"]["reason"] == "not_found":
                     self.logger.error(
@@ -428,7 +428,8 @@ def update_user_from_remote(
                     return user, remote_data, [], {}
                 elif remote_data["error"]["reason"] == "invalid_response":
                     self.logger.error(
-                        "Invalid response fetching user data from remote server."
+                        "Invalid response fetching user data from remote "
+                        "server."
                     )
                     return user, remote_data, [], {}
                 else:
@@ -560,6 +561,7 @@ def fetch_from_remote_api(
                     "status_code": 408,
                     "text": "Request timed out",
                 }
+        return remote_data
 
     def compare_remote_with_local(
         self, user: User, remote_data: dict, idp: str, **kwargs