From 2e4d7d0db43a6da43e7b413ce351074718112e0b Mon Sep 17 00:00:00 2001 From: agessaman Date: Thu, 12 Feb 2026 20:31:14 -0800 Subject: [PATCH] Refactor configuration handling for joke commands - Standardized the configuration keys for joke commands by replacing `joke_enabled` and `dadjoke_enabled` with a unified `enabled` key in the configuration files. - Updated the validation logic to warn users when both legacy `[Jokes]` and new `[Joke_Command]`/`[DadJoke_Command]` sections are present, suggesting the removal of the legacy section. - Enhanced the command classes to support fallback mechanisms for legacy configuration keys, ensuring backward compatibility. - Added tests to verify the correct behavior of the new configuration handling and legacy support. --- config.ini.example | 195 +++++++++++++++------------- config.ini.minimal-example | 9 +- modules/commands/base_command.py | 26 ++++ modules/commands/dadjoke_command.py | 6 +- modules/commands/joke_command.py | 6 +- modules/config_validation.py | 18 ++- tests/commands/test_base_command.py | 36 +++++ tests/test_config_validation.py | 31 +++++ 8 files changed, 222 insertions(+), 105 deletions(-) diff --git a/config.ini.example b/config.ini.example index 0f199de..4e59838 100644 --- a/config.ini.example +++ b/config.ini.example @@ -232,7 +232,7 @@ companion_min_inactive_days = 30 # Enable or disable the joke command # true: Joke command is available (default) # false: Joke command is disabled -joke_enabled = true +enabled = true # Enable seasonal joke defaults (October: spooky, December: Christmas) # true: Seasonal defaults are applied (default) @@ -245,13 +245,13 @@ seasonal_jokes = true long_jokes = false # Channels where joke command is allowed (omit to use global monitor_channels) -# channels = general,#bot,#jokes +# channels = #bot,#jokes [DadJoke_Command] # Enable or disable the dad joke command # true: Dad joke command is available (default) # false: Dad joke command is disabled -dadjoke_enabled = true +enabled = true # Handle long jokes (over 130 characters) # false: Fetch new jokes until we get a short one (default) @@ -259,7 +259,7 @@ dadjoke_enabled = true long_jokes = false # Channels where dadjoke command is allowed (omit to use global monitor_channels) -# channels = general,#bot,#jokes +# channels = #bot,#jokes [Keywords] # Available placeholders (message-based): @@ -549,95 +549,6 @@ precipitation_unit = inch # Note: Only one 'default' source can be set per provider type. # Future providers (e.g., custom.pwaweather.location) will follow the same pattern. -# Command stubs (enable/channels only; omit to use global monitor_channels) -[Wx_Command] -enabled = true -# channels = - -[WebViewer_Command] -enabled = true -# channels = - -[Sun_Command] -enabled = true -# channels = - -[Solar_Command] -enabled = true -# channels = - -[Solarforecast_Command] -enabled = true -# channels = - -[Satpass_Command] -enabled = true -# channels = - -[Roll_Command] -enabled = true -# channels = - -[Repeater_Command] -enabled = true -# channels = - -[Ping_Command] -enabled = true -# channels = - -[Moon_Command] -enabled = true -# channels = - -[Magic8_Command] -enabled = true -# channels = - -[Hfcond_Command] -enabled = true -# channels = - -[Help_Command] -enabled = true -# channels = - -[Hello_Command] -enabled = true -# channels = - -[Dice_Command] -enabled = true -# channels = - -[Cmd_Command] -enabled = true -# channels = - -[Channels_Command] -enabled = true -# channels = - -[Catfact_Command] -enabled = true -# channels = - -[Aqi_Command] -enabled = true -# channels = - -[Advert_Command] -enabled = true -# channels = - -[Test_Command] -enabled = true -# channels = - -[Feed_Command] -enabled = true -# channels = - [Solar_Config] # URL timeout for external API calls (seconds) url_timeout = 10 @@ -1098,6 +1009,104 @@ max_results = 10 # API request timeout in seconds url_timeout = 10 +# Command stubs (enable/channels only; omit to use global monitor_channels) +[Wx_Command] +enabled = true +# channels = + +[WebViewer_Command] +enabled = true +# channels = + +[Sun_Command] +enabled = true +# channels = + +[Solar_Command] +enabled = true +# channels = + +[Solarforecast_Command] +enabled = true +# channels = + +[Satpass_Command] +enabled = true +# channels = + +[Roll_Command] +enabled = true +# channels = + +[Repeater_Command] +enabled = true +# channels = + +[Ping_Command] +enabled = true +# channels = + +[Moon_Command] +enabled = true +# channels = + +[Magic8_Command] +enabled = true +# channels = + +[Hfcond_Command] +enabled = true +# channels = + +[Help_Command] +enabled = true +# channels = + +[Hello_Command] +enabled = true +# channels = + +[Dice_Command] +enabled = true +# channels = + +[Cmd_Command] +enabled = true +# channels = + +[Channels_Command] +enabled = true +# channels = + +[Catfact_Command] +enabled = true +# channels = + +[Aqi_Command] +enabled = true +# channels = + +[Advert_Command] +enabled = true +# channels = + +[Test_Command] +enabled = true +# channels = + +[Feed_Command] +enabled = false +# channels = + +#################################################################################################### +# # +# Web Viewer Configuration # +# # +# Settings for web administration backend # +# # +#################################################################################################### + + [Web_Viewer] # Enable or disable the web data viewer # SECURITY NOTE: Web viewer has NO AUTHENTICATION built-in diff --git a/config.ini.minimal-example b/config.ini.minimal-example index 0b313a4..390f6ed 100644 --- a/config.ini.minimal-example +++ b/config.ini.minimal-example @@ -373,10 +373,11 @@ enabled = false # Enable or disable the cmd command enabled = false -[Jokes] -# Enable or disable joke commands -joke_enabled = false -dadjoke_enabled = false +[Joke_Command] +enabled = false + +[DadJoke_Command] +enabled = false [Sports_Command] # Enable or disable the sports command diff --git a/modules/commands/base_command.py b/modules/commands/base_command.py index 1a7b3d8..223d144 100644 --- a/modules/commands/base_command.py +++ b/modules/commands/base_command.py @@ -105,6 +105,11 @@ 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) + legacy_key_alias = { + ('Joke_Command', 'enabled'): ('Jokes', 'joke_enabled'), + ('DadJoke_Command', 'enabled'): ('Jokes', 'dadjoke_enabled'), + } legacy_section_fallback = { ('Joke_Command', 'joke_enabled'): 'Jokes', ('Joke_Command', 'seasonal_jokes'): 'Jokes', @@ -170,6 +175,27 @@ class BaseCommand(ABC): self.logger.debug(f"Error reading config {sec}.{key}: {e}") continue + # 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}") + return fallback @abstractmethod diff --git a/modules/commands/dadjoke_command.py b/modules/commands/dadjoke_command.py index ec84f98..f2a9d59 100644 --- a/modules/commands/dadjoke_command.py +++ b/modules/commands/dadjoke_command.py @@ -41,8 +41,10 @@ class DadJokeCommand(BaseCommand): """ super().__init__(bot) - # Load configuration (DadJoke_Command; legacy [Jokes] supported via get_config_value) - self.dadjoke_enabled = self.get_config_value('DadJoke_Command', 'dadjoke_enabled', fallback=True, value_type='bool') + # Load configuration (enabled standard; dadjoke_enabled legacy from [DadJoke_Command] or [Jokes]) + self.dadjoke_enabled = self.get_config_value('DadJoke_Command', 'enabled', fallback=None, value_type='bool') + if self.dadjoke_enabled is None: + self.dadjoke_enabled = self.get_config_value('DadJoke_Command', 'dadjoke_enabled', fallback=True, value_type='bool') self.long_jokes = self.get_config_value('DadJoke_Command', 'long_jokes', fallback=False, value_type='bool') def get_help_text(self) -> str: diff --git a/modules/commands/joke_command.py b/modules/commands/joke_command.py index 9733307..ae18e63 100644 --- a/modules/commands/joke_command.py +++ b/modules/commands/joke_command.py @@ -56,8 +56,10 @@ class JokeCommand(BaseCommand): """ super().__init__(bot) - # Load configuration (Joke_Command; legacy [Jokes] supported via get_config_value) - self.joke_enabled = self.get_config_value('Joke_Command', 'joke_enabled', fallback=True, value_type='bool') + # Load configuration (enabled standard; joke_enabled legacy from [Joke_Command] or [Jokes]) + self.joke_enabled = self.get_config_value('Joke_Command', 'enabled', fallback=None, value_type='bool') + if self.joke_enabled is None: + self.joke_enabled = self.get_config_value('Joke_Command', 'joke_enabled', fallback=True, value_type='bool') self.seasonal_jokes = self.get_config_value('Joke_Command', 'seasonal_jokes', fallback=True, value_type='bool') self.long_jokes = self.get_config_value('Joke_Command', 'long_jokes', fallback=False, value_type='bool') diff --git a/modules/config_validation.py b/modules/config_validation.py index 7601e3d..6cc8330 100644 --- a/modules/config_validation.py +++ b/modules/config_validation.py @@ -213,10 +213,20 @@ def validate_config(config_path: str) -> List[Tuple[str, str]]: # Check typo map for known non-standard names if section_stripped in SECTION_TYPO_MAP: suggestion = SECTION_TYPO_MAP[section_stripped] - results.append(( - SEVERITY_WARNING, - f"Non-standard section [{section_stripped}]; did you mean [{suggestion}]?", - )) + # Special case: [Jokes] + [Joke_Command]/[DadJoke_Command] overlap + if section_stripped == "Jokes" and ( + "Joke_Command" in sections_present or "DadJoke_Command" in sections_present + ): + results.append(( + SEVERITY_WARNING, + "Both [Jokes] and [Joke_Command]/[DadJoke_Command] are present; " + "the *_Command sections take precedence. Consider removing [Jokes] to avoid confusion.", + )) + else: + results.append(( + SEVERITY_WARNING, + f"Non-standard section [{section_stripped}]; did you mean [{suggestion}]?", + )) else: # Check if section looks like a command name (e.g. [Stats] -> [Stats_Command]) if prefix_to_section is None: diff --git a/tests/commands/test_base_command.py b/tests/commands/test_base_command.py index 19241d2..3549136 100644 --- a/tests/commands/test_base_command.py +++ b/tests/commands/test_base_command.py @@ -6,6 +6,7 @@ from unittest.mock import MagicMock 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.models import MeshMessage from tests.conftest import mock_message @@ -88,6 +89,41 @@ class TestGetConfigValue: val = cmd.get_config_value("Hacker_Command", "enabled", fallback=False, value_type="bool") assert val is True + def test_joke_command_enabled_standard(self, command_mock_bot): + """Joke_Command uses enabled (standard) when present.""" + command_mock_bot.config.add_section("Joke_Command") + command_mock_bot.config.set("Joke_Command", "enabled", "false") + cmd = JokeCommand(command_mock_bot) + assert cmd.joke_enabled is False + + def test_joke_command_joke_enabled_legacy(self, command_mock_bot): + """Joke_Command falls back to joke_enabled when enabled absent.""" + command_mock_bot.config.add_section("Joke_Command") + command_mock_bot.config.set("Joke_Command", "joke_enabled", "false") + cmd = JokeCommand(command_mock_bot) + assert cmd.joke_enabled is False + + def test_joke_command_legacy_jokes_section(self, command_mock_bot): + """Joke_Command reads joke_enabled from legacy [Jokes] when enabled absent.""" + command_mock_bot.config.add_section("Jokes") + command_mock_bot.config.set("Jokes", "joke_enabled", "false") + cmd = JokeCommand(command_mock_bot) + assert cmd.joke_enabled is False + + def test_dadjoke_command_enabled_standard(self, command_mock_bot): + """DadJoke_Command uses enabled (standard) when present.""" + command_mock_bot.config.add_section("DadJoke_Command") + command_mock_bot.config.set("DadJoke_Command", "enabled", "false") + cmd = DadJokeCommand(command_mock_bot) + assert cmd.dadjoke_enabled is False + + def test_dadjoke_command_dadjoke_enabled_legacy(self, command_mock_bot): + """DadJoke_Command falls back to dadjoke_enabled when enabled absent.""" + command_mock_bot.config.add_section("DadJoke_Command") + command_mock_bot.config.set("DadJoke_Command", "dadjoke_enabled", "false") + cmd = DadJokeCommand(command_mock_bot) + assert cmd.dadjoke_enabled is False + class TestCanExecute: """Tests for can_execute().""" diff --git a/tests/test_config_validation.py b/tests/test_config_validation.py index 2896830..8e35e2a 100644 --- a/tests/test_config_validation.py +++ b/tests/test_config_validation.py @@ -119,6 +119,37 @@ enabled = true infos = [r for r in results if r[0] == SEVERITY_INFO] assert any("Stats" in r[1] and "Stats_Command" in r[1] for r in infos) + def test_jokes_overlap_suggests_removal(self, tmp_path): + """When both [Jokes] and [Joke_Command]/[DadJoke_Command] exist, suggest removing [Jokes].""" + config = tmp_path / "config.ini" + config.write_text("""[Connection] +connection_type = serial +serial_port = /dev/ttyUSB0 + +[Bot] +bot_name = TestBot +db_path = {db_path} + +[Channels] +monitor_channels = general +respond_to_dms = true + +[Jokes] +joke_enabled = true + +[Joke_Command] +enabled = true + +[Keywords] +test = ack +""".format(db_path=str(tmp_path / "meshcore_bot.db"))) + results = validate_config(str(config)) + warnings = [r for r in results if r[0] == SEVERITY_WARNING] + assert any( + "Both [Jokes]" in r[1] and "Consider removing [Jokes]" in r[1] + for r in warnings + ) + class TestPathValidation: """Tests for path writability validation."""