From cc91b6ee7ab52b6f48d48818a84130afcffaa1e5 Mon Sep 17 00:00:00 2001 From: agessaman Date: Thu, 12 Feb 2026 20:51:52 -0800 Subject: [PATCH] Refactor command configuration handling - Standardized the configuration keys for various commands by replacing specific `*_enabled` keys with a unified `enabled` key across configuration files. - Updated command classes to support fallback mechanisms for legacy configuration keys, ensuring backward compatibility. - Enhanced the logic in the `BaseCommand` class to handle both standard and legacy keys for command enabling. - Added tests to verify the correct behavior of the new configuration handling and legacy support for commands including Stats, Sports, Hacker, and Alert. --- README.md | 2 +- config.ini.example | 8 ++-- config.ini.minimal-example | 12 ++---- modules/commands/alert_command.py | 11 ++++-- modules/commands/base_command.py | 51 +++++++++++++++--------- modules/commands/hacker_command.py | 4 +- modules/commands/sports_command.py | 9 +++-- modules/commands/stats_command.py | 4 +- tests/commands/test_base_command.py | 60 +++++++++++++++++++++++++++++ tests/conftest.py | 23 +++++++++++ 10 files changed, 144 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index 3aaef09..c1dadf1 100644 --- a/README.md +++ b/README.md @@ -200,7 +200,7 @@ airnow_api_key = # Air quality data ### Alert Command ```ini [Alert_Command] -alert_enabled = true # Enable/disable alert command +enabled = true # Enable/disable alert command max_incident_age_hours = 24 # Maximum age for incidents (hours) max_distance_km = 20.0 # Maximum distance for proximity queries (km) agency.city. = # City-specific agency IDs (e.g., agency.city.seattle = 17D20,17M15) diff --git a/config.ini.example b/config.ini.example index 4e59838..fc35a0f 100644 --- a/config.ini.example +++ b/config.ini.example @@ -610,7 +610,7 @@ emergency.hamradio = Amateur radio emergency net # Enable or disable the sports command # true: Sports command is available # false: Sports command is disabled -sports_enabled = true +enabled = true # Default teams to show when 'sports' command is used without arguments # Comma-separated list of team names (use lowercase) @@ -636,7 +636,7 @@ api_timeout = 10 # Enable or disable the stats command # true: Stats command is available # false: Stats command is disabled -stats_enabled = true +enabled = true # Data retention settings # Number of days to keep stats data (older data will be automatically cleaned up) @@ -849,7 +849,7 @@ star_bias_multiplier = 2.5 # Enable or disable the hacker command # true: Hacker command is available (responds to Linux commands with supervillain mainframe errors) # false: Hacker command is disabled -hacker_enabled = false +enabled = false [Multitest_Command] # Response format for multitest command results @@ -933,7 +933,7 @@ backfill_lookback_days = 30 # Enable or disable the alert command # true: Alert command is available # false: Alert command is disabled -alert_enabled = false +enabled = false # PulsePoint agency IDs by county/region # Format: agency. = comma-separated agency IDs diff --git a/config.ini.minimal-example b/config.ini.minimal-example index 390f6ed..92c8dcc 100644 --- a/config.ini.minimal-example +++ b/config.ini.minimal-example @@ -380,24 +380,20 @@ enabled = false enabled = false [Sports_Command] -# Enable or disable the sports command -sports_enabled = false +enabled = false [Stats_Command] -# Enable or disable the stats command -stats_enabled = false +enabled = false [Hacker_Command] -# Enable or disable the hacker command -hacker_enabled = false +enabled = false [Greeter_Command] # Enable or disable the greeter command enabled = false [Alert_Command] -# Enable or disable the alert command -alert_enabled = false +enabled = false [Announcements_Command] # Enable or disable the announcements command diff --git a/modules/commands/alert_command.py b/modules/commands/alert_command.py index 9a29ed1..9fcf4c3 100644 --- a/modules/commands/alert_command.py +++ b/modules/commands/alert_command.py @@ -202,7 +202,12 @@ class AlertCommand(BaseCommand): # Get max incident age in hours (default 24 hours) - filter out incidents older than this self.max_incident_age_hours = self.get_config_value('Alert_Command', 'max_incident_age_hours', fallback=24.0, value_type='float') - + + # Load enabled (standard enabled; alert_enabled legacy) + self.alert_enabled = self.get_config_value('Alert_Command', 'enabled', fallback=None, value_type='bool') + if self.alert_enabled is None: + self.alert_enabled = self.get_config_value('Alert_Command', 'alert_enabled', fallback=True, value_type='bool') + def can_execute(self, message: MeshMessage) -> bool: """Check if this command can be executed with the given message. @@ -212,9 +217,7 @@ class AlertCommand(BaseCommand): Returns: bool: True if command is enabled and checks pass, False otherwise. """ - # Check if alert command is enabled - alert_enabled = self.get_config_value('Alert_Command', 'alert_enabled', fallback=True, value_type='bool') - if not alert_enabled: + if not self.alert_enabled: return False # Call parent can_execute() which includes channel checking, cooldown, etc. diff --git a/modules/commands/base_command.py b/modules/commands/base_command.py index 223d144..f82b00c 100644 --- a/modules/commands/base_command.py +++ b/modules/commands/base_command.py @@ -105,10 +105,24 @@ class BaseCommand(ABC): 'Weather': 'Wx_Command', # wx command reads from [Wx_Command]; [Weather] is legacy } # Legacy [Jokes] -> [Joke_Command] / [DadJoke_Command]: (requested_section, key) -> legacy section - # For 'enabled', also try legacy key in [Jokes]: (section, key) -> (legacy_section, legacy_key) + # For 'enabled', also try legacy key: (section, key) -> (legacy_section, legacy_key) or list of same legacy_key_alias = { ('Joke_Command', 'enabled'): ('Jokes', 'joke_enabled'), ('DadJoke_Command', 'enabled'): ('Jokes', 'dadjoke_enabled'), + # Standard enabled with *_enabled fallback (same section, then old section) + ('Stats_Command', 'enabled'): [ + ('Stats_Command', 'stats_enabled'), + ('Stats', 'stats_enabled'), + ], + ('Sports_Command', 'enabled'): [ + ('Sports_Command', 'sports_enabled'), + ('Sports', 'sports_enabled'), + ], + ('Hacker_Command', 'enabled'): [ + ('Hacker_Command', 'hacker_enabled'), + ('Hacker', 'hacker_enabled'), + ], + ('Alert_Command', 'enabled'): [('Alert_Command', 'alert_enabled')], } legacy_section_fallback = { ('Joke_Command', 'joke_enabled'): 'Jokes', @@ -178,23 +192,24 @@ class BaseCommand(ABC): # Try legacy key alias (e.g. [Jokes] joke_enabled when requesting Joke_Command enabled) alias = legacy_key_alias.get((section, key)) if alias: - legacy_sec, legacy_key = alias - if self.bot.config.has_section(legacy_sec) and self.bot.config.has_option(legacy_sec, legacy_key): - try: - if value_type == 'bool': - value = self.bot.config.getboolean(legacy_sec, legacy_key, fallback=fallback) - elif value_type == 'int': - value = self.bot.config.getint(legacy_sec, legacy_key, fallback=fallback) - elif value_type == 'float': - value = self.bot.config.getfloat(legacy_sec, legacy_key, fallback=fallback) - elif value_type == 'list': - raw = self.bot.config.get(legacy_sec, legacy_key) - value = [item.strip() for item in raw.split(',') if item.strip()] - else: - value = self.bot.config.get(legacy_sec, legacy_key) - return value - except (ValueError, TypeError) as e: - self.logger.debug(f"Config conversion error for {legacy_sec}.{legacy_key}: {e}") + aliases = [alias] if isinstance(alias, tuple) else alias + for legacy_sec, legacy_key in aliases: + if self.bot.config.has_section(legacy_sec) and self.bot.config.has_option(legacy_sec, legacy_key): + try: + if value_type == 'bool': + value = self.bot.config.getboolean(legacy_sec, legacy_key, fallback=fallback) + elif value_type == 'int': + value = self.bot.config.getint(legacy_sec, legacy_key, fallback=fallback) + elif value_type == 'float': + value = self.bot.config.getfloat(legacy_sec, legacy_key, fallback=fallback) + elif value_type == 'list': + raw = self.bot.config.get(legacy_sec, legacy_key) + value = [item.strip() for item in raw.split(',') if item.strip()] + else: + value = self.bot.config.get(legacy_sec, legacy_key) + return value + except (ValueError, TypeError) as e: + self.logger.debug(f"Config conversion error for {legacy_sec}.{legacy_key}: {e}") return fallback diff --git a/modules/commands/hacker_command.py b/modules/commands/hacker_command.py index 0675533..a3c186d 100644 --- a/modules/commands/hacker_command.py +++ b/modules/commands/hacker_command.py @@ -34,7 +34,9 @@ class HackerCommand(BaseCommand): bot: The bot instance. """ super().__init__(bot) - self.enabled = self.get_config_value('Hacker_Command', 'hacker_enabled', fallback=False, value_type='bool') + self.enabled = self.get_config_value('Hacker_Command', 'enabled', fallback=None, value_type='bool') + if self.enabled is None: + self.enabled = self.get_config_value('Hacker_Command', 'hacker_enabled', fallback=False, value_type='bool') def get_help_text(self) -> str: """Get help text for the hacker command. diff --git a/modules/commands/sports_command.py b/modules/commands/sports_command.py index 65d530c..152f92c 100644 --- a/modules/commands/sports_command.py +++ b/modules/commands/sports_command.py @@ -69,6 +69,11 @@ class SportsCommand(BaseCommand): super().__init__(bot) self.url_timeout = 10 # seconds + # Load enabled (standard enabled; sports_enabled legacy) + self.sports_enabled = self.get_config_value('Sports_Command', 'enabled', fallback=None, value_type='bool') + if self.sports_enabled is None: + self.sports_enabled = self.get_config_value('Sports_Command', 'sports_enabled', fallback=True, value_type='bool') + # Initialize API clients self.espn_client = ESPNClient(logger=self.logger, timeout=self.url_timeout) self.thesportsdb_client = TheSportsDBClient(logger=self.logger) @@ -128,9 +133,7 @@ class SportsCommand(BaseCommand): def can_execute(self, message: MeshMessage) -> bool: """Check if this command can execute with the given message""" - # Check if sports command is enabled - sports_enabled = self.get_config_value('Sports_Command', 'sports_enabled', fallback=True, value_type='bool') - if not sports_enabled: + if not self.sports_enabled: return False # Channel access and cooldown are now handled by BaseCommand.can_execute() diff --git a/modules/commands/stats_command.py b/modules/commands/stats_command.py index 3c122bb..73fc8bf 100644 --- a/modules/commands/stats_command.py +++ b/modules/commands/stats_command.py @@ -45,7 +45,9 @@ class StatsCommand(BaseCommand): def _load_config(self) -> None: """Load configuration settings for stats command.""" - self.stats_enabled = self.get_config_value('Stats_Command', 'stats_enabled', fallback=True, value_type='bool') + self.stats_enabled = self.get_config_value('Stats_Command', 'enabled', fallback=None, value_type='bool') + if self.stats_enabled is None: + self.stats_enabled = self.get_config_value('Stats_Command', 'stats_enabled', fallback=True, value_type='bool') self.data_retention_days = self.get_config_value('Stats_Command', 'data_retention_days', fallback=7, value_type='int') self.auto_cleanup = self.get_config_value('Stats_Command', 'auto_cleanup', fallback=True, value_type='bool') self.track_all_messages = self.get_config_value('Stats_Command', 'track_all_messages', fallback=True, value_type='bool') diff --git a/tests/commands/test_base_command.py b/tests/commands/test_base_command.py index 3549136..117e0ff 100644 --- a/tests/commands/test_base_command.py +++ b/tests/commands/test_base_command.py @@ -7,6 +7,10 @@ from modules.commands.base_command import BaseCommand from modules.commands.ping_command import PingCommand from modules.commands.dadjoke_command import DadJokeCommand from modules.commands.joke_command import JokeCommand +from modules.commands.stats_command import StatsCommand +from modules.commands.hacker_command import HackerCommand +from modules.commands.sports_command import SportsCommand +from modules.commands.alert_command import AlertCommand from modules.models import MeshMessage from tests.conftest import mock_message @@ -124,6 +128,62 @@ class TestGetConfigValue: cmd = DadJokeCommand(command_mock_bot) assert cmd.dadjoke_enabled is False + def test_stats_command_enabled_standard(self, command_mock_bot_with_db): + """Stats_Command uses enabled (standard) when present.""" + command_mock_bot_with_db.config.add_section("Stats_Command") + command_mock_bot_with_db.config.set("Stats_Command", "enabled", "false") + cmd = StatsCommand(command_mock_bot_with_db) + assert cmd.stats_enabled is False + + def test_stats_command_stats_enabled_legacy(self, command_mock_bot_with_db): + """Stats_Command falls back to stats_enabled when enabled absent.""" + command_mock_bot_with_db.config.add_section("Stats_Command") + command_mock_bot_with_db.config.set("Stats_Command", "stats_enabled", "false") + cmd = StatsCommand(command_mock_bot_with_db) + assert cmd.stats_enabled is False + + def test_hacker_command_enabled_standard(self, command_mock_bot): + """Hacker_Command uses enabled (standard) when present.""" + command_mock_bot.config.add_section("Hacker_Command") + command_mock_bot.config.set("Hacker_Command", "enabled", "true") + cmd = HackerCommand(command_mock_bot) + assert cmd.enabled is True + + def test_hacker_command_hacker_enabled_legacy(self, command_mock_bot): + """Hacker_Command falls back to hacker_enabled when enabled absent.""" + command_mock_bot.config.add_section("Hacker_Command") + command_mock_bot.config.set("Hacker_Command", "hacker_enabled", "true") + cmd = HackerCommand(command_mock_bot) + assert cmd.enabled is True + + def test_sports_command_enabled_standard(self, command_mock_bot): + """Sports_Command uses enabled (standard) when present.""" + command_mock_bot.config.add_section("Sports_Command") + command_mock_bot.config.set("Sports_Command", "enabled", "false") + cmd = SportsCommand(command_mock_bot) + assert cmd.sports_enabled is False + + def test_sports_command_sports_enabled_legacy(self, command_mock_bot): + """Sports_Command falls back to sports_enabled when enabled absent.""" + command_mock_bot.config.add_section("Sports_Command") + command_mock_bot.config.set("Sports_Command", "sports_enabled", "false") + cmd = SportsCommand(command_mock_bot) + assert cmd.sports_enabled is False + + def test_alert_command_enabled_standard(self, command_mock_bot): + """Alert_Command uses enabled (standard) when present.""" + command_mock_bot.config.add_section("Alert_Command") + command_mock_bot.config.set("Alert_Command", "enabled", "false") + cmd = AlertCommand(command_mock_bot) + assert cmd.alert_enabled is False + + def test_alert_command_alert_enabled_legacy(self, command_mock_bot): + """Alert_Command falls back to alert_enabled when enabled absent.""" + command_mock_bot.config.add_section("Alert_Command") + command_mock_bot.config.set("Alert_Command", "alert_enabled", "false") + cmd = AlertCommand(command_mock_bot) + assert cmd.alert_enabled is False + class TestCanExecute: """Tests for can_execute().""" diff --git a/tests/conftest.py b/tests/conftest.py index 1d73582..f993e59 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -75,6 +75,29 @@ def command_mock_bot(mock_logger, minimal_config): return bot +@pytest.fixture +def command_mock_bot_with_db(mock_logger, minimal_config, tmp_path): + """Command mock bot with db_manager for commands that need DB (e.g. StatsCommand).""" + bot = MagicMock() + bot.logger = mock_logger + bot.config = minimal_config + bot.translator = MagicMock() + + def _mock_translate(key, **kwargs): + if kwargs: + return key + " " + " ".join(f"{k}={v}" for k, v in sorted(kwargs.items())) + return key + + bot.translator.translate = Mock(side_effect=_mock_translate) + bot.translator.get_value = Mock(return_value=None) + bot.command_manager = MagicMock() + bot.command_manager.monitor_channels = ["general", "test", "emergency"] + bot.command_manager.send_response = AsyncMock(return_value=True) + bot.db_manager = MagicMock() + bot.db_manager.db_path = str(tmp_path / "test.db") + return bot + + @pytest.fixture def mock_logger(): """Create a mock logger for testing."""