fix(config): return None if env var is empty or non-existent (#315)

* fix(config): return None if env var is empty or non-existent
* chore: fix pylint issues
* fix: add unit test for non-existent env vars

app/commands/exit.py

@@ -1,4 +1,4 @@
-#!/usr/bin/env python3
+"""Exit command."""
 
 import logging
 
@@ -8,7 +8,10 @@ log: logging.Logger = logging.getLogger(__name__)
 
 
 class ExitCommand(Command):
+    """Exit command class."""
+
     def __init__(self) -> None:
+        """Exit command class."""
         super().__init__(
             command_keyword="exit",
             help_message="Exit",
@@ -17,10 +20,10 @@ class ExitCommand(Command):
         self.sender: str = ""
 
     def pre_execute(self, message, attachment_actions, activity) -> None:
-        pass
+        """Pre-execute method."""
 
     def execute(self, message, attachment_actions, activity) -> None:
-        pass
+        """Execute method."""
 
     def post_execute(self, message, attachment_actions, activity) -> None:
-        pass
+        """Post-execute method."""

app/commands/submit_task.py

@@ -1,20 +1,12 @@
-#!/usr/bin/env python3
+"""Submit task command."""
 
 import logging
-import sentry_sdk
 
+import sentry_sdk
 from webex_bot.models.command import Command
 from webex_bot.models.response import Response, response_from_adaptive_card
-from webexteamssdk.models.cards import (
+from webexteamssdk.models.cards import (AdaptiveCard, Column, ColumnSet, Date,
-    AdaptiveCard,
+                                        FontSize, FontWeight, Text, TextBlock)
-    Column,
-    ColumnSet,
-    Date,
-    FontSize,
-    FontWeight,
-    Text,
-    TextBlock,
-)
 from webexteamssdk.models.cards.actions import Submit
 
 from app.utils.config import config
@@ -24,7 +16,10 @@ log: logging.Logger = logging.getLogger(__name__)
 
 
 class SubmitTaskCommand(Command):
+    """Submit task command."""
+
     def __init__(self) -> None:
+        """Submit task command."""
         super().__init__(
             command_keyword="submit_feedback_dstgmyn",
             help_message="Submit Task",
@@ -34,9 +29,11 @@ class SubmitTaskCommand(Command):
         self.sender: str = ""
 
     def pre_execute(self, message, attachment_actions, activity) -> None:
+        """Pre-execute method."""
         self.sender = activity.get("actor").get("id")
 
     def execute(self, message, attachment_actions, activity) -> Response:
+        """Execute method."""
         card_body: list = [
             ColumnSet(
                 columns=[
@@ -48,7 +45,8 @@ class SubmitTaskCommand(Command):
                                 size=FontSize.MEDIUM,
                             ),
                             TextBlock(
-                                f"Add a task to {config.admin_first_name}'s To Do list. All fields are required. Please don't use special characters.",
+                                f"Add a task to {config.admin_first_name}'s To Do list. "
+                                + "All fields are required. Please don't use special characters.",
                                 wrap=True,
                                 isSubtle=True,
                             ),
@@ -62,7 +60,9 @@ class SubmitTaskCommand(Command):
                     Column(
                         width=2,
                         items=[
-                            Text(id="issue_title", placeholder="Summary", maxLength=100),
+                            Text(
+                                id="issue_title", placeholder="Summary", maxLength=100
+                            ),
                             Text(
                                 id="issue_description",
                                 placeholder="Description",
@@ -85,7 +85,8 @@ class SubmitTaskCommand(Command):
                             items=[
                                 Text(
                                     id="issue_requester",
-                                    placeholder="Requester Email (leave blank to submit for yourself)",
+                                    placeholder="Requester Email "
+                                    + "(leave blank to submit for yourself)",
                                     maxLength=100,
                                 ),
                             ],
@@ -119,19 +120,26 @@ class SubmitTaskCommand(Command):
 
 
 class SubmitTaskCallback(Command):
+    """Submit task callback."""
+
     def __init__(self) -> None:
+        """Submit task callback."""
         super().__init__(
-            card_callback_keyword="submit_task_callback_rbamzfyx", delete_previous_message=True
+            card_callback_keyword="submit_task_callback_rbamzfyx",
+            delete_previous_message=True,
         )
         self.msg: str = ""
 
     def pre_execute(self, message, attachment_actions, activity) -> None:
+        """Pre-execute method."""
         issue_title: str = attachment_actions.inputs.get("issue_title")
         issue_description: str = attachment_actions.inputs.get("issue_description")
         completion_date: str = attachment_actions.inputs.get("completion_date")
 
         sender: str = attachment_actions.inputs.get("sender")
-        issue_requester: str = attachment_actions.inputs.get("issue_requester") or sender
+        issue_requester: str = (
+            attachment_actions.inputs.get("issue_requester") or sender
+        )
 
         if not issue_title or not issue_description or not completion_date:
             self.msg = "Please complete all fields."
@@ -145,29 +153,38 @@ class SubmitTaskCallback(Command):
         )
 
         self.msg = (
-            "Submitting your task..." if result else "Failed to submit task. Please try again."
+            "Submitting your task..."
+            if result
+            else "Failed to submit task. Please try again."
         )
 
     def execute(self, message, attachment_actions, activity) -> str:
+        """Execute method."""
         with sentry_sdk.start_transaction(name="submit_task_callback"):
             return self.msg
 
 
 class MyTasksCallback(Command):
+    """My tasks callback."""
+
     def __init__(self) -> None:
+        """My tasks callback."""
         super().__init__(
-            card_callback_keyword="my_tasks_callback_rbamzfyx", delete_previous_message=True
+            card_callback_keyword="my_tasks_callback_rbamzfyx",
+            delete_previous_message=True,
         )
         self.msg: str = ""
 
     def pre_execute(self, message, attachment_actions, activity) -> str:
+        """Pre-execute method."""
         with sentry_sdk.start_transaction(name="my_tasks_preexec"):
             return "Getting your tasks..."
 
     def execute(self, message, attachment_actions, activity) -> str | None:
+        """Execute method."""
         sender: str = attachment_actions.inputs.get("sender")
         result: bool = get_tasks(requestor=sender)
         with sentry_sdk.start_transaction(name="my_tasks_exec"):
             if not result:
                 return "Failed to get tasks. Please try again."
-            return
+            return None

app/main.py

@@ -1,4 +1,6 @@
-#!/usr/bin/env python3
+"""Main module."""
+
+import sys
 
 import sentry_sdk
 from sentry_sdk.integrations.stdlib import StdlibIntegration
@@ -43,4 +45,4 @@ if __name__ == "__main__":
         bot.run()
     except KeyboardInterrupt:
         print("Shutting down bot...")
-        exit()
+        sys.exit(0)

app/utils/config.py

@@ -69,23 +69,32 @@ class Config:
         return os.environ["N8N_WEBHOOK_URL"]
 
     @property
-    def approved_users(self) -> list:
+    def approved_users(self) -> list | None:
         """Returns a list of approved users."""
-        emails: list[str] = os.environ.get("APPROVED_USERS", "").split(",")
+        _emails: list[str] = os.environ.get("APPROVED_USERS", "").split(",")
-        emails = [i.strip() for i in emails if validate_email_syntax(i.strip())]
+        _emails: list[str] = [i.strip() for i in _emails if i]
+        if not _emails:
+            return None
+        emails = [i for i in _emails if validate_email_syntax(i)]
         return emails
 
     @property
-    def approved_rooms(self) -> list:
+    def approved_rooms(self) -> list | None:
         """Returns a list of approved rooms."""
-        rooms: list[str] = os.environ.get("APPROVED_ROOMS", "").split(",")
+        _rooms: list[str] = os.environ.get("APPROVED_ROOMS", "").split(",")
-        return [i.strip() for i in rooms]
+        rooms: list[str] = [i.strip() for i in _rooms if i]
+        if not rooms:
+            return None
+        return rooms
 
     @property
-    def approved_domains(self) -> list:
+    def approved_domains(self) -> list | None:
         """Returns a list of approved domains."""
-        domains: list[str] = os.environ.get("APPROVED_DOMAINS", "").split(",")
+        _domains: list[str] = os.environ.get("APPROVED_DOMAINS", "").split(",")
-        return [i.strip() for i in domains]
+        domains: list[str] = [i.strip() for i in _domains if i]
+        if not domains:
+            return None
+        return domains
 
 
 config: Config = Config()

app/utils/datetime.py

@@ -1,4 +1,4 @@
-#!/usr/bin/env python3
+"""Provides functions for converting timestamps to dates."""
 
 from datetime import datetime
 from zoneinfo import ZoneInfo

app/utils/n8n.py

@@ -1,3 +1,5 @@
+"""N8N utils module."""
+
 import requests
 import sentry_sdk
 

tests/test_config.py

@@ -1,46 +0,0 @@
-#!/usr/bin/env python3
-
-# ruff: noqa: E402 pylint: disable=wrong-import-position
-
-"""Provides test cases for app/utils/config.py."""
-
-import os
-
-vars: dict = {
-    "APP_VERSION": "dev",
-    "BOT_NAME": "TestBot",
-    "WEBEX_API_KEY": "testing",
-    "ADMIN_FIRST_NAME": "Test",
-    "ADMIN_EMAIL": "test@test.com",
-    "N8N_WEBHOOK_URL": "https://n8n.test.com/webhook/abcdefg",
-    "SENTRY_ENABLED": "false",
-    "SENTRY_DSN": "http://localhost",
-    "APPROVED_USERS": "test@test.com",
-    "APPROVED_DOMAINS": "test.com",
-    "APPROVED_ROOMS": "test",
-}
-
-
-for var, value in vars.items():
-    os.environ[var] = value
-
-# needs to be imported AFTER environment variables are set
-from app.utils.config import config  # pragma: no cover
-
-
-def test_config() -> None:
-    assert config.admin_emails == vars["ADMIN_EMAIL"].split(",")
-    assert config.admin_first_name == vars["ADMIN_FIRST_NAME"]
-    assert config.approved_domains == vars["APPROVED_DOMAINS"].split(",")
-    assert config.approved_rooms == vars["APPROVED_ROOMS"].split(",")
-    assert config.approved_users == vars["APPROVED_USERS"].split(",")
-    assert config.bot_name == vars["BOT_NAME"]
-    assert config.n8n_webhook_url == vars["N8N_WEBHOOK_URL"]
-    assert config.sentry_enabled == bool(vars["SENTRY_ENABLED"].upper() == "TRUE")
-    assert config.version == vars["APP_VERSION"]
-    assert config.webex_token == vars["WEBEX_API_KEY"]
-
-    if config.sentry_enabled:
-        assert config.sentry_dsn == vars["SENTRY_DSN"]
-    else:
-        assert config.sentry_dsn == ""

tests/test_config_1.py

@@ -0,0 +1,52 @@
+#!/usr/bin/env python3
+
+# ruff: noqa: E402 pylint: disable=wrong-import-position
+
+"""Provides test cases for app/utils/config.py."""
+
+import os
+
+
+def test_config() -> None:
+    """Test config module."""
+
+    config_vars: dict = {
+        "APP_VERSION": "dev",
+        "BOT_NAME": "TestBot",
+        "WEBEX_API_KEY": "testing",
+        "ADMIN_FIRST_NAME": "Test",
+        "ADMIN_EMAIL": "test@test.com",
+        "N8N_WEBHOOK_URL": "https://n8n.test.com/webhook/abcdefg",
+        "SENTRY_ENABLED": "false",
+        "SENTRY_DSN": "http://localhost",
+        "APPROVED_USERS": "test@test.com",
+        "APPROVED_DOMAINS": "test.com",
+        "APPROVED_ROOMS": "test",
+    }
+
+    for config_var, value in config_vars.items():
+        os.environ[config_var] = value
+
+    # needs to be imported AFTER environment variables are set
+    from app.utils.config import config  # pragma: no cover
+
+    assert config.admin_emails == config_vars["ADMIN_EMAIL"].split(",")
+    assert config.admin_first_name == config_vars["ADMIN_FIRST_NAME"]
+    assert config.approved_domains == config_vars["APPROVED_DOMAINS"].split(",")
+    assert config.approved_rooms == config_vars["APPROVED_ROOMS"].split(",")
+    assert config.approved_users == config_vars["APPROVED_USERS"].split(",")
+    assert config.bot_name == config_vars["BOT_NAME"]
+    assert config.n8n_webhook_url == config_vars["N8N_WEBHOOK_URL"]
+    assert config.sentry_enabled == bool(
+        config_vars["SENTRY_ENABLED"].upper() == "TRUE"
+    )
+    assert config.version == config_vars["APP_VERSION"]
+    assert config.webex_token == config_vars["WEBEX_API_KEY"]
+
+    if config.sentry_enabled:
+        assert config.sentry_dsn == config_vars["SENTRY_DSN"]
+    else:
+        assert config.sentry_dsn == ""
+
+    for config_var in config_vars:
+        os.environ.pop(config_var, None)

tests/test_config_2.py

@@ -0,0 +1,49 @@
+#!/usr/bin/env python3
+
+# ruff: noqa: E402 pylint: disable=wrong-import-position
+
+"""Provides test cases for app/utils/config.py."""
+
+import os
+
+
+def test_config_no_admin_vars() -> None:
+    """Test config module."""
+
+    config_vars: dict = {
+        "APP_VERSION": "dev",
+        "BOT_NAME": "TestBot",
+        "WEBEX_API_KEY": "testing",
+        "ADMIN_FIRST_NAME": "Test",
+        "ADMIN_EMAIL": "test@test.com",
+        "N8N_WEBHOOK_URL": "https://n8n.test.com/webhook/abcdefg",
+        "SENTRY_ENABLED": "false",
+        "SENTRY_DSN": "http://localhost",
+    }
+
+    for config_var, value in config_vars.items():
+        os.environ[config_var] = value
+
+    # needs to be imported AFTER environment variables are set
+    from app.utils.config import config  # pragma: no cover
+
+    assert config.approved_domains is None
+    assert config.approved_rooms is None
+    assert config.approved_users is None
+    assert config.admin_emails == config_vars["ADMIN_EMAIL"].split(",")
+    assert config.admin_first_name == config_vars["ADMIN_FIRST_NAME"]
+    assert config.bot_name == config_vars["BOT_NAME"]
+    assert config.n8n_webhook_url == config_vars["N8N_WEBHOOK_URL"]
+    assert config.sentry_enabled == bool(
+        config_vars["SENTRY_ENABLED"].upper() == "TRUE"
+    )
+    assert config.version == config_vars["APP_VERSION"]
+    assert config.webex_token == config_vars["WEBEX_API_KEY"]
+
+    if config.sentry_enabled:
+        assert config.sentry_dsn == config_vars["SENTRY_DSN"]
+    else:
+        assert config.sentry_dsn == ""
+
+    for config_var in config_vars:
+        os.environ.pop(config_var, None)

tests/test_datetime.py

@@ -8,12 +8,14 @@ from app.utils.datetime import timestamp_to_date  # pragma: no cover
 
 
 def test_correct() -> None:
+    """Test timestamp_to_date() with a correct timestamp."""
     timestamp: int = 1680722218
     result: str = timestamp_to_date(timestamp)
     assert result == "2023-04-05"
 
 
 def test_invalid() -> None:
+    """Test timestamp_to_date() with an invalid timestamp."""
     timestamp: str = "hello"
     with pytest.raises(TypeError) as excinfo:
         timestamp_to_date(timestamp)