get player connection history command for owners only#251
Conversation
… with repository integration
sonikro
force-pushed
the
owner-only-command
branch
from
f8a8e6c to
224bcb2
Compare
There was a problem hiding this comment.
Pull request overview
Adds a Discord slash command to fetch a player’s stored connection history, and introduces an “owner-only” command registration path so sensitive commands can be scoped to a specific guild.
Changes:
- Add
findBySteamId3to the corePlayerConnectionHistoryRepositorycontract and implement it in the SQLite provider. - Introduce
get-player-connection-historyDiscord slash command (definition, handler, tests). - Split Discord command registration into global vs owner-only (guild-scoped) commands via
DISCORD_OWNER_GUILD_ID.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/providers/src/repository/SQlitePlayerConnectionHistoryRepository.ts | Adds findBySteamId3 query to fetch connection history rows. |
| packages/providers/src/repository/SQlitePlayerConnectionHistoryRepository.test.ts | Adds repository tests for saving and querying connection history. |
| packages/entrypoints/src/discordBot.ts | Registers regular commands globally and owner-only commands to a configured guild. |
| packages/entrypoints/src/discordBot.test.ts | Updates tests to expect separate global vs guild command registration. |
| packages/entrypoints/src/commands/index.ts | Adds ownerOnly?: boolean to command metadata and wires new command/dependency. |
| packages/entrypoints/src/commands/GetPlayerConnectionHistory/index.ts | Re-exports command definition/handler. |
| packages/entrypoints/src/commands/GetPlayerConnectionHistory/definition.ts | Defines the new slash command and its required player_steam_id3 option. |
| packages/entrypoints/src/commands/GetPlayerConnectionHistory/handler.ts | Implements the interaction handler to query and display connection history. |
| packages/entrypoints/src/commands/GetPlayerConnectionHistory/handler.test.ts | Adds handler tests for “no history” and “history found” replies. |
| packages/core/src/repository/PlayerConnectionHistoryRepository.ts | Extends the repository interface with findBySteamId3. |
| packages/core/src/domain/PlayerConnectionHistory.ts | Adds optional timestamp to the domain type. |
| .env.example | Documents DISCORD_OWNER_GUILD_ID for owner-only command registration. |
| // Separate regular commands from owner-only commands | ||
| const regularCommands = allCommands | ||
| .filter(cmd => !cmd.ownerOnly) | ||
| .map(cmd => cmd.definition); | ||
|
|
There was a problem hiding this comment.
The new ownerOnly split only affects registration, not execution-time authorization. Any member in the owner guild who’s granted permission to use the command could still run it and access sensitive results (e.g., IPs). Consider adding a runtime guard (e.g., in the interactionCreate handler) that denies command.ownerOnly unless the caller is an allowed owner (guild owner / allowlist env var), replying ephemerally on denial.
| const playerSteamId3 = interaction.options.getString('player_steam_id3', true); | ||
|
|
||
| const history = await dependencies.playerConnectionHistoryRepository.findBySteamId3({ | ||
| steamId3: playerSteamId3, | ||
| }); |
There was a problem hiding this comment.
steamId3 values persisted from SRCDS logs appear to be stored without brackets (e.g., U:1:29162964). If users enter [U:1:...] here, findBySteamId3 will not match and will misleadingly return no history. Consider normalizing the input (strip surrounding [] / optional U: prefix variants) before querying, and/or clarify the required format in the command UX.
| const historyLines = history.map((entry, index) => { | ||
| const timestamp = entry.timestamp ? new Date(entry.timestamp).toLocaleString() : 'Unknown'; | ||
| return `${index + 1}. **${entry.nickname}** - IP: \`${entry.ipAddress}\` - ${timestamp}`; | ||
| }); |
There was a problem hiding this comment.
This builds one reply containing all history entries. If a player has many connections, the message can exceed Discord’s 2000-character limit and the reply will fail. Consider capping entries (most recent N) and/or adding a length check/pagination similar to GetMyServers.
| const rows = await knex("player_connection_history") | ||
| .where("steam_id_3", steamId3) | ||
| .orderBy("timestamp", "desc") | ||
| .select("id", "steam_id_3", "ip_address", "nickname", "timestamp"); |
There was a problem hiding this comment.
findBySteamId3 currently returns all rows for a player with no limit. Since this is used for a Discord response, it risks large queries and oversized replies. Consider adding a limit param (with a safe default) and applying .limit(...) (and possibly .offset(...) for pagination).
| connectionHistory: { | ||
| steamId3: "[U:1:12345678]", | ||
| ipAddress: "192.168.1.1", | ||
| nickname: "TestPlayer", | ||
| }, |
There was a problem hiding this comment.
These tests use bracketed SteamID3 values ([U:1:...]), but the SRCDS log parser appears to persist SteamID3 without brackets (U:1:...). Using the production format in tests would better validate real behavior and avoid masking format-mismatch bugs in the command/query path.
| const steamId3 = "[U:1:12345678]"; | ||
|
|
||
| when(interactionMock.options.getString) | ||
| .calledWith('player_steam_id3', true) | ||
| .thenReturn(steamId3); |
There was a problem hiding this comment.
The handler tests use bracketed SteamID3 ([U:1:...]), but production ingestion stores steamId3 without brackets (e.g., U:1:29162964). Updating the test inputs (or explicitly testing normalization of bracketed inputs) will better reflect real usage.
No description provided.