DiscordBot/Fixes.md

Major problems found

1. Database / SQL issues

• sql_commands.py

  • execute_query() returns copy(cursor) after closing the cursor/connection, which is invalid and likely breaks fetch operations.
  • insert() parses SQL text to build ON DUPLICATE KEY UPDATE; this is brittle and unsafe for many query forms.
  • delete() builds SQL using f-strings with table_name and column names from caller input, so it can be SQL-injection vulnerable if those values are not controlled.
  • create_table_if_not_exists() also uses unescaped table/schema names via f-string.
  • bulk_insert() assumes params are list of dicts and rebuilds SQL based on keys; this is fragile and can break on mixed column orders.
  • DatabaseManager is instantiated in many modules/cogs, creating multiple separate connection pools instead of reusing one singleton.

• bank_functions.py

  • db = DatabaseManager("economy") is created at import time and may use incorrect/duplicate env loading.
  • bank_data() treats fetch_one() result inconsistently; if it returns {} instead of None, account creation may not happen.
  • update_money() uses insert(..., overwrite=True) to update balances, which is a weird and brittle upsert pattern.
  • record_transaction() calls db.insert("transactions", ...) without a proper query skeleton.

• npc_memory.py

  • Uses sqlite3.connect('npc_memory.db') with a hardcoded path and no thread-safety handling.
  • Single SQLite connection is reused without controlling concurrency or isolation.

2. Secret/configuration handling

• app.py
  • app.secret_key = os.getenv("SECRET_KEY") can be None, breaking Flask session security.
  • Required env-vars are checked, but DISCORD_BOT_TOKEN is loaded and never used; config is inconsistent with actual bot startup.
• mail.py
  • Loads email credentials from env and may fail silently if missing.
• bot.py
  • Prints the bot token with print(token); this is a secret leak risk.
• Multiple files call load_dotenv() repeatedly instead of once at startup.

3. Bot startup / architecture

• bot.py

  • iterate_prefix("Vamoc") and iterate_prefix("!V") generate all uppercase/lowercase permutations, which is extremely inefficient and unnecessary.
  • self.iterate_prefix("Vamoc")+self.iterate_prefix("!V") creates huge prefixes list and prints it.
  • A Flask dev server is started in a daemon thread with app.run(...), which is not suitable for production and may cause shutdown issues.
  • token missing path handling is minimal and should use explicit error handling.

• app.py

  • Uses Flask dev server, OAuth flows, and session storage without clear production readiness.
  • Raw requests calls to Discord API are used without timeouts in some cases.

4. Code quality / maintainability

• economy.py

  • from utils.bank_functions import * is bad practice and obscures imports.
  • Duplicate from discord.ext import commands import.
  • Many methods have weak validation and inconsistent return semantics.
  • check_transfer() has odd parameter names and does not cleanly separate logic from reply behavior.
  • daily command uses inconsistent datetime handling and stores timestamp as float in DB without schema enforcement.

• sql_commands.py

  • Uses pandas only for fetch_as_dataframe, which is heavy for a bot and likely unnecessary.
  • pool_reset_session config uses bool(os.getenv(...)), which returns True for any non-empty string, not only "True".

• README.md

  • The documented repo structure does not match actual workspace structure.
  • It claims SQLite usage while sql_commands.py uses MySQL.
  • It references files/paths that are not present (e.g. extras/, LICENSE).

5. Dependency / packaging

• requirements.txt
  • Includes large packages like numpy, pandas, and mysql-connector even though the bot only needs lightweight DB access and small utilities.
  • mysql-connector==2.2.9 is very old; upgrade or use mysql-connector-python / mysqlclient.

6. Security / robustness

• app.py

  • OAuth redirect URL and query parameters are concatenated without URL encoding.
  • Session data stores access tokens directly, which is sensitive.
  • get_user_transactions() builds ORDER BY using string interpolation; it is validated but could be improved with safer mapping.

• mail.py

  • HTML email is generated via large f-strings; this is hard to maintain and could be moved to a template.

• Many files mix business logic and I/O without error boundaries, so failures can cascade.

7. Other issues / enhancements

• Several experimental files under Experimantal and implementing use input() and are not production-ready.
• npc_memory.py and sql_commands.py should expose a shared data access layer rather than ad-hoc DB wrappers in each module.
• Missing tests, linting, formatting, and structured error logging across the repo.
• app.py has duplicated code patterns for Discord API requests and could benefit from helper functions.
• bot.py and bot_development.py are almost duplicates; consolidate startup logic.

Recommended enhancements

• Centralize configuration and database initialization.
• Use a shared singleton DatabaseManager or dependency injection.
• Add strong type hints and remove wildcard imports.
• Replace Flask dev thread with proper web hosting or decouple the web interface.
• Validate all environment variables at startup.
• Add command-level checks and better error messages.
• Remove or replace heavy dependencies if they are not needed.
• Add tests for DB operations, command behavior, and web routes.
• Clean up README to match actual repository state.

Note: this is a high-level audit, not a full code rewrite. If you want, I can now create a prioritized issue list or fix the worst bugs first.